From 32691373a76f0cba47bedc8f0daca10adeb77e9b Mon Sep 17 00:00:00 2001
From: Alberto Panizzo <alberto@amarulasolutions.com>
Date: Wed, 4 Sep 2024 20:22:44 +0200
Subject: [PATCH] Fix possible infinite loop when loading cert chains from Java
 P11KeyStore

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  | 13 ++++++++++++-
 pkcs11/src/backend/session.rs | 17 +++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/pkcs11/src/backend/object.rs b/pkcs11/src/backend/object.rs
index ec5f0b13..6c83fda5 100644
--- a/pkcs11/src/backend/object.rs
+++ b/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,
         }),
     }
 }
diff --git a/pkcs11/src/backend/session.rs b/pkcs11/src/backend/session.rs
index 69330516..104f3e63 100644
--- a/pkcs11/src/backend/session.rs
+++ b/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
                 }