Simplify and unify checking EC point

Signed-off-by: Simo Sorce <[email protected]>
latchset · Nov 27, 2024 · 7f93a14 · 7f93a14
1 parent c7d7935
commit 7f93a14
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 45 deletions.
diff --git a/src/ec/ecdsa.rs b/src/ec/ecdsa.rs
@@ -42,7 +42,7 @@ impl ECCPubFactory {
 
 impl ObjectFactory for ECCPubFactory {
     fn create(&self, template: &[CK_ATTRIBUTE]) -> Result<Object> {
-        let obj = self.default_object_create(template)?;
+        let mut obj = self.default_object_create(template)?;
 
         /* According to PKCS#11 v3.1 6.3.3:
          * CKA_EC_PARAMS, Byte array,
@@ -64,18 +64,13 @@ impl ObjectFactory for ECCPubFactory {
         /* According to PKCS#11 v3.1 6.3.3:
          * CKA_EC_POINT, Byte array,
          * DER-encoding of ANSI X9.62 ECPoint value Q */
-        let point = get_ec_point_from_obj(&obj).map_err(|e| {
+        check_ec_point_from_obj(&oid, &mut obj).map_err(|e| {
             if e.attr_not_found() {
                 Error::ck_rv_from_error(CKR_TEMPLATE_INCOMPLETE, e)
-            } else if e.rv() != CKR_ATTRIBUTE_VALUE_INVALID {
-                Error::ck_rv_from_error(CKR_ATTRIBUTE_VALUE_INVALID, e)
             } else {
-                general_error(e)
+                e
             }
         })?;
-        if point.len() != ec_point_size(&oid)? {
-            return Err(CKR_ATTRIBUTE_VALUE_INVALID)?;
-        }
 
         Ok(obj)
     }

diff --git a/src/ec/eddsa.rs b/src/ec/eddsa.rs
@@ -68,24 +68,13 @@ impl ObjectFactory for EDDSAPubFactory {
         /* According to PKCS#11 v3.1 6.3.5:
          * CKA_EC_POINT, Byte array,
          * Public key bytes in little endian order as defined in RFC 8032 */
-        let point = get_ec_point_from_obj(&obj).map_err(|e| {
+        check_ec_point_from_obj(&oid, &mut obj).map_err(|e| {
             if e.attr_not_found() {
                 Error::ck_rv_from_error(CKR_TEMPLATE_INCOMPLETE, e)
-            } else if e.rv() != CKR_ATTRIBUTE_VALUE_INVALID {
-                Error::ck_rv_from_error(CKR_ATTRIBUTE_VALUE_INVALID, e)
             } else {
-                general_error(e)
+                e
             }
         })?;
-        let rawsize = ec_point_size(&oid)?;
-        if point.len() != rawsize {
-            /* For compatibility with applications that use DER encoding */
-            if point.len() == rawsize + 2 {
-                convert_ec_point_from_30(&mut obj)?;
-            } else {
-                return Err(CKR_ATTRIBUTE_VALUE_INVALID)?;
-            }
-        }
 
         Ok(obj)
     }

diff --git a/src/ec/mod.rs b/src/ec/mod.rs
@@ -134,17 +134,6 @@ pub fn get_ec_point_from_obj(key: &Object) -> Result<Vec<u8>> {
     Ok(octet.to_vec())
 }
 
-pub fn convert_ec_point_from_30(key: &mut Object) -> Result<()> {
-    match key.get_attr_as_ulong(CKA_KEY_TYPE).map_err(general_error)? {
-        CKK_EC_EDWARDS | CKK_EC_MONTGOMERY => (),
-        _ => return Err(CKR_GENERAL_ERROR)?,
-    }
-    let point = key.get_attr_as_bytes(CKA_EC_POINT)?;
-    let decoded = asn1::parse_single::<&[u8]>(point).map_err(device_error)?;
-    key.set_attr(Attribute::from_bytes(CKA_EC_POINT, decoded.to_vec()))
-        .map_err(general_error)
-}
-
 #[cfg(test)]
 pub fn curvename_to_key_size(name: &str) -> Result<usize> {
     ec_key_size(&curvename_to_oid(name)?)
@@ -184,3 +173,40 @@ pub fn point_buf_to_der(buf: &[u8], bufsize: usize) -> Result<Option<Vec<u8>>> {
         _ => Ok(None),
     }
 }
+
+#[cfg(feature = "ecc")]
+pub fn check_ec_point_from_obj(
+    oid: &asn1::ObjectIdentifier,
+    key: &mut Object,
+) -> Result<()> {
+    let point = key.get_attr_as_bytes(CKA_EC_POINT)?;
+    let size = ec_point_size(&oid)?;
+
+    let octet: &[u8];
+    let compat: bool;
+    match oid {
+        &EC_SECP256R1 | &EC_SECP384R1 | &EC_SECP521R1 => {
+            octet = asn1::parse_single::<&[u8]>(point).map_err(device_error)?;
+            compat = false;
+        }
+        &ED25519_OID | &ED448_OID | &X25519_OID | &X448_OID => {
+            octet = point.as_slice();
+            compat = true;
+        }
+        _ => return Err(CKR_GENERAL_ERROR)?,
+    }
+
+    if octet.len() == size {
+        return Ok(());
+    }
+
+    if compat && octet.len() == size + 2 {
+        /* Compatibility with applications that use DER encoding */
+        let raw = asn1::parse_single::<&[u8]>(octet).map_err(device_error)?;
+        key.set_attr(Attribute::from_bytes(CKA_EC_POINT, raw.to_vec()))
+            .map_err(general_error)?;
+        return Ok(());
+    }
+
+    Err(CKR_ATTRIBUTE_VALUE_INVALID)?
+}
diff --git a/src/ec/montgomery.rs b/src/ec/montgomery.rs
@@ -69,24 +69,13 @@ impl ObjectFactory for ECMontgomeryPubFactory {
         /* According to PKCS#11 v3.1 6.3.7:
          * CKA_EC_POINT, Byte array,
          * Public key bytes in little endian order as defined in RFC 7748 */
-        let point = get_ec_point_from_obj(&obj).map_err(|e| {
+        check_ec_point_from_obj(&oid, &mut obj).map_err(|e| {
             if e.attr_not_found() {
                 Error::ck_rv_from_error(CKR_TEMPLATE_INCOMPLETE, e)
-            } else if e.rv() != CKR_ATTRIBUTE_VALUE_INVALID {
-                Error::ck_rv_from_error(CKR_ATTRIBUTE_VALUE_INVALID, e)
             } else {
-                general_error(e)
+                e
             }
         })?;
-        let rawsize = ec_point_size(&oid)?;
-        if point.len() != rawsize {
-            /* For compatibility with applications that use DER encoding */
-            if point.len() == rawsize + 2 {
-                convert_ec_point_from_30(&mut obj)?;
-            } else {
-                return Err(CKR_ATTRIBUTE_VALUE_INVALID)?;
-            }
-        }
 
         Ok(obj)
     }