Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CK_DATE - memory allocation error #244

Closed
vjardin opened this issue Sep 10, 2019 · 12 comments
Closed

CK_DATE - memory allocation error #244

vjardin opened this issue Sep 10, 2019 · 12 comments

Comments

@vjardin
Copy link

vjardin commented Sep 10, 2019

Hi,

When I have a CK_DATE, then we get this memory failure that is recorded here https://github.com/p11-glue/p11-kit/blob/master/p11-kit/rpc-message.c#L988

For this issue, value_length is 0 byte which of course is not a validé one. How/when should value_length be set ?

Thank you,

@ueno
Copy link
Member

ueno commented Sep 10, 2019

It shall be set by the caller of any PKCS#11 function that takes a CK_DATE attribute (typically a part of attribute array, e.g., C_CreateObject). How did you reproduce it?

@vjardin
Copy link
Author

vjardin commented Sep 10, 2019

Using pkcs-dump dump command.
The Dump() function calls C_GetAttributeValue ()

@ueno
Copy link
Member

ueno commented Sep 10, 2019

Do you mean this?
Note that the code path is only taken if you set up a module through p11-kit remote or p11-kit server. Do you have such configuration?

@vjardin
Copy link
Author

vjardin commented Sep 10, 2019

Yes, correct for this pkcs-dump repo. It is a good stress test.

I use p11-kit remote (over ssh).

@ueno
Copy link
Member

ueno commented Sep 10, 2019

Which PKCS#11 module do you actually remote (e.g., opensc-pkcs11.so, something proprietary)?

@vjardin
Copy link
Author

vjardin commented Sep 10, 2019

I use a proprietary one ; I do not have an opensc compatible card to try with.

From the code I do not understand why a wrong value can lead to such memory issue. Moreover, according to oasis pkcs11 specification, I remember that a request with a value 0 means "get/return the size of the memory", doesn't it ?

@vjardin
Copy link
Author

vjardin commented Sep 10, 2019

For details, see some traces I added from the pkcs11-dump/Dump() function:
Dump:1149 C_GetAttributeValue(17, 1, {type=272, p=0x7fffc3d71af0, ulValueLen=10240}, 1)

then, we get these traces:

(p11-kit:2638) write_at: ok: wrote block of 33
(p11-kit:2638) read_at: ok: read block of 12
(p11-kit:2638) read_at: ok: read block of 6
(p11-kit:2638) read_at: ok: read block of 40
(p11-kit:2638) rpc_C_GetAttributeValue: GetAttributeValue: enter
proto_read_attribute_buffer:276 ulValueLen=10240/value
p11_rpc_buffer_add_attribute:1066
p11_rpc_buffer_add_attribute:1068
p11_rpc_buffer_add_attribute:1070 value_type=4 attr->ulValueLen=0
p11_rpc_buffer_add_date_value:1006 buffer_fail value_length=0 sizeof (CK_DATE)=8
p11_rpc_buffer_add_attribute:1075
(p11-kit:2638) rpc_C_GetAttributeValue: ret: 49
(p11-kit:2638) message: out of memory error putting together message
(p11-kit:2638) message: unexpected error handling rpc message
(p11-kit:2638) p11_kit_module_release: in
(p11-kit:2638) p11_kit_module_release: out
(p11-kit:2638) uninit_common: uninitializing library

I'll keep investigating.

@vjardin
Copy link
Author

vjardin commented Sep 10, 2019

FYI, when we do a direct access with pkcs-dump on this card, we can see that CKA_START_DATE is an empty value, so it happens that the value is empty.

CKA_DERIVE: FALSE
Dump:1149 C_GetAttributeValue(1, 1, {type=272, p=0x7ffe0f4c0a20, ulValueLen=10240}, 1)
CKA_START_DATE:
Dump:1149 C_GetAttributeValue(1, 1, {type=273, p=0x7ffe0f4c0a20, ulValueLen=10240}, 1)
CKA_END_DATE:
Dump:1149 C_GetAttributeValue(1, 1, {type=288, p=0x7ffe0f4c0a20, ulValueLen=10240}, 1)
CKA_MODULUS:
xyz

More debugs from pkcs-dump (without p11-remote) shows that: ulValueLen is 0:

   case attrtypeCK_DATE:
      printf ("XXXDATE len=%ld   %s", t[0].ulValueLen, FixedStringToString (t[0].pValue, t[0].ulValueLen).c_str ());
    break;

the output is:CKA_START_DATE: XXXDATE len=0

vjardin added a commit to vjardin/p11-kit that referenced this issue Sep 10, 2019
Fix the issue p11-glue#244 : one should return an empty buffer whenever
its length is 0.
@vjardin
Copy link
Author

vjardin commented Sep 10, 2019

Maybe this fix could be generalized for any cases when the length is 0. What do you think ?

@ueno
Copy link
Member

ueno commented Sep 11, 2019

Thank you for the investigation. Yes, indeed the PKCS#11 spec suggests that ulValueLen should be 0 for the attributes that are specified to be empty, and CKA_START_DATE and CKA_END_DATE are two of them.

@vjardin
Copy link
Author

vjardin commented Sep 11, 2019

Then, should we turn my patch into a generic one for all the cases when ulValueLen is 0 ?

vjardin added a commit to vjardin/p11-kit that referenced this issue Sep 16, 2019
Fix the issue p11-glue#244 : one should return an empty buffer whenever
its length is 0.
ueno added a commit to ueno/p11-kit that referenced this issue Sep 25, 2019
Unlike other data types, CK_DATE value may be empty (and that is the
default).  Treat it as a valid value and serialize/deserialize
accordingly.

Reported by Vincent JARDIN in:
p11-glue#244
@vjardin
Copy link
Author

vjardin commented Sep 28, 2019

This issue is fixed by the pull request #253 , it can be closed once it'll be pushed.

ueno added a commit that referenced this issue Sep 30, 2019
Unlike other data types, CK_DATE value may be empty (and that is the
default).  Treat it as a valid value and serialize/deserialize
accordingly.

Reported by Vincent JARDIN in:
#244
@ueno ueno closed this as completed Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants