From 5bbaecfaa7bf6fd6ed96288a17d0816afbf7900f Mon Sep 17 00:00:00 2001 From: Praneeth Sarode Date: Thu, 30 Oct 2025 22:37:22 +0530 Subject: [PATCH] feat(pki): extend the sshsig API to support security keys along with tests Signed-off-by: Praneeth Sarode Reviewed-by: Jakub Jelen Reviewed-by: Eshan Kelkar --- include/libssh/libssh.h | 1 + src/pki.c | 91 ++++++++-- tests/unittests/torture_pki_sshsig.c | 237 ++++++++++++++++++++++++++- 3 files changed, 313 insertions(+), 16 deletions(-) diff --git a/include/libssh/libssh.h b/include/libssh/libssh.h index 6546f584..6dff4d0e 100644 --- a/include/libssh/libssh.h +++ b/include/libssh/libssh.h @@ -912,6 +912,7 @@ enum sshsig_digest_e { LIBSSH_API int sshsig_sign(const void *data, size_t data_length, ssh_key privkey, + ssh_pki_ctx pki_context, const char *sig_namespace, enum sshsig_digest_e hash_alg, char **signature); diff --git a/src/pki.c b/src/pki.c index 6f76f815..17f13377 100644 --- a/src/pki.c +++ b/src/pki.c @@ -3415,9 +3415,15 @@ cleanup: * @param data The data to sign * @param data_length The length of the data * @param privkey The private key to sign with + * @param pki_context The PKI context. For non-SK keys, this parameter is + * ignored and can be NULL. For SK keys, can be NULL in + * which case a default context with default callbacks + * will be used. If provided, the context must have + * sk_callbacks set with a valid sign callback + * implementation. See ssh_pki_ctx_set_sk_callbacks(). + * @param sig_namespace The signature namespace (e.g. "file", "email", etc.) * @param hash_alg The hash algorithm to use (SSHSIG_DIGEST_SHA2_256 or * SSHSIG_DIGEST_SHA2_512) - * @param sig_namespace The signature namespace (e.g. "file", "email", etc.) * @param signature Pointer to store the allocated signature string in the * armored format. Must be freed with * ssh_string_free_char() @@ -3427,6 +3433,7 @@ cleanup: int sshsig_sign(const void *data, size_t data_length, ssh_key privkey, + ssh_pki_ctx pki_context, const char *sig_namespace, enum sshsig_digest_e hash_alg, char **signature) @@ -3436,6 +3443,8 @@ int sshsig_sign(const void *data, ssh_signature sig = NULL; ssh_string sig_string = NULL; ssh_string pub_blob = NULL; + ssh_pki_ctx temp_ctx = NULL; + ssh_pki_ctx ctx_to_use = NULL; enum ssh_digest_e digest_type; const char *hash_alg_str = NULL; int rc = SSH_ERROR; @@ -3453,6 +3462,31 @@ int sshsig_sign(const void *data, return SSH_ERROR; } + /* Check if this is an SK key that requires a PKI context */ + if (is_sk_key_type(privkey->type)) { + /* If no context provided, create a temporary default one */ + if (pki_context == NULL) { + SSH_LOG(SSH_LOG_INFO, + "No PKI context provided, using the default one"); + + temp_ctx = ssh_pki_ctx_new(); + if (temp_ctx == NULL) { + SSH_LOG(SSH_LOG_WARN, "Failed to create temporary PKI context"); + return SSH_ERROR; + } + ctx_to_use = temp_ctx; + } else { + ctx_to_use = pki_context; + } + + /* Verify that we have valid SK callbacks */ + if (ctx_to_use->sk_callbacks == NULL) { + SSH_LOG(SSH_LOG_WARN, + "Security Key callbacks not configured in PKI context"); + goto cleanup; + } + } + *signature = NULL; if (hash_alg == SSHSIG_DIGEST_SHA2_256) { @@ -3474,11 +3508,24 @@ int sshsig_sign(const void *data, goto cleanup; } - digest_type = key_type_to_hash(ssh_key_type_plain(privkey->type)); - sig = pki_sign_data(privkey, - digest_type, - ssh_buffer_get(tosign), - ssh_buffer_get_len(tosign)); + /* Use appropriate signing method based on key type */ + if (is_sk_key_type(privkey->type)) { +#ifdef WITH_FIDO2 + sig = pki_sk_do_sign(ctx_to_use, + privkey, + ssh_buffer_get(tosign), + ssh_buffer_get_len(tosign)); +#else + SSH_LOG(SSH_LOG_WARN, SK_NOT_SUPPORTED_MSG); + goto cleanup; +#endif + } else { + digest_type = key_type_to_hash(ssh_key_type_plain(privkey->type)); + sig = pki_sign_data(privkey, + digest_type, + ssh_buffer_get(tosign), + ssh_buffer_get_len(tosign)); + } if (sig == NULL) { SSH_LOG(SSH_LOG_TRACE, "Failed to sign data with private key"); goto cleanup; @@ -3530,6 +3577,11 @@ cleanup: SSH_STRING_FREE(sig_string); SSH_STRING_FREE(pub_blob); + /* Clean up temporary context if we created one */ + if (temp_ctx != NULL) { + SSH_PKI_CTX_FREE(temp_ctx); + } + return rc; } @@ -3665,10 +3717,29 @@ int sshsig_verify(const void *data, goto cleanup; } - rc = pki_verify_data_signature(signature_obj, - key, - ssh_buffer_get(tosign), - ssh_buffer_get_len(tosign)); + if (is_sk_key_type(key->type)) { + ssh_buffer sk_buffer = NULL; + rc = pki_sk_signature_buffer_prepare(key, + signature_obj, + ssh_buffer_get(tosign), + ssh_buffer_get_len(tosign), + &sk_buffer); + if (rc != SSH_OK) { + SSH_LOG(SSH_LOG_TRACE, "Failed to prepare sk signature buffer"); + goto cleanup; + } + + rc = pki_verify_data_signature(signature_obj, + key, + ssh_buffer_get(sk_buffer), + ssh_buffer_get_len(sk_buffer)); + SSH_BUFFER_FREE(sk_buffer); + } else { + rc = pki_verify_data_signature(signature_obj, + key, + ssh_buffer_get(tosign), + ssh_buffer_get_len(tosign)); + } if (rc != SSH_OK) { SSH_LOG(SSH_LOG_TRACE, "Signature verification failed"); goto cleanup; diff --git a/tests/unittests/torture_pki_sshsig.c b/tests/unittests/torture_pki_sshsig.c index 465eb347..85b07a22 100644 --- a/tests/unittests/torture_pki_sshsig.c +++ b/tests/unittests/torture_pki_sshsig.c @@ -8,10 +8,25 @@ #include "torture_key.h" #include "torture_pki.h" +#ifdef WITH_FIDO2 +#include "libssh/libssh.h" +#include "torture_sk.h" +#endif + #include #include #include +/** + * The tests for the sk-type keys can also be configured to run with + * the sk-usbhid callbacks instead of the default sk-dummy callbacks which can + * run in a CI environment. + * + * To run these tests with the sk-usbhid callbacks, at least one FIDO2 device + * must be connected and the environment variables TORTURE_SK_USBHID must be + * set. + */ + static const char template[] = "tmp_XXXXXX"; static const char input[] = "Test input\0string with null byte"; static const size_t input_len = sizeof(input) - 1; /* -1 to exclude final \0 */ @@ -38,6 +53,13 @@ struct sshsig_st { ssh_key rsa_key; ssh_key ed25519_key; ssh_key ecdsa_key; + +#ifdef WITH_FIDO2 + ssh_pki_ctx pki_ctx; + ssh_key sk_ecdsa_key; + ssh_key sk_ed25519_key; +#endif + const char *ssh_keygen_path; const struct key_hash_combo *test_combinations; size_t num_combinations; @@ -51,6 +73,15 @@ static struct key_hash_combo test_combinations[] = { #ifdef HAVE_ECC {SSH_KEYTYPE_ECDSA_P256, SSHSIG_DIGEST_SHA2_256, "ecdsa"}, {SSH_KEYTYPE_ECDSA_P256, SSHSIG_DIGEST_SHA2_512, "ecdsa"}, +# ifdef WITH_FIDO2 + {SSH_KEYTYPE_SK_ECDSA, SSHSIG_DIGEST_SHA2_256, "sk_ecdsa"}, + {SSH_KEYTYPE_SK_ECDSA, SSHSIG_DIGEST_SHA2_512, "sk_ecdsa"}, +# endif /* WITH_FIDO2 */ +#endif /* HAVE_ECC */ + +#ifdef WITH_FIDO2 + {SSH_KEYTYPE_SK_ED25519, SSHSIG_DIGEST_SHA2_256, "sk_ed25519"}, + {SSH_KEYTYPE_SK_ED25519, SSHSIG_DIGEST_SHA2_512, "sk_ed25519"}, #endif }; @@ -69,6 +100,19 @@ static ssh_key get_test_key(struct sshsig_st *test_state, #ifdef HAVE_ECC case SSH_KEYTYPE_ECDSA_P256: return test_state->ecdsa_key; +# ifdef WITH_FIDO2 + case SSH_KEYTYPE_SK_ECDSA: + return test_state->sk_ecdsa_key; +# endif /* WITH_FIDO2 */ +#endif /* HAVE_ECC */ + +#ifdef WITH_FIDO2 + case SSH_KEYTYPE_SK_ED25519: + if (ssh_fips_mode()) { + return NULL; + } else { + return test_state->sk_ed25519_key; + } #endif default: return NULL; @@ -82,6 +126,10 @@ static int setup_sshsig_compat(void **state) char *temp_dir = NULL; int rc = 0; +#ifdef WITH_FIDO2 + const struct ssh_sk_callbacks_struct *sk_callbacks = NULL; +#endif + test_state = calloc(1, sizeof(struct sshsig_st)); assert_non_null(test_state); @@ -142,6 +190,35 @@ static int setup_sshsig_compat(void **state) assert_int_equal(rc, SSH_OK); #endif +#ifdef WITH_FIDO2 + /* Create and configure PKI context for SK operations */ + sk_callbacks = torture_get_sk_callbacks(); + if (sk_callbacks != NULL) { + test_state->pki_ctx = ssh_pki_ctx_new(); + assert_non_null(test_state->pki_ctx); + + rc = ssh_pki_ctx_options_set(test_state->pki_ctx, + SSH_PKI_OPTION_SK_CALLBACKS, + sk_callbacks); + assert_int_equal(rc, SSH_OK); + +# ifdef HAVE_ECC + rc = ssh_pki_generate_key(SSH_KEYTYPE_SK_ECDSA, + test_state->pki_ctx, + &test_state->sk_ecdsa_key); + assert_int_equal(rc, SSH_OK); +# endif /* HAVE_ECC */ + + if (!ssh_fips_mode()) { + rc = ssh_pki_generate_key(SSH_KEYTYPE_SK_ED25519, + test_state->pki_ctx, + &test_state->sk_ed25519_key); + assert_int_equal(rc, SSH_OK); + } + } + +#endif /* WITH_FIDO2 */ + /* Write keys to files for openssh compatibility testing */ if (test_state->ssh_keygen_path != NULL) { torture_write_file("test_rsa", torture_get_testkey(SSH_KEYTYPE_RSA, 0)); @@ -161,7 +238,56 @@ static int setup_sshsig_compat(void **state) torture_get_testkey(SSH_KEYTYPE_ECDSA_P256, 0)); torture_write_file("test_ecdsa.pub", torture_get_testkey_pub(SSH_KEYTYPE_ECDSA_P256)); -#endif +#endif /* HAVE_ECC */ + +#ifdef WITH_FIDO2 +# ifdef HAVE_ECC + /* Write SK keys to files if they were successfully generated */ + if (test_state->sk_ecdsa_key != NULL) { + char *sk_ecdsa_priv = NULL; + char *sk_ecdsa_pub = NULL; + + rc = ssh_pki_export_privkey_base64(test_state->sk_ecdsa_key, + NULL, + NULL, + NULL, + &sk_ecdsa_priv); + assert_int_equal(rc, SSH_OK); + + rc = ssh_pki_export_pubkey_base64(test_state->sk_ecdsa_key, + &sk_ecdsa_pub); + assert_int_equal(rc, SSH_OK); + + torture_write_file("test_sk_ecdsa", sk_ecdsa_priv); + torture_write_file("test_sk_ecdsa.pub", sk_ecdsa_pub); + + SAFE_FREE(sk_ecdsa_priv); + SAFE_FREE(sk_ecdsa_pub); + } +# endif /* HAVE_ECC */ + + if (!ssh_fips_mode() && test_state->sk_ed25519_key != NULL) { + char *sk_ed25519_priv = NULL; + char *sk_ed25519_pub = NULL; + + rc = ssh_pki_export_privkey_base64(test_state->sk_ed25519_key, + NULL, + NULL, + NULL, + &sk_ed25519_priv); + assert_int_equal(rc, SSH_OK); + + rc = ssh_pki_export_pubkey_base64(test_state->sk_ed25519_key, + &sk_ed25519_pub); + assert_int_equal(rc, SSH_OK); + + torture_write_file("test_sk_ed25519", sk_ed25519_priv); + torture_write_file("test_sk_ed25519.pub", sk_ed25519_pub); + + SAFE_FREE(sk_ed25519_priv); + SAFE_FREE(sk_ed25519_pub); + } +#endif /* WITH_FIDO2 */ rc = chmod("test_rsa", 0600); assert_return_code(rc, errno); @@ -173,6 +299,20 @@ static int setup_sshsig_compat(void **state) rc = chmod("test_ecdsa", 0600); assert_return_code(rc, errno); #endif + +#ifdef WITH_FIDO2 + /* Set permissions for SK key files */ +# ifdef HAVE_ECC + if (test_state->sk_ecdsa_key != NULL) { + rc = chmod("test_sk_ecdsa", 0600); + assert_return_code(rc, errno); + } +# endif /* HAVE_ECC */ + if (!ssh_fips_mode() && test_state->sk_ed25519_key != NULL) { + rc = chmod("test_sk_ed25519", 0600); + assert_return_code(rc, errno); + } +#endif /* WITH_FIDO2 */ } return 0; @@ -189,6 +329,12 @@ static int teardown_sshsig_compat(void **state) ssh_key_free(test_state->ed25519_key); ssh_key_free(test_state->ecdsa_key); +#ifdef WITH_FIDO2 + SSH_PKI_CTX_FREE(test_state->pki_ctx); + ssh_key_free(test_state->sk_ecdsa_key); + ssh_key_free(test_state->sk_ed25519_key); +#endif + rc = torture_change_dir(test_state->original_cwd); assert_int_equal(rc, 0); @@ -204,7 +350,25 @@ static int teardown_sshsig_compat(void **state) static int run_openssh_command(const char *cmd) { - int rc = system(cmd); + char full_cmd[2048]; + int rc; + +#if defined(WITH_FIDO2) && defined(SK_DUMMY_LIBRARY_PATH) + /* Set SSH_SK_PROVIDER to sk-dummy library when using sk-dummy callbacks */ + if (torture_sk_is_using_sk_dummy()) { + snprintf(full_cmd, + sizeof(full_cmd), + "SSH_SK_PROVIDER=\"%s\" %s", + SK_DUMMY_LIBRARY_PATH, + cmd); + } else { + snprintf(full_cmd, sizeof(full_cmd), "%s", cmd); + } +#else + snprintf(full_cmd, sizeof(full_cmd), "%s", cmd); +#endif + + rc = system(full_cmd); return WIFEXITED(rc) ? WEXITSTATUS(rc) : -1; } @@ -287,18 +451,35 @@ static void test_libssh_sign_verify_combo(struct sshsig_st *test_state, char *signature = NULL; ssh_key verify_key = NULL; ssh_key test_key = NULL; + ssh_pki_ctx pki_context = NULL; int rc; - if (combo->key_type == SSH_KEYTYPE_ED25519 && ssh_fips_mode()) { + if ((combo->key_type == SSH_KEYTYPE_ED25519 || + combo->key_type == SSH_KEYTYPE_SK_ED25519) && + ssh_fips_mode()) { skip(); } test_key = get_test_key(test_state, combo->key_type); + if (is_sk_key_type(combo->key_type) && test_key == NULL) { + /* Skip if SK key type is requested but SK callbacks are not available + */ + skip(); + } + assert_non_null(test_key); +#ifdef WITH_FIDO2 + /* Use PKI context for SK keys */ + if (is_sk_key_type(combo->key_type)) { + pki_context = test_state->pki_ctx; + } +#endif + rc = sshsig_sign(input, input_len, test_key, + pki_context, test_namespace, combo->hash_alg, &signature); @@ -324,10 +505,20 @@ test_openssh_sign_libssh_verify_combo(struct sshsig_st *test_state, char cmd[1024]; char *openssh_sig = NULL; ssh_key verify_key = NULL; + ssh_key test_key = NULL; FILE *fp = NULL; int rc; - if (combo->key_type == SSH_KEYTYPE_ED25519 && ssh_fips_mode()) { + if ((combo->key_type == SSH_KEYTYPE_ED25519 || + combo->key_type == SSH_KEYTYPE_SK_ED25519) && + ssh_fips_mode()) { + skip(); + } + + test_key = get_test_key(test_state, combo->key_type); + if (is_sk_key_type(combo->key_type) && test_key == NULL) { + /* Skip if SK key type is requested but SK callbacks are not available + */ skip(); } @@ -377,15 +568,30 @@ test_libssh_sign_openssh_verify_combo(struct sshsig_st *test_state, int rc; char *pubkey_b64 = NULL; ssh_key test_key = NULL; + ssh_pki_ctx pki_context = NULL; - if (combo->key_type == SSH_KEYTYPE_ED25519 && ssh_fips_mode()) { + if ((combo->key_type == SSH_KEYTYPE_ED25519 || + combo->key_type == SSH_KEYTYPE_SK_ED25519) && + ssh_fips_mode()) { skip(); } printf("Testing key type: %s\n", combo->key_name); test_key = get_test_key(test_state, combo->key_type); + if (is_sk_key_type(combo->key_type) && test_key == NULL) { + /* Skip if SK key type is requested but SK callbacks are not available + */ + skip(); + } assert_non_null(test_key); +#ifdef WITH_FIDO2 + /* Use PKI context for SK keys */ + if (is_sk_key_type(combo->key_type)) { + pki_context = test_state->pki_ctx; + } +#endif + fp = fopen("test_message.txt", "wb"); assert_non_null(fp); /* Write binary data including null byte */ @@ -397,6 +603,7 @@ test_libssh_sign_openssh_verify_combo(struct sshsig_st *test_state, rc = sshsig_sign(input, input_len, test_key, + pki_context, test_namespace, combo->hash_alg, &libssh_sig); @@ -494,17 +701,32 @@ static void torture_sshsig_error_cases_all_combinations(void **state) for (i = 0; i < test_state->num_combinations; i++) { const struct key_hash_combo *combo = &test_state->test_combinations[i]; ssh_key test_key = NULL; + ssh_pki_ctx pki_context = NULL; - if (combo->key_type == SSH_KEYTYPE_ED25519 && ssh_fips_mode()) { + if ((combo->key_type == SSH_KEYTYPE_ED25519 || + combo->key_type == SSH_KEYTYPE_SK_ED25519) && + ssh_fips_mode()) { continue; } test_key = get_test_key(test_state, combo->key_type); + if (is_sk_key_type(combo->key_type) && test_key == NULL) { + /* Skip if SK key type is requested but SK callbacks are not + * available */ + continue; + } assert_non_null(test_key); +#ifdef WITH_FIDO2 + if (is_sk_key_type(combo->key_type)) { + pki_context = test_state->pki_ctx; + } +#endif + rc = sshsig_sign(input, input_len, test_key, + pki_context, "", /* Test empty string namespace */ combo->hash_alg, &signature); @@ -514,6 +736,7 @@ static void torture_sshsig_error_cases_all_combinations(void **state) rc = sshsig_sign(input, input_len, test_key, + pki_context, test_namespace, combo->hash_alg, &signature); @@ -552,6 +775,7 @@ static void torture_sshsig_error_cases_all_combinations(void **state) rc = sshsig_sign(input, input_len, test_state->rsa_key, + NULL, /* pki_context */ test_namespace, 2, &signature); @@ -561,6 +785,7 @@ static void torture_sshsig_error_cases_all_combinations(void **state) rc = sshsig_sign(input, input_len, NULL, + NULL, /* pki_context */ test_namespace, SSHSIG_DIGEST_SHA2_256, &signature);