Corrected logic for handling CK_UNAVAILABLE_INFORMATION to properly check the length returned

Author: Micah N Gorrell

pkcs11/_pkcs11.pyx

@@ -938,8 +938,14 @@ class Object(types.Object):
         # Find out the attribute size
         with nogil:
             retval = _funclist.C_GetAttributeValue(handle, obj, &template, 1)
-        if retval == CK_UNAVAILABLE_INFORMATION:
-            return None
+        if retval == CKR_OK and \
+                template.ulValueLen == CK_UNAVAILABLE_INFORMATION:
+            # The spec prohibits returning CK_UNAVAILABLE_INFORMATION
+            #  together with CKR_OK, but some tokens do that anyway.
+            #  Let's be defensive and map that to a proper error,
+            #  otherwise CK_UNAVAILABLE_INFORMATION will be treated
+            #  as a length value, which causes issues.
+            retval = CKR_FUNCTION_FAILED
         assertRV(retval)
 
         if template.ulValueLen == 0:
@@ -952,8 +958,6 @@ class Object(types.Object):
         # Request the value
         with nogil:
             retval = _funclist.C_GetAttributeValue(handle, obj, &template, 1)
-        if retval == CK_UNAVAILABLE_INFORMATION:
-            return None
         assertRV(retval)
 
         return _unpack_attributes(key, value)

tests/test_sessions.py

@@ -3,6 +3,7 @@
 """
 
 import pkcs11
+from pkcs11 import Attribute, AttributeSensitive, AttributeTypeInvalid
 
 from . import FIXME, TOKEN_PIN, TOKEN_SO_PIN, Not, Only, TestCase, requires
 
@@ -157,3 +158,14 @@ def test_generate_random(self):
             self.assertEqual(len(random), 16)
             # Ensure we didn't get 16 bytes of zeros
             self.assertTrue(all(c != "\0" for c in random))
+
+    def test_attribute_reading_failures(self):
+        with self.token.open(user_pin=TOKEN_PIN) as session:
+            key = session.generate_key(pkcs11.KeyType.AES, 128, label="SAMPLE KEY")
+            def try_read_value():
+                return key[Attribute.VALUE]
+            self.assertRaises(AttributeSensitive, try_read_value)
+
+            def try_read_irrelevant():
+                return key[Attribute.CERTIFICATE_TYPE]
+            self.assertRaises(AttributeTypeInvalid, try_read_irrelevant)