Correctly handle CK_UNAVAILABLE_INFORMATION (2nd attempt)

[minego]

This is a 2nd attempt, because the last rebase in my previous pull request (#200) broke the logic and I missed it. This corrects that oversight.

The PKCS#11 specification states

The constant CK_UNAVAILABLE_INFORMATION is used in the ulValueLen field to denote an invalid or unavailable value. See C_GetAttributeValue for further details.

This MR exposes the CK_UNAVAILABLE_INFORMATION constant definition (along with CK_EFFECTIVELY_INFINITE which is defined next to it, for completeness sake) and checks for that result after calling C_GetAttributeValue

This corrects a crash when a PKCS#11 module returns this. I believe this will fix the following issues:
#139
#60

Prior to this change the returned value was being interpreted as an actual size, which resulted in attempting to allocate an array of size -1 which causes an overflow:

  File "pkcs11/_pkcs11.pyx", line 726, in pkcs11._pkcs11.Object.__getitem__
  File "pkcs11/_utils.pyx", line 11, in pkcs11._pkcs11.CK_BYTE_buffer
  File "stringsource", line 152, in View.MemoryView.array.__cinit__
OverflowError: Python int too large to convert to C ssize_t

[MatthiasValvekens]

Hi, I rebased your changes, but the tests fail. Looking into the reasons why, it has to do with some library functions not coping well with these None responses.

Copying the relevant text of the spec below: https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/csd01/pkcs11-spec-v3.1-csd01.html#_Toc98177195

---

1. If the specified attribute (i.e., the attribute specified by the type field) for the object cannot be revealed because the object is sensitive or unextractable, then the ulValueLen field in that triple is modified to hold the value CK_UNAVAILABLE_INFORMATION.

2. Otherwise, if the specified value for the object is invalid (the object does not possess such an attribute), then the ulValueLen field in that triple is modified to hold the value CK_UNAVAILABLE_INFORMATION.

3. Otherwise, if the pValue field has the value NULL_PTR, then the ulValueLen field is modified to hold the exact length of the specified attribute for the object.

4. Otherwise, if the length specified in ulValueLen is large enough to hold the value of the specified attribute for the object, then that attribute is copied into the buffer located at pValue, and the ulValueLen field is modified to hold the exact length of the attribute.

5. Otherwise, the ulValueLen field is modified to hold the value CK_UNAVAILABLE_INFORMATION.

If case 1 applies to any of the requested attributes, then the call should return the value CKR_ATTRIBUTE_SENSITIVE. If case 2 applies to any of the requested attributes, then the call should return the value CKR_ATTRIBUTE_TYPE_INVALID. If case 5 applies to any of the requested attributes, then the call should return the value CKR_BUFFER_TOO_SMALL. As usual, if more than one of these error codes is applicable, Cryptoki may return any of them. Only if none of them applies to any of the requested attributes will CKR_OK be returned.

---

The bolding is mine. My takeaway here is the following:

• CKR_OK with CK_UNAVAILABLE_INFORMATION in ulValueLen is not a compliant PKCS#11 response (although I'm sure that there are tokens that get this wrong, as evidenced by the bug reports complaining about this)
• assertRV(retval) on the result of C_GetAttributeValue covers this case for compliant tokens.
• Since returning None evidently breaks stuff in various places, we should cover this case by raising an exception instead. How about overriding the CKR_OK with CKR_FUNCTION_FAILED if ulValueLen is CK_UNAVAILABLE_INFORMATION?

WDYT?

[MatthiasValvekens]

How about this instead of returning None? (see bigger comment)

        if retval == CKR_OK and template.ulValueLen == CK_UNAVAILABLE_INFORMATION:
            retval = CKR_FUNCTION_FAILED

We can also add a comment then that this is specifically to deal with noncompliant tokens.

[MatthiasValvekens]

This branch shouldn't be necessary.

[minego]

I don't agree with your logic about case 2. The spec doesn't indicate that it should return CKR_ATTRIBUTE_TYPE_INVALID, and in fact it seems that it would be wrong to do so if the attribute type specified is valid, but not available for that key. Regardless, treating that value as a length is not appropriate. That was the root bug. Since the API doesn't appear to handle returning Null I think returning an empty buffer or an exception would be the next best options.

On Mon, Jun 23, 2025, at 5:12 PM, Matthias Valvekens wrote: ***@***.**** requested changes on this pull request. Hi, I rebased your changes, but the tests fail. Looking into the reasons why, it has to do with some library functions not coping well with these `None` responses. Copying the relevant text of the spec below: https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/csd01/pkcs11-spec-v3.1-csd01.html#_Toc98177195 1. `If the specified attribute (i.e., the attribute specified by the type field) for the object cannot be revealed because the object is sensitive or unextractable, then the ulValueLen field in that triple is modified to hold the value CK_UNAVAILABLE_INFORMATION.

`

2. `Otherwise, if the specified value for the object is invalid (the object does not possess such an attribute), then the ulValueLen field in that triple is modified to hold the value CK_UNAVAILABLE_INFORMATION.

`

3. `Otherwise, if the pValue field has the value NULL_PTR, then the ulValueLen field is modified to hold the exact length of the specified attribute for the object.

`

4. `Otherwise, if the length specified in ulValueLen is large enough to hold the value of the specified attribute for the object, then that attribute is copied into the buffer located at pValue, and the ulValueLen field is modified to hold the exact length of the attribute.

`

5. `Otherwise, the ulValueLen field is modified to hold the value CK_UNAVAILABLE_INFORMATION.

`

*If case 1 applies to any of the requested attributes, then the call should return the value CKR_ATTRIBUTE_SENSITIVE. If case 2 applies to any of the requested attributes, then the call should return the value CKR_ATTRIBUTE_TYPE_INVALID. If case 5 applies to any of the requested attributes, then the call should return the value CKR_BUFFER_TOO_SMALL.* As usual, if more than one of these error codes is applicable, Cryptoki may return any of them. Only if none of them applies to any of the requested attributes will CKR_OK be returned. The bolding is mine. My takeaway here is the following: • `CKR_OK` with `CK_UNAVAILABLE_INFORMATION` in `ulValueLen` is not a compliant PKCS#11 response (although I'm sure that there are tokens that get this wrong, as evidenced by the bug reports complaining about this) • `assertRV(retval)` on the result of `C_GetAttributeValue` covers this case for compliant tokens. • Since returning `None` evidently breaks stuff in various places, we should cover this case by raising an exception instead. How about overriding the `CKR_OK` with `CKR_FUNCTION_FAILED` if `ulValueLen` is `CK_UNAVAILABLE_INFORMATION`? WDYT? In pkcs11/_pkcs11.pyx <#207 (comment)>: > @@ -938,7 +938,7 @@ class Object(types.Object): # Find out the attribute size with nogil: retval = _funclist.C_GetAttributeValue(handle, obj, &template, 1) - if retval == CK_UNAVAILABLE_INFORMATION: + if template.ulValueLen == CK_UNAVAILABLE_INFORMATION: How about this instead of returning `None`? (see bigger comment) ` if retval == CKR_OK and template.ulValueLen == CK_UNAVAILABLE_INFORMATION: retval = CKR_FUNCTION_FAILED

`

…

We can also add a comment then that this is specifically to deal with noncompliant tokens. In pkcs11/_pkcs11.pyx <#207 (comment)>: > @@ -952,7 +952,7 @@ class Object(types.Object): # Request the value with nogil: retval = _funclist.C_GetAttributeValue(handle, obj, &template, 1) - if retval == CK_UNAVAILABLE_INFORMATION: + if template.ulValueLen == CK_UNAVAILABLE_INFORMATION: This branch shouldn't be necessary. — Reply to this email directly, view it on GitHub <#207 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADSRCF44FEM2OG5IRFZI733FCJXPAVCNFSM6AAAAAB75527QKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSNJRHAZDONRQGY>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[MatthiasValvekens]

I don't agree with your logic about case 2. The spec doesn't indicate that it should return CKR_ATTRIBUTE_TYPE_INVALID, and in fact it seems that it would be wrong to do so if the attribute type specified is valid, but not available for that key.

It does, though 😉 See this excerpt from the passage I quoted above:

If case 2 applies to any of the requested attributes, then the call should return the value CKR_ATTRIBUTE_TYPE_INVALID.

All the situations enumerated in the spec where CK_UNAVAILABLE_INFORMATION is returned have a CKR_... error code associated with them. The spec doesn't say anything about returning CKR_OK with CK_UNAVAILABLE_INFORMATION, so that's undefined behaviour at best (i.e. up to the implementation). As such, I think CKR_FUNCTION_FAILED is appropriate in this case.

[minego]

I'll have to read through the spec more carefully in a bit when I'm actually at a computer. That wording is clear, so I have to agree with you based on that. That means our module is incorrect here and I'll create a bug on our side and get it fixed. But, in the mean time I think it is still worth it to handle this better here. Since you know this library much better than I do, I'll be happy with whatever you decide as the correct way to handle a module that behaves this way, as long as it doesn't try to allocate that value :-) Thanks!

…

On Tue, Jun 24, 2025, at 8:42 AM, Matthias Valvekens wrote: *MatthiasValvekens* left a comment (pyauth/python-pkcs11#207) <#207 (comment)> > I don't agree with your logic about case 2. The spec doesn't indicate that it should return CKR_ATTRIBUTE_TYPE_INVALID, and in fact it seems that it would be wrong to do so if the attribute type specified is valid, but not available for that key. > It does, though 😉 See this excerpt from the passage I quoted above: > *If case 2 applies to any of the requested attributes, then the call should return the value CKR_ATTRIBUTE_TYPE_INVALID.* > All the situations enumerated in the spec where `CK_UNAVAILABLE_INFORMATION` is returned have a `CKR_...` error code associated with them. The spec doesn't say anything about returning `CKR_OK` with `CK_UNAVAILABLE_INFORMATION`, so that's undefined behaviour at best. As such, I think `CKR_FUNCTION_FAILED` is appropriate in this case. — Reply to this email directly, view it on GitHub <#207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADSRCDDFZXUQWI5HQCX5F33FFWUXAVCNFSM6AAAAAB75527QKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMBRGAYDOMBWGU>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[MatthiasValvekens]

On closer inspection, actually emitting CKR_OK with CK_UNAVAILABLE_INFORMATION isn't even undefined behaviour, it's strictly non-compliant on the token's part. If you follow the algorithm in the spec, CKR_OK is only allowed as the result of step 3 or 4. And in both of those cases, the spec explicitly prescribes what you should put in ulValueLen, namely the actual size of the attribute. So yes, that looks like a bug 🙂 .

I totally agree that the original behaviour in python-pkcs11 was dangerous, let's fix that by all means. Bailing with CKR_FUNCTION_FAILED certainly seems like an appropriate defensive response that is consistent with other error handling in this lib 🙂.

[minego]

I'm happy with that solution. Would you like me to update my PR, or would you like to make the change?

…

On Tue, Jun 24, 2025, at 8:49 AM, Matthias Valvekens wrote: *MatthiasValvekens* left a comment (pyauth/python-pkcs11#207) <#207 (comment)> On closer inspection, actually emitting `CKR_OK` with `CK_UNAVAILABLE_INFORMATION` isn't even undefined behaviour, it's strictly non-compliant on the token's part. If you follow the algorithm in the spec <https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/csd01/pkcs11-spec-v3.1-csd01.html#_Toc98177133>, `CKR_OK` is only allowed as the result of step 3 or 4. And in both of those cases, the spec explicitly prescribes what you should put in `ulValueLen`, namely the actual size of the attribute. So yes, that looks like a bug 🙂 . I totally agree that the original behaviour in python-pkcs11 was dangerous, let's fix that by all means. Bailing with `CKR_FUNCTION_FAILED` certainly seems like an appropriate defensive response that is consistent with other error handling in this lib 🙂. — Reply to this email directly, view it on GitHub <#207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADSRCBBN72BDJJLN62I7OT3FFXRDAVCNFSM6AAAAAB75527QKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMBRGAZDSNJVGY>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[MatthiasValvekens]

Either is fine with me. I should have some time a few hours from now, but if you want to update the PR in the meantime, that also works for me.

[minego]

I'll let you then. I won't have a chance until later today. Thank you!

…

On Tue, Jun 24, 2025, at 8:54 AM, Matthias Valvekens wrote: *MatthiasValvekens* left a comment (pyauth/python-pkcs11#207) <#207 (comment)> Either is fine with me. I should have some time a few hours from now, but if you want to update the PR in the meantime, that also works for me. — Reply to this email directly, view it on GitHub <#207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADSRCB2IUOXGHOFPFPKRMT3FFYE5AVCNFSM6AAAAAB75527QKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMBRGA2DOOJWGI>. You are receiving this because you authored the thread.Message ID: ***@***.***>