Fix possible infinite loop when loading cert chains from Java P11KeyS…

…tore

When HSM contains certificate chains, the JDK P11KeyStore
tries to load the full chain within loadChain() method.

This action is performed in a while(true) loop as:

  while (true) {
    CK_ATTRIBUTE[] attrs = new CK_ATTRIBUTE[] {
      ATTR_TOKEN_TRUE,
      ATTR_CLASS_CERT,
      new CK_ATTRIBUTE(CKA_SUBJECT,
          next.getIssuerX500Principal().getEncoded()) };
    long[] ch = findObjects(session, attrs);
    if (ch == null || ch.length == 0) {
        // done
        break;
    } else {
        // Just take the first
        next = loadCert(session, ch[0]);
        lChain.add(next);
        if (next.getSubjectX500Principal().equals
              (next.getIssuerX500Principal())) {
            // self signed
            break;
        }
    }
  }

Here, supporting filtering certificates by CKA_SUBJECT is crucial
otherwise the while true loop would continue forever (until findObjects
returns some certificates and first one is not self signed)

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>

pkcs11/src/backend/object.rs

@@ -1,4 +1,4 @@
-use cryptoki_sys::{CKA_CLASS, CKA_ID, CKA_LABEL, CK_OBJECT_CLASS, CK_SESSION_HANDLE};
+use cryptoki_sys::{CKA_CLASS, CKA_ID, CKA_LABEL, CKA_SUBJECT, CK_OBJECT_CLASS, CK_SESSION_HANDLE};
 use log::{debug, trace};
 
 use super::{
@@ -22,6 +22,7 @@ pub struct KeyRequirements {
     pub kind: Option<ObjectKind>,
     pub id: Option<String>,
     pub raw_id: Option<Vec<u8>>,
+    pub cka_subject: Option<Vec<u8>>,
 }
 
 fn parse_key_requirements(template: Option<CkRawAttrTemplate>) -> Result<KeyRequirements, Error> {
@@ -30,6 +31,7 @@ fn parse_key_requirements(template: Option<CkRawAttrTemplate>) -> Result<KeyRequ
             let mut key_id = None;
             let mut kind = None;
             let mut raw_id = None;
+            let mut cka_subject = None;
             for attr in template.iter() {
                 debug!("attr {:?}: {:?}", attr.type_(), attr.val_bytes());
 
@@ -59,17 +61,26 @@ fn parse_key_requirements(template: Option<CkRawAttrTemplate>) -> Result<KeyRequ
                 if attr.type_() == CKA_LABEL && key_id.is_none() {
                     key_id = Some(parse_str_from_attr(&attr)?);
                 }
+
+                if attr.type_() == CKA_SUBJECT {
+                    let bytes = attr
+                        .val_bytes()
+                        .ok_or(Error::InvalidAttribute(attr.type_()))?;
+                    cka_subject = Some(bytes.to_vec());
+                }
             }
             Ok(KeyRequirements {
                 kind,
                 id: key_id,
                 raw_id,
+                cka_subject,
             })
         }
         None => Ok(KeyRequirements {
             kind: None,
             id: None,
             raw_id: None,
+            cka_subject: None,
         }),
     }
 }

pkcs11/src/backend/session.rs

@@ -5,7 +5,7 @@ use std::{
 
 use cryptoki_sys::{
     CKR_OK, CK_FLAGS, CK_OBJECT_HANDLE, CK_RV, CK_SESSION_HANDLE, CK_SESSION_INFO, CK_SLOT_ID,
-    CK_USER_TYPE,
+    CK_USER_TYPE, CKA_SUBJECT,
 };
 use log::{debug, error, trace};
 use nethsm_sdk_rs::apis::default_api;
@@ -479,7 +479,20 @@ impl Session {
             .into_iter()
             .filter(|(_, obj)| {
                 if let Some(kind) = requirements.kind {
-                    kind == obj.kind
+                    // kind must match
+                    if kind != obj.kind {
+                        false
+                    // extra checks if kind is Cerificate
+                    } else if kind == ObjectKind::Certificate {
+                        // When Subject is provided as requirement, it must match
+                        requirements.cka_subject.is_none() ||
+                            obj.attr(CKA_SUBJECT)
+                                .map(|attr| attr.as_bytes())
+                                    == requirements.cka_subject.as_deref()
+                    // On other kinds, no need for extra checks
+                    } else {
+                        true
+                    }
                 } else {
                     true
                 }