pki: add security key fields to ssh_key_struct and update compare, copying and cleaning functions

Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>

include/libssh/pki.h

@@ -86,6 +86,11 @@ struct ssh_key_struct {
     ssh_string sk_application;
     ssh_buffer cert;
     enum ssh_keytypes_e cert_type;
+
+    /* Security Key specific private data */
+    uint8_t sk_flags;
+    ssh_string sk_key_handle;
+    ssh_string sk_reserved;
 };
 
 struct ssh_signature_struct {
@@ -137,6 +142,11 @@ enum ssh_digest_e ssh_key_hash_from_name(const char *name);
     ((kt) >= SSH_KEYTYPE_ECDSA_P256_CERT01 &&\
      (kt) <= SSH_KEYTYPE_ED25519_CERT01))
 
+#define is_sk_key_type(kt)                                             \
+    ((kt) == SSH_KEYTYPE_SK_ECDSA || (kt) == SSH_KEYTYPE_SK_ED25519 || \
+     (kt) == SSH_KEYTYPE_SK_ECDSA_CERT01 ||                            \
+     (kt) == SSH_KEYTYPE_SK_ED25519_CERT01)
+
 /* SSH Signature Functions */
 ssh_signature ssh_signature_new(void);
 void ssh_signature_free(ssh_signature sign);

include/libssh/pki_priv.h

@@ -61,6 +61,7 @@ enum ssh_digest_e ssh_key_type_to_hash(ssh_session session,
                                        enum ssh_keytypes_e type);
 
 /* SSH Key Functions */
+ssh_key pki_key_dup_common_init(const ssh_key key, int demote);
 ssh_key pki_key_dup(const ssh_key key, int demote);
 int pki_key_generate_rsa(ssh_key key, int parameter);
 int pki_key_generate_ecdsa(ssh_key key, int parameter);

src/pki.c

@@ -116,6 +116,78 @@ ssh_key ssh_key_new (void)
     return ptr;
 }
 
+/**
+ * @internal
+ *
+ * @brief Initialize a new SSH key by duplicating common fields from an existing
+ * key.
+ *
+ * This function creates a new SSH key and copies the common fields from the
+ * source key, including the key type, type string, flags, and security key
+ * fields if applicable. This is a helper function used by key duplication
+ * routines.
+ *
+ * @param[in] key     The source ssh_key to copy common fields from.
+ * @param[in] demote  Whether to demote the new key to public only. If non-zero,
+ *                    only the public fields will be copied and the flags will
+ *                    be set accordingly.
+ *
+ * @return            A new ssh_key with common fields initialized, or NULL on
+ * error.
+ *
+ * @note The caller is responsible for freeing the returned key with
+ * ssh_key_free().
+ */
+ssh_key pki_key_dup_common_init(const ssh_key key, int demote)
+{
+    ssh_key new = NULL;
+
+    if (key == NULL) {
+        return NULL;
+    }
+
+    new = ssh_key_new();
+    if (new == NULL) {
+        return NULL;
+    }
+
+    new->type = key->type;
+    new->type_c = key->type_c;
+    if (demote) {
+        new->flags = SSH_KEY_FLAG_PUBLIC;
+    } else {
+        new->flags = key->flags;
+    }
+
+    /* Copy security key fields if present */
+    if (is_sk_key_type(key->type)) {
+        new->sk_application = ssh_string_copy(key->sk_application);
+        if (new->sk_application == NULL) {
+            goto fail;
+        }
+
+        if (!demote) {
+            new->sk_flags = key->sk_flags;
+
+            new->sk_key_handle = ssh_string_copy(key->sk_key_handle);
+            if (new->sk_key_handle == NULL) {
+                goto fail;
+            }
+
+            new->sk_reserved = ssh_string_copy(key->sk_reserved);
+            if (new->sk_reserved == NULL) {
+                goto fail;
+            }
+        }
+    }
+
+    return new;
+
+fail:
+    SSH_KEY_FREE(new);
+    return NULL;
+}
+
 /**
  * @brief duplicates the key
  *
@@ -153,12 +225,14 @@ void ssh_key_clean (ssh_key key)
     if (key->cert != NULL) {
         SSH_BUFFER_FREE(key->cert);
     }
-    if (key->type == SSH_KEYTYPE_SK_ECDSA ||
+    if (is_sk_key_type(key->type)) {
-        key->type == SSH_KEYTYPE_SK_ED25519 ||
-        key->type == SSH_KEYTYPE_SK_ECDSA_CERT01 ||
-        key->type == SSH_KEYTYPE_SK_ED25519_CERT01) {
         ssh_string_burn(key->sk_application);
         ssh_string_free(key->sk_application);
+        ssh_string_burn(key->sk_key_handle);
+        ssh_string_free(key->sk_key_handle);
+        ssh_string_burn(key->sk_reserved);
+        ssh_string_free(key->sk_reserved);
+        key->sk_flags = 0;
     }
     key->cert_type = SSH_KEYTYPE_UNKNOWN;
     key->flags = SSH_KEY_FLAG_EMPTY;
@@ -706,13 +780,24 @@ int ssh_key_cmp(const ssh_key k1,
         }
     }
 
-    if (k1->type == SSH_KEYTYPE_SK_ECDSA ||
+    if (is_sk_key_type(k1->type)) {
-        k1->type == SSH_KEYTYPE_SK_ED25519) {
+        if (ssh_string_cmp(k1->sk_application, k2->sk_application) != 0) {
-        if (strncmp(ssh_string_get_char(k1->sk_application),
-                ssh_string_get_char(k2->sk_application),
-                ssh_string_len(k2->sk_application)) != 0) {
             return 1;
         }
+
+        if (what == SSH_KEY_CMP_PRIVATE) {
+            if (k1->sk_flags != k2->sk_flags) {
+                return 1;
+            }
+
+            if (ssh_string_cmp(k1->sk_key_handle, k2->sk_key_handle) != 0) {
+                return 1;
+            }
+
+            if (ssh_string_cmp(k1->sk_reserved, k2->sk_reserved) != 0) {
+                return 1;
+            }
+        }
     }
 
     if (what == SSH_KEY_CMP_CERTIFICATE) {
@@ -732,9 +817,10 @@ int ssh_key_cmp(const ssh_key k1,
     }
 
 #ifndef HAVE_LIBCRYPTO
-    if (k1->type == SSH_KEYTYPE_ED25519 ||
+    if (k1->type == SSH_KEYTYPE_ED25519) {
-        k1->type == SSH_KEYTYPE_SK_ED25519) {
         return pki_ed25519_key_cmp(k1, k2, what);
+    } else if (k1->type == SSH_KEYTYPE_SK_ED25519) {
+        return pki_ed25519_key_cmp(k1, k2, SSH_KEY_CMP_PUBLIC);
     }
 #endif
 
@@ -2635,11 +2721,7 @@ int ssh_pki_signature_verify(ssh_session session,
         return SSH_ERROR;
     }
 
-    if (key->type == SSH_KEYTYPE_SK_ECDSA ||
+    if (is_sk_key_type(key->type)) {
-        key->type == SSH_KEYTYPE_SK_ECDSA_CERT01 ||
-        key->type == SSH_KEYTYPE_SK_ED25519 ||
-        key->type == SSH_KEYTYPE_SK_ED25519_CERT01) {
-
         ssh_buffer sk_buffer = NULL;
         SHA256CTX ctx = NULL;
         unsigned char application_hash[SHA256_DIGEST_LEN] = {0};

src/pki_crypto.c

@@ -485,19 +485,11 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
     ssh_key new = NULL;
     int rc;
 
-    new = ssh_key_new();
+    new = pki_key_dup_common_init(key, demote);
     if (new == NULL) {
         return NULL;
     }
 
-    new->type = key->type;
-    new->type_c = key->type_c;
-    if (demote) {
-        new->flags = SSH_KEY_FLAG_PUBLIC;
-    } else {
-        new->flags = key->flags;
-    }
-
     switch (key->type) {
     case SSH_KEYTYPE_RSA:
     case SSH_KEYTYPE_RSA1: {
@@ -646,6 +638,7 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
     case SSH_KEYTYPE_ECDSA_P256:
     case SSH_KEYTYPE_ECDSA_P384:
     case SSH_KEYTYPE_ECDSA_P521:
+    case SSH_KEYTYPE_SK_ECDSA:
 #ifdef HAVE_OPENSSL_ECC
         new->ecdsa_nid = key->ecdsa_nid;
 #ifdef WITH_PKCS11_URI
@@ -715,7 +708,8 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
 #endif /* OPENSSL_VERSION_NUMBER */
         break;
 #endif /* HAVE_OPENSSL_ECC */
-    case SSH_KEYTYPE_ED25519: {
+    case SSH_KEYTYPE_ED25519:
+    case SSH_KEYTYPE_SK_ED25519: {
 #if OPENSSL_VERSION_NUMBER < 0x30000000L
         /* Take the PKCS#11 keys as they are */
         if (key->flags & SSH_KEY_FLAG_PKCS11_URI && !demote) {
@@ -727,7 +721,8 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
             return new;
         }
 
-        if (!demote && (key->flags & SSH_KEY_FLAG_PRIVATE)) {
+        if (!demote && (key->flags & SSH_KEY_FLAG_PRIVATE) &&
+            key->type == SSH_KEYTYPE_ED25519) {
             unsigned char *ed25519_privkey = NULL;
             size_t key_len = 0;
 
@@ -1019,7 +1014,7 @@ int pki_key_compare(const ssh_key k1, const ssh_key k2, enum ssh_keycmp_e what)
             return 1;
         }
 
-        if (what == SSH_KEY_CMP_PRIVATE) {
+        if (what == SSH_KEY_CMP_PRIVATE && !is_sk_key_type(k1->type)) {
             if (bignum_cmp(EC_KEY_get0_private_key(ec1),
                            EC_KEY_get0_private_key(ec2))) {
                 return 1;

src/pki_gcrypt.c

@@ -1155,17 +1155,10 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
 
     gcry_sexp_t curve = NULL;
 
-    new = ssh_key_new();
+    new = pki_key_dup_common_init(key, demote);
     if (new == NULL) {
         return NULL;
     }
-    new->type = key->type;
-    new->type_c = key->type_c;
-    if (demote) {
-        new->flags = SSH_KEY_FLAG_PUBLIC;
-    } else {
-        new->flags = key->flags;
-    }
 
     switch (key->type) {
     case SSH_KEYTYPE_RSA:
@@ -1203,6 +1196,7 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
         }
         break;
     case SSH_KEYTYPE_ED25519:
+    case SSH_KEYTYPE_SK_ED25519:
         rc = pki_ed25519_key_dup(new, key);
         if (rc != SSH_OK) {
             ssh_key_free(new);
@@ -1213,6 +1207,7 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
     case SSH_KEYTYPE_ECDSA_P256:
     case SSH_KEYTYPE_ECDSA_P384:
     case SSH_KEYTYPE_ECDSA_P521:
+    case SSH_KEYTYPE_SK_ECDSA:
 #ifdef HAVE_GCRYPT_ECC
         new->ecdsa_nid = key->ecdsa_nid;
 
@@ -1226,7 +1221,8 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
             break;
         }
 
-        if (!demote && (key->flags & SSH_KEY_FLAG_PRIVATE)) {
+        if (!demote && (key->flags & SSH_KEY_FLAG_PRIVATE) &&
+            !is_sk_key_type(key->type)) {
             err = gcry_sexp_build(&new->ecdsa,
                                   NULL,
                                   "(private-key(ecdsa %S (d %m)(q %m)))",
@@ -1427,7 +1423,7 @@ int pki_key_compare(const ssh_key k1, const ssh_key k2, enum ssh_keycmp_e what)
             return 1;
         }
 
-        if (what == SSH_KEY_CMP_PRIVATE) {
+        if (what == SSH_KEY_CMP_PRIVATE && !is_sk_key_type(k1->type)) {
             if (_bignum_cmp(k1->ecdsa, k2->ecdsa, "d") != 0) {
                 return 1;
             }

src/pki_mbedcrypto.c

@@ -380,19 +380,11 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
     mbedtls_mpi Q;
 #endif
 
-    new = ssh_key_new();
+    new = pki_key_dup_common_init(key, demote);
     if (new == NULL) {
         return NULL;
     }
 
-    new->type = key->type;
-    new->type_c = key->type_c;
-    if (demote) {
-        new->flags = SSH_KEY_FLAG_PUBLIC;
-    } else {
-        new->flags = key->flags;
-    }
-
 #if MBEDTLS_VERSION_MAJOR > 2
     mbedtls_mpi_init(&N);
     mbedtls_mpi_init(&E);
@@ -512,6 +504,7 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
         case SSH_KEYTYPE_ECDSA_P256:
         case SSH_KEYTYPE_ECDSA_P384:
         case SSH_KEYTYPE_ECDSA_P521:
+        case SSH_KEYTYPE_SK_ECDSA:
             new->ecdsa_nid = key->ecdsa_nid;
 
             new->ecdsa = malloc(sizeof(mbedtls_ecdsa_context));
@@ -522,7 +515,8 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
 
             mbedtls_ecdsa_init(new->ecdsa);
 
-            if (demote && ssh_key_is_private(key)) {
+            if ((demote && ssh_key_is_private(key)) ||
+                is_sk_key_type(key->type)) {
                 rc = mbedtls_ecp_copy(&new->ecdsa->MBEDTLS_PRIVATE(Q),
                                 &key->ecdsa->MBEDTLS_PRIVATE(Q));
                 if (rc != 0) {
@@ -540,6 +534,7 @@ ssh_key pki_key_dup(const ssh_key key, int demote)
 
             break;
         case SSH_KEYTYPE_ED25519:
+        case SSH_KEYTYPE_SK_ED25519:
             rc = pki_ed25519_key_dup(new, key);
             if (rc != SSH_OK) {
                 goto fail;
@@ -768,7 +763,8 @@ int pki_key_compare(const ssh_key k1, const ssh_key k2, enum ssh_keycmp_e what)
                 goto cleanup;
             }
 
-            if (what == SSH_KEY_CMP_PRIVATE) {
+            if (what == SSH_KEY_CMP_PRIVATE &&
+                k1->type != SSH_KEYTYPE_SK_ECDSA) {
                 if (mbedtls_mpi_cmp_mpi(&ecdsa1->MBEDTLS_PRIVATE(d),
                     &ecdsa2->MBEDTLS_PRIVATE(d)))
                 {