From 79e721f200a856ff9f2981fe4eeef4bddb0cee6a Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Fri, 17 Jul 2026 10:34:30 +0200
Subject: [PATCH 01/16] Support PKCS#11 certificates in Windows ssh-agent
Bring the Windows agent implementation in line with upstream support for
associated-certs-v00@openssh.com.
Match supplied certificates to PKCS#11-backed RSA and ECDSA keys,
persist and reload certificate identities, and support certificate-only
loading. Enable the existing -C option in the Windows ssh-add argument
parser.
Persist certificate identities separately from their underlying keys
and reconstruct their PKCS#11 signing state after an agent restart.
Allow individual certificate deletion while preserving provider-wide
removal behavior.
Keep the existing provider allowlist, remote-provider restrictions, and
encrypted PIN storage intact. Reject malformed, duplicate, truncated,
and oversized certificate constraints.
Add Windows unit and Pester coverage for RSA and ECDSA certificates,
certificate-only loading, unmatched certificates, deletion, provider
removal, and service restart.
---
contrib/win32/openssh/ssh-agent.vcxproj | 2 +
.../openssh/unittest-win32compat.vcxproj | 10 +
contrib/win32/win32compat/pkcs11-cert.c | 157 +++++++
contrib/win32/win32compat/pkcs11-cert.h | 27 ++
.../win32compat/ssh-agent/keyagent-request.c | 389 +++++++++++++-----
regress/pesterTests/KeyUtils.Tests.ps1 | 93 ++++-
regress/pesterTests/README.md | 16 +
.../unittests/win32compat/pkcs11_cert_tests.c | 269 ++++++++++++
regress/unittests/win32compat/tests.c | 1 +
regress/unittests/win32compat/tests.h | 1 +
ssh-add.c | 2 +-
11 files changed, 867 insertions(+), 100 deletions(-)
create mode 100644 contrib/win32/win32compat/pkcs11-cert.c
create mode 100644 contrib/win32/win32compat/pkcs11-cert.h
create mode 100644 regress/unittests/win32compat/pkcs11_cert_tests.c
diff --git a/contrib/win32/openssh/ssh-agent.vcxproj b/contrib/win32/openssh/ssh-agent.vcxproj
index cc6e9b5e4813..8a5b1e6e13e3 100644
--- a/contrib/win32/openssh/ssh-agent.vcxproj
+++ b/contrib/win32/openssh/ssh-agent.vcxproj
@@ -416,12 +416,14 @@
+
+
diff --git a/contrib/win32/openssh/unittest-win32compat.vcxproj b/contrib/win32/openssh/unittest-win32compat.vcxproj
index bbc3ed9b73f4..b52de4bfd072 100644
--- a/contrib/win32/openssh/unittest-win32compat.vcxproj
+++ b/contrib/win32/openssh/unittest-win32compat.vcxproj
@@ -36,6 +36,15 @@
+
+ true
+
+
+ true
+
+
+ true
+
true
@@ -65,6 +74,7 @@
+
diff --git a/contrib/win32/win32compat/pkcs11-cert.c b/contrib/win32/win32compat/pkcs11-cert.c
new file mode 100644
index 000000000000..e88aa5eab7f7
--- /dev/null
+++ b/contrib/win32/win32compat/pkcs11-cert.c
@@ -0,0 +1,157 @@
+/*
+ * Copyright (c) 2026 Sebastian Ott. All rights reserved.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "includes.h"
+
+#include "authfd.h"
+#include "digest.h"
+#include "log.h"
+#include "sshbuf.h"
+#include "ssherr.h"
+#include "sshkey.h"
+#include "xmalloc.h"
+
+#include "pkcs11-cert.h"
+
+char *
+pkcs11_identity_name(const struct sshkey *key, const u_char *blob,
+ size_t blob_len)
+{
+ u_char digest[SSH_DIGEST_MAX_LENGTH];
+ char *name;
+ size_t i, digest_len;
+
+ if (!sshkey_is_cert(key))
+ return sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT,
+ SSH_FP_DEFAULT);
+ digest_len = ssh_digest_bytes(SSH_DIGEST_SHA256);
+ if (ssh_digest_memory(SSH_DIGEST_SHA256, blob, blob_len, digest,
+ sizeof(digest)) != 0)
+ return NULL;
+ name = xmalloc(5 + digest_len * 2 + 1);
+ memcpy(name, "cert-", 5);
+ for (i = 0; i < digest_len; i++)
+ snprintf(name + 5 + i * 2, 3, "%02x", digest[i]);
+ return name;
+}
+
+void
+free_pkcs11_certs(struct sshkey **certs, size_t ncerts)
+{
+ size_t i;
+
+ for (i = 0; i < ncerts; i++)
+ sshkey_free(certs[i]);
+ free(certs);
+}
+
+int
+parse_pkcs11_add_constraints(struct sshbuf *m, int *cert_onlyp,
+ struct sshkey ***certsp, size_t *ncertsp)
+{
+ struct sshbuf *b = NULL;
+ struct sshkey *key = NULL;
+ char *ext_name = NULL;
+ u_char ctype, value;
+ u_int seconds;
+ int r, seen = 0, seen_lifetime = 0, seen_confirm = 0;
+ int seen_destinations = 0;
+
+ if (m == NULL || cert_onlyp == NULL || certsp == NULL ||
+ ncertsp == NULL || *certsp != NULL || *ncertsp != 0)
+ return SSH_ERR_INVALID_ARGUMENT;
+ *cert_onlyp = 0;
+
+ while (sshbuf_len(m) != 0) {
+ if ((r = sshbuf_get_u8(m, &ctype)) != 0)
+ goto out;
+ if (ctype == SSH_AGENT_CONSTRAIN_LIFETIME) {
+ if (seen_lifetime ||
+ (r = sshbuf_get_u32(m, &seconds)) != 0) {
+ r = SSH_ERR_INVALID_FORMAT;
+ goto out;
+ }
+ seen_lifetime = 1;
+ continue;
+ }
+ if (ctype == SSH_AGENT_CONSTRAIN_CONFIRM) {
+ if (seen_confirm) {
+ r = SSH_ERR_INVALID_FORMAT;
+ goto out;
+ }
+ seen_confirm = 1;
+ continue;
+ }
+ if (ctype != SSH_AGENT_CONSTRAIN_EXTENSION) {
+ error_f("unsupported smartcard constraint %u", ctype);
+ r = SSH_ERR_FEATURE_UNSUPPORTED;
+ goto out;
+ }
+ if ((r = sshbuf_get_cstring(m, &ext_name, NULL)) != 0)
+ goto out;
+ if (strcmp(ext_name,
+ "restrict-destination-v00@openssh.com") == 0) {
+ if (seen_destinations || sshbuf_skip_string(m) != 0) {
+ r = SSH_ERR_INVALID_FORMAT;
+ goto out;
+ }
+ seen_destinations = 1;
+ free(ext_name);
+ ext_name = NULL;
+ continue;
+ }
+ if (strcmp(ext_name,
+ "associated-certs-v00@openssh.com") != 0) {
+ error_f("unsupported smartcard constraint \"%s\"",
+ ext_name);
+ r = SSH_ERR_FEATURE_UNSUPPORTED;
+ goto out;
+ }
+ if (seen) {
+ error_f("%s already set", ext_name);
+ r = SSH_ERR_INVALID_FORMAT;
+ goto out;
+ }
+ seen = 1;
+ if ((r = sshbuf_get_u8(m, &value)) != 0 ||
+ (r = sshbuf_froms(m, &b)) != 0)
+ goto out;
+ *cert_onlyp = value != 0;
+ while (sshbuf_len(b) != 0) {
+ if (*ncertsp >= AGENT_MAX_EXT_CERTS) {
+ error_f("too many %s constraints", ext_name);
+ r = SSH_ERR_INVALID_FORMAT;
+ goto out;
+ }
+ if ((r = sshkey_froms(b, &key)) != 0)
+ goto out;
+ *certsp = xrecallocarray(*certsp, *ncertsp,
+ *ncertsp + 1, sizeof(**certsp));
+ (*certsp)[(*ncertsp)++] = key;
+ key = NULL;
+ }
+ sshbuf_free(b);
+ b = NULL;
+ free(ext_name);
+ ext_name = NULL;
+ }
+ r = 0;
+ out:
+ sshkey_free(key);
+ sshbuf_free(b);
+ free(ext_name);
+ return r;
+}
diff --git a/contrib/win32/win32compat/pkcs11-cert.h b/contrib/win32/win32compat/pkcs11-cert.h
new file mode 100644
index 000000000000..42f0c750e8d6
--- /dev/null
+++ b/contrib/win32/win32compat/pkcs11-cert.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2026 Sebastian Ott. All rights reserved.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#pragma once
+
+#include "sshbuf.h"
+#include "sshkey.h"
+
+#define AGENT_MAX_EXT_CERTS 1024
+
+char *pkcs11_identity_name(const struct sshkey *, const u_char *, size_t);
+int parse_pkcs11_add_constraints(struct sshbuf *, int *,
+ struct sshkey ***, size_t *);
+void free_pkcs11_certs(struct sshkey **, size_t);
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index b9b4b036e19b..05f4c61c6995 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -36,6 +36,7 @@
#include
#ifdef ENABLE_PKCS11
#include "ssh-pkcs11.h"
+#include "pkcs11-cert.h"
#endif
#include "xmalloc.h"
@@ -365,6 +366,205 @@ process_add_identity(struct sshbuf* request, struct sshbuf* response, struct age
return r;
}
+#ifdef ENABLE_PKCS11
+static int
+store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
+ const char *provider)
+{
+ SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
+ HKEY reg = NULL, sub = NULL;
+ u_char *blob = NULL;
+ size_t blob_len;
+ char *thumbprint = NULL;
+ int success = 0;
+
+ sa.nLength = sizeof(sa);
+ if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
+ SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength) ||
+ sshkey_to_blob(key, &blob, &blob_len) != 0 ||
+ blob_len == 0 || blob_len > MAX_MESSAGE_SIZE ||
+ (thumbprint = pkcs11_identity_name(key, blob, blob_len)) == NULL ||
+ RegCreateKeyExW(user_root, SSH_KEYS_ROOT, 0, NULL, 0,
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
+ RegCreateKeyExA(reg, thumbprint, 0, NULL, 0,
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub, NULL) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, NULL, 0, REG_BINARY, blob,
+ (DWORD)blob_len) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"pub", 0, REG_BINARY, blob,
+ (DWORD)blob_len) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"type", 0, REG_DWORD,
+ (const BYTE *)&key->type, sizeof(key->type)) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"comment", 0, REG_BINARY,
+ (const BYTE *)provider, (DWORD)strlen(provider)) != ERROR_SUCCESS) {
+ error_f("failed to persist PKCS11 identity");
+ goto out;
+ }
+ success = 1;
+ out:
+ if (sub != NULL) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ if (!success && reg != NULL && thumbprint != NULL)
+ RegDeleteTreeA(reg, thumbprint);
+ if (reg != NULL)
+ RegCloseKey(reg);
+ if (sa.lpSecurityDescriptor != NULL)
+ LocalFree(sa.lpSecurityDescriptor);
+ free(thumbprint);
+ free(blob);
+ return success ? 0 : -1;
+}
+
+static int
+store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
+ const char *provider, const char *pin, size_t pin_len)
+{
+ SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
+ HKEY reg = NULL, sub = NULL;
+ char *epin = NULL;
+ DWORD epin_len = 0;
+ int success = 0;
+
+ sa.nLength = sizeof(sa);
+ if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
+ SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength) ||
+ convert_blob(con, pin, (DWORD)pin_len, &epin, &epin_len, TRUE) != 0 ||
+ RegCreateKeyExW(user_root, SSH_PKCS11_PROVIDERS_ROOT, 0, NULL, 0,
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
+ RegCreateKeyExA(reg, provider, 0, NULL, 0,
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub, NULL) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"provider", 0, REG_BINARY,
+ (const BYTE *)provider, (DWORD)strlen(provider)) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"pin", 0, REG_BINARY, (const BYTE *)epin,
+ epin_len) != ERROR_SUCCESS) {
+ error_f("failed to persist PKCS11 provider");
+ goto out;
+ }
+ success = 1;
+ out:
+ if (epin != NULL) {
+ SecureZeroMemory(epin, epin_len);
+ free(epin);
+ }
+ if (sub != NULL) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ if (!success && reg != NULL)
+ RegDeleteTreeA(reg, provider);
+ if (reg != NULL)
+ RegCloseKey(reg);
+ if (sa.lpSecurityDescriptor != NULL)
+ LocalFree(sa.lpSecurityDescriptor);
+ return success ? 0 : -1;
+}
+
+static int
+load_pkcs11_identities(HKEY user_root, const char *provider,
+ struct sshkey **token_keys, int nkeys)
+{
+ HKEY root = NULL, sub = NULL;
+ wchar_t sub_name[MAX_KEY_LENGTH];
+ DWORD sub_name_len, blob_len, comment_len;
+ u_char *blob = NULL;
+ char *comment = NULL;
+ struct sshkey *registered = NULL, *cert = NULL;
+ u_char *plain_added = NULL;
+ size_t provider_len = strlen(provider);
+ int i, index = 0, loaded = 0;
+ LSTATUS status;
+
+ if (nkeys > 0)
+ plain_added = xcalloc((size_t)nkeys, sizeof(*plain_added));
+ status = RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
+ KEY_ENUMERATE_SUB_KEYS | KEY_QUERY_VALUE | KEY_WOW64_64KEY, &root);
+ if (status == ERROR_FILE_NOT_FOUND)
+ goto out;
+ if (status != ERROR_SUCCESS) {
+ error_f("failed to open persisted identities: %ld", status);
+ loaded = -1;
+ goto out;
+ }
+ for (;;) {
+ sub_name_len = MAX_KEY_LENGTH;
+ if (sub != NULL) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ status = RegEnumKeyExW(root, index++, sub_name, &sub_name_len,
+ NULL, NULL, NULL, NULL);
+ if (status == ERROR_NO_MORE_ITEMS)
+ break;
+ if (status != ERROR_SUCCESS)
+ continue;
+ if (RegOpenKeyExW(root, sub_name, 0,
+ KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS ||
+ RegQueryValueExW(sub, L"pub", NULL, NULL, NULL,
+ &blob_len) != ERROR_SUCCESS ||
+ RegQueryValueExW(sub, L"comment", NULL, NULL, NULL,
+ &comment_len) != ERROR_SUCCESS ||
+ blob_len == 0 || blob_len > MAX_MESSAGE_SIZE ||
+ comment_len != provider_len)
+ continue;
+ free(blob);
+ free(comment);
+ blob = xmalloc(blob_len);
+ comment = xmalloc((size_t)comment_len + 1);
+ if (RegQueryValueExW(sub, L"pub", NULL, NULL, blob,
+ &blob_len) != ERROR_SUCCESS ||
+ RegQueryValueExW(sub, L"comment", NULL, NULL,
+ (BYTE *)comment, &comment_len) != ERROR_SUCCESS)
+ continue;
+ comment[comment_len] = '\0';
+ if (memcmp(comment, provider, provider_len) != 0)
+ continue;
+ sshkey_free(registered);
+ registered = NULL;
+ if (sshkey_from_blob(blob, blob_len, ®istered) != 0)
+ continue;
+ for (i = 0; i < nkeys; i++) {
+ if (token_keys[i] == NULL)
+ continue;
+ if (sshkey_is_cert(registered)) {
+ if (!sshkey_equal_public(token_keys[i], registered))
+ continue;
+ if (pkcs11_make_cert(token_keys[i], registered,
+ &cert) != 0)
+ continue;
+ add_key(cert, (char *)provider);
+ cert = NULL;
+ loaded++;
+ break;
+ }
+ if (!plain_added[i] &&
+ sshkey_equal(token_keys[i], registered)) {
+ plain_added[i] = 1;
+ break;
+ }
+ }
+ }
+ for (i = 0; i < nkeys; i++) {
+ if (!plain_added[i] || token_keys[i] == NULL)
+ continue;
+ add_key(token_keys[i], (char *)provider);
+ token_keys[i] = NULL;
+ loaded++;
+ }
+ out:
+ sshkey_free(cert);
+ sshkey_free(registered);
+ free(plain_added);
+ free(comment);
+ free(blob);
+ if (sub != NULL)
+ RegCloseKey(sub);
+ if (root != NULL)
+ RegCloseKey(root);
+ return loaded;
+}
+#endif /* ENABLE_PKCS11 */
+
static int sign_blob(const struct sshkey *pubkey, u_char ** sig, size_t *siglen,
const u_char *blob, size_t blen, u_int flags, struct agent_connection* con)
{
@@ -454,7 +654,7 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
struct sshkey *key = NULL;
#ifdef ENABLE_PKCS11
- int i, count = 0, index = 0;;
+ int i, count = 0, index = 0, loaded = 0;
wchar_t sub_name[MAX_KEY_LENGTH];
DWORD sub_name_len = MAX_KEY_LENGTH;
DWORD pin_len, epin_len, provider_len;
@@ -497,10 +697,16 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
pin = npin;
pin[pin_len] = '\0';
count = pkcs11_add_provider(provider, pin, &keys, NULL);
- for (i = 0; i < count; i++) {
- add_key(keys[i], provider);
- }
+ if (count <= 0)
+ goto done;
+ loaded = load_pkcs11_identities(user_root, provider,
+ keys, count);
+ for (i = 0; i < count; i++)
+ sshkey_free(keys[i]);
free(keys);
+ keys = NULL;
+ if (loaded < 0)
+ goto done;
if (provider)
free(provider);
if (pin) {
@@ -553,6 +759,11 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
if (signature)
free(signature);
#ifdef ENABLE_PKCS11
+ if (keys != NULL) {
+ for (i = 0; i < count; i++)
+ sshkey_free(keys[i]);
+ free(keys);
+ }
del_all_keys();
pkcs11_terminate();
if (provider)
@@ -590,7 +801,12 @@ process_remove_key(struct sshbuf* request, struct sshbuf* response, struct agent
goto done;
}
- if ((thumbprint = sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT, SSH_FP_DEFAULT)) == NULL ||
+ if ((thumbprint =
+#ifdef ENABLE_PKCS11
+ pkcs11_identity_name(key, (const u_char *)blob, blen)) == NULL ||
+#else
+ sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT, SSH_FP_DEFAULT)) == NULL ||
+#endif
get_user_root(con, &user_root) != 0 ||
RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_QUERY_VALUE | KEY_WOW64_64KEY, &root) != 0 ||
@@ -641,28 +857,34 @@ process_remove_all(struct sshbuf* request, struct sshbuf* response, struct agent
}
#ifdef ENABLE_PKCS11
-int process_add_smartcard_key(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
+int
+process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
+ struct agent_connection *con)
{
- char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX];
- int i, count = 0, r = 0, request_invalid = 0, success = 0;
- struct sshkey **keys = NULL;
- struct sshkey* key = NULL;
- size_t pubkey_blob_len, provider_len, pin_len, epin_len;
- u_char *pubkey_blob = NULL;
- char *thumbprint = NULL;
- char *epin = NULL;
- HKEY reg = 0, sub = 0, user_root = 0;
- SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
+ char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX] = { 0 };
+ char allowed_provider[PATH_MAX];
+ int i, j, count = 0, r = 0, request_invalid = 0, success = 0;
+ int cert_only = 0, identities_stored = 0;
+ struct sshkey **keys = NULL, **certs = NULL, *cert = NULL;
+ size_t pin_len = 0, ncerts = 0;
+ HKEY user_root = NULL;
pkcs11_init(0);
- if ((r = sshbuf_get_cstring(request, &provider, &provider_len)) != 0 ||
- (r = sshbuf_get_cstring(request, &pin, &pin_len)) != 0 ||
- pin_len > 256) {
+ if ((r = sshbuf_get_cstring(request, &provider, NULL)) != 0 ||
+ (r = sshbuf_get_cstring(request, &pin, &pin_len)) != 0 ||
+ pin_len > 256) {
error("add smartcard request is invalid");
request_invalid = 1;
goto done;
}
+ if (sshbuf_len(request) != 0 &&
+ parse_pkcs11_add_constraints(request, &cert_only, &certs,
+ &ncerts) != 0) {
+ error("add smartcard constraints are invalid");
+ request_invalid = 1;
+ goto done;
+ }
if (con->nsession_ids != 0 && !remote_add_provider) {
verbose("failed PKCS#11 add of \"%.100s\": remote addition of "
@@ -677,76 +899,66 @@ int process_add_smartcard_key(struct sshbuf* request, struct sshbuf* response, s
goto done;
}
- to_lower_case(provider);
- verbose("provider realpath: \"%.100s\"", provider);
+ /* Remove the leading slash from the canonical Windows drive path. */
+ if (canonical_provider[0] == '/')
+ memmove(canonical_provider, canonical_provider + 1,
+ strlen(canonical_provider));
+ strcpy_s(allowed_provider, sizeof(allowed_provider), canonical_provider);
+ for (i = 0; allowed_provider[i] != '\0'; i++) {
+ if (allowed_provider[i] == '/')
+ allowed_provider[i] = '\\';
+ }
+ to_lower_case(allowed_provider);
+ verbose("provider realpath: \"%.100s\"", canonical_provider);
verbose("allowed provider paths: \"%.100s\"", allowed_providers);
- if (match_pattern_list(provider, allowed_providers, 1) != 1) {
+ if (match_pattern_list(allowed_provider, allowed_providers, 1) != 1) {
verbose("refusing PKCS#11 add of \"%.100s\": "
- "provider not allowed", provider);
+ "provider not allowed", canonical_provider);
goto done;
}
- // Remove 'drive root' if exists
- if (canonical_provider[0] == '/')
- memmove(canonical_provider, canonical_provider + 1, strlen(canonical_provider));
-
count = pkcs11_add_provider(canonical_provider, pin, &keys, NULL);
if (count <= 0) {
- error_f("failed to add key to store. count:%d", count);
+ error_f("failed to load provider keys: count:%d", count);
goto done;
}
- // If HKCU registry already has the provider then remove the provider and associated keys.
- // This allows customers to add new keys.
- if (get_user_root(con, &user_root) != 0 ||
- is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT, canonical_provider)) {
- remove_matching_subkeys_from_registry(user_root, SSH_KEYS_ROOT, L"comment", canonical_provider);
- remove_matching_subkeys_from_registry(user_root, SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider);
+ /* Replace the provider and all of its persisted identities. */
+ if (get_user_root(con, &user_root) != 0)
+ goto done;
+ if (is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT,
+ canonical_provider)) {
+ remove_matching_subkeys_from_registry(user_root, SSH_KEYS_ROOT,
+ L"comment", canonical_provider);
+ remove_matching_subkeys_from_registry(user_root,
+ SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider);
}
for (i = 0; i < count; i++) {
- key = keys[i];
- if (sa.lpSecurityDescriptor)
- LocalFree(sa.lpSecurityDescriptor);
- if (reg) {
- RegCloseKey(reg);
- reg = NULL;
- }
- if (sub) {
- RegCloseKey(sub);
- sub = NULL;
+ for (j = 0; j < (int)ncerts; j++) {
+ if (!sshkey_is_cert(certs[j]) ||
+ !sshkey_equal_public(keys[i], certs[j]))
+ continue;
+ if (pkcs11_make_cert(keys[i], certs[j], &cert) != 0)
+ continue;
+ if (store_pkcs11_identity(user_root, cert,
+ canonical_provider) != 0)
+ goto done;
+ sshkey_free(cert);
+ cert = NULL;
+ identities_stored++;
}
- memset(&sa, 0, sizeof(SECURITY_ATTRIBUTES));
- sa.nLength = sizeof(sa);
- if ((!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL, SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength)) ||
- sshkey_to_blob(key, &pubkey_blob, &pubkey_blob_len) != 0 ||
- ((thumbprint = sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT, SSH_FP_DEFAULT)) == NULL) ||
- RegCreateKeyExW(user_root, SSH_KEYS_ROOT, 0, 0, 0, KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != 0 ||
- RegCreateKeyExA(reg, thumbprint, 0, 0, 0, KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub, NULL) != 0 ||
- RegSetValueExW(sub, NULL, 0, REG_BINARY, pubkey_blob, (DWORD)pubkey_blob_len) != 0 ||
- RegSetValueExW(sub, L"pub", 0, REG_BINARY, pubkey_blob, (DWORD)pubkey_blob_len) != 0 ||
- RegSetValueExW(sub, L"type", 0, REG_DWORD, (BYTE*)&key->type, 4) != 0 ||
- RegSetValueExW(sub, L"comment", 0, REG_BINARY, canonical_provider, (DWORD)strlen(canonical_provider)) != 0) {
- error_f("failed to add key to store");
+ if (!cert_only && store_pkcs11_identity(user_root, keys[i],
+ canonical_provider) == 0)
+ identities_stored++;
+ else if (!cert_only)
goto done;
- }
}
- debug("added smartcard keys to store");
-
- memset(&sa, 0, sizeof(SECURITY_ATTRIBUTES));
- sa.nLength = sizeof(sa);
- if ((!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL, SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength)) ||
- convert_blob(con, pin, (DWORD)pin_len, &epin, (DWORD*)&epin_len, 1) != 0 ||
- RegCreateKeyExW(user_root, SSH_PKCS11_PROVIDERS_ROOT, 0, 0, 0, KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != 0 ||
- RegCreateKeyExA(reg, canonical_provider, 0, 0, 0, KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub, NULL) != 0 ||
- RegSetValueExW(sub, L"provider", 0, REG_BINARY, canonical_provider, (DWORD)strlen(canonical_provider)) != 0 ||
- RegSetValueExW(sub, L"pin", 0, REG_BINARY, epin, (DWORD)epin_len) != 0) {
- error("failed to add pkcs11 provider to store");
+ if (identities_stored == 0 || store_pkcs11_provider(user_root, con,
+ canonical_provider, pin, pin_len) != 0)
goto done;
- }
-
- debug("added pkcs11 provider to store");
+ debug("added PKCS11 provider and identities to store");
success = 1;
done:
r = 0;
@@ -755,44 +967,27 @@ int process_add_smartcard_key(struct sshbuf* request, struct sshbuf* response, s
else if (sshbuf_put_u8(response, success ? SSH_AGENT_SUCCESS : SSH_AGENT_FAILURE) != 0)
r = -1;
- /* delete created reg keys if not succeeded*/
- if ((success == 0) && reg) {
- if (thumbprint)
- RegDeleteKeyExA(reg, thumbprint, KEY_WOW64_64KEY, 0);
- if (canonical_provider)
- RegDeleteKeyExA(reg, canonical_provider, KEY_WOW64_64KEY, 0);
+ if (!success && user_root != NULL && canonical_provider[0] != '\0') {
+ remove_matching_subkeys_from_registry(user_root, SSH_KEYS_ROOT,
+ L"comment", canonical_provider);
+ remove_matching_subkeys_from_registry(user_root,
+ SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider);
}
pkcs11_terminate();
- if (sa.lpSecurityDescriptor)
- LocalFree(sa.lpSecurityDescriptor);
+ sshkey_free(cert);
for (i = 0; i < count; i++)
sshkey_free(keys[i]);
- if (keys)
- free(keys);
- if (thumbprint)
- free(thumbprint);
- if (pubkey_blob)
- free(pubkey_blob);
- if (provider)
- free(provider);
- if (allowed_providers)
- free(allowed_providers);
+ free(keys);
+ free_pkcs11_certs(certs, ncerts);
+ free(provider);
if (pin) {
SecureZeroMemory(pin, (DWORD)pin_len);
free(pin);
}
- if (epin) {
- SecureZeroMemory(epin, (DWORD)epin_len);
- free(epin);
- }
if (user_root)
RegCloseKey(user_root);
- if (reg)
- RegCloseKey(reg);
- if (sub)
- RegCloseKey(sub);
return r;
}
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index 5881f509d57f..1194d81e2fa9 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -301,10 +301,15 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
}
It "$tC.$tI - ssh-add - pkcs11 library (if available)" {
- $pkcs11Path = "C:\\Program Files\\OpenSC Project\\OpenSC\\pkcs11\\opensc-pkcs11.dll"
+ $pkcs11Path = $env:OPENSSH_TEST_PKCS11_PROVIDER
+ if (-not $pkcs11Path) {
+ $pkcs11Path = "C:\\Program Files\\OpenSC Project\\OpenSC\\pkcs11\\opensc-pkcs11.dll"
+ }
if (Test-Path $pkcs11Path) {
#set up SSH_ASKPASS
- Add-PasswordSetting -Pass $pkcs11Pin
+ $testPin = $env:OPENSSH_TEST_PKCS11_PIN
+ if (-not $testPin) { $testPin = $pkcs11Pin }
+ Add-PasswordSetting -Pass $testPin
ssh-add -s "$pkcs11Path"
$LASTEXITCODE | Should Be 0
@@ -326,6 +331,90 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
Write-Host "skipping pkcs11 test because provider not found"
}
}
+
+ It "$tC.$tI - ssh-add - pkcs11 certificates (if configured)" {
+ $pkcs11Path = $env:OPENSSH_TEST_PKCS11_PROVIDER
+ $publicKeyPaths = @($env:OPENSSH_TEST_PKCS11_PUBLIC_KEYS -split ';' |
+ Where-Object { $_ })
+ if (-not $pkcs11Path -or -not (Test-Path $pkcs11Path) -or
+ $publicKeyPaths.Count -eq 0) {
+ Write-Host "skipping pkcs11 certificate test because provider and public keys are not configured"
+ return
+ }
+
+ foreach ($publicKeyPath in $publicKeyPaths) {
+ Test-Path $publicKeyPath | Should Be $true
+ }
+ $testPin = $env:OPENSSH_TEST_PKCS11_PIN
+ if (-not $testPin) { $testPin = $pkcs11Pin }
+ Add-PasswordSetting -Pass $testPin
+
+ $ca = Join-Path $testDir "pkcs11-ca"
+ Remove-Item "$ca*" -Force -ErrorAction SilentlyContinue
+ & ssh-keygen -q -t ed25519 -N '""' -f $ca
+ $LASTEXITCODE | Should Be 0
+
+ $certPaths = @()
+ $copiedPublicKeyPaths = @()
+ $serial = 1
+ foreach ($publicKeyPath in $publicKeyPaths) {
+ $copiedPublicKeyPath = Join-Path $testDir "pkcs11-$serial.pub"
+ Copy-Item $publicKeyPath $copiedPublicKeyPath -Force
+ & ssh-keygen -q -s $ca -I "pkcs11-$serial" -n $env:USERNAME `
+ -z $serial $copiedPublicKeyPath
+ $LASTEXITCODE | Should Be 0
+ $copiedPublicKeyPaths += $copiedPublicKeyPath
+ $certPaths += $copiedPublicKeyPath.Replace(".pub", "-cert.pub")
+ $serial++
+ }
+ & ssh-keygen -q -s $ca -I "pkcs11-unmatched" -n $env:USERNAME `
+ -z 999 "$ca.pub"
+ $LASTEXITCODE | Should Be 0
+ $unmatchedCertPath = "$ca-cert.pub"
+ $associatedCertPaths = $certPaths + $unmatchedCertPath
+
+ $addArguments = @("-s", $pkcs11Path) + $associatedCertPaths
+ & ssh-add @addArguments
+ $LASTEXITCODE | Should Be 0
+ $allKeys = @(ssh-add -L)
+ foreach ($keyPath in $copiedPublicKeyPaths + $certPaths) {
+ $keyBlob = (Get-Content $keyPath).Split(' ')[1]
+ @($allKeys | Where-Object { $_.Contains($keyBlob) }).Count | Should Be 1
+ & ssh-add -T $keyPath
+ $LASTEXITCODE | Should Be 0
+ }
+
+ Restart-Service ssh-agent
+ WaitForStatus -ServiceName ssh-agent -Status "Running"
+ foreach ($certPath in $certPaths) {
+ & ssh-add -T $certPath
+ $LASTEXITCODE | Should Be 0
+ }
+ & ssh-add -d $certPaths[0]
+ $LASTEXITCODE | Should Be 0
+ $deletedKeyBlob = (Get-Content $certPaths[0]).Split(' ')[1]
+ @((ssh-add -L) | Where-Object { $_.Contains($deletedKeyBlob) }).Count |
+ Should Be 0
+
+ ssh-add -D
+ $LASTEXITCODE | Should Be 0
+ $addArguments = @("-s", $pkcs11Path, "-C") + $associatedCertPaths
+ & ssh-add @addArguments
+ $LASTEXITCODE | Should Be 0
+ $allKeys = @(ssh-add -L)
+ $allKeys.Count | Should Be $certPaths.Count
+ foreach ($certPath in $certPaths) {
+ $keyBlob = (Get-Content $certPath).Split(' ')[1]
+ @($allKeys | Where-Object { $_.Contains($keyBlob) }).Count | Should Be 1
+ & ssh-add -T $certPath
+ $LASTEXITCODE | Should Be 0
+ }
+
+ & ssh-add -e $pkcs11Path
+ $LASTEXITCODE | Should Be 0
+ @(ssh-add -L) -match "The agent has no identities." | Should Be $true
+ Remove-PasswordSetting
+ }
}
Context "$tC ssh-keygen known_hosts operations" {
diff --git a/regress/pesterTests/README.md b/regress/pesterTests/README.md
index afd636e76c6d..4d9de131c5b3 100644
--- a/regress/pesterTests/README.md
+++ b/regress/pesterTests/README.md
@@ -64,3 +64,19 @@ Follow these simple steps for test case indexing
AfterAll{$tC++}
```
- Prefix any test out file with $tC.$tI. You may use pre-created $stderrFile, $stdoutFile, $logFile for this purpose
+
+#### PKCS#11 certificate tests
+
+The PKCS#11 certificate scenario in `KeyUtils.Tests.ps1` is enabled when the
+following environment variables are set before running the E2E tests:
+
+* `OPENSSH_TEST_PKCS11_PROVIDER`: absolute path to a PKCS#11 provider DLL.
+* `OPENSSH_TEST_PKCS11_PIN`: token PIN.
+* `OPENSSH_TEST_PKCS11_PUBLIC_KEYS`: semicolon-separated public-key files whose
+ corresponding private keys are present on the token.
+
+The test creates short-lived OpenSSH certificates for the supplied public
+keys. It verifies plain and certificate identities, signing, agent service
+restart, individual certificate deletion, cert-only loading, an unmatched
+certificate, and provider removal. Never use production token credentials in
+CI.
diff --git a/regress/unittests/win32compat/pkcs11_cert_tests.c b/regress/unittests/win32compat/pkcs11_cert_tests.c
new file mode 100644
index 000000000000..f0e1cfb74de9
--- /dev/null
+++ b/regress/unittests/win32compat/pkcs11_cert_tests.c
@@ -0,0 +1,269 @@
+/*
+ * Copyright (c) 2026 Sebastian Ott. All rights reserved.
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "includes.h"
+
+#include "authfd.h"
+#include "sshbuf.h"
+#include "ssherr.h"
+#include "sshkey.h"
+#include "xmalloc.h"
+
+#include "contrib/win32/win32compat/pkcs11-cert.h"
+#include "../test_helper/test_helper.h"
+#include "tests.h"
+
+#define TEST_CERT \
+ "ecdsa-sha2-nistp256-cert-v01@openssh.com " \
+ "AAAAKGVjZHNhLXNoYTItbmlzdHAyNTYtY2VydC12MDFAb3BlbnNzaC5jb20AAAAg" \
+ "OtFRnMigkGliaYfPmX5IidVWfV3tRH6lqRXv0l8bvKoAAAAIbmlzdHAyNTYAAABB" \
+ "BAxZW5ZDq1vcnSlYbTPvQGN3PbGgRO0ht5Rcd/JwWr5AAw2iPY4d/5Lxvybfb6" \
+ "ZttqsKJJUwhg38wpF5CCmlpQcAAAAAAAAABwAAAAIAAAAGanVsaXVzAAAAEgAAAA" \
+ "Vob3N0MQAAAAVob3N0MgAAAAA2jAHwAAAAAE0eYHAAAAAAAAAAAAAAAAAAAABoAA" \
+ "AAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBAxZW5ZDq1vcnS" \
+ "lYbTPvQGN3PbGgRO0ht5Rcd/JwWr5AAw2iPY4d/5Lxvybfb6ZttqsKJJUwhg38w" \
+ "pF5CCmlpQcAAABkAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAABJAAAAIHbxGwTnu" \
+ "e7KxhHXGFvRcxBnekhQ3Qx84vV/Vs4oVCrpAAAAIQC7vk2+d14aS7td7kVXLQn3" \
+ "92oALjEBzMZoDvT1vT/zOA== test"
+
+static struct sshkey *
+load_test_cert(void)
+{
+ struct sshkey *key = NULL;
+ char *line = NULL, *cp;
+
+ line = xstrdup(TEST_CERT);
+ cp = line;
+ key = sshkey_new(KEY_UNSPEC);
+ if (key == NULL || sshkey_read(key, &cp) != 0) {
+ sshkey_free(key);
+ key = NULL;
+ }
+ free(line);
+ return key;
+}
+
+static int
+put_associated_certs(struct sshbuf *m, int cert_only,
+ struct sshkey *cert, size_t ncerts)
+{
+ struct sshbuf *b = NULL;
+ size_t i;
+ int r;
+
+ if ((b = sshbuf_new()) == NULL)
+ return SSH_ERR_ALLOC_FAIL;
+ for (i = 0; i < ncerts; i++) {
+ if ((r = sshkey_puts(cert, b)) != 0)
+ goto out;
+ }
+ if ((r = sshbuf_put_u8(m, SSH_AGENT_CONSTRAIN_EXTENSION)) != 0 ||
+ (r = sshbuf_put_cstring(m,
+ "associated-certs-v00@openssh.com")) != 0 ||
+ (r = sshbuf_put_u8(m, cert_only != 0)) != 0 ||
+ (r = sshbuf_put_stringb(m, b)) != 0)
+ goto out;
+ r = 0;
+ out:
+ sshbuf_free(b);
+ return r;
+}
+
+static void
+test_pkcs11_cert_constraints_valid(void)
+{
+ struct sshbuf *m = NULL;
+ struct sshkey *cert = NULL, **certs = NULL;
+ size_t ncerts = 0;
+ int cert_only = 0, r;
+
+ TEST_START("PKCS11 associated certificate constraint");
+ ASSERT_PTR_NE(cert = load_test_cert(), NULL);
+ ASSERT_PTR_NE(m = sshbuf_new(), NULL);
+ ASSERT_INT_EQ(put_associated_certs(m, 1, cert, 1), 0);
+ ASSERT_INT_EQ(r = parse_pkcs11_add_constraints(m, &cert_only,
+ &certs, &ncerts), 0);
+ ASSERT_INT_EQ(cert_only, 1);
+ ASSERT_SIZE_T_EQ(ncerts, 1);
+ ASSERT_INT_EQ(sshkey_equal(cert, certs[0]), 1);
+ free_pkcs11_certs(certs, ncerts);
+ sshkey_free(cert);
+ sshbuf_free(m);
+ TEST_DONE();
+}
+
+static void
+test_pkcs11_cert_constraints_compatible(void)
+{
+ struct sshbuf *m = NULL, *destinations = NULL;
+ struct sshkey *cert = NULL, **certs = NULL;
+ size_t ncerts = 0;
+ int cert_only = 0;
+
+ TEST_START("PKCS11 certificate with existing constraints");
+ ASSERT_PTR_NE(cert = load_test_cert(), NULL);
+ ASSERT_PTR_NE(m = sshbuf_new(), NULL);
+ ASSERT_PTR_NE(destinations = sshbuf_new(), NULL);
+ ASSERT_INT_EQ(sshbuf_put_u8(m, SSH_AGENT_CONSTRAIN_LIFETIME), 0);
+ ASSERT_INT_EQ(sshbuf_put_u32(m, 60), 0);
+ ASSERT_INT_EQ(sshbuf_put_u8(m, SSH_AGENT_CONSTRAIN_CONFIRM), 0);
+ ASSERT_INT_EQ(sshbuf_put_u8(m, SSH_AGENT_CONSTRAIN_EXTENSION), 0);
+ ASSERT_INT_EQ(sshbuf_put_cstring(m,
+ "restrict-destination-v00@openssh.com"), 0);
+ ASSERT_INT_EQ(sshbuf_put_stringb(m, destinations), 0);
+ ASSERT_INT_EQ(put_associated_certs(m, 1, cert, 1), 0);
+ ASSERT_INT_EQ(parse_pkcs11_add_constraints(m, &cert_only,
+ &certs, &ncerts), 0);
+ ASSERT_INT_EQ(cert_only, 1);
+ ASSERT_SIZE_T_EQ(ncerts, 1);
+ free_pkcs11_certs(certs, ncerts);
+ sshkey_free(cert);
+ sshbuf_free(destinations);
+ sshbuf_free(m);
+ TEST_DONE();
+}
+
+static void
+test_pkcs11_cert_identity_name(void)
+{
+ struct sshkey *cert = NULL, *plain = NULL;
+ u_char *cert_blob = NULL, *plain_blob = NULL;
+ size_t cert_blob_len = 0, plain_blob_len = 0;
+ char *cert_name = NULL, *cert_name_again = NULL, *plain_name = NULL;
+
+ TEST_START("distinct PKCS11 certificate registry identity");
+ ASSERT_PTR_NE(cert = load_test_cert(), NULL);
+ ASSERT_INT_EQ(sshkey_from_private(cert, &plain), 0);
+ ASSERT_INT_EQ(sshkey_drop_cert(plain), 0);
+ ASSERT_INT_EQ(sshkey_to_blob(cert, &cert_blob, &cert_blob_len), 0);
+ ASSERT_INT_EQ(sshkey_to_blob(plain, &plain_blob, &plain_blob_len), 0);
+ ASSERT_PTR_NE(cert_name = pkcs11_identity_name(cert, cert_blob,
+ cert_blob_len), NULL);
+ ASSERT_PTR_NE(cert_name_again = pkcs11_identity_name(cert, cert_blob,
+ cert_blob_len), NULL);
+ ASSERT_PTR_NE(plain_name = pkcs11_identity_name(plain, plain_blob,
+ plain_blob_len), NULL);
+ ASSERT_INT_EQ(strncmp(cert_name, "cert-", 5), 0);
+ ASSERT_STRING_EQ(cert_name, cert_name_again);
+ ASSERT_STRING_NE(cert_name, plain_name);
+ free(plain_name);
+ free(cert_name_again);
+ free(cert_name);
+ free(plain_blob);
+ free(cert_blob);
+ sshkey_free(plain);
+ sshkey_free(cert);
+ TEST_DONE();
+}
+
+static void
+test_pkcs11_cert_constraints_duplicate(void)
+{
+ struct sshbuf *m = NULL;
+ struct sshkey *cert = NULL, **certs = NULL;
+ size_t ncerts = 0;
+ int cert_only = 0;
+
+ TEST_START("duplicate PKCS11 certificate constraint");
+ ASSERT_PTR_NE(cert = load_test_cert(), NULL);
+ ASSERT_PTR_NE(m = sshbuf_new(), NULL);
+ ASSERT_INT_EQ(put_associated_certs(m, 0, cert, 1), 0);
+ ASSERT_INT_EQ(put_associated_certs(m, 0, cert, 1), 0);
+ ASSERT_INT_NE(parse_pkcs11_add_constraints(m, &cert_only,
+ &certs, &ncerts), 0);
+ free_pkcs11_certs(certs, ncerts);
+ sshkey_free(cert);
+ sshbuf_free(m);
+ TEST_DONE();
+}
+
+static void
+test_pkcs11_cert_constraints_truncated(void)
+{
+ struct sshbuf *m = NULL;
+ struct sshkey **certs = NULL;
+ size_t ncerts = 0;
+ int cert_only = 0;
+
+ TEST_START("truncated PKCS11 certificate constraint");
+ ASSERT_PTR_NE(m = sshbuf_new(), NULL);
+ ASSERT_INT_EQ(sshbuf_put_u8(m, SSH_AGENT_CONSTRAIN_EXTENSION), 0);
+ ASSERT_INT_EQ(sshbuf_put_cstring(m,
+ "associated-certs-v00@openssh.com"), 0);
+ ASSERT_INT_NE(parse_pkcs11_add_constraints(m, &cert_only,
+ &certs, &ncerts), 0);
+ free_pkcs11_certs(certs, ncerts);
+ sshbuf_free(m);
+ TEST_DONE();
+}
+
+static void
+test_pkcs11_cert_constraints_malformed(void)
+{
+ struct sshbuf *m = NULL, *b = NULL;
+ struct sshkey **certs = NULL;
+ size_t ncerts = 0;
+ int cert_only = 0;
+
+ TEST_START("malformed PKCS11 certificate constraint");
+ ASSERT_PTR_NE(m = sshbuf_new(), NULL);
+ ASSERT_PTR_NE(b = sshbuf_new(), NULL);
+ ASSERT_INT_EQ(sshbuf_put_string(b, "bad", 3), 0);
+ ASSERT_INT_EQ(sshbuf_put_u8(m, SSH_AGENT_CONSTRAIN_EXTENSION), 0);
+ ASSERT_INT_EQ(sshbuf_put_cstring(m,
+ "associated-certs-v00@openssh.com"), 0);
+ ASSERT_INT_EQ(sshbuf_put_u8(m, 0), 0);
+ ASSERT_INT_EQ(sshbuf_put_stringb(m, b), 0);
+ ASSERT_INT_NE(parse_pkcs11_add_constraints(m, &cert_only,
+ &certs, &ncerts), 0);
+ free_pkcs11_certs(certs, ncerts);
+ sshbuf_free(b);
+ sshbuf_free(m);
+ TEST_DONE();
+}
+
+static void
+test_pkcs11_cert_constraints_oversized(void)
+{
+ struct sshbuf *m = NULL;
+ struct sshkey *cert = NULL, **certs = NULL;
+ size_t ncerts = 0;
+ int cert_only = 0;
+
+ TEST_START("oversized PKCS11 certificate constraint");
+ ASSERT_PTR_NE(cert = load_test_cert(), NULL);
+ ASSERT_PTR_NE(m = sshbuf_new(), NULL);
+ ASSERT_INT_EQ(put_associated_certs(m, 0, cert,
+ AGENT_MAX_EXT_CERTS + 1), 0);
+ ASSERT_INT_NE(parse_pkcs11_add_constraints(m, &cert_only,
+ &certs, &ncerts), 0);
+ free_pkcs11_certs(certs, ncerts);
+ sshkey_free(cert);
+ sshbuf_free(m);
+ TEST_DONE();
+}
+
+void
+pkcs11_cert_tests(void)
+{
+ test_pkcs11_cert_constraints_valid();
+ test_pkcs11_cert_constraints_compatible();
+ test_pkcs11_cert_identity_name();
+ test_pkcs11_cert_constraints_duplicate();
+ test_pkcs11_cert_constraints_truncated();
+ test_pkcs11_cert_constraints_malformed();
+ test_pkcs11_cert_constraints_oversized();
+}
diff --git a/regress/unittests/win32compat/tests.c b/regress/unittests/win32compat/tests.c
index 756d6b8b394c..34cced7a643e 100644
--- a/regress/unittests/win32compat/tests.c
+++ b/regress/unittests/win32compat/tests.c
@@ -19,6 +19,7 @@ tests()
{
_set_abort_behavior(0, 1);
log_init(NULL, 7, 2, 0);
+ pkcs11_cert_tests();
signal_tests();
socket_tests();
file_tests();
diff --git a/regress/unittests/win32compat/tests.h b/regress/unittests/win32compat/tests.h
index 580cf063f859..64e06f09f0f8 100644
--- a/regress/unittests/win32compat/tests.h
+++ b/regress/unittests/win32compat/tests.h
@@ -3,6 +3,7 @@ void signal_tests();
void socket_tests();
void file_tests();
void miscellaneous_tests();
+void pkcs11_cert_tests(void);
char *dup_str(char *inStr);
void delete_dir_recursive(char *full_dir_path);
diff --git a/ssh-add.c b/ssh-add.c
index e7ac10799e16..b229ae3fb18a 100644
--- a/ssh-add.c
+++ b/ssh-add.c
@@ -861,7 +861,7 @@ main(int argc, char **argv)
skprovider = getenv("SSH_SK_PROVIDER");
#ifdef WINDOWS
- while ((ch = getopt(argc, argv, "vkKlLNcdDTxXE:e:M:m:Qqs:S:t:")) != -1) {
+ while ((ch = getopt(argc, argv, "vkKlLNCcdDTxXE:e:M:m:Qqs:S:t:")) != -1) {
#else
while ((ch = getopt(argc, argv, "vkKlLNCcdDTxXE:e:h:H:M:m:Qqs:S:t:")) != -1) {
#endif
From a396ef8b65ea7ebcf20f4367840131b2683545e7 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Fri, 17 Jul 2026 13:54:00 +0200
Subject: [PATCH 02/16] Preserve PKCS#11 identities on provider add
Treat repeated provider additions as additive and idempotent because the Windows service persists providers and identities across client and service lifetimes. Unlike the in-memory Unix agent, this allows certificates to be added in separate requests without replacing earlier identities.
Roll back only identities created by the failed request and preserve all previously persisted state.
---
.../win32compat/ssh-agent/keyagent-request.c | 98 ++++++++++++++-----
regress/pesterTests/CommonUtils.psm1 | 2 +-
regress/pesterTests/KeyUtils.Tests.ps1 | 81 ++++++++++++---
regress/pesterTests/README.md | 3 +-
4 files changed, 143 insertions(+), 41 deletions(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index 05f4c61c6995..721423a9742f 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -369,15 +369,19 @@ process_add_identity(struct sshbuf* request, struct sshbuf* response, struct age
#ifdef ENABLE_PKCS11
static int
store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
- const char *provider)
+ const char *provider, char **created_identityp)
{
SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
HKEY reg = NULL, sub = NULL;
u_char *blob = NULL;
size_t blob_len;
char *thumbprint = NULL;
+ DWORD disposition = 0;
int success = 0;
+ if (created_identityp == NULL)
+ return -1;
+ *created_identityp = NULL;
sa.nLength = sizeof(sa);
if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength) ||
@@ -387,8 +391,16 @@ store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
RegCreateKeyExW(user_root, SSH_KEYS_ROOT, 0, NULL, 0,
KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
RegCreateKeyExA(reg, thumbprint, 0, NULL, 0,
- KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub, NULL) != ERROR_SUCCESS ||
- RegSetValueExW(sub, NULL, 0, REG_BINARY, blob,
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub,
+ &disposition) != ERROR_SUCCESS) {
+ error_f("failed to persist PKCS11 identity");
+ goto out;
+ }
+ if (disposition == REG_OPENED_EXISTING_KEY) {
+ success = 1;
+ goto out;
+ }
+ if (RegSetValueExW(sub, NULL, 0, REG_BINARY, blob,
(DWORD)blob_len) != ERROR_SUCCESS ||
RegSetValueExW(sub, L"pub", 0, REG_BINARY, blob,
(DWORD)blob_len) != ERROR_SUCCESS ||
@@ -399,13 +411,15 @@ store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
error_f("failed to persist PKCS11 identity");
goto out;
}
+ *created_identityp = xstrdup(thumbprint);
success = 1;
out:
if (sub != NULL) {
RegCloseKey(sub);
sub = NULL;
}
- if (!success && reg != NULL && thumbprint != NULL)
+ if (!success && disposition == REG_CREATED_NEW_KEY && reg != NULL &&
+ thumbprint != NULL)
RegDeleteTreeA(reg, thumbprint);
if (reg != NULL)
RegCloseKey(reg);
@@ -424,6 +438,7 @@ store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
HKEY reg = NULL, sub = NULL;
char *epin = NULL;
DWORD epin_len = 0;
+ DWORD disposition = 0;
int success = 0;
sa.nLength = sizeof(sa);
@@ -433,7 +448,8 @@ store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
RegCreateKeyExW(user_root, SSH_PKCS11_PROVIDERS_ROOT, 0, NULL, 0,
KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
RegCreateKeyExA(reg, provider, 0, NULL, 0,
- KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub, NULL) != ERROR_SUCCESS ||
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub,
+ &disposition) != ERROR_SUCCESS ||
RegSetValueExW(sub, L"provider", 0, REG_BINARY,
(const BYTE *)provider, (DWORD)strlen(provider)) != ERROR_SUCCESS ||
RegSetValueExW(sub, L"pin", 0, REG_BINARY, (const BYTE *)epin,
@@ -451,7 +467,7 @@ store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
RegCloseKey(sub);
sub = NULL;
}
- if (!success && reg != NULL)
+ if (!success && disposition == REG_CREATED_NEW_KEY && reg != NULL)
RegDeleteTreeA(reg, provider);
if (reg != NULL)
RegCloseKey(reg);
@@ -460,6 +476,28 @@ store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
return success ? 0 : -1;
}
+static void
+rollback_pkcs11_identities(HKEY user_root, char **identities,
+ size_t nidentities)
+{
+ HKEY reg = NULL;
+ size_t i;
+
+ if (nidentities == 0)
+ return;
+ if (RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
+ DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY,
+ ®) != ERROR_SUCCESS) {
+ error_f("failed to open PKCS11 identities for rollback");
+ return;
+ }
+ for (i = 0; i < nidentities; i++) {
+ if (RegDeleteTreeA(reg, identities[i]) != ERROR_SUCCESS)
+ error_f("failed to roll back PKCS11 identity");
+ }
+ RegCloseKey(reg);
+}
+
static int
load_pkcs11_identities(HKEY user_root, const char *provider,
struct sshkey **token_keys, int nkeys)
@@ -862,11 +900,12 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
struct agent_connection *con)
{
char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX] = { 0 };
- char allowed_provider[PATH_MAX];
+ char allowed_provider[PATH_MAX], *created_identity = NULL;
+ char **created_identities = NULL;
int i, j, count = 0, r = 0, request_invalid = 0, success = 0;
int cert_only = 0, identities_stored = 0;
struct sshkey **keys = NULL, **certs = NULL, *cert = NULL;
- size_t pin_len = 0, ncerts = 0;
+ size_t k, pin_len = 0, ncerts = 0, ncreated_identities = 0;
HKEY user_root = NULL;
pkcs11_init(0);
@@ -923,16 +962,8 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
goto done;
}
- /* Replace the provider and all of its persisted identities. */
if (get_user_root(con, &user_root) != 0)
goto done;
- if (is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT,
- canonical_provider)) {
- remove_matching_subkeys_from_registry(user_root, SSH_KEYS_ROOT,
- L"comment", canonical_provider);
- remove_matching_subkeys_from_registry(user_root,
- SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider);
- }
for (i = 0; i < count; i++) {
for (j = 0; j < (int)ncerts; j++) {
@@ -942,16 +973,32 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
if (pkcs11_make_cert(keys[i], certs[j], &cert) != 0)
continue;
if (store_pkcs11_identity(user_root, cert,
- canonical_provider) != 0)
+ canonical_provider, &created_identity) != 0)
goto done;
+ if (created_identity != NULL) {
+ created_identities = xrecallocarray(created_identities,
+ ncreated_identities, ncreated_identities + 1,
+ sizeof(*created_identities));
+ created_identities[ncreated_identities++] =
+ created_identity;
+ created_identity = NULL;
+ }
sshkey_free(cert);
cert = NULL;
identities_stored++;
}
if (!cert_only && store_pkcs11_identity(user_root, keys[i],
- canonical_provider) == 0)
+ canonical_provider, &created_identity) == 0) {
+ if (created_identity != NULL) {
+ created_identities = xrecallocarray(created_identities,
+ ncreated_identities, ncreated_identities + 1,
+ sizeof(*created_identities));
+ created_identities[ncreated_identities++] =
+ created_identity;
+ created_identity = NULL;
+ }
identities_stored++;
- else if (!cert_only)
+ } else if (!cert_only)
goto done;
}
@@ -967,16 +1014,17 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
else if (sshbuf_put_u8(response, success ? SSH_AGENT_SUCCESS : SSH_AGENT_FAILURE) != 0)
r = -1;
- if (!success && user_root != NULL && canonical_provider[0] != '\0') {
- remove_matching_subkeys_from_registry(user_root, SSH_KEYS_ROOT,
- L"comment", canonical_provider);
- remove_matching_subkeys_from_registry(user_root,
- SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider);
- }
+ if (!success && user_root != NULL)
+ rollback_pkcs11_identities(user_root, created_identities,
+ ncreated_identities);
pkcs11_terminate();
sshkey_free(cert);
+ free(created_identity);
+ for (k = 0; k < ncreated_identities; k++)
+ free(created_identities[k]);
+ free(created_identities);
for (i = 0; i < count; i++)
sshkey_free(keys[i]);
free(keys);
diff --git a/regress/pesterTests/CommonUtils.psm1 b/regress/pesterTests/CommonUtils.psm1
index 67c696e99dbf..a4222185025a 100644
--- a/regress/pesterTests/CommonUtils.psm1
+++ b/regress/pesterTests/CommonUtils.psm1
@@ -67,7 +67,7 @@ function Set-FilePermission
function Add-PasswordSetting
{
param([string] $pass)
- if ($IsWindows) {
+ if ($IsWindows -or $env:OS -eq "Windows_NT") {
if (-not($env:DISPLAY)) {$env:DISPLAY = 1}
$askpass_util = Join-Path $PSScriptRoot "utilities\askpass_util\askpass_util.exe"
$env:SSH_ASKPASS=$askpass_util
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index 1194d81e2fa9..69827d6f4318 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -215,6 +215,7 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
}
}
AfterAll{$tC++}
+ AfterEach { Remove-PasswordSetting }
# Executing ssh-agent will start agent service
# This is to support typical Unix scenarios where
@@ -310,11 +311,9 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$testPin = $env:OPENSSH_TEST_PKCS11_PIN
if (-not $testPin) { $testPin = $pkcs11Pin }
Add-PasswordSetting -Pass $testPin
-
+ $env:SSH_ASKPASS_REQUIRE = "force"
ssh-add -s "$pkcs11Path"
$LASTEXITCODE | Should Be 0
- #remove SSH_ASKPASS
- Remove-PasswordSetting
#ensure added keys are listed
$allkeys = ssh-add -L
@@ -347,11 +346,9 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
}
$testPin = $env:OPENSSH_TEST_PKCS11_PIN
if (-not $testPin) { $testPin = $pkcs11Pin }
- Add-PasswordSetting -Pass $testPin
-
$ca = Join-Path $testDir "pkcs11-ca"
Remove-Item "$ca*" -Force -ErrorAction SilentlyContinue
- & ssh-keygen -q -t ed25519 -N '""' -f $ca
+ & ssh-keygen -q -t ed25519 -N $keypassphrase -f $ca
$LASTEXITCODE | Should Be 0
$certPaths = @()
@@ -360,19 +357,29 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
foreach ($publicKeyPath in $publicKeyPaths) {
$copiedPublicKeyPath = Join-Path $testDir "pkcs11-$serial.pub"
Copy-Item $publicKeyPath $copiedPublicKeyPath -Force
- & ssh-keygen -q -s $ca -I "pkcs11-$serial" -n $env:USERNAME `
- -z $serial $copiedPublicKeyPath
+ & ssh-keygen -q -s $ca -P $keypassphrase -I "pkcs11-$serial" `
+ -n $env:USERNAME -z $serial $copiedPublicKeyPath
$LASTEXITCODE | Should Be 0
$copiedPublicKeyPaths += $copiedPublicKeyPath
$certPaths += $copiedPublicKeyPath.Replace(".pub", "-cert.pub")
$serial++
}
- & ssh-keygen -q -s $ca -I "pkcs11-unmatched" -n $env:USERNAME `
- -z 999 "$ca.pub"
+ & ssh-keygen -q -s $ca -P $keypassphrase -I "pkcs11-unmatched" `
+ -n $env:USERNAME -z 999 "$ca.pub"
$LASTEXITCODE | Should Be 0
$unmatchedCertPath = "$ca-cert.pub"
$associatedCertPaths = $certPaths + $unmatchedCertPath
+ $sequentialPublicKeyPath = Join-Path $testDir "pkcs11-sequential.pub"
+ Copy-Item $publicKeyPaths[0] $sequentialPublicKeyPath -Force
+ & ssh-keygen -q -s $ca -P $keypassphrase -I "pkcs11-sequential" `
+ -n $env:USERNAME -z 1000 $sequentialPublicKeyPath
+ $LASTEXITCODE | Should Be 0
+ $sequentialCertPath = $sequentialPublicKeyPath.Replace(".pub", "-cert.pub")
+
+ Add-PasswordSetting -Pass $testPin
+ $env:SSH_ASKPASS_REQUIRE = "force"
+
$addArguments = @("-s", $pkcs11Path) + $associatedCertPaths
& ssh-add @addArguments
$LASTEXITCODE | Should Be 0
@@ -398,22 +405,68 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
ssh-add -D
$LASTEXITCODE | Should Be 0
- $addArguments = @("-s", $pkcs11Path, "-C") + $associatedCertPaths
+
+ # Separate additions for the same token key must merge certificates.
+ $addArguments = @("-s", $pkcs11Path, "-C", $certPaths[0])
+ & ssh-add @addArguments
+ $LASTEXITCODE | Should Be 0
+ $addArguments = @("-s", $pkcs11Path, "-C", $sequentialCertPath)
& ssh-add @addArguments
$LASTEXITCODE | Should Be 0
$allKeys = @(ssh-add -L)
- $allKeys.Count | Should Be $certPaths.Count
- foreach ($certPath in $certPaths) {
+ foreach ($certPath in @($certPaths[0], $sequentialCertPath)) {
$keyBlob = (Get-Content $certPath).Split(' ')[1]
@($allKeys | Where-Object { $_.Contains($keyBlob) }).Count | Should Be 1
& ssh-add -T $certPath
$LASTEXITCODE | Should Be 0
}
+ # Re-adding an exact certificate is successful and idempotent.
+ & ssh-add @addArguments
+ $LASTEXITCODE | Should Be 0
+ $sequentialCertBlob = (Get-Content $sequentialCertPath).Split(' ')[1]
+ @((ssh-add -L) | Where-Object { $_.Contains($sequentialCertBlob) }).Count |
+ Should Be 1
+
+ ssh-add -D
+ $LASTEXITCODE | Should Be 0
+
+ # cert-only applies to this request and must preserve plain identities.
+ & ssh-add -s $pkcs11Path
+ $LASTEXITCODE | Should Be 0
+ & ssh-add -s $pkcs11Path -C $certPaths[0]
+ $LASTEXITCODE | Should Be 0
+ $allKeys = @(ssh-add -L)
+ foreach ($keyPath in $copiedPublicKeyPaths + $certPaths[0]) {
+ $keyBlob = (Get-Content $keyPath).Split(' ')[1]
+ @($allKeys | Where-Object { $_.Contains($keyBlob) }).Count | Should Be 1
+ }
+
+ # A failed unmatched add must not change existing persisted identities.
+ $identitiesBefore = @(ssh-add -L | Sort-Object)
+ & ssh-add -s $pkcs11Path -C $unmatchedCertPath
+ $LASTEXITCODE | Should Not Be 0
+ $identitiesAfter = @(ssh-add -L | Sort-Object)
+ @(Compare-Object $identitiesBefore $identitiesAfter).Count | Should Be 0
+
+ Restart-Service ssh-agent
+ WaitForStatus -ServiceName ssh-agent -Status "Running"
+ foreach ($keyPath in $copiedPublicKeyPaths + $certPaths[0]) {
+ & ssh-add -T $keyPath
+ $LASTEXITCODE | Should Be 0
+ }
+
+ & ssh-add -d $certPaths[0]
+ $LASTEXITCODE | Should Be 0
+ foreach ($keyPath in $copiedPublicKeyPaths) {
+ $keyBlob = (Get-Content $keyPath).Split(' ')[1]
+ @((ssh-add -L) | Where-Object { $_.Contains($keyBlob) }).Count |
+ Should Be 1
+ }
+
& ssh-add -e $pkcs11Path
$LASTEXITCODE | Should Be 0
@(ssh-add -L) -match "The agent has no identities." | Should Be $true
- Remove-PasswordSetting
}
}
diff --git a/regress/pesterTests/README.md b/regress/pesterTests/README.md
index 4d9de131c5b3..b0885e3249fa 100644
--- a/regress/pesterTests/README.md
+++ b/regress/pesterTests/README.md
@@ -79,4 +79,5 @@ The test creates short-lived OpenSSH certificates for the supplied public
keys. It verifies plain and certificate identities, signing, agent service
restart, individual certificate deletion, cert-only loading, an unmatched
certificate, and provider removal. Never use production token credentials in
-CI.
+CI. PKCS#11 PIN input is forced through the test askpass helper so an
+unattended run cannot block on an interactive prompt.
From 244b7977b95bbbd1aadbff51734befe7ac00eff8 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Fri, 17 Jul 2026 15:10:09 +0200
Subject: [PATCH 03/16] Restore software certificate deletion
Try the regular fingerprint-backed Registry entry before falling back to the PKCS#11 certificate identity name. This preserves individual removal for both software-backed and token-backed certificates when PKCS#11 support is enabled.
Add Windows regression coverage for RSA and ECDSA software certificates.
---
.../win32compat/ssh-agent/keyagent-request.c | 74 +++++++++++++++++--
regress/pesterTests/KeyUtils.Tests.ps1 | 50 +++++++++++++
2 files changed, 116 insertions(+), 8 deletions(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index 721423a9742f..099a790baf15 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -824,14 +824,59 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
return r;
}
+static LSTATUS
+delete_matching_identity(HKEY root, const char *name, const u_char *blob,
+ size_t blob_len)
+{
+ HKEY sub = NULL;
+ u_char *stored_blob = NULL;
+ DWORD stored_blob_len = 0;
+ LSTATUS status;
+
+ status = RegOpenKeyExA(root, name, 0,
+ KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub);
+ if (status != ERROR_SUCCESS)
+ return status;
+ status = RegQueryValueExW(sub, L"pub", NULL, NULL, NULL,
+ &stored_blob_len);
+ if (status != ERROR_SUCCESS)
+ goto out;
+ if (stored_blob_len > MAX_MESSAGE_SIZE) {
+ status = ERROR_INVALID_DATA;
+ goto out;
+ }
+ stored_blob = xmalloc(stored_blob_len == 0 ? 1 : stored_blob_len);
+ status = RegQueryValueExW(sub, L"pub", NULL, NULL, stored_blob,
+ &stored_blob_len);
+ if (status != ERROR_SUCCESS)
+ goto out;
+ if (stored_blob_len != blob_len ||
+ memcmp(stored_blob, blob, blob_len) != 0) {
+ status = ERROR_FILE_NOT_FOUND;
+ goto out;
+ }
+ RegCloseKey(sub);
+ sub = NULL;
+ status = RegDeleteTreeA(root, name);
+ out:
+ free(stored_blob);
+ if (sub != NULL)
+ RegCloseKey(sub);
+ return status;
+}
+
int
process_remove_key(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
{
HKEY user_root = 0, root = 0;
char *blob, *thumbprint = NULL;
+#ifdef ENABLE_PKCS11
+ char *pkcs11_name = NULL;
+#endif
size_t blen;
int r = 0, success = 0, request_invalid = 0;
struct sshkey *key = NULL;
+ LSTATUS status;
if (sshbuf_get_string_direct(request, &blob, &blen) != 0 ||
sshkey_from_blob(blob, blen, &key) != 0) {
@@ -839,16 +884,26 @@ process_remove_key(struct sshbuf* request, struct sshbuf* response, struct agent
goto done;
}
- if ((thumbprint =
-#ifdef ENABLE_PKCS11
- pkcs11_identity_name(key, (const u_char *)blob, blen)) == NULL ||
-#else
- sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT, SSH_FP_DEFAULT)) == NULL ||
-#endif
+ if ((thumbprint = sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT,
+ SSH_FP_DEFAULT)) == NULL ||
get_user_root(con, &user_root) != 0 ||
RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
- DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_QUERY_VALUE | KEY_WOW64_64KEY, &root) != 0 ||
- RegDeleteTreeA(root, thumbprint) != 0)
+ DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_QUERY_VALUE |
+ KEY_WOW64_64KEY, &root) != 0)
+ goto done;
+ status = delete_matching_identity(root, thumbprint,
+ (const u_char *)blob, blen);
+#ifdef ENABLE_PKCS11
+ if (status == ERROR_FILE_NOT_FOUND && sshkey_is_cert(key)) {
+ pkcs11_name = pkcs11_identity_name(key,
+ (const u_char *)blob, blen);
+ if (pkcs11_name == NULL)
+ goto done;
+ status = delete_matching_identity(root, pkcs11_name,
+ (const u_char *)blob, blen);
+ }
+#endif
+ if (status != ERROR_SUCCESS)
goto done;
success = 1;
done:
@@ -866,6 +921,9 @@ process_remove_key(struct sshbuf* request, struct sshbuf* response, struct agent
RegCloseKey(root);
if (thumbprint)
free(thumbprint);
+#ifdef ENABLE_PKCS11
+ free(pkcs11_name);
+#endif
return r;
}
int
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index 69827d6f4318..6385b80d3cd2 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -301,6 +301,56 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
ValidateRegistryACL -count $allkeys.count
}
+ It "$tC.$tI - ssh-add - remove software certificates" {
+ if ($NoLibreSSL) {
+ Write-Host "skipping software certificate removal test without LibreSSL"
+ return
+ }
+
+ $ca = Join-Path $testDir "software-cert-ca"
+ $nullFile = Join-Path $testDir "$tC.$tI.nullfile"
+ $null > $nullFile
+ Remove-Item "$ca*" -Force -ErrorAction SilentlyContinue
+ & ssh-keygen -q -t ed25519 -N $keypassphrase -f $ca
+ $LASTEXITCODE | Should Be 0
+
+ try {
+ ssh-add -D
+ $LASTEXITCODE | Should Be 0
+ Add-PasswordSetting -Pass $keypassphrase
+ $env:SSH_ASKPASS_REQUIRE = "force"
+
+ foreach ($type in @("rsa", "ecdsa")) {
+ $keyPath = Join-Path $testDir "id_$type"
+ & ssh-keygen -q -s $ca -P $keypassphrase `
+ -I "software-$type" -n $env:USERNAME "$keyPath.pub"
+ $LASTEXITCODE | Should Be 0
+ $certPath = "$keyPath-cert.pub"
+
+ cmd /c "ssh-add `"$keyPath`" < `"$nullFile`""
+ $LASTEXITCODE | Should Be 0
+ & ssh-add -T $certPath
+ $LASTEXITCODE | Should Be 0
+
+ $certBlob = (Get-Content $certPath).Split(' ')[1]
+ @((ssh-add -L) | Where-Object { $_.Contains($certBlob) }).Count |
+ Should Be 1
+ & ssh-add -d $certPath
+ $LASTEXITCODE | Should Be 0
+ @((ssh-add -L) | Where-Object { $_.Contains($certBlob) }).Count |
+ Should Be 0
+ }
+ }
+ finally {
+ ssh-add -D | Out-Null
+ Remove-Item "$ca*" -Force -ErrorAction SilentlyContinue
+ foreach ($type in @("rsa", "ecdsa")) {
+ Remove-Item (Join-Path $testDir "id_$type-cert.pub") `
+ -Force -ErrorAction SilentlyContinue
+ }
+ }
+ }
+
It "$tC.$tI - ssh-add - pkcs11 library (if available)" {
$pkcs11Path = $env:OPENSSH_TEST_PKCS11_PROVIDER
if (-not $pkcs11Path) {
From 2ee905b5546873c6ba9504cd4d23f99cd997eabc Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Fri, 17 Jul 2026 15:16:20 +0200
Subject: [PATCH 04/16] Keep unrelated PKCS#11 providers from blocking
signatures
Continue past persisted providers that cannot be loaded while rebuilding the transient PKCS#11 key set for a sign request. The requested identity still fails when its backing key is unavailable, while software keys and identities from other providers remain usable.
Cover signing with a stale provider record in the Windows PKCS#11 E2E tests.
---
.../win32compat/ssh-agent/keyagent-request.c | 89 +++++++++--------
regress/pesterTests/KeyUtils.Tests.ps1 | 98 +++++++++++++++++++
2 files changed, 147 insertions(+), 40 deletions(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index 099a790baf15..e9ad1f33ea2e 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -601,6 +601,32 @@ load_pkcs11_identities(HKEY user_root, const char *provider,
RegCloseKey(root);
return loaded;
}
+
+static void
+free_pkcs11_sign_provider(char **providerp, char **pinp, DWORD pin_len,
+ char **epinp, DWORD epin_len, struct sshkey ***keysp, int nkeys)
+{
+ int i;
+
+ if (*keysp != NULL) {
+ for (i = 0; i < nkeys; i++)
+ sshkey_free((*keysp)[i]);
+ free(*keysp);
+ *keysp = NULL;
+ }
+ free(*providerp);
+ *providerp = NULL;
+ if (*pinp != NULL) {
+ SecureZeroMemory(*pinp, pin_len);
+ free(*pinp);
+ *pinp = NULL;
+ }
+ if (*epinp != NULL) {
+ SecureZeroMemory(*epinp, epin_len);
+ free(*epinp);
+ *epinp = NULL;
+ }
+}
#endif /* ENABLE_PKCS11 */
static int sign_blob(const struct sshkey *pubkey, u_char ** sig, size_t *siglen,
@@ -692,10 +718,11 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
struct sshkey *key = NULL;
#ifdef ENABLE_PKCS11
- int i, count = 0, index = 0, loaded = 0;
+ int count = 0, index = 0, loaded = 0;
wchar_t sub_name[MAX_KEY_LENGTH];
DWORD sub_name_len = MAX_KEY_LENGTH;
- DWORD pin_len, epin_len, provider_len;
+ DWORD pin_len = 0, epin_len = 0, provider_len = 0;
+ DWORD epin_alloc_len = 0;
char *pin = NULL, *npin = NULL, *epin = NULL, *provider = NULL;
HKEY root = 0, sub = 0, user_root = 0;
struct sshkey **keys = NULL;
@@ -713,6 +740,7 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
while (1) {
sub_name_len = MAX_KEY_LENGTH;
+ pin_len = epin_len = provider_len = 0;
if (sub) {
RegCloseKey(sub);
sub = NULL;
@@ -721,43 +749,37 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
if (RegOpenKeyExW(root, sub_name, 0, KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) == 0 &&
RegQueryValueExW(sub, L"provider", 0, NULL, NULL, &provider_len) == 0 &&
RegQueryValueExW(sub, L"pin", 0, NULL, NULL, &epin_len) == 0) {
- if ((epin = malloc(epin_len + 1)) == NULL ||
+ epin_alloc_len = epin_len;
+ if ((epin = malloc(epin_alloc_len + 1)) == NULL ||
(provider = malloc(provider_len + 1)) == NULL ||
RegQueryValueExW(sub, L"provider", 0, NULL, provider, &provider_len) != 0 ||
- RegQueryValueExW(sub, L"pin", 0, NULL, epin, &epin_len) != 0)
- goto done;
+ RegQueryValueExW(sub, L"pin", 0, NULL, epin, &epin_len) != 0) {
+ free_pkcs11_sign_provider(&provider, &pin, pin_len,
+ &epin, epin_alloc_len, &keys, count);
+ continue;
+ }
provider[provider_len] = '\0';
epin[epin_len] = '\0';
if (convert_blob(con, epin, epin_len, &pin, &pin_len, 0) != 0 ||
(npin = realloc(pin, pin_len + 1)) == NULL) {
- goto done;
+ free_pkcs11_sign_provider(&provider, &pin, pin_len,
+ &epin, epin_len, &keys, count);
+ continue;
}
pin = npin;
pin[pin_len] = '\0';
count = pkcs11_add_provider(provider, pin, &keys, NULL);
- if (count <= 0)
- goto done;
+ if (count <= 0) {
+ free_pkcs11_sign_provider(&provider, &pin, pin_len,
+ &epin, epin_len, &keys, count);
+ continue;
+ }
loaded = load_pkcs11_identities(user_root, provider,
keys, count);
- for (i = 0; i < count; i++)
- sshkey_free(keys[i]);
- free(keys);
- keys = NULL;
+ free_pkcs11_sign_provider(&provider, &pin, pin_len,
+ &epin, epin_len, &keys, count);
if (loaded < 0)
goto done;
- if (provider)
- free(provider);
- if (pin) {
- SecureZeroMemory(pin, (DWORD)pin_len);
- free(pin);
- }
- if (epin) {
- SecureZeroMemory(epin, (DWORD)epin_len);
- free(epin);
- }
- provider = NULL;
- pin = NULL;
- epin = NULL;
}
}
else
@@ -797,23 +819,10 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
if (signature)
free(signature);
#ifdef ENABLE_PKCS11
- if (keys != NULL) {
- for (i = 0; i < count; i++)
- sshkey_free(keys[i]);
- free(keys);
- }
+ free_pkcs11_sign_provider(&provider, &pin, pin_len, &epin, epin_len,
+ &keys, count);
del_all_keys();
pkcs11_terminate();
- if (provider)
- free(provider);
- if (pin) {
- SecureZeroMemory(pin, (DWORD)pin_len);
- free(pin);
- }
- if (epin) {
- SecureZeroMemory(epin, (DWORD)epin_len);
- free(epin);
- }
if (user_root)
RegCloseKey(user_root);
if (root)
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index 6385b80d3cd2..5296ab77263a 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -518,6 +518,104 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$LASTEXITCODE | Should Be 0
@(ssh-add -L) -match "The agent has no identities." | Should Be $true
}
+
+ It "$tC.$tI - ssh-add - stale pkcs11 provider isolation (if configured)" {
+ $pkcs11Path = $env:OPENSSH_TEST_PKCS11_PROVIDER
+ $publicKeyPaths = @($env:OPENSSH_TEST_PKCS11_PUBLIC_KEYS -split ';' |
+ Where-Object { $_ })
+ if (-not $pkcs11Path -or -not (Test-Path $pkcs11Path) -or
+ $publicKeyPaths.Count -eq 0) {
+ Write-Host "skipping stale provider test because provider and public keys are not configured"
+ return
+ }
+
+ $testPin = $env:OPENSSH_TEST_PKCS11_PIN
+ if (-not $testPin) { $testPin = $pkcs11Pin }
+ $softwareKeyPath = Join-Path $testDir "id_rsa"
+ $unavailableKeyPath = Join-Path $testDir "id_ecdsa.pub"
+ $nullFile = Join-Path $testDir "$tC.$tI.nullfile"
+ $null > $nullFile
+ $providerRoot = $null
+ $validProviderKey = $null
+ $staleProviderKey = $null
+ $staleProviderPath = Join-Path $testDir `
+ "nonexistent\openssh-stale-provider.dll"
+ $staleProvider = [IO.Path]::GetFullPath($staleProviderPath).Replace(
+ '\', '/')
+ $corruptProviderPath = Join-Path $testDir `
+ "nonexistent\openssh-corrupt-provider.dll"
+ $corruptProvider = [IO.Path]::GetFullPath(
+ $corruptProviderPath).Replace('\', '/')
+
+ try {
+ ssh-add -D
+ $LASTEXITCODE | Should Be 0
+
+ Add-PasswordSetting -Pass $keypassphrase
+ $env:SSH_ASKPASS_REQUIRE = "force"
+ cmd /c "ssh-add `"$softwareKeyPath`" < `"$nullFile`""
+ $LASTEXITCODE | Should Be 0
+ Remove-PasswordSetting
+
+ Add-PasswordSetting -Pass $testPin
+ $env:SSH_ASKPASS_REQUIRE = "force"
+ & ssh-add -s $pkcs11Path
+ $LASTEXITCODE | Should Be 0
+ & ssh-add -T $softwareKeyPath
+ $LASTEXITCODE | Should Be 0
+ & ssh-add -T $publicKeyPaths[0]
+ $LASTEXITCODE | Should Be 0
+
+ $providerRootPath = "$currentUserSid\Software\OpenSSH\Agent\PKCS11_Providers"
+ $providerRoot = [Microsoft.Win32.Registry]::Users.OpenSubKey(
+ $providerRootPath, $true)
+ $providerRoot | Should Not Be $null
+ $canonicalProvider = [IO.Path]::GetFullPath($pkcs11Path).Replace('\', '/')
+ $validProviderKey = $providerRoot.OpenSubKey($canonicalProvider)
+ $validProviderKey | Should Not Be $null
+ $encryptedPin = $validProviderKey.GetValue("pin")
+ $encryptedPin -is [byte[]] | Should Be $true
+
+ $staleProviderKey = $providerRoot.CreateSubKey($staleProvider)
+ $staleProviderKey.SetValue("provider",
+ [Text.Encoding]::UTF8.GetBytes($staleProvider),
+ [Microsoft.Win32.RegistryValueKind]::Binary)
+ $staleProviderKey.SetValue("pin", $encryptedPin,
+ [Microsoft.Win32.RegistryValueKind]::Binary)
+
+ & ssh-add -T $softwareKeyPath
+ $LASTEXITCODE | Should Be 0
+ & ssh-add -T $publicKeyPaths[0]
+ $LASTEXITCODE | Should Be 0
+ & ssh-add -T $unavailableKeyPath
+ $LASTEXITCODE | Should Not Be 0
+
+ $staleProviderKey.Dispose()
+ $staleProviderKey = $providerRoot.CreateSubKey($corruptProvider)
+ $staleProviderKey.SetValue("provider",
+ [Text.Encoding]::UTF8.GetBytes($corruptProvider),
+ [Microsoft.Win32.RegistryValueKind]::Binary)
+ $staleProviderKey.SetValue("pin",
+ [Text.Encoding]::UTF8.GetBytes("invalid encrypted pin"),
+ [Microsoft.Win32.RegistryValueKind]::Binary)
+
+ & ssh-add -T $softwareKeyPath
+ $LASTEXITCODE | Should Be 0
+ & ssh-add -T $publicKeyPaths[0]
+ $LASTEXITCODE | Should Be 0
+ }
+ finally {
+ if ($staleProviderKey) { $staleProviderKey.Dispose() }
+ if ($validProviderKey) { $validProviderKey.Dispose() }
+ if ($providerRoot) {
+ $providerRoot.DeleteSubKeyTree($staleProvider, $false)
+ $providerRoot.DeleteSubKeyTree($corruptProvider, $false)
+ $providerRoot.Dispose()
+ }
+ ssh-add -D | Out-Null
+ Remove-PasswordSetting
+ }
+ }
}
Context "$tC ssh-keygen known_hosts operations" {
From b969ee5d5a3b9fe25996374f7a060ba45d3cc177 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Fri, 17 Jul 2026 15:37:15 +0200
Subject: [PATCH 05/16] Use PKCS#11 labels for Windows agent identities
Persist the PKCS#11 provider association separately from the user-visible identity comment and use the label returned by the provider. Empty labels continue to fall back to the canonical provider path.
Keep legacy Registry entries readable and removable, migrate their metadata on a repeated add, and preserve labels across merged certificate additions and service restarts.
---
contrib/win32/win32compat/pkcs11-cert.c | 6 +
contrib/win32/win32compat/pkcs11-cert.h | 1 +
.../win32compat/ssh-agent/keyagent-request.c | 318 ++++++++++++++----
regress/pesterTests/KeyUtils.Tests.ps1 | 135 ++++++++
regress/pesterTests/README.md | 3 +
.../unittests/win32compat/pkcs11_cert_tests.c | 14 +
6 files changed, 418 insertions(+), 59 deletions(-)
diff --git a/contrib/win32/win32compat/pkcs11-cert.c b/contrib/win32/win32compat/pkcs11-cert.c
index e88aa5eab7f7..ff789e7b96b8 100644
--- a/contrib/win32/win32compat/pkcs11-cert.c
+++ b/contrib/win32/win32compat/pkcs11-cert.c
@@ -48,6 +48,12 @@ pkcs11_identity_name(const struct sshkey *key, const u_char *blob,
return name;
}
+const char *
+pkcs11_identity_comment(const char *provider, const char *label)
+{
+ return label == NULL || *label == '\0' ? provider : label;
+}
+
void
free_pkcs11_certs(struct sshkey **certs, size_t ncerts)
{
diff --git a/contrib/win32/win32compat/pkcs11-cert.h b/contrib/win32/win32compat/pkcs11-cert.h
index 42f0c750e8d6..c6019672cc61 100644
--- a/contrib/win32/win32compat/pkcs11-cert.h
+++ b/contrib/win32/win32compat/pkcs11-cert.h
@@ -22,6 +22,7 @@
#define AGENT_MAX_EXT_CERTS 1024
char *pkcs11_identity_name(const struct sshkey *, const u_char *, size_t);
+const char *pkcs11_identity_comment(const char *, const char *);
int parse_pkcs11_add_constraints(struct sshbuf *, int *,
struct sshkey ***, size_t *);
void free_pkcs11_certs(struct sshkey **, size_t);
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index e9ad1f33ea2e..b98d34cbc82d 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -367,21 +367,99 @@ process_add_identity(struct sshbuf* request, struct sshbuf* response, struct age
}
#ifdef ENABLE_PKCS11
+struct pkcs11_identity_change {
+ char *name;
+ int created;
+ int had_provider;
+ DWORD provider_type;
+ u_char *provider;
+ DWORD provider_len;
+ int had_comment;
+ DWORD comment_type;
+ u_char *comment;
+ DWORD comment_len;
+};
+
+static void
+free_pkcs11_identity_change(struct pkcs11_identity_change *change)
+{
+ if (change == NULL)
+ return;
+ free(change->name);
+ free(change->provider);
+ free(change->comment);
+ free(change);
+}
+
+static int
+read_optional_reg_value(HKEY key, const wchar_t *name, int *presentp,
+ DWORD *typep, u_char **datap, DWORD *lenp)
+{
+ LSTATUS status;
+
+ *presentp = 0;
+ *datap = NULL;
+ *lenp = 0;
+ status = RegQueryValueExW(key, name, NULL, typep, NULL, lenp);
+ if (status == ERROR_FILE_NOT_FOUND)
+ return 0;
+ if (status != ERROR_SUCCESS || *lenp > MAX_MESSAGE_SIZE)
+ return -1;
+ *datap = xmalloc(*lenp == 0 ? 1 : *lenp);
+ if (RegQueryValueExW(key, name, NULL, typep, *datap,
+ lenp) != ERROR_SUCCESS) {
+ free(*datap);
+ *datap = NULL;
+ return -1;
+ }
+ *presentp = 1;
+ return 0;
+}
+
+static int
+restore_optional_reg_value(HKEY key, const wchar_t *name, int present,
+ DWORD type, const u_char *data, DWORD len)
+{
+ LSTATUS status;
+
+ if (present)
+ return RegSetValueExW(key, name, 0, type, data, len) ==
+ ERROR_SUCCESS ? 0 : -1;
+ status = RegDeleteValueW(key, name);
+ return status == ERROR_SUCCESS || status == ERROR_FILE_NOT_FOUND ? 0 : -1;
+}
+
+static int
+restore_pkcs11_identity_metadata(HKEY key,
+ const struct pkcs11_identity_change *change)
+{
+ int r1, r2;
+
+ r1 = restore_optional_reg_value(key, L"provider",
+ change->had_provider, change->provider_type, change->provider,
+ change->provider_len);
+ r2 = restore_optional_reg_value(key, L"comment", change->had_comment,
+ change->comment_type, change->comment, change->comment_len);
+ return r1 == 0 && r2 == 0 ? 0 : -1;
+}
+
static int
store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
- const char *provider, char **created_identityp)
+ const char *provider, const char *comment,
+ struct pkcs11_identity_change **changep)
{
SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
HKEY reg = NULL, sub = NULL;
u_char *blob = NULL;
size_t blob_len;
char *thumbprint = NULL;
+ struct pkcs11_identity_change *change = NULL;
DWORD disposition = 0;
int success = 0;
- if (created_identityp == NULL)
+ if (changep == NULL || provider == NULL || comment == NULL)
return -1;
- *created_identityp = NULL;
+ *changep = NULL;
sa.nLength = sizeof(sa);
if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength) ||
@@ -391,27 +469,54 @@ store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
RegCreateKeyExW(user_root, SSH_KEYS_ROOT, 0, NULL, 0,
KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
RegCreateKeyExA(reg, thumbprint, 0, NULL, 0,
- KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub,
+ KEY_WRITE | KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sa, &sub,
&disposition) != ERROR_SUCCESS) {
error_f("failed to persist PKCS11 identity");
goto out;
}
+ change = xcalloc(1, sizeof(*change));
+ change->name = xstrdup(thumbprint);
if (disposition == REG_OPENED_EXISTING_KEY) {
- success = 1;
- goto out;
- }
- if (RegSetValueExW(sub, NULL, 0, REG_BINARY, blob,
- (DWORD)blob_len) != ERROR_SUCCESS ||
- RegSetValueExW(sub, L"pub", 0, REG_BINARY, blob,
- (DWORD)blob_len) != ERROR_SUCCESS ||
- RegSetValueExW(sub, L"type", 0, REG_DWORD,
- (const BYTE *)&key->type, sizeof(key->type)) != ERROR_SUCCESS ||
- RegSetValueExW(sub, L"comment", 0, REG_BINARY,
- (const BYTE *)provider, (DWORD)strlen(provider)) != ERROR_SUCCESS) {
- error_f("failed to persist PKCS11 identity");
- goto out;
+ if (read_optional_reg_value(sub, L"provider",
+ &change->had_provider, &change->provider_type,
+ &change->provider, &change->provider_len) != 0 ||
+ read_optional_reg_value(sub, L"comment",
+ &change->had_comment, &change->comment_type,
+ &change->comment, &change->comment_len) != 0) {
+ error_f("failed to read PKCS11 identity metadata");
+ goto out;
+ }
+ if (RegSetValueExW(sub, L"provider", 0, REG_BINARY,
+ (const BYTE *)provider, (DWORD)strlen(provider)) !=
+ ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"comment", 0, REG_BINARY,
+ (const BYTE *)comment, (DWORD)strlen(comment)) !=
+ ERROR_SUCCESS) {
+ error_f("failed to update PKCS11 identity metadata");
+ if (restore_pkcs11_identity_metadata(sub, change) != 0)
+ error_f("failed to restore PKCS11 identity metadata");
+ goto out;
+ }
+ } else {
+ change->created = 1;
+ if (RegSetValueExW(sub, NULL, 0, REG_BINARY, blob,
+ (DWORD)blob_len) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"pub", 0, REG_BINARY, blob,
+ (DWORD)blob_len) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"type", 0, REG_DWORD,
+ (const BYTE *)&key->type, sizeof(key->type)) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"provider", 0, REG_BINARY,
+ (const BYTE *)provider, (DWORD)strlen(provider)) !=
+ ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"comment", 0, REG_BINARY,
+ (const BYTE *)comment, (DWORD)strlen(comment)) !=
+ ERROR_SUCCESS) {
+ error_f("failed to persist PKCS11 identity");
+ goto out;
+ }
}
- *created_identityp = xstrdup(thumbprint);
+ *changep = change;
+ change = NULL;
success = 1;
out:
if (sub != NULL) {
@@ -425,6 +530,7 @@ store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
RegCloseKey(reg);
if (sa.lpSecurityDescriptor != NULL)
LocalFree(sa.lpSecurityDescriptor);
+ free_pkcs11_identity_change(change);
free(thumbprint);
free(blob);
return success ? 0 : -1;
@@ -477,10 +583,11 @@ store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
}
static void
-rollback_pkcs11_identities(HKEY user_root, char **identities,
+rollback_pkcs11_identities(HKEY user_root,
+ struct pkcs11_identity_change **changes,
size_t nidentities)
{
- HKEY reg = NULL;
+ HKEY reg = NULL, sub = NULL;
size_t i;
if (nidentities == 0)
@@ -491,26 +598,99 @@ rollback_pkcs11_identities(HKEY user_root, char **identities,
error_f("failed to open PKCS11 identities for rollback");
return;
}
- for (i = 0; i < nidentities; i++) {
- if (RegDeleteTreeA(reg, identities[i]) != ERROR_SUCCESS)
- error_f("failed to roll back PKCS11 identity");
+ for (i = nidentities; i > 0; i--) {
+ if (changes[i - 1]->created) {
+ if (RegDeleteTreeA(reg, changes[i - 1]->name) !=
+ ERROR_SUCCESS)
+ error_f("failed to roll back PKCS11 identity");
+ continue;
+ }
+ if (RegOpenKeyExA(reg, changes[i - 1]->name, 0,
+ KEY_SET_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS ||
+ restore_pkcs11_identity_metadata(sub, changes[i - 1]) != 0)
+ error_f("failed to roll back PKCS11 identity metadata");
+ if (sub != NULL) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
}
RegCloseKey(reg);
}
+static int
+remove_pkcs11_identities(HKEY user_root, const char *provider)
+{
+ HKEY root = NULL, sub = NULL;
+ wchar_t sub_name[MAX_KEY_LENGTH];
+ DWORD sub_name_len, type, data_len;
+ u_char *data = NULL;
+ size_t provider_len = strlen(provider);
+ int index = 0, present, remove;
+ LSTATUS status;
+
+ status = RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
+ DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY, &root);
+ if (status == ERROR_FILE_NOT_FOUND)
+ return 0;
+ if (status != ERROR_SUCCESS)
+ return -1;
+ for (;;) {
+ sub_name_len = MAX_KEY_LENGTH;
+ status = RegEnumKeyExW(root, index, sub_name, &sub_name_len,
+ NULL, NULL, NULL, NULL);
+ if (status == ERROR_NO_MORE_ITEMS)
+ break;
+ if (status != ERROR_SUCCESS) {
+ index++;
+ continue;
+ }
+ if (RegOpenKeyExW(root, sub_name, 0,
+ KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS) {
+ index++;
+ continue;
+ }
+ free(data);
+ data = NULL;
+ if (read_optional_reg_value(sub, L"provider", &present, &type,
+ &data, &data_len) != 0 ||
+ (!present && read_optional_reg_value(sub, L"comment",
+ &present, &type, &data, &data_len) != 0)) {
+ RegCloseKey(sub);
+ sub = NULL;
+ index++;
+ continue;
+ }
+ remove = present && data_len == provider_len &&
+ memcmp(data, provider, provider_len) == 0;
+ RegCloseKey(sub);
+ sub = NULL;
+ if (remove) {
+ if (RegDeleteTreeW(root, sub_name) != ERROR_SUCCESS) {
+ RegCloseKey(root);
+ free(data);
+ return -1;
+ }
+ } else
+ index++;
+ }
+ RegCloseKey(root);
+ free(data);
+ return 0;
+}
+
static int
load_pkcs11_identities(HKEY user_root, const char *provider,
struct sshkey **token_keys, int nkeys)
{
HKEY root = NULL, sub = NULL;
wchar_t sub_name[MAX_KEY_LENGTH];
- DWORD sub_name_len, blob_len, comment_len;
+ DWORD sub_name_len, blob_len, comment_len, association_len;
u_char *blob = NULL;
- char *comment = NULL;
+ char *comment = NULL, *association = NULL;
struct sshkey *registered = NULL, *cert = NULL;
u_char *plain_added = NULL;
size_t provider_len = strlen(provider);
- int i, index = 0, loaded = 0;
+ int i, index = 0, legacy, loaded = 0;
LSTATUS status;
if (nkeys > 0)
@@ -543,19 +723,37 @@ load_pkcs11_identities(HKEY user_root, const char *provider,
RegQueryValueExW(sub, L"comment", NULL, NULL, NULL,
&comment_len) != ERROR_SUCCESS ||
blob_len == 0 || blob_len > MAX_MESSAGE_SIZE ||
- comment_len != provider_len)
+ comment_len > MAX_MESSAGE_SIZE)
+ continue;
+ status = RegQueryValueExW(sub, L"provider", NULL, NULL, NULL,
+ &association_len);
+ if (status == ERROR_FILE_NOT_FOUND) {
+ legacy = 1;
+ association_len = comment_len;
+ } else if (status == ERROR_SUCCESS &&
+ association_len <= MAX_MESSAGE_SIZE)
+ legacy = 0;
+ else
continue;
free(blob);
free(comment);
+ free(association);
blob = xmalloc(blob_len);
comment = xmalloc((size_t)comment_len + 1);
+ association = xmalloc((size_t)association_len + 1);
if (RegQueryValueExW(sub, L"pub", NULL, NULL, blob,
&blob_len) != ERROR_SUCCESS ||
RegQueryValueExW(sub, L"comment", NULL, NULL,
- (BYTE *)comment, &comment_len) != ERROR_SUCCESS)
+ (BYTE *)comment, &comment_len) != ERROR_SUCCESS ||
+ (!legacy && RegQueryValueExW(sub, L"provider", NULL, NULL,
+ (BYTE *)association, &association_len) != ERROR_SUCCESS))
continue;
comment[comment_len] = '\0';
- if (memcmp(comment, provider, provider_len) != 0)
+ if (legacy)
+ memcpy(association, comment, comment_len);
+ association[association_len] = '\0';
+ if (association_len != provider_len ||
+ memcmp(association, provider, provider_len) != 0)
continue;
sshkey_free(registered);
registered = NULL;
@@ -593,6 +791,7 @@ load_pkcs11_identities(HKEY user_root, const char *provider,
sshkey_free(cert);
sshkey_free(registered);
free(plain_added);
+ free(association);
free(comment);
free(blob);
if (sub != NULL)
@@ -967,12 +1166,14 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
struct agent_connection *con)
{
char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX] = { 0 };
- char allowed_provider[PATH_MAX], *created_identity = NULL;
- char **created_identities = NULL;
+ char allowed_provider[PATH_MAX], **labels = NULL;
+ const char *comment;
int i, j, count = 0, r = 0, request_invalid = 0, success = 0;
int cert_only = 0, identities_stored = 0;
struct sshkey **keys = NULL, **certs = NULL, *cert = NULL;
- size_t k, pin_len = 0, ncerts = 0, ncreated_identities = 0;
+ struct pkcs11_identity_change *identity_change = NULL;
+ struct pkcs11_identity_change **identity_changes = NULL;
+ size_t k, pin_len = 0, ncerts = 0, nidentity_changes = 0;
HKEY user_root = NULL;
pkcs11_init(0);
@@ -1023,7 +1224,7 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
goto done;
}
- count = pkcs11_add_provider(canonical_provider, pin, &keys, NULL);
+ count = pkcs11_add_provider(canonical_provider, pin, &keys, &labels);
if (count <= 0) {
error_f("failed to load provider keys: count:%d", count);
goto done;
@@ -1033,6 +1234,7 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
goto done;
for (i = 0; i < count; i++) {
+ comment = pkcs11_identity_comment(canonical_provider, labels[i]);
for (j = 0; j < (int)ncerts; j++) {
if (!sshkey_is_cert(certs[j]) ||
!sshkey_equal_public(keys[i], certs[j]))
@@ -1040,30 +1242,24 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
if (pkcs11_make_cert(keys[i], certs[j], &cert) != 0)
continue;
if (store_pkcs11_identity(user_root, cert,
- canonical_provider, &created_identity) != 0)
+ canonical_provider, comment, &identity_change) != 0)
goto done;
- if (created_identity != NULL) {
- created_identities = xrecallocarray(created_identities,
- ncreated_identities, ncreated_identities + 1,
- sizeof(*created_identities));
- created_identities[ncreated_identities++] =
- created_identity;
- created_identity = NULL;
- }
+ identity_changes = xrecallocarray(identity_changes,
+ nidentity_changes, nidentity_changes + 1,
+ sizeof(*identity_changes));
+ identity_changes[nidentity_changes++] = identity_change;
+ identity_change = NULL;
sshkey_free(cert);
cert = NULL;
identities_stored++;
}
if (!cert_only && store_pkcs11_identity(user_root, keys[i],
- canonical_provider, &created_identity) == 0) {
- if (created_identity != NULL) {
- created_identities = xrecallocarray(created_identities,
- ncreated_identities, ncreated_identities + 1,
- sizeof(*created_identities));
- created_identities[ncreated_identities++] =
- created_identity;
- created_identity = NULL;
- }
+ canonical_provider, comment, &identity_change) == 0) {
+ identity_changes = xrecallocarray(identity_changes,
+ nidentity_changes, nidentity_changes + 1,
+ sizeof(*identity_changes));
+ identity_changes[nidentity_changes++] = identity_change;
+ identity_change = NULL;
identities_stored++;
} else if (!cert_only)
goto done;
@@ -1082,19 +1278,22 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
r = -1;
if (!success && user_root != NULL)
- rollback_pkcs11_identities(user_root, created_identities,
- ncreated_identities);
+ rollback_pkcs11_identities(user_root, identity_changes,
+ nidentity_changes);
pkcs11_terminate();
sshkey_free(cert);
- free(created_identity);
- for (k = 0; k < ncreated_identities; k++)
- free(created_identities[k]);
- free(created_identities);
+ free_pkcs11_identity_change(identity_change);
+ for (k = 0; k < nidentity_changes; k++)
+ free_pkcs11_identity_change(identity_changes[k]);
+ free(identity_changes);
for (i = 0; i < count; i++)
sshkey_free(keys[i]);
free(keys);
+ for (i = 0; i < count; i++)
+ free(labels[i]);
+ free(labels);
free_pkcs11_certs(certs, ncerts);
free(provider);
if (pin) {
@@ -1134,8 +1333,9 @@ int process_remove_smartcard_key(struct sshbuf* request, struct sshbuf* response
!is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT, canonical_provider))
goto done;
- if (remove_matching_subkeys_from_registry(user_root, SSH_KEYS_ROOT, L"comment", canonical_provider) != 0 ||
- remove_matching_subkeys_from_registry(user_root, SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider) != 0) {
+ if (remove_pkcs11_identities(user_root, canonical_provider) != 0 ||
+ remove_matching_subkeys_from_registry(user_root,
+ SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider) != 0) {
goto done;
}
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index 5296ab77263a..c33b2579c793 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -394,6 +394,44 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
foreach ($publicKeyPath in $publicKeyPaths) {
Test-Path $publicKeyPath | Should Be $true
}
+ Test-Path Env:OPENSSH_TEST_PKCS11_LABELS | Should Be $true
+ $pkcs11Labels = @($env:OPENSSH_TEST_PKCS11_LABELS.Split(
+ [char[]]@(';'), [StringSplitOptions]::None))
+ $pkcs11Labels.Count | Should Be $publicKeyPaths.Count
+ $canonicalProvider = [IO.Path]::GetFullPath($pkcs11Path).Replace('\', '/')
+ $expectedComments = @($pkcs11Labels | ForEach-Object {
+ if ($_) { $_ } else { $canonicalProvider }
+ })
+
+ function Assert-Pkcs11IdentityComments {
+ param([string[]]$KeyPaths, [string[]]$Comments)
+
+ $longListing = @(ssh-add -L)
+ $shortListing = @(ssh-add -l)
+ $KeyPaths.Count | Should Be $Comments.Count
+ $fingerprints = @($KeyPaths | ForEach-Object {
+ ((ssh-keygen -lf $_) -split ' ')[1]
+ })
+ for ($index = 0; $index -lt $KeyPaths.Count; $index++) {
+ $keyBlob = (Get-Content $KeyPaths[$index]).Split(' ')[1]
+ $longEntry = @($longListing | Where-Object {
+ $_.Contains($keyBlob)
+ })
+ $longEntry.Count | Should Be 1
+ ($longEntry[0] -split ' ', 3)[2] | Should Be $Comments[$index]
+
+ $fingerprint = $fingerprints[$index]
+ $shortEntry = @($shortListing | Where-Object {
+ $_.Contains(" $fingerprint ")
+ })
+ $shortEntry.Count | Should Be @($fingerprints |
+ Where-Object { $_ -eq $fingerprint }).Count
+ foreach ($entry in $shortEntry) {
+ $entry | Should Match (" " +
+ [regex]::Escape($Comments[$index]) + " \([^)]+\)$")
+ }
+ }
+ }
$testPin = $env:OPENSSH_TEST_PKCS11_PIN
if (-not $testPin) { $testPin = $pkcs11Pin }
$ca = Join-Path $testDir "pkcs11-ca"
@@ -440,6 +478,9 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
& ssh-add -T $keyPath
$LASTEXITCODE | Should Be 0
}
+ Assert-Pkcs11IdentityComments `
+ ($copiedPublicKeyPaths + $certPaths) `
+ ($expectedComments + $expectedComments)
Restart-Service ssh-agent
WaitForStatus -ServiceName ssh-agent -Status "Running"
@@ -447,6 +488,9 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
& ssh-add -T $certPath
$LASTEXITCODE | Should Be 0
}
+ Assert-Pkcs11IdentityComments `
+ ($copiedPublicKeyPaths + $certPaths) `
+ ($expectedComments + $expectedComments)
& ssh-add -d $certPaths[0]
$LASTEXITCODE | Should Be 0
$deletedKeyBlob = (Get-Content $certPaths[0]).Split(' ')[1]
@@ -477,6 +521,9 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$sequentialCertBlob = (Get-Content $sequentialCertPath).Split(' ')[1]
@((ssh-add -L) | Where-Object { $_.Contains($sequentialCertBlob) }).Count |
Should Be 1
+ Assert-Pkcs11IdentityComments `
+ @($certPaths[0], $sequentialCertPath) `
+ @($expectedComments[0], $expectedComments[0])
ssh-add -D
$LASTEXITCODE | Should Be 0
@@ -491,6 +538,9 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$keyBlob = (Get-Content $keyPath).Split(' ')[1]
@($allKeys | Where-Object { $_.Contains($keyBlob) }).Count | Should Be 1
}
+ Assert-Pkcs11IdentityComments `
+ ($copiedPublicKeyPaths + $certPaths[0]) `
+ ($expectedComments + $expectedComments[0])
# A failed unmatched add must not change existing persisted identities.
$identitiesBefore = @(ssh-add -L | Sort-Object)
@@ -505,6 +555,9 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
& ssh-add -T $keyPath
$LASTEXITCODE | Should Be 0
}
+ Assert-Pkcs11IdentityComments `
+ ($copiedPublicKeyPaths + $certPaths[0]) `
+ ($expectedComments + $expectedComments[0])
& ssh-add -d $certPaths[0]
$LASTEXITCODE | Should Be 0
@@ -514,6 +567,88 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
Should Be 1
}
+ # Legacy identities used comment for their provider association.
+ $identityRootPath = "$currentUserSid\Software\OpenSSH\Agent\Keys"
+ $identityRoot = [Microsoft.Win32.Registry]::Users.OpenSubKey(
+ $identityRootPath, $true)
+ $identityRoot | Should Not Be $null
+ $plainBlob = (Get-Content $copiedPublicKeyPaths[0]).Split(' ')[1]
+ $identityKey = $null
+ foreach ($identityName in $identityRoot.GetSubKeyNames()) {
+ $candidate = $identityRoot.OpenSubKey($identityName, $true)
+ $storedBlob = $candidate.GetValue("pub")
+ if ($storedBlob -is [byte[]] -and
+ [Convert]::ToBase64String($storedBlob) -eq $plainBlob) {
+ $identityKey = $candidate
+ break
+ }
+ $candidate.Dispose()
+ }
+ $identityKey | Should Not Be $null
+ $providerBytes = [Text.Encoding]::UTF8.GetBytes($canonicalProvider)
+ $identityKey.DeleteValue("provider", $false)
+ $identityKey.SetValue("comment", $providerBytes,
+ [Microsoft.Win32.RegistryValueKind]::Binary)
+
+ Restart-Service ssh-agent
+ WaitForStatus -ServiceName ssh-agent -Status "Running"
+ Assert-Pkcs11IdentityComments @($copiedPublicKeyPaths[0]) `
+ @($canonicalProvider)
+ & ssh-add -T $copiedPublicKeyPaths[0]
+ $LASTEXITCODE | Should Be 0
+ $identityKey.GetValue("provider", $null) | Should Be $null
+ [Text.Encoding]::UTF8.GetString($identityKey.GetValue("comment")) |
+ Should Be $canonicalProvider
+
+ # A failed provider update must roll legacy metadata back.
+ $providerRootPath = "$currentUserSid\Software\OpenSSH\Agent\PKCS11_Providers"
+ $providerRoot = [Microsoft.Win32.Registry]::Users.OpenSubKey(
+ $providerRootPath, $true)
+ $providerRoot | Should Not Be $null
+ $providerKey = $providerRoot.OpenSubKey($canonicalProvider,
+ [Microsoft.Win32.RegistryKeyPermissionCheck]::ReadWriteSubTree,
+ [Security.AccessControl.RegistryRights]::FullControl)
+ $providerKey | Should Not Be $null
+ $blockedAcl = $providerKey.GetAccessControl()
+ $denySetValue = New-Object `
+ System.Security.AccessControl.RegistryAccessRule(
+ $systemSid,
+ [System.Security.AccessControl.RegistryRights]::SetValue,
+ [System.Security.AccessControl.AccessControlType]::Deny)
+ $blockedAcl.AddAccessRule($denySetValue) | Out-Null
+ try {
+ $providerKey.SetAccessControl($blockedAcl)
+ & ssh-add -s $pkcs11Path
+ $LASTEXITCODE | Should Not Be 0
+ $identityKey.GetValue("provider", $null) | Should Be $null
+ [Text.Encoding]::UTF8.GetString(
+ $identityKey.GetValue("comment")) |
+ Should Be $canonicalProvider
+ }
+ finally {
+ $blockedAcl.RemoveAccessRuleSpecific($denySetValue)
+ $providerKey.SetAccessControl($blockedAcl)
+ }
+
+ # Re-adding migrates metadata without replacing the key entry.
+ & ssh-add -s $pkcs11Path
+ $LASTEXITCODE | Should Be 0
+ [Text.Encoding]::UTF8.GetString($identityKey.GetValue("provider")) |
+ Should Be $canonicalProvider
+ [Text.Encoding]::UTF8.GetString($identityKey.GetValue("comment")) |
+ Should Be $expectedComments[0]
+ Assert-Pkcs11IdentityComments @($copiedPublicKeyPaths[0]) `
+ @($expectedComments[0])
+
+ # Provider removal accepts both migrated and legacy identities.
+ $identityKey.DeleteValue("provider", $false)
+ $identityKey.SetValue("comment", $providerBytes,
+ [Microsoft.Win32.RegistryValueKind]::Binary)
+ $providerKey.Dispose()
+ $providerRoot.Dispose()
+ $identityKey.Dispose()
+ $identityRoot.Dispose()
+
& ssh-add -e $pkcs11Path
$LASTEXITCODE | Should Be 0
@(ssh-add -L) -match "The agent has no identities." | Should Be $true
diff --git a/regress/pesterTests/README.md b/regress/pesterTests/README.md
index b0885e3249fa..cd9ac12d0871 100644
--- a/regress/pesterTests/README.md
+++ b/regress/pesterTests/README.md
@@ -74,6 +74,9 @@ following environment variables are set before running the E2E tests:
* `OPENSSH_TEST_PKCS11_PIN`: token PIN.
* `OPENSSH_TEST_PKCS11_PUBLIC_KEYS`: semicolon-separated public-key files whose
corresponding private keys are present on the token.
+* `OPENSSH_TEST_PKCS11_LABELS`: semicolon-separated labels corresponding
+ positionally to `OPENSSH_TEST_PKCS11_PUBLIC_KEYS`. An empty item expects the
+ canonical provider path fallback.
The test creates short-lived OpenSSH certificates for the supplied public
keys. It verifies plain and certificate identities, signing, agent service
diff --git a/regress/unittests/win32compat/pkcs11_cert_tests.c b/regress/unittests/win32compat/pkcs11_cert_tests.c
index f0e1cfb74de9..c863d7e7d51a 100644
--- a/regress/unittests/win32compat/pkcs11_cert_tests.c
+++ b/regress/unittests/win32compat/pkcs11_cert_tests.c
@@ -169,6 +169,19 @@ test_pkcs11_cert_identity_name(void)
TEST_DONE();
}
+static void
+test_pkcs11_identity_comment(void)
+{
+ const char *provider = "C:/provider.dll";
+
+ TEST_START("PKCS11 identity comment fallback");
+ ASSERT_STRING_EQ(pkcs11_identity_comment(provider, "token label"),
+ "token label");
+ ASSERT_STRING_EQ(pkcs11_identity_comment(provider, ""), provider);
+ ASSERT_STRING_EQ(pkcs11_identity_comment(provider, NULL), provider);
+ TEST_DONE();
+}
+
static void
test_pkcs11_cert_constraints_duplicate(void)
{
@@ -262,6 +275,7 @@ pkcs11_cert_tests(void)
test_pkcs11_cert_constraints_valid();
test_pkcs11_cert_constraints_compatible();
test_pkcs11_cert_identity_name();
+ test_pkcs11_identity_comment();
test_pkcs11_cert_constraints_duplicate();
test_pkcs11_cert_constraints_truncated();
test_pkcs11_cert_constraints_malformed();
From ef4a6a7084339fa967ce88f580c3e9d8e634fd7b Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Fri, 17 Jul 2026 18:25:10 +0200
Subject: [PATCH 06/16] Harden persisted PKCS#11 provider reload
Skip persisted provider records with invalid or oversized Registry metadata while rebuilding the transient signing key set. This keeps corrupt records from causing unbounded allocations or blocking unrelated identities.
Extend the stale-provider regression to cover oversized encrypted PIN data.
---
.../win32compat/ssh-agent/keyagent-request.c | 6 +++++-
regress/pesterTests/KeyUtils.Tests.ps1 | 18 ++++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index b98d34cbc82d..c66ad9e74efe 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -940,6 +940,7 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
while (1) {
sub_name_len = MAX_KEY_LENGTH;
pin_len = epin_len = provider_len = 0;
+ epin_alloc_len = 0;
if (sub) {
RegCloseKey(sub);
sub = NULL;
@@ -948,6 +949,9 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
if (RegOpenKeyExW(root, sub_name, 0, KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) == 0 &&
RegQueryValueExW(sub, L"provider", 0, NULL, NULL, &provider_len) == 0 &&
RegQueryValueExW(sub, L"pin", 0, NULL, NULL, &epin_len) == 0) {
+ if (provider_len == 0 || provider_len >= PATH_MAX ||
+ epin_len == 0 || epin_len > MAX_MESSAGE_SIZE)
+ continue;
epin_alloc_len = epin_len;
if ((epin = malloc(epin_alloc_len + 1)) == NULL ||
(provider = malloc(provider_len + 1)) == NULL ||
@@ -1018,7 +1022,7 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
if (signature)
free(signature);
#ifdef ENABLE_PKCS11
- free_pkcs11_sign_provider(&provider, &pin, pin_len, &epin, epin_len,
+ free_pkcs11_sign_provider(&provider, &pin, pin_len, &epin, epin_alloc_len,
&keys, count);
del_all_keys();
pkcs11_terminate();
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index c33b2579c793..14dc13fb24d9 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -681,6 +681,10 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
"nonexistent\openssh-corrupt-provider.dll"
$corruptProvider = [IO.Path]::GetFullPath(
$corruptProviderPath).Replace('\', '/')
+ $oversizedProviderPath = Join-Path $testDir `
+ "nonexistent\openssh-oversized-provider.dll"
+ $oversizedProvider = [IO.Path]::GetFullPath(
+ $oversizedProviderPath).Replace('\', '/')
try {
ssh-add -D
@@ -738,6 +742,19 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
$LASTEXITCODE | Should Be 0
& ssh-add -T $publicKeyPaths[0]
$LASTEXITCODE | Should Be 0
+
+ $staleProviderKey.Dispose()
+ $staleProviderKey = $providerRoot.CreateSubKey($oversizedProvider)
+ $staleProviderKey.SetValue("provider",
+ [Text.Encoding]::UTF8.GetBytes($oversizedProvider),
+ [Microsoft.Win32.RegistryValueKind]::Binary)
+ $staleProviderKey.SetValue("pin", (New-Object byte[] 11000),
+ [Microsoft.Win32.RegistryValueKind]::Binary)
+
+ & ssh-add -T $softwareKeyPath
+ $LASTEXITCODE | Should Be 0
+ & ssh-add -T $publicKeyPaths[0]
+ $LASTEXITCODE | Should Be 0
}
finally {
if ($staleProviderKey) { $staleProviderKey.Dispose() }
@@ -745,6 +762,7 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
if ($providerRoot) {
$providerRoot.DeleteSubKeyTree($staleProvider, $false)
$providerRoot.DeleteSubKeyTree($corruptProvider, $false)
+ $providerRoot.DeleteSubKeyTree($oversizedProvider, $false)
$providerRoot.Dispose()
}
ssh-add -D | Out-Null
From 1cb677e99b3c0c2a67df11de7e335632d5ad7b66 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Fri, 17 Jul 2026 19:31:26 +0200
Subject: [PATCH 07/16] Harden Windows PKCS#11 registry helpers
Keep SECURITY_ATTRIBUTES.nLength stable when converting Registry security descriptors by using a separate descriptor-size variable. Also wipe the full encrypted PIN allocation during provider reload cleanup.
---
.../win32/win32compat/ssh-agent/keyagent-request.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index c66ad9e74efe..4e3088b16371 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -455,6 +455,7 @@ store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
char *thumbprint = NULL;
struct pkcs11_identity_change *change = NULL;
DWORD disposition = 0;
+ ULONG sd_len = 0;
int success = 0;
if (changep == NULL || provider == NULL || comment == NULL)
@@ -462,7 +463,7 @@ store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
*changep = NULL;
sa.nLength = sizeof(sa);
if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
- SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength) ||
+ SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sd_len) ||
sshkey_to_blob(key, &blob, &blob_len) != 0 ||
blob_len == 0 || blob_len > MAX_MESSAGE_SIZE ||
(thumbprint = pkcs11_identity_name(key, blob, blob_len)) == NULL ||
@@ -545,11 +546,12 @@ store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
char *epin = NULL;
DWORD epin_len = 0;
DWORD disposition = 0;
+ ULONG sd_len = 0;
int success = 0;
sa.nLength = sizeof(sa);
if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
- SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength) ||
+ SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sd_len) ||
convert_blob(con, pin, (DWORD)pin_len, &epin, &epin_len, TRUE) != 0 ||
RegCreateKeyExW(user_root, SSH_PKCS11_PROVIDERS_ROOT, 0, NULL, 0,
KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
@@ -966,7 +968,7 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
if (convert_blob(con, epin, epin_len, &pin, &pin_len, 0) != 0 ||
(npin = realloc(pin, pin_len + 1)) == NULL) {
free_pkcs11_sign_provider(&provider, &pin, pin_len,
- &epin, epin_len, &keys, count);
+ &epin, epin_alloc_len, &keys, count);
continue;
}
pin = npin;
@@ -974,13 +976,13 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
count = pkcs11_add_provider(provider, pin, &keys, NULL);
if (count <= 0) {
free_pkcs11_sign_provider(&provider, &pin, pin_len,
- &epin, epin_len, &keys, count);
+ &epin, epin_alloc_len, &keys, count);
continue;
}
loaded = load_pkcs11_identities(user_root, provider,
keys, count);
free_pkcs11_sign_provider(&provider, &pin, pin_len,
- &epin, epin_len, &keys, count);
+ &epin, epin_alloc_len, &keys, count);
if (loaded < 0)
goto done;
}
From 05838248114e468c1c8780f6f0314145da580c71 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 15:37:12 +0200
Subject: [PATCH 08/16] Fix PKCS#11 helper cleanup ordering
---
contrib/win32/win32compat/ssh-agent/keyagent-request.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index 4e3088b16371..74b333b0b2fc 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -1287,8 +1287,6 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
rollback_pkcs11_identities(user_root, identity_changes,
nidentity_changes);
- pkcs11_terminate();
-
sshkey_free(cert);
free_pkcs11_identity_change(identity_change);
for (k = 0; k < nidentity_changes; k++)
@@ -1301,6 +1299,7 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
free(labels[i]);
free(labels);
free_pkcs11_certs(certs, ncerts);
+ pkcs11_terminate();
free(provider);
if (pin) {
SecureZeroMemory(pin, (DWORD)pin_len);
From 678cc42c4d703fd8cf6fb93d3a8b0d2924f28651 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 15:37:35 +0200
Subject: [PATCH 09/16] Enable PKCS#11 certificate regressions on Windows
---
contrib/win32/openssh/bash_tests_iterator.ps1 | 17 +++
regress/ssh-pkcs11.sh | 9 ++
regress/test-exec.sh | 110 ++++++++++++++----
3 files changed, 111 insertions(+), 25 deletions(-)
diff --git a/contrib/win32/openssh/bash_tests_iterator.ps1 b/contrib/win32/openssh/bash_tests_iterator.ps1
index 570173df715e..beabd9d8a3f0 100644
--- a/contrib/win32/openssh/bash_tests_iterator.ps1
+++ b/contrib/win32/openssh/bash_tests_iterator.ps1
@@ -17,6 +17,7 @@ $ErrorActionPreference = 'Continue'
# Resolve the relative paths
$OpenSSHBinPath = Resolve-Path $OpenSSHBinPath -ErrorAction Stop | select -ExpandProperty Path
$BashTestsPath = Resolve-Path $BashTestsPath -ErrorAction Stop | select -ExpandProperty Path
+$BashTestsWindowsPath = $BashTestsPath
$ShellPath = Resolve-Path $ShellPath -ErrorAction Stop | select -ExpandProperty Path
$ArtifactsDirectoryPath = Resolve-Path $ArtifactsDirectoryPath -ErrorAction Stop | select -ExpandProperty Path
if ($TestFilePath) {
@@ -25,6 +26,7 @@ if ($TestFilePath) {
$TestFilePath = $TestFilePath -replace "\\","/"
}
$OriginalSystemPath = [System.Environment]::GetEnvironmentVariable('Path', [System.EnvironmentVariableTarget]::Machine)
+$OriginalUserSoftHsmConf = [System.Environment]::GetEnvironmentVariable('SOFTHSM2_CONF', [System.EnvironmentVariableTarget]::User)
# Make sure config.h exists. It is used in some bashstests (Ex - sftp-glob.sh, cfgparse.sh)
# first check in $BashTestsPath folder. If not then it's parent folder. If not then in the $OpenSSHBinPath
@@ -62,6 +64,12 @@ if(!$SkipInstallSSHD) {
# We need ssh-agent to be installed as service to run some bash tests.
& "$OpenSSHBinPath\install-sshd.ps1"
+ if (-not [string]::IsNullOrEmpty($env:TEST_SSH_PKCS11_PROVIDER)) {
+ $testProvider = (& $ShellPath -c "cygpath -w '$env:TEST_SSH_PKCS11_PROVIDER'").Trim()
+ $agentImagePath = '"{0}" -P "{1}"' -f (Join-Path $OpenSSHBinPath 'ssh-agent.exe'), $testProvider
+ Set-ItemProperty -Path 'HKLM:\SYSTEM\CurrentControlSet\Services\ssh-agent' `
+ -Name ImagePath -Value $agentImagePath -Force
+ }
}
try
@@ -147,6 +155,10 @@ try
$env:TEST_SSH_SFTP = $OpenSSHBinPath_shell_fmt+"/sftp.exe"
$env:TEST_SSH_SFTPSERVER = $OpenSSHBinPath_shell_fmt+"/sftp-server.exe"
$env:TEST_SSH_SCP = $OpenSSHBinPath_shell_fmt+"/scp.exe"
+ $env:TEST_SSH_OPENSSL = (&$ShellPath -c "command -v openssl").Trim()
+ if ([string]::IsNullOrEmpty($env:TEST_SSH_OPENSSL)) {
+ throw "openssl was not found in the test shell"
+ }
$env:BUILDDIR = $BUILDDIR
$env:TEST_WINDOWS_SSH = 1
$env:TEST_SSH_ASKPASS = $TEST_SSH_ASKPASS
@@ -173,6 +185,10 @@ try
$temp_test_path = "temp_test"
$null = Remove-Item -Recurse -Force $temp_test_path -ErrorAction SilentlyContinue
$null = New-Item -ItemType directory -Path $temp_test_path -Force -ErrorAction Stop
+ if (-not [string]::IsNullOrEmpty($env:TEST_SSH_PKCS11_PROVIDER)) {
+ $testSoftHsmConf = Join-Path $BashTestsWindowsPath "$temp_test_path\SOFTHSM\softhsm2.conf"
+ [System.Environment]::SetEnvironmentVariable('SOFTHSM2_CONF', $testSoftHsmConf, [System.EnvironmentVariableTarget]::User)
+ }
# remove the summary, output files.
$bash_test_summary = "$ArtifactsDirectoryPath\bash_tests_summary.txt"
@@ -271,6 +287,7 @@ finally
{
# Restore User Path variable in the registry once the tests finish running.
[System.Environment]::SetEnvironmentVariable('Path', $OriginalSystemPath, [System.EnvironmentVariableTarget]::Machine)
+ [System.Environment]::SetEnvironmentVariable('SOFTHSM2_CONF', $OriginalUserSoftHsmConf, [System.EnvironmentVariableTarget]::User)
# remove temp test directory
if (!$SkipCleanup)
{
diff --git a/regress/ssh-pkcs11.sh b/regress/ssh-pkcs11.sh
index 96680fca9f74..dab012d212f0 100644
--- a/regress/ssh-pkcs11.sh
+++ b/regress/ssh-pkcs11.sh
@@ -17,6 +17,15 @@ check_all() {
for k in $ED25519 $RSA $EC; do
kshort=`basename "$k"`
verbose "$tag: $kshort"
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ if test "$expect_success" = "y"; then
+ ASKPASS_PASSWORD="$TEST_SSH_PIN"
+ else
+ ASKPASS_PASSWORD="0000"
+ fi
+ export ASKPASS_PASSWORD
+ pinsh="$TEST_SSH_ASKPASS"
+ fi
pub="$k.pub"
cp $pub $OBJ/key.pub
chmod 0600 $OBJ/key.pub
diff --git a/regress/test-exec.sh b/regress/test-exec.sh
index 965018fcbe7e..38635a8cf9d3 100644
--- a/regress/test-exec.sh
+++ b/regress/test-exec.sh
@@ -1028,6 +1028,17 @@ p11_find_lib() {
done
}
+p11_make_public() {
+ chmod 600 "$1" || fatal "chmod private key failed"
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ /usr/bin/ssh-keygen -y -f "$1" > "$1.pub" || \
+ fatal "Cygwin ssh-keygen public key extraction failed"
+ else
+ ${SSHKEYGEN} -y -f "$1" > "$1.pub" || \
+ fatal "ssh-keygen public key extraction failed"
+ fi
+}
+
# Perform PKCS#11 setup: prepares a softhsm2 token configuration, generated
# keys and loads them into the virtual token.
PKCS11_OK=
@@ -1036,10 +1047,22 @@ p11_setup() {
# XXX we could potentially test ed25519 only in the absence of
# RSA and ECDSA support.
$SSH -Q key | grep ssh-rsa >/dev/null || return 1
- p11_find_lib \
- /usr/local/lib/softhsm/libsofthsm2.so \
- /usr/lib64/pkcs11/libsofthsm2.so \
- /usr/lib/x86_64-linux-gnu/softhsm/libsofthsm2.so
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ if test -n "$TEST_SSH_PKCS11_PROVIDER"; then
+ p11_find_lib "$TEST_SSH_PKCS11_PROVIDER"
+ else
+ p11_find_lib \
+ "/cygdrive/c/Program Files/SoftHSM2/lib/softhsm2-x64.dll" \
+ "/cygdrive/c/Program Files/SoftHSM2/lib/softhsm2.dll"
+ fi
+ SOFTHSM2_UTIL="${TEST_SSH_SOFTHSM2_UTIL:-/cygdrive/c/Program Files/SoftHSM2/bin/softhsm2-util.exe}"
+ else
+ p11_find_lib \
+ /usr/local/lib/softhsm/libsofthsm2.so \
+ /usr/lib64/pkcs11/libsofthsm2.so \
+ /usr/lib/x86_64-linux-gnu/softhsm/libsofthsm2.so
+ SOFTHSM2_UTIL=softhsm2-util
+ fi
test -z "$TEST_SSH_PKCS11" && return 1
trace "using token library $TEST_SSH_PKCS11"
TEST_SSH_PIN=1234
@@ -1056,18 +1079,26 @@ p11_setup() {
TOKEN=$SSH_SOFTHSM_DIR/tokendir
mkdir -p $TOKEN
SOFTHSM2_CONF=$SSH_SOFTHSM_DIR/softhsm2.conf
+ SOFTHSM2_CONF_FILE=$SOFTHSM2_CONF
+ TOKEN_CONFIG=$TOKEN
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ TOKEN_CONFIG=$(cygpath -w "$TOKEN")
+ SOFTHSM2_CONF=$(cygpath -w "$SOFTHSM2_CONF")
+ TEST_SSH_PKCS11=$(cygpath -w "$TEST_SSH_PKCS11")
+ fi
export SOFTHSM2_CONF
- cat > $SOFTHSM2_CONF << EOF
+ cat > "$SOFTHSM2_CONF_FILE" << EOF
# SoftHSM v2 configuration file
-directories.tokendir = ${TOKEN}
+directories.tokendir = ${TOKEN_CONFIG}
objectstore.backend = file
# ERROR, WARNING, INFO, DEBUG
log.level = DEBUG
# If CKF_REMOVABLE_DEVICE flag should be set
slots.removable = false
EOF
- out=$(softhsm2-util --init-token --free --label token-slot-0 --pin "$TEST_SSH_PIN" --so-pin "$TEST_SSH_SOPIN")
- slot=$(echo -- $out | sed 's/.* //')
+ out=$("$SOFTHSM2_UTIL" --module "$TEST_SSH_PKCS11" --init-token --free \
+ --label token-slot-0 --pin "$TEST_SSH_PIN" --so-pin "$TEST_SSH_SOPIN")
+ slot=$(echo -- $out | tr -d '\r' | sed 's/.* //')
trace "generating keys"
# RSA key
RSA=${SSH_SOFTHSM_DIR}/RSA
@@ -1075,10 +1106,14 @@ EOF
$OPENSSL_BIN genpkey -algorithm rsa > $RSA 2>/dev/null || \
fatal "genpkey RSA fail"
$OPENSSL_BIN pkcs8 -nocrypt -in $RSA > $RSAP8 || fatal "pkcs8 RSA fail"
- softhsm2-util --slot "$slot" --label 01 --id 01 --pin "$TEST_SSH_PIN" \
- --import $RSAP8 >/dev/null || fatal "softhsm import RSA fail"
- chmod 600 $RSA
- ${SSHKEYGEN} -y -f $RSA > ${RSA}.pub
+ RSAP8_IMPORT=$RSAP8
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ RSAP8_IMPORT=$(cygpath -w "$RSAP8")
+ fi
+ "$SOFTHSM2_UTIL" --module "$TEST_SSH_PKCS11" --slot "$slot" \
+ --label 01 --id 01 --pin "$TEST_SSH_PIN" \
+ --import "$RSAP8_IMPORT" >/dev/null || fatal "softhsm import RSA fail"
+ p11_make_public $RSA
# ECDSA key
ECPARAM=${SSH_SOFTHSM_DIR}/ECPARAM
EC=${SSH_SOFTHSM_DIR}/EC
@@ -1089,10 +1124,14 @@ EOF
$OPENSSL_BIN genpkey -paramfile $ECPARAM > $EC || \
fatal "genpkey EC fail"
$OPENSSL_BIN pkcs8 -nocrypt -in $EC > $ECP8 || fatal "pkcs8 EC fail"
- softhsm2-util --slot "$slot" --label 02 --id 02 --pin "$TEST_SSH_PIN" \
- --import $ECP8 >/dev/null || fatal "softhsm import EC fail"
- chmod 600 $EC
- ${SSHKEYGEN} -y -f $EC > ${EC}.pub
+ ECP8_IMPORT=$ECP8
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ ECP8_IMPORT=$(cygpath -w "$ECP8")
+ fi
+ "$SOFTHSM2_UTIL" --module "$TEST_SSH_PKCS11" --slot "$slot" \
+ --label 02 --id 02 --pin "$TEST_SSH_PIN" \
+ --import "$ECP8_IMPORT" >/dev/null || fatal "softhsm import EC fail"
+ p11_make_public $EC
# Ed25519 key
ED25519=${SSH_SOFTHSM_DIR}/ED25519
ED25519P8=${SSH_SOFTHSM_DIR}/ED25519P8
@@ -1100,11 +1139,15 @@ EOF
fatal "genpkey Ed25519 fail"
$OPENSSL_BIN pkcs8 -nocrypt -in $ED25519 > $ED25519P8 || \
fatal "pkcs8 Ed25519 fail"
- softhsm2-util --slot "$slot" --label 03 --id 03 --pin "$TEST_SSH_PIN" \
- --import $ED25519P8 >/dev/null || \
+ ED25519P8_IMPORT=$ED25519P8
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ ED25519P8_IMPORT=$(cygpath -w "$ED25519P8")
+ fi
+ "$SOFTHSM2_UTIL" --module "$TEST_SSH_PKCS11" --slot "$slot" \
+ --label 03 --id 03 --pin "$TEST_SSH_PIN" \
+ --import "$ED25519P8_IMPORT" >/dev/null || \
fatal "softhsm import ed25519 fail"
- chmod 600 $ED25519
- ${SSHKEYGEN} -y -f $ED25519 > ${ED25519}.pub
+ p11_make_public $ED25519
# Prepare some askpass scripts to load PINs.
PIN_SH=$SSH_SOFTHSM_DIR/pin.sh
cat > $PIN_SH << EOF
@@ -1128,7 +1171,12 @@ EOF
# Peforms ssh-add with the right token PIN.
p11_ssh_add() {
- env SSH_ASKPASS="$PIN_SH" SSH_ASKPASS_REQUIRE=force ${SSHADD} "$@"
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ env ASKPASS_PASSWORD="$TEST_SSH_PIN" SSH_ASKPASS="$TEST_SSH_ASKPASS" \
+ SSH_ASKPASS_REQUIRE=force ${SSHADD} "$@"
+ else
+ env SSH_ASKPASS="$PIN_SH" SSH_ASKPASS_REQUIRE=force ${SSHADD} "$@"
+ fi
}
start_ssh_agent() {
@@ -1140,10 +1188,22 @@ start_ssh_agent() {
export SSH_AUTH_SOCK
rm -f $SSH_AUTH_SOCK $OBJ/agent.log
trace "start agent"
- ${SSHAGENT} ${EXTRA_AGENT_ARGS} -d -a $SSH_AUTH_SOCK \
- > $OBJ/agent.log 2>&1 &
- AGENT_PID=$!
- trap "kill $AGENT_PID" EXIT
+ if test "x$TEST_WINDOWS_SSH" = "x1"; then
+ unset SSH_AUTH_SOCK
+ ${SSHAGENT} > $OBJ/agent.log 2>&1
+ if test "$PKCS11_OK" = "yes"; then
+ ${SSHADD} -e "$TEST_SSH_PKCS11" >/dev/null 2>&1
+ powershell.exe -NoProfile -NonInteractive -Command \
+ "Stop-Service ssh-agent -Force" >/dev/null 2>&1 || \
+ fatal "failed to reset ssh-agent service"
+ ${SSHAGENT} >> $OBJ/agent.log 2>&1
+ fi
+ else
+ ${SSHAGENT} ${EXTRA_AGENT_ARGS} -d -a $SSH_AUTH_SOCK \
+ > $OBJ/agent.log 2>&1 &
+ AGENT_PID=$!
+ trap "kill $AGENT_PID" EXIT
+ fi
for x in 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 ; do
# Give it a chance to start
${SSHADD} -l > /dev/null 2>&1
From 0fda304cb1aaf767f849df3d3126ae8dcde29ae6 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 16:28:22 +0200
Subject: [PATCH 10/16] Compare PKCS#11 provider paths case-insensitively
Provider paths are case-insensitive on Windows, but identities were
matched against the stored provider byte for byte, and the provider
entry itself by prefix. Re-adding a provider with different casing
orphaned its identities or left the provider and its encrypted PIN
behind after removal.
Match the complete stored value case-insensitively everywhere through
pkcs11_provider_equal().
Co-Authored-By: Claude Sonnet 5.5
---
contrib/win32/win32compat/pkcs11-cert.c | 15 +++++++++++
contrib/win32/win32compat/pkcs11-cert.h | 1 +
.../win32compat/ssh-agent/keyagent-request.c | 14 +++++-----
regress/pesterTests/KeyUtils.Tests.ps1 | 21 +++++++++++++++
.../unittests/win32compat/pkcs11_cert_tests.c | 26 +++++++++++++++++++
5 files changed, 69 insertions(+), 8 deletions(-)
diff --git a/contrib/win32/win32compat/pkcs11-cert.c b/contrib/win32/win32compat/pkcs11-cert.c
index ff789e7b96b8..2b5ed161301a 100644
--- a/contrib/win32/win32compat/pkcs11-cert.c
+++ b/contrib/win32/win32compat/pkcs11-cert.c
@@ -54,6 +54,21 @@ pkcs11_identity_comment(const char *provider, const char *label)
return label == NULL || *label == '\0' ? provider : label;
}
+/*
+ * Compare a provider path stored in the Registry, which is not NUL
+ * terminated, with a canonical provider path. Windows paths are case
+ * insensitive, and the whole value must match.
+ */
+int
+pkcs11_provider_equal(const u_char *stored, size_t stored_len,
+ const char *provider)
+{
+ if (stored == NULL || provider == NULL || stored_len == 0 ||
+ strlen(provider) != stored_len)
+ return 0;
+ return strncasecmp((const char *)stored, provider, stored_len) == 0;
+}
+
void
free_pkcs11_certs(struct sshkey **certs, size_t ncerts)
{
diff --git a/contrib/win32/win32compat/pkcs11-cert.h b/contrib/win32/win32compat/pkcs11-cert.h
index c6019672cc61..9c7e22b2db9e 100644
--- a/contrib/win32/win32compat/pkcs11-cert.h
+++ b/contrib/win32/win32compat/pkcs11-cert.h
@@ -23,6 +23,7 @@
char *pkcs11_identity_name(const struct sshkey *, const u_char *, size_t);
const char *pkcs11_identity_comment(const char *, const char *);
+int pkcs11_provider_equal(const u_char *, size_t, const char *);
int parse_pkcs11_add_constraints(struct sshbuf *, int *,
struct sshkey ***, size_t *);
void free_pkcs11_certs(struct sshkey **, size_t);
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index 74b333b0b2fc..9720a0996784 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -34,9 +34,9 @@
#include "config.h"
#include "match.h"
#include
+#include "pkcs11-cert.h"
#ifdef ENABLE_PKCS11
#include "ssh-pkcs11.h"
-#include "pkcs11-cert.h"
#endif
#include "xmalloc.h"
@@ -172,7 +172,8 @@ remove_matching_subkeys_from_registry(HKEY user_root, wchar_t const* key_name, w
RegQueryValueExW(sub, value_name_to_remove, 0, NULL, data, &data_len) != 0)
goto done;
data[data_len] = '\0';
- if (strncmp(data, value_data_to_remove, data_len) == 0) {
+ if (pkcs11_provider_equal((u_char *)data, data_len,
+ value_data_to_remove)) {
if (RegDeleteTreeW(root, sub_name) != 0)
goto done;
--index;
@@ -626,7 +627,6 @@ remove_pkcs11_identities(HKEY user_root, const char *provider)
wchar_t sub_name[MAX_KEY_LENGTH];
DWORD sub_name_len, type, data_len;
u_char *data = NULL;
- size_t provider_len = strlen(provider);
int index = 0, present, remove;
LSTATUS status;
@@ -662,8 +662,7 @@ remove_pkcs11_identities(HKEY user_root, const char *provider)
index++;
continue;
}
- remove = present && data_len == provider_len &&
- memcmp(data, provider, provider_len) == 0;
+ remove = present && pkcs11_provider_equal(data, data_len, provider);
RegCloseKey(sub);
sub = NULL;
if (remove) {
@@ -691,7 +690,6 @@ load_pkcs11_identities(HKEY user_root, const char *provider,
char *comment = NULL, *association = NULL;
struct sshkey *registered = NULL, *cert = NULL;
u_char *plain_added = NULL;
- size_t provider_len = strlen(provider);
int i, index = 0, legacy, loaded = 0;
LSTATUS status;
@@ -754,8 +752,8 @@ load_pkcs11_identities(HKEY user_root, const char *provider,
if (legacy)
memcpy(association, comment, comment_len);
association[association_len] = '\0';
- if (association_len != provider_len ||
- memcmp(association, provider, provider_len) != 0)
+ if (!pkcs11_provider_equal((u_char *)association, association_len,
+ provider))
continue;
sshkey_free(registered);
registered = NULL;
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index 14dc13fb24d9..8641b26d8ad7 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -491,6 +491,27 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
Assert-Pkcs11IdentityComments `
($copiedPublicKeyPaths + $certPaths) `
($expectedComments + $expectedComments)
+
+ # Provider paths and Registry key names are case-insensitive on
+ # Windows. Re-adding only one certificate with alternate casing
+ # must not orphan the identities that retain the original path.
+ $caseVariantProvider = ([IO.Path]::GetFullPath(
+ $pkcs11Path)).ToUpperInvariant()
+ & ssh-add -s $caseVariantProvider -C $certPaths[0]
+ $LASTEXITCODE | Should Be 0
+ Restart-Service ssh-agent
+ WaitForStatus -ServiceName ssh-agent -Status "Running"
+ foreach ($keyPath in $copiedPublicKeyPaths + $certPaths) {
+ & ssh-add -T $keyPath
+ $LASTEXITCODE | Should Be 0
+ }
+ & ssh-add -e $caseVariantProvider
+ $LASTEXITCODE | Should Be 0
+ @(ssh-add -L) -match "The agent has no identities." | Should Be $true
+
+ # Restore the complete set for the remaining deletion scenarios.
+ & ssh-add @addArguments
+ $LASTEXITCODE | Should Be 0
& ssh-add -d $certPaths[0]
$LASTEXITCODE | Should Be 0
$deletedKeyBlob = (Get-Content $certPaths[0]).Split(' ')[1]
diff --git a/regress/unittests/win32compat/pkcs11_cert_tests.c b/regress/unittests/win32compat/pkcs11_cert_tests.c
index c863d7e7d51a..30c05fdada41 100644
--- a/regress/unittests/win32compat/pkcs11_cert_tests.c
+++ b/regress/unittests/win32compat/pkcs11_cert_tests.c
@@ -269,6 +269,31 @@ test_pkcs11_cert_constraints_oversized(void)
TEST_DONE();
}
+static void
+test_pkcs11_provider_equal(void)
+{
+ /* Registry data is not NUL terminated. */
+ const u_char stored[] = { 'C', ':', '\\', 'T', 'o', 'k', 'e', 'n',
+ '.', 'd', 'l', 'l' };
+
+ TEST_START("PKCS11 provider comparison");
+ ASSERT_INT_EQ(pkcs11_provider_equal(stored, sizeof(stored),
+ "C:\\Token.dll"), 1);
+ ASSERT_INT_EQ(pkcs11_provider_equal(stored, sizeof(stored),
+ "c:\\TOKEN.DLL"), 1);
+ /* The whole value must match, not a prefix of either side. */
+ ASSERT_INT_EQ(pkcs11_provider_equal(stored, sizeof(stored),
+ "C:\\Token.dll.old"), 0);
+ ASSERT_INT_EQ(pkcs11_provider_equal(stored, sizeof(stored) - 1,
+ "C:\\Token.dll"), 0);
+ ASSERT_INT_EQ(pkcs11_provider_equal(stored, sizeof(stored),
+ "C:\\Other.dll"), 0);
+ ASSERT_INT_EQ(pkcs11_provider_equal(NULL, 0, "C:\\Token.dll"), 0);
+ ASSERT_INT_EQ(pkcs11_provider_equal(stored, sizeof(stored), NULL), 0);
+ ASSERT_INT_EQ(pkcs11_provider_equal(stored, 0, ""), 0);
+ TEST_DONE();
+}
+
void
pkcs11_cert_tests(void)
{
@@ -276,6 +301,7 @@ pkcs11_cert_tests(void)
test_pkcs11_cert_constraints_compatible();
test_pkcs11_cert_identity_name();
test_pkcs11_identity_comment();
+ test_pkcs11_provider_equal();
test_pkcs11_cert_constraints_duplicate();
test_pkcs11_cert_constraints_truncated();
test_pkcs11_cert_constraints_malformed();
From 05ee9fbc39dc25f915cdc063692b9fa55152e5a3 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 16:33:50 +0200
Subject: [PATCH 11/16] Validate existing Registry identity before PKCS#11
reuse
Adding a PKCS#11 provider overwrote the provider and comment of any
Registry identity with the same name without looking at it. A software
key with the same public key as a token key was thereby adopted by the
provider and deleted with it on provider removal, and an identity of a
different provider could be taken over.
Reuse an existing identity only if its public key, default value, key
type and provider match. Software identities keep their encrypted
private key as default value and are refused, which fails the add and
rolls back the identities stored so far. The token label remains
refreshable.
Co-Authored-By: Claude Sonnet 5.5
---
contrib/win32/win32compat/pkcs11-cert.c | 35 +++++++++
contrib/win32/win32compat/pkcs11-cert.h | 9 +++
.../win32compat/ssh-agent/keyagent-request.c | 52 +++++++++++++
regress/pesterTests/KeyUtils.Tests.ps1 | 54 ++++++++++++++
regress/pesterTests/README.md | 5 ++
.../unittests/win32compat/pkcs11_cert_tests.c | 73 +++++++++++++++++++
6 files changed, 228 insertions(+)
diff --git a/contrib/win32/win32compat/pkcs11-cert.c b/contrib/win32/win32compat/pkcs11-cert.c
index 2b5ed161301a..edd278c0db72 100644
--- a/contrib/win32/win32compat/pkcs11-cert.c
+++ b/contrib/win32/win32compat/pkcs11-cert.c
@@ -69,6 +69,41 @@ pkcs11_provider_equal(const u_char *stored, size_t stored_len,
return strncasecmp((const char *)stored, provider, stored_len) == 0;
}
+/*
+ * Decide whether an existing Registry identity may be reused for the PKCS#11
+ * identity (blob, key_type) of provider. Only identities previously created
+ * for the same key by the same provider qualify: a software key that happens
+ * to have the same public key stores its private key as default value and
+ * must not be adopted by, and later removed with, a provider.
+ */
+int
+pkcs11_identity_entry_matches(const struct pkcs11_identity_entry *e,
+ const u_char *blob, size_t blob_len, int key_type, const char *provider)
+{
+ const u_char *association;
+ size_t association_len;
+
+ if (e == NULL || blob == NULL || blob_len == 0 || provider == NULL)
+ return 0;
+ if (e->pub == NULL || e->pub_len != blob_len ||
+ memcmp(e->pub, blob, blob_len) != 0)
+ return 0;
+ if (e->dflt == NULL || e->dflt_len != blob_len ||
+ memcmp(e->dflt, blob, blob_len) != 0)
+ return 0;
+ if (!e->has_type || e->type != key_type)
+ return 0;
+ /* Entries created before the provider value existed use the comment. */
+ if (e->provider != NULL) {
+ association = e->provider;
+ association_len = e->provider_len;
+ } else {
+ association = e->comment;
+ association_len = e->comment_len;
+ }
+ return pkcs11_provider_equal(association, association_len, provider);
+}
+
void
free_pkcs11_certs(struct sshkey **certs, size_t ncerts)
{
diff --git a/contrib/win32/win32compat/pkcs11-cert.h b/contrib/win32/win32compat/pkcs11-cert.h
index 9c7e22b2db9e..f38bc96ce99f 100644
--- a/contrib/win32/win32compat/pkcs11-cert.h
+++ b/contrib/win32/win32compat/pkcs11-cert.h
@@ -21,9 +21,18 @@
#define AGENT_MAX_EXT_CERTS 1024
+/* Values of an existing Registry identity, NULL when not present. */
+struct pkcs11_identity_entry {
+ const u_char *pub, *dflt, *provider, *comment;
+ size_t pub_len, dflt_len, provider_len, comment_len;
+ int has_type, type;
+};
+
char *pkcs11_identity_name(const struct sshkey *, const u_char *, size_t);
const char *pkcs11_identity_comment(const char *, const char *);
int pkcs11_provider_equal(const u_char *, size_t, const char *);
+int pkcs11_identity_entry_matches(const struct pkcs11_identity_entry *,
+ const u_char *, size_t, int, const char *);
int parse_pkcs11_add_constraints(struct sshbuf *, int *,
struct sshkey ***, size_t *);
void free_pkcs11_certs(struct sshkey **, size_t);
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index 9720a0996784..3a9e5cf69cef 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -444,6 +444,52 @@ restore_pkcs11_identity_metadata(HKEY key,
return r1 == 0 && r2 == 0 ? 0 : -1;
}
+static int
+pkcs11_identity_reusable(HKEY sub, const struct pkcs11_identity_change *change,
+ const u_char *blob, size_t blob_len, int key_type, const char *provider)
+{
+ struct pkcs11_identity_entry entry;
+ u_char *pub = NULL, *dflt = NULL;
+ DWORD pub_type, dflt_type, pub_len, dflt_len, type, type_kind;
+ DWORD type_len = sizeof(type);
+ int has_pub, has_dflt, reusable = 0;
+
+ memset(&entry, 0, sizeof(entry));
+ if (read_optional_reg_value(sub, L"pub", &has_pub, &pub_type, &pub,
+ &pub_len) != 0 ||
+ read_optional_reg_value(sub, NULL, &has_dflt, &dflt_type, &dflt,
+ &dflt_len) != 0)
+ goto out;
+ if (has_pub && pub_type == REG_BINARY) {
+ entry.pub = pub;
+ entry.pub_len = pub_len;
+ }
+ if (has_dflt && dflt_type == REG_BINARY) {
+ entry.dflt = dflt;
+ entry.dflt_len = dflt_len;
+ }
+ if (RegQueryValueExW(sub, L"type", NULL, &type_kind, (BYTE *)&type,
+ &type_len) == ERROR_SUCCESS && type_kind == REG_DWORD &&
+ type_len == sizeof(type)) {
+ entry.has_type = 1;
+ entry.type = (int)type;
+ }
+ if (change->had_provider) {
+ entry.provider = change->provider;
+ entry.provider_len = change->provider_len;
+ }
+ if (change->had_comment) {
+ entry.comment = change->comment;
+ entry.comment_len = change->comment_len;
+ }
+ reusable = pkcs11_identity_entry_matches(&entry, blob, blob_len,
+ key_type, provider);
+ out:
+ free(pub);
+ free(dflt);
+ return reusable;
+}
+
static int
store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
const char *provider, const char *comment,
@@ -488,6 +534,12 @@ store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
error_f("failed to read PKCS11 identity metadata");
goto out;
}
+ if (!pkcs11_identity_reusable(sub, change, blob, blob_len,
+ key->type, provider)) {
+ error_f("refusing to replace existing identity %s "
+ "not created for this provider", thumbprint);
+ goto out;
+ }
if (RegSetValueExW(sub, L"provider", 0, REG_BINARY,
(const BYTE *)provider, (DWORD)strlen(provider)) !=
ERROR_SUCCESS ||
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index 8641b26d8ad7..17fb7ff7d746 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -675,6 +675,60 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
@(ssh-add -L) -match "The agent has no identities." | Should Be $true
}
+ It "$tC.$tI - ssh-add - pkcs11 add keeps software identity with the same key (if configured)" {
+ $pkcs11Path = $env:OPENSSH_TEST_PKCS11_PROVIDER
+ $publicKeyPaths = @($env:OPENSSH_TEST_PKCS11_PUBLIC_KEYS -split ';' |
+ Where-Object { $_ })
+ $softwareKeySource = $env:OPENSSH_TEST_PKCS11_SOFTWARE_KEY
+ if (-not $pkcs11Path -or -not (Test-Path $pkcs11Path) -or
+ $publicKeyPaths.Count -eq 0 -or -not $softwareKeySource -or
+ -not (Test-Path $softwareKeySource)) {
+ Write-Host "skipping pkcs11 software identity test because provider, public keys and OPENSSH_TEST_PKCS11_SOFTWARE_KEY are not configured"
+ return
+ }
+
+ $testPin = $env:OPENSSH_TEST_PKCS11_PIN
+ if (-not $testPin) { $testPin = $pkcs11Pin }
+ $softwareKeyPath = Join-Path $testDir "pkcs11-software"
+ $nullFile = Join-Path $testDir "$tC.$tI.nullfile"
+ $null > $nullFile
+ Copy-Item $softwareKeySource $softwareKeyPath -Force
+ Repair-UserKeyPermission $softwareKeyPath -confirm:$false
+ $softwareBlob = ((ssh-keygen -y -f $softwareKeyPath) -split ' ')[1]
+ $softwareBlob | Should Be ((Get-Content $publicKeyPaths[0]).Split(' ')[1])
+
+ ssh-add -D
+ $LASTEXITCODE | Should Be 0
+ iex "cmd /c `"ssh-add $softwareKeyPath < $nullFile 2> nul `""
+ @((ssh-add -L) | Where-Object { $_.Contains($softwareBlob) }).Count |
+ Should Be 1
+
+ # The token key has the same public key as the software identity,
+ # so it must not silently take over the software identity.
+ Add-PasswordSetting -Pass $testPin
+ $env:SSH_ASKPASS_REQUIRE = "force"
+ & ssh-add -s $pkcs11Path
+ $LASTEXITCODE | Should Not Be 0
+ Remove-PasswordSetting
+ @((ssh-add -L) | Where-Object { $_.Contains($softwareBlob) }).Count |
+ Should Be 1
+ & ssh-add -T "$softwareKeyPath.pub"
+ $LASTEXITCODE | Should Be 0
+
+ # After removing the software identity the provider can be added.
+ & ssh-add -d $softwareKeyPath
+ $LASTEXITCODE | Should Be 0
+ Add-PasswordSetting -Pass $testPin
+ $env:SSH_ASKPASS_REQUIRE = "force"
+ & ssh-add -s $pkcs11Path
+ $LASTEXITCODE | Should Be 0
+ @((ssh-add -L) | Where-Object { $_.Contains($softwareBlob) }).Count |
+ Should Be 1
+ & ssh-add -e $pkcs11Path
+ $LASTEXITCODE | Should Be 0
+ @(ssh-add -L) -match "The agent has no identities." | Should Be $true
+ }
+
It "$tC.$tI - ssh-add - stale pkcs11 provider isolation (if configured)" {
$pkcs11Path = $env:OPENSSH_TEST_PKCS11_PROVIDER
$publicKeyPaths = @($env:OPENSSH_TEST_PKCS11_PUBLIC_KEYS -split ';' |
diff --git a/regress/pesterTests/README.md b/regress/pesterTests/README.md
index cd9ac12d0871..e438a00746f3 100644
--- a/regress/pesterTests/README.md
+++ b/regress/pesterTests/README.md
@@ -77,6 +77,11 @@ following environment variables are set before running the E2E tests:
* `OPENSSH_TEST_PKCS11_LABELS`: semicolon-separated labels corresponding
positionally to `OPENSSH_TEST_PKCS11_PUBLIC_KEYS`. An empty item expects the
canonical provider path fallback.
+* `OPENSSH_TEST_PKCS11_SOFTWARE_KEY` (optional): unencrypted private key file
+ whose public key equals the first entry of `OPENSSH_TEST_PKCS11_PUBLIC_KEYS`,
+ for example the key that was imported into a SoftHSM token. It enables the
+ scenario that a provider must not adopt a software identity with the same
+ public key.
The test creates short-lived OpenSSH certificates for the supplied public
keys. It verifies plain and certificate identities, signing, agent service
diff --git a/regress/unittests/win32compat/pkcs11_cert_tests.c b/regress/unittests/win32compat/pkcs11_cert_tests.c
index 30c05fdada41..cdc8ce40fbc6 100644
--- a/regress/unittests/win32compat/pkcs11_cert_tests.c
+++ b/regress/unittests/win32compat/pkcs11_cert_tests.c
@@ -294,6 +294,78 @@ test_pkcs11_provider_equal(void)
TEST_DONE();
}
+static void
+test_pkcs11_identity_entry_matches(void)
+{
+ const u_char blob[] = "public-key-blob";
+ const u_char other[] = "other-key-blob";
+ const u_char encrypted[] = "encrypted-private-key";
+ const char *provider = "C:\\Token.dll";
+ struct pkcs11_identity_entry e;
+
+ TEST_START("existing PKCS11 registry identity validation");
+ memset(&e, 0, sizeof(e));
+ e.pub = blob; e.pub_len = sizeof(blob);
+ e.dflt = blob; e.dflt_len = sizeof(blob);
+ e.has_type = 1; e.type = KEY_RSA;
+ e.provider = (const u_char *)provider; e.provider_len = strlen(provider);
+ e.comment = (const u_char *)"token label"; e.comment_len = 11;
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 1);
+ /* The provider path is case insensitive. */
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, "c:\\TOKEN.DLL"), 1);
+ /* Another provider must not take over the identity. */
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, "C:\\Other.dll"), 0);
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_ECDSA, provider), 0);
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, other, sizeof(other),
+ KEY_RSA, provider), 0);
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(NULL, blob, sizeof(blob),
+ KEY_RSA, provider), 0);
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, NULL, 0,
+ KEY_RSA, provider), 0);
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, NULL), 0);
+
+ /* A software key keeps its encrypted private key as default value. */
+ e.dflt = encrypted; e.dflt_len = sizeof(encrypted);
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 0);
+ e.dflt = blob; e.dflt_len = sizeof(blob);
+
+ /* Stored public key, type, or default value missing or different. */
+ e.pub = other; e.pub_len = sizeof(other);
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 0);
+ e.pub = NULL; e.pub_len = 0;
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 0);
+ e.pub = blob; e.pub_len = sizeof(blob);
+ e.dflt = NULL; e.dflt_len = 0;
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 0);
+ e.dflt = blob; e.dflt_len = sizeof(blob);
+ e.has_type = 0;
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 0);
+ e.has_type = 1;
+
+ /* Entries from before the provider value existed use the comment. */
+ e.provider = NULL; e.provider_len = 0;
+ e.comment = (const u_char *)provider; e.comment_len = strlen(provider);
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 1);
+ e.comment = (const u_char *)"user comment"; e.comment_len = 12;
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 0);
+ e.comment = NULL; e.comment_len = 0;
+ ASSERT_INT_EQ(pkcs11_identity_entry_matches(&e, blob, sizeof(blob),
+ KEY_RSA, provider), 0);
+ TEST_DONE();
+}
+
void
pkcs11_cert_tests(void)
{
@@ -302,6 +374,7 @@ pkcs11_cert_tests(void)
test_pkcs11_cert_identity_name();
test_pkcs11_identity_comment();
test_pkcs11_provider_equal();
+ test_pkcs11_identity_entry_matches();
test_pkcs11_cert_constraints_duplicate();
test_pkcs11_cert_constraints_truncated();
test_pkcs11_cert_constraints_malformed();
From 61b535667886ba41a356868010c92613b1613059 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 16:40:22 +0200
Subject: [PATCH 12/16] Detach software identity from PKCS#11 provider on add
Adding a software key over a Registry identity that was persisted for a
PKCS#11 provider kept the stale provider value. The software key then
counted as a provider identity and was deleted when the provider was
removed.
Delete the provider value when a software key is stored.
Co-Authored-By: Claude Sonnet 5.5
---
.../win32compat/ssh-agent/keyagent-request.c | 6 ++-
regress/pesterTests/KeyUtils.Tests.ps1 | 44 +++++++++++++++++++
2 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index 3a9e5cf69cef..c9e23b7e204c 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -300,6 +300,7 @@ process_add_identity(struct sshbuf* request, struct sshbuf* response, struct age
char* eblob = NULL;
HKEY reg = 0, sub = 0, user_root = 0;
SECURITY_ATTRIBUTES sa;
+ LSTATUS status;
/* parse input request */
memset(&sa, 0, sizeof(SECURITY_ATTRIBUTES));
@@ -330,7 +331,10 @@ process_add_identity(struct sshbuf* request, struct sshbuf* response, struct age
RegSetValueExW(sub, NULL, 0, REG_BINARY, eblob, eblob_len) != 0 ||
RegSetValueExW(sub, L"pub", 0, REG_BINARY, pubkey_blob, (DWORD)pubkey_blob_len) != 0 ||
RegSetValueExW(sub, L"type", 0, REG_DWORD, (BYTE*)&key->type, 4) != 0 ||
- RegSetValueExW(sub, L"comment", 0, REG_BINARY, comment, (DWORD)comment_len) != 0 ) {
+ RegSetValueExW(sub, L"comment", 0, REG_BINARY, comment, (DWORD)comment_len) != 0 ||
+ /* a software key does not belong to a PKCS#11 provider */
+ ((status = RegDeleteValueW(sub, L"provider")) != ERROR_SUCCESS &&
+ status != ERROR_FILE_NOT_FOUND)) {
error("failed to add key to store");
goto done;
}
diff --git a/regress/pesterTests/KeyUtils.Tests.ps1 b/regress/pesterTests/KeyUtils.Tests.ps1
index 17fb7ff7d746..1c6cc5ed994a 100644
--- a/regress/pesterTests/KeyUtils.Tests.ps1
+++ b/regress/pesterTests/KeyUtils.Tests.ps1
@@ -729,6 +729,50 @@ Describe "E2E scenarios for ssh key management" -Tags "CI" {
@(ssh-add -L) -match "The agent has no identities." | Should Be $true
}
+ It "$tC.$tI - ssh-add - software add detaches identity from pkcs11 provider (if configured)" {
+ $pkcs11Path = $env:OPENSSH_TEST_PKCS11_PROVIDER
+ $publicKeyPaths = @($env:OPENSSH_TEST_PKCS11_PUBLIC_KEYS -split ';' |
+ Where-Object { $_ })
+ $softwareKeySource = $env:OPENSSH_TEST_PKCS11_SOFTWARE_KEY
+ if (-not $pkcs11Path -or -not (Test-Path $pkcs11Path) -or
+ $publicKeyPaths.Count -eq 0 -or -not $softwareKeySource -or
+ -not (Test-Path $softwareKeySource)) {
+ Write-Host "skipping pkcs11 software detach test because provider, public keys and OPENSSH_TEST_PKCS11_SOFTWARE_KEY are not configured"
+ return
+ }
+
+ $testPin = $env:OPENSSH_TEST_PKCS11_PIN
+ if (-not $testPin) { $testPin = $pkcs11Pin }
+ $softwareKeyPath = Join-Path $testDir "pkcs11-software"
+ $nullFile = Join-Path $testDir "$tC.$tI.nullfile"
+ $null > $nullFile
+ Copy-Item $softwareKeySource $softwareKeyPath -Force
+ Repair-UserKeyPermission $softwareKeyPath -confirm:$false
+ $softwareBlob = ((ssh-keygen -y -f $softwareKeyPath) -split ' ')[1]
+ $softwareBlob | Should Be ((Get-Content $publicKeyPaths[0]).Split(' ')[1])
+
+ ssh-add -D
+ $LASTEXITCODE | Should Be 0
+ Add-PasswordSetting -Pass $testPin
+ $env:SSH_ASKPASS_REQUIRE = "force"
+ & ssh-add -s $pkcs11Path
+ $LASTEXITCODE | Should Be 0
+ Remove-PasswordSetting
+
+ # Adding the same key as software key makes it a software identity.
+ # Removing the provider must no longer delete it.
+ iex "cmd /c `"ssh-add $softwareKeyPath < $nullFile 2> nul `""
+ & ssh-add -e $pkcs11Path
+ $LASTEXITCODE | Should Be 0
+ @((ssh-add -L) | Where-Object { $_.Contains($softwareBlob) }).Count |
+ Should Be 1
+ & ssh-add -T "$softwareKeyPath.pub"
+ $LASTEXITCODE | Should Be 0
+
+ ssh-add -D
+ $LASTEXITCODE | Should Be 0
+ }
+
It "$tC.$tI - ssh-add - stale pkcs11 provider isolation (if configured)" {
$pkcs11Path = $env:OPENSSH_TEST_PKCS11_PROVIDER
$publicKeyPaths = @($env:OPENSSH_TEST_PKCS11_PUBLIC_KEYS -split ';' |
From 471eb4868d31c54faeac60e86785abfd46c3c9e1 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 17:38:14 +0200
Subject: [PATCH 13/16] Fix Windows SoftHSM regression prerequisites
---
contrib/win32/openssh/bash_tests_iterator.ps1 | 39 +++++++++++++++----
regress/test-exec.sh | 26 +++++++++++--
2 files changed, 53 insertions(+), 12 deletions(-)
diff --git a/contrib/win32/openssh/bash_tests_iterator.ps1 b/contrib/win32/openssh/bash_tests_iterator.ps1
index beabd9d8a3f0..e591908064e1 100644
--- a/contrib/win32/openssh/bash_tests_iterator.ps1
+++ b/contrib/win32/openssh/bash_tests_iterator.ps1
@@ -26,7 +26,10 @@ if ($TestFilePath) {
$TestFilePath = $TestFilePath -replace "\\","/"
}
$OriginalSystemPath = [System.Environment]::GetEnvironmentVariable('Path', [System.EnvironmentVariableTarget]::Machine)
-$OriginalUserSoftHsmConf = [System.Environment]::GetEnvironmentVariable('SOFTHSM2_CONF', [System.EnvironmentVariableTarget]::User)
+$AgentServiceRegistryPath = 'HKLM:\SYSTEM\CurrentControlSet\Services\ssh-agent'
+$AgentEnvironmentConfigured = $false
+$OriginalAgentEnvironmentPresent = $false
+$OriginalAgentEnvironment = $null
# Make sure config.h exists. It is used in some bashstests (Ex - sftp-glob.sh, cfgparse.sh)
# first check in $BashTestsPath folder. If not then it's parent folder. If not then in the $OpenSSHBinPath
@@ -67,7 +70,7 @@ if(!$SkipInstallSSHD) {
if (-not [string]::IsNullOrEmpty($env:TEST_SSH_PKCS11_PROVIDER)) {
$testProvider = (& $ShellPath -c "cygpath -w '$env:TEST_SSH_PKCS11_PROVIDER'").Trim()
$agentImagePath = '"{0}" -P "{1}"' -f (Join-Path $OpenSSHBinPath 'ssh-agent.exe'), $testProvider
- Set-ItemProperty -Path 'HKLM:\SYSTEM\CurrentControlSet\Services\ssh-agent' `
+ Set-ItemProperty -Path $AgentServiceRegistryPath `
-Name ImagePath -Value $agentImagePath -Force
}
}
@@ -155,10 +158,7 @@ try
$env:TEST_SSH_SFTP = $OpenSSHBinPath_shell_fmt+"/sftp.exe"
$env:TEST_SSH_SFTPSERVER = $OpenSSHBinPath_shell_fmt+"/sftp-server.exe"
$env:TEST_SSH_SCP = $OpenSSHBinPath_shell_fmt+"/scp.exe"
- $env:TEST_SSH_OPENSSL = (&$ShellPath -c "command -v openssl").Trim()
- if ([string]::IsNullOrEmpty($env:TEST_SSH_OPENSSL)) {
- throw "openssl was not found in the test shell"
- }
+ $env:TEST_SSH_OPENSSL = ([string](&$ShellPath -c "command -v openssl")).Trim()
$env:BUILDDIR = $BUILDDIR
$env:TEST_WINDOWS_SSH = 1
$env:TEST_SSH_ASKPASS = $TEST_SSH_ASKPASS
@@ -187,7 +187,21 @@ try
$null = New-Item -ItemType directory -Path $temp_test_path -Force -ErrorAction Stop
if (-not [string]::IsNullOrEmpty($env:TEST_SSH_PKCS11_PROVIDER)) {
$testSoftHsmConf = Join-Path $BashTestsWindowsPath "$temp_test_path\SOFTHSM\softhsm2.conf"
- [System.Environment]::SetEnvironmentVariable('SOFTHSM2_CONF', $testSoftHsmConf, [System.EnvironmentVariableTarget]::User)
+ $agentEnvironmentProperty = Get-ItemProperty -Path $AgentServiceRegistryPath `
+ -Name Environment -ErrorAction SilentlyContinue
+ if ($null -ne $agentEnvironmentProperty) {
+ $OriginalAgentEnvironmentPresent = $true
+ $OriginalAgentEnvironment = @($agentEnvironmentProperty.Environment)
+ }
+ $agentEnvironment = @($OriginalAgentEnvironment | Where-Object {
+ -not ([string]$_).StartsWith('SOFTHSM2_CONF=',
+ [StringComparison]::OrdinalIgnoreCase)
+ })
+ $agentEnvironment += "SOFTHSM2_CONF=$testSoftHsmConf"
+ New-ItemProperty -Path $AgentServiceRegistryPath -Name Environment `
+ -PropertyType MultiString -Value $agentEnvironment -Force `
+ -ErrorAction Stop | Out-Null
+ $AgentEnvironmentConfigured = $true
}
# remove the summary, output files.
@@ -287,7 +301,16 @@ finally
{
# Restore User Path variable in the registry once the tests finish running.
[System.Environment]::SetEnvironmentVariable('Path', $OriginalSystemPath, [System.EnvironmentVariableTarget]::Machine)
- [System.Environment]::SetEnvironmentVariable('SOFTHSM2_CONF', $OriginalUserSoftHsmConf, [System.EnvironmentVariableTarget]::User)
+ if ($AgentEnvironmentConfigured) {
+ if ($OriginalAgentEnvironmentPresent) {
+ New-ItemProperty -Path $AgentServiceRegistryPath -Name Environment `
+ -PropertyType MultiString -Value $OriginalAgentEnvironment `
+ -Force -ErrorAction SilentlyContinue | Out-Null
+ } else {
+ Remove-ItemProperty -Path $AgentServiceRegistryPath -Name Environment `
+ -ErrorAction SilentlyContinue
+ }
+ }
# remove temp test directory
if (!$SkipCleanup)
{
diff --git a/regress/test-exec.sh b/regress/test-exec.sh
index 38635a8cf9d3..3ae801db1910 100644
--- a/regress/test-exec.sh
+++ b/regress/test-exec.sh
@@ -1039,6 +1039,14 @@ p11_make_public() {
fi
}
+p11_softhsm2_util() {
+ if test -n "$SOFTHSM2_MODULE"; then
+ "$SOFTHSM2_UTIL" --module "$SOFTHSM2_MODULE" "$@"
+ else
+ "$SOFTHSM2_UTIL" "$@"
+ fi
+}
+
# Perform PKCS#11 setup: prepares a softhsm2 token configuration, generated
# keys and loads them into the virtual token.
PKCS11_OK=
@@ -1047,6 +1055,8 @@ p11_setup() {
# XXX we could potentially test ed25519 only in the absence of
# RSA and ECDSA support.
$SSH -Q key | grep ssh-rsa >/dev/null || return 1
+ test -n "$OPENSSL_BIN" || return 1
+ "$OPENSSL_BIN" version >/dev/null 2>&1 || return 1
if test "x$TEST_WINDOWS_SSH" = "x1"; then
if test -n "$TEST_SSH_PKCS11_PROVIDER"; then
p11_find_lib "$TEST_SSH_PKCS11_PROVIDER"
@@ -1056,12 +1066,19 @@ p11_setup() {
"/cygdrive/c/Program Files/SoftHSM2/lib/softhsm2.dll"
fi
SOFTHSM2_UTIL="${TEST_SSH_SOFTHSM2_UTIL:-/cygdrive/c/Program Files/SoftHSM2/bin/softhsm2-util.exe}"
+ SOFTHSM2_MODULE="${TEST_SSH_SOFTHSM2_MODULE:-$TEST_SSH_PKCS11}"
+ case "$SOFTHSM2_MODULE" in
+ *softhsm2-x64.dll)
+ SOFTHSM2_MODULE="${SOFTHSM2_MODULE%-x64.dll}.dll"
+ ;;
+ esac
else
p11_find_lib \
/usr/local/lib/softhsm/libsofthsm2.so \
/usr/lib64/pkcs11/libsofthsm2.so \
/usr/lib/x86_64-linux-gnu/softhsm/libsofthsm2.so
SOFTHSM2_UTIL=softhsm2-util
+ SOFTHSM2_MODULE=
fi
test -z "$TEST_SSH_PKCS11" && return 1
trace "using token library $TEST_SSH_PKCS11"
@@ -1085,6 +1102,7 @@ p11_setup() {
TOKEN_CONFIG=$(cygpath -w "$TOKEN")
SOFTHSM2_CONF=$(cygpath -w "$SOFTHSM2_CONF")
TEST_SSH_PKCS11=$(cygpath -w "$TEST_SSH_PKCS11")
+ SOFTHSM2_MODULE=$(cygpath -w "$SOFTHSM2_MODULE")
fi
export SOFTHSM2_CONF
cat > "$SOFTHSM2_CONF_FILE" << EOF
@@ -1096,7 +1114,7 @@ log.level = DEBUG
# If CKF_REMOVABLE_DEVICE flag should be set
slots.removable = false
EOF
- out=$("$SOFTHSM2_UTIL" --module "$TEST_SSH_PKCS11" --init-token --free \
+ out=$(p11_softhsm2_util --init-token --free \
--label token-slot-0 --pin "$TEST_SSH_PIN" --so-pin "$TEST_SSH_SOPIN")
slot=$(echo -- $out | tr -d '\r' | sed 's/.* //')
trace "generating keys"
@@ -1110,7 +1128,7 @@ EOF
if test "x$TEST_WINDOWS_SSH" = "x1"; then
RSAP8_IMPORT=$(cygpath -w "$RSAP8")
fi
- "$SOFTHSM2_UTIL" --module "$TEST_SSH_PKCS11" --slot "$slot" \
+ p11_softhsm2_util --slot "$slot" \
--label 01 --id 01 --pin "$TEST_SSH_PIN" \
--import "$RSAP8_IMPORT" >/dev/null || fatal "softhsm import RSA fail"
p11_make_public $RSA
@@ -1128,7 +1146,7 @@ EOF
if test "x$TEST_WINDOWS_SSH" = "x1"; then
ECP8_IMPORT=$(cygpath -w "$ECP8")
fi
- "$SOFTHSM2_UTIL" --module "$TEST_SSH_PKCS11" --slot "$slot" \
+ p11_softhsm2_util --slot "$slot" \
--label 02 --id 02 --pin "$TEST_SSH_PIN" \
--import "$ECP8_IMPORT" >/dev/null || fatal "softhsm import EC fail"
p11_make_public $EC
@@ -1143,7 +1161,7 @@ EOF
if test "x$TEST_WINDOWS_SSH" = "x1"; then
ED25519P8_IMPORT=$(cygpath -w "$ED25519P8")
fi
- "$SOFTHSM2_UTIL" --module "$TEST_SSH_PKCS11" --slot "$slot" \
+ p11_softhsm2_util --slot "$slot" \
--label 03 --id 03 --pin "$TEST_SSH_PIN" \
--import "$ED25519P8_IMPORT" >/dev/null || \
fatal "softhsm import ed25519 fail"
From fb905662e4a4a4f77b3340c41053fedd49506286 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 18:11:21 +0200
Subject: [PATCH 14/16] Split keyagent-request.c into registry and PKCS#11
modules
keyagent-request.c grew to more than 1600 lines, mostly PKCS#11
persistence in the Registry. Move the code without changing behavior:
- keyagent-registry.c/h: user hive lookup, DPAPI blob conversion and
generic Registry helpers
- keyagent-pkcs11.c/h: PKCS#11 identity/provider store, rollback,
reload helpers and the add/remove smartcard handlers
- keyagent-request.c: request handlers for software keys, signing,
identity listing and the session-bind extension
Only the helpers shared between the files lose their static linkage.
Co-Authored-By: Claude Sonnet 5.5
---
contrib/win32/openssh/ssh-agent.vcxproj | 4 +
.../win32compat/ssh-agent/keyagent-pkcs11.c | 723 ++++++++++++++
.../win32compat/ssh-agent/keyagent-pkcs11.h | 16 +
.../win32compat/ssh-agent/keyagent-registry.c | 228 +++++
.../win32compat/ssh-agent/keyagent-registry.h | 30 +
.../win32compat/ssh-agent/keyagent-request.c | 883 +-----------------
6 files changed, 1003 insertions(+), 881 deletions(-)
create mode 100644 contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
create mode 100644 contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.h
create mode 100644 contrib/win32/win32compat/ssh-agent/keyagent-registry.c
create mode 100644 contrib/win32/win32compat/ssh-agent/keyagent-registry.h
diff --git a/contrib/win32/openssh/ssh-agent.vcxproj b/contrib/win32/openssh/ssh-agent.vcxproj
index 8a5b1e6e13e3..243658da090f 100644
--- a/contrib/win32/openssh/ssh-agent.vcxproj
+++ b/contrib/win32/openssh/ssh-agent.vcxproj
@@ -417,11 +417,15 @@
+
+
+
+
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
new file mode 100644
index 000000000000..65ca886d0ec8
--- /dev/null
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
@@ -0,0 +1,723 @@
+/*
+ * Author: Manoj Ampalam
+ * ssh-agent implementation on Windows
+ *
+ * Copyright (c) 2015 Microsoft Corp.
+ * All rights reserved
+ *
+ * Microsoft openssh win32 port
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "agent.h"
+#include "agent-request.h"
+#include "config.h"
+#include "match.h"
+#include
+#include "pkcs11-cert.h"
+#ifdef ENABLE_PKCS11
+#include "ssh-pkcs11.h"
+#endif
+#include "xmalloc.h"
+#include "keyagent-registry.h"
+#include "keyagent-pkcs11.h"
+
+#ifdef ENABLE_PKCS11
+
+#pragma warning(push, 3)
+
+extern char* allowed_providers;
+extern int remote_add_provider;
+
+extern void
+add_key(struct sshkey *k, char *name);
+
+struct pkcs11_identity_change {
+ char *name;
+ int created;
+ int had_provider;
+ DWORD provider_type;
+ u_char *provider;
+ DWORD provider_len;
+ int had_comment;
+ DWORD comment_type;
+ u_char *comment;
+ DWORD comment_len;
+};
+
+static void
+free_pkcs11_identity_change(struct pkcs11_identity_change *change)
+{
+ if (change == NULL)
+ return;
+ free(change->name);
+ free(change->provider);
+ free(change->comment);
+ free(change);
+}
+
+static int
+restore_pkcs11_identity_metadata(HKEY key,
+ const struct pkcs11_identity_change *change)
+{
+ int r1, r2;
+
+ r1 = restore_optional_reg_value(key, L"provider",
+ change->had_provider, change->provider_type, change->provider,
+ change->provider_len);
+ r2 = restore_optional_reg_value(key, L"comment", change->had_comment,
+ change->comment_type, change->comment, change->comment_len);
+ return r1 == 0 && r2 == 0 ? 0 : -1;
+}
+
+static int
+pkcs11_identity_reusable(HKEY sub, const struct pkcs11_identity_change *change,
+ const u_char *blob, size_t blob_len, int key_type, const char *provider)
+{
+ struct pkcs11_identity_entry entry;
+ u_char *pub = NULL, *dflt = NULL;
+ DWORD pub_type, dflt_type, pub_len, dflt_len, type, type_kind;
+ DWORD type_len = sizeof(type);
+ int has_pub, has_dflt, reusable = 0;
+
+ memset(&entry, 0, sizeof(entry));
+ if (read_optional_reg_value(sub, L"pub", &has_pub, &pub_type, &pub,
+ &pub_len) != 0 ||
+ read_optional_reg_value(sub, NULL, &has_dflt, &dflt_type, &dflt,
+ &dflt_len) != 0)
+ goto out;
+ if (has_pub && pub_type == REG_BINARY) {
+ entry.pub = pub;
+ entry.pub_len = pub_len;
+ }
+ if (has_dflt && dflt_type == REG_BINARY) {
+ entry.dflt = dflt;
+ entry.dflt_len = dflt_len;
+ }
+ if (RegQueryValueExW(sub, L"type", NULL, &type_kind, (BYTE *)&type,
+ &type_len) == ERROR_SUCCESS && type_kind == REG_DWORD &&
+ type_len == sizeof(type)) {
+ entry.has_type = 1;
+ entry.type = (int)type;
+ }
+ if (change->had_provider) {
+ entry.provider = change->provider;
+ entry.provider_len = change->provider_len;
+ }
+ if (change->had_comment) {
+ entry.comment = change->comment;
+ entry.comment_len = change->comment_len;
+ }
+ reusable = pkcs11_identity_entry_matches(&entry, blob, blob_len,
+ key_type, provider);
+ out:
+ free(pub);
+ free(dflt);
+ return reusable;
+}
+
+static int
+store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
+ const char *provider, const char *comment,
+ struct pkcs11_identity_change **changep)
+{
+ SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
+ HKEY reg = NULL, sub = NULL;
+ u_char *blob = NULL;
+ size_t blob_len;
+ char *thumbprint = NULL;
+ struct pkcs11_identity_change *change = NULL;
+ DWORD disposition = 0;
+ ULONG sd_len = 0;
+ int success = 0;
+
+ if (changep == NULL || provider == NULL || comment == NULL)
+ return -1;
+ *changep = NULL;
+ sa.nLength = sizeof(sa);
+ if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
+ SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sd_len) ||
+ sshkey_to_blob(key, &blob, &blob_len) != 0 ||
+ blob_len == 0 || blob_len > MAX_MESSAGE_SIZE ||
+ (thumbprint = pkcs11_identity_name(key, blob, blob_len)) == NULL ||
+ RegCreateKeyExW(user_root, SSH_KEYS_ROOT, 0, NULL, 0,
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
+ RegCreateKeyExA(reg, thumbprint, 0, NULL, 0,
+ KEY_WRITE | KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sa, &sub,
+ &disposition) != ERROR_SUCCESS) {
+ error_f("failed to persist PKCS11 identity");
+ goto out;
+ }
+ change = xcalloc(1, sizeof(*change));
+ change->name = xstrdup(thumbprint);
+ if (disposition == REG_OPENED_EXISTING_KEY) {
+ if (read_optional_reg_value(sub, L"provider",
+ &change->had_provider, &change->provider_type,
+ &change->provider, &change->provider_len) != 0 ||
+ read_optional_reg_value(sub, L"comment",
+ &change->had_comment, &change->comment_type,
+ &change->comment, &change->comment_len) != 0) {
+ error_f("failed to read PKCS11 identity metadata");
+ goto out;
+ }
+ if (!pkcs11_identity_reusable(sub, change, blob, blob_len,
+ key->type, provider)) {
+ error_f("refusing to replace existing identity %s "
+ "not created for this provider", thumbprint);
+ goto out;
+ }
+ if (RegSetValueExW(sub, L"provider", 0, REG_BINARY,
+ (const BYTE *)provider, (DWORD)strlen(provider)) !=
+ ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"comment", 0, REG_BINARY,
+ (const BYTE *)comment, (DWORD)strlen(comment)) !=
+ ERROR_SUCCESS) {
+ error_f("failed to update PKCS11 identity metadata");
+ if (restore_pkcs11_identity_metadata(sub, change) != 0)
+ error_f("failed to restore PKCS11 identity metadata");
+ goto out;
+ }
+ } else {
+ change->created = 1;
+ if (RegSetValueExW(sub, NULL, 0, REG_BINARY, blob,
+ (DWORD)blob_len) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"pub", 0, REG_BINARY, blob,
+ (DWORD)blob_len) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"type", 0, REG_DWORD,
+ (const BYTE *)&key->type, sizeof(key->type)) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"provider", 0, REG_BINARY,
+ (const BYTE *)provider, (DWORD)strlen(provider)) !=
+ ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"comment", 0, REG_BINARY,
+ (const BYTE *)comment, (DWORD)strlen(comment)) !=
+ ERROR_SUCCESS) {
+ error_f("failed to persist PKCS11 identity");
+ goto out;
+ }
+ }
+ *changep = change;
+ change = NULL;
+ success = 1;
+ out:
+ if (sub != NULL) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ if (!success && disposition == REG_CREATED_NEW_KEY && reg != NULL &&
+ thumbprint != NULL)
+ RegDeleteTreeA(reg, thumbprint);
+ if (reg != NULL)
+ RegCloseKey(reg);
+ if (sa.lpSecurityDescriptor != NULL)
+ LocalFree(sa.lpSecurityDescriptor);
+ free_pkcs11_identity_change(change);
+ free(thumbprint);
+ free(blob);
+ return success ? 0 : -1;
+}
+
+static int
+store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
+ const char *provider, const char *pin, size_t pin_len)
+{
+ SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
+ HKEY reg = NULL, sub = NULL;
+ char *epin = NULL;
+ DWORD epin_len = 0;
+ DWORD disposition = 0;
+ ULONG sd_len = 0;
+ int success = 0;
+
+ sa.nLength = sizeof(sa);
+ if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
+ SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sd_len) ||
+ convert_blob(con, pin, (DWORD)pin_len, &epin, &epin_len, TRUE) != 0 ||
+ RegCreateKeyExW(user_root, SSH_PKCS11_PROVIDERS_ROOT, 0, NULL, 0,
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
+ RegCreateKeyExA(reg, provider, 0, NULL, 0,
+ KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub,
+ &disposition) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"provider", 0, REG_BINARY,
+ (const BYTE *)provider, (DWORD)strlen(provider)) != ERROR_SUCCESS ||
+ RegSetValueExW(sub, L"pin", 0, REG_BINARY, (const BYTE *)epin,
+ epin_len) != ERROR_SUCCESS) {
+ error_f("failed to persist PKCS11 provider");
+ goto out;
+ }
+ success = 1;
+ out:
+ if (epin != NULL) {
+ SecureZeroMemory(epin, epin_len);
+ free(epin);
+ }
+ if (sub != NULL) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ if (!success && disposition == REG_CREATED_NEW_KEY && reg != NULL)
+ RegDeleteTreeA(reg, provider);
+ if (reg != NULL)
+ RegCloseKey(reg);
+ if (sa.lpSecurityDescriptor != NULL)
+ LocalFree(sa.lpSecurityDescriptor);
+ return success ? 0 : -1;
+}
+
+static void
+rollback_pkcs11_identities(HKEY user_root,
+ struct pkcs11_identity_change **changes,
+ size_t nidentities)
+{
+ HKEY reg = NULL, sub = NULL;
+ size_t i;
+
+ if (nidentities == 0)
+ return;
+ if (RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
+ DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY,
+ ®) != ERROR_SUCCESS) {
+ error_f("failed to open PKCS11 identities for rollback");
+ return;
+ }
+ for (i = nidentities; i > 0; i--) {
+ if (changes[i - 1]->created) {
+ if (RegDeleteTreeA(reg, changes[i - 1]->name) !=
+ ERROR_SUCCESS)
+ error_f("failed to roll back PKCS11 identity");
+ continue;
+ }
+ if (RegOpenKeyExA(reg, changes[i - 1]->name, 0,
+ KEY_SET_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS ||
+ restore_pkcs11_identity_metadata(sub, changes[i - 1]) != 0)
+ error_f("failed to roll back PKCS11 identity metadata");
+ if (sub != NULL) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ }
+ RegCloseKey(reg);
+}
+
+static int
+remove_pkcs11_identities(HKEY user_root, const char *provider)
+{
+ HKEY root = NULL, sub = NULL;
+ wchar_t sub_name[MAX_KEY_LENGTH];
+ DWORD sub_name_len, type, data_len;
+ u_char *data = NULL;
+ int index = 0, present, remove;
+ LSTATUS status;
+
+ status = RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
+ DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY, &root);
+ if (status == ERROR_FILE_NOT_FOUND)
+ return 0;
+ if (status != ERROR_SUCCESS)
+ return -1;
+ for (;;) {
+ sub_name_len = MAX_KEY_LENGTH;
+ status = RegEnumKeyExW(root, index, sub_name, &sub_name_len,
+ NULL, NULL, NULL, NULL);
+ if (status == ERROR_NO_MORE_ITEMS)
+ break;
+ if (status != ERROR_SUCCESS) {
+ index++;
+ continue;
+ }
+ if (RegOpenKeyExW(root, sub_name, 0,
+ KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS) {
+ index++;
+ continue;
+ }
+ free(data);
+ data = NULL;
+ if (read_optional_reg_value(sub, L"provider", &present, &type,
+ &data, &data_len) != 0 ||
+ (!present && read_optional_reg_value(sub, L"comment",
+ &present, &type, &data, &data_len) != 0)) {
+ RegCloseKey(sub);
+ sub = NULL;
+ index++;
+ continue;
+ }
+ remove = present && pkcs11_provider_equal(data, data_len, provider);
+ RegCloseKey(sub);
+ sub = NULL;
+ if (remove) {
+ if (RegDeleteTreeW(root, sub_name) != ERROR_SUCCESS) {
+ RegCloseKey(root);
+ free(data);
+ return -1;
+ }
+ } else
+ index++;
+ }
+ RegCloseKey(root);
+ free(data);
+ return 0;
+}
+
+int
+load_pkcs11_identities(HKEY user_root, const char *provider,
+ struct sshkey **token_keys, int nkeys)
+{
+ HKEY root = NULL, sub = NULL;
+ wchar_t sub_name[MAX_KEY_LENGTH];
+ DWORD sub_name_len, blob_len, comment_len, association_len;
+ u_char *blob = NULL;
+ char *comment = NULL, *association = NULL;
+ struct sshkey *registered = NULL, *cert = NULL;
+ u_char *plain_added = NULL;
+ int i, index = 0, legacy, loaded = 0;
+ LSTATUS status;
+
+ if (nkeys > 0)
+ plain_added = xcalloc((size_t)nkeys, sizeof(*plain_added));
+ status = RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
+ KEY_ENUMERATE_SUB_KEYS | KEY_QUERY_VALUE | KEY_WOW64_64KEY, &root);
+ if (status == ERROR_FILE_NOT_FOUND)
+ goto out;
+ if (status != ERROR_SUCCESS) {
+ error_f("failed to open persisted identities: %ld", status);
+ loaded = -1;
+ goto out;
+ }
+ for (;;) {
+ sub_name_len = MAX_KEY_LENGTH;
+ if (sub != NULL) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ status = RegEnumKeyExW(root, index++, sub_name, &sub_name_len,
+ NULL, NULL, NULL, NULL);
+ if (status == ERROR_NO_MORE_ITEMS)
+ break;
+ if (status != ERROR_SUCCESS)
+ continue;
+ if (RegOpenKeyExW(root, sub_name, 0,
+ KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS ||
+ RegQueryValueExW(sub, L"pub", NULL, NULL, NULL,
+ &blob_len) != ERROR_SUCCESS ||
+ RegQueryValueExW(sub, L"comment", NULL, NULL, NULL,
+ &comment_len) != ERROR_SUCCESS ||
+ blob_len == 0 || blob_len > MAX_MESSAGE_SIZE ||
+ comment_len > MAX_MESSAGE_SIZE)
+ continue;
+ status = RegQueryValueExW(sub, L"provider", NULL, NULL, NULL,
+ &association_len);
+ if (status == ERROR_FILE_NOT_FOUND) {
+ legacy = 1;
+ association_len = comment_len;
+ } else if (status == ERROR_SUCCESS &&
+ association_len <= MAX_MESSAGE_SIZE)
+ legacy = 0;
+ else
+ continue;
+ free(blob);
+ free(comment);
+ free(association);
+ blob = xmalloc(blob_len);
+ comment = xmalloc((size_t)comment_len + 1);
+ association = xmalloc((size_t)association_len + 1);
+ if (RegQueryValueExW(sub, L"pub", NULL, NULL, blob,
+ &blob_len) != ERROR_SUCCESS ||
+ RegQueryValueExW(sub, L"comment", NULL, NULL,
+ (BYTE *)comment, &comment_len) != ERROR_SUCCESS ||
+ (!legacy && RegQueryValueExW(sub, L"provider", NULL, NULL,
+ (BYTE *)association, &association_len) != ERROR_SUCCESS))
+ continue;
+ comment[comment_len] = '\0';
+ if (legacy)
+ memcpy(association, comment, comment_len);
+ association[association_len] = '\0';
+ if (!pkcs11_provider_equal((u_char *)association, association_len,
+ provider))
+ continue;
+ sshkey_free(registered);
+ registered = NULL;
+ if (sshkey_from_blob(blob, blob_len, ®istered) != 0)
+ continue;
+ for (i = 0; i < nkeys; i++) {
+ if (token_keys[i] == NULL)
+ continue;
+ if (sshkey_is_cert(registered)) {
+ if (!sshkey_equal_public(token_keys[i], registered))
+ continue;
+ if (pkcs11_make_cert(token_keys[i], registered,
+ &cert) != 0)
+ continue;
+ add_key(cert, (char *)provider);
+ cert = NULL;
+ loaded++;
+ break;
+ }
+ if (!plain_added[i] &&
+ sshkey_equal(token_keys[i], registered)) {
+ plain_added[i] = 1;
+ break;
+ }
+ }
+ }
+ for (i = 0; i < nkeys; i++) {
+ if (!plain_added[i] || token_keys[i] == NULL)
+ continue;
+ add_key(token_keys[i], (char *)provider);
+ token_keys[i] = NULL;
+ loaded++;
+ }
+ out:
+ sshkey_free(cert);
+ sshkey_free(registered);
+ free(plain_added);
+ free(association);
+ free(comment);
+ free(blob);
+ if (sub != NULL)
+ RegCloseKey(sub);
+ if (root != NULL)
+ RegCloseKey(root);
+ return loaded;
+}
+
+void
+free_pkcs11_sign_provider(char **providerp, char **pinp, DWORD pin_len,
+ char **epinp, DWORD epin_len, struct sshkey ***keysp, int nkeys)
+{
+ int i;
+
+ if (*keysp != NULL) {
+ for (i = 0; i < nkeys; i++)
+ sshkey_free((*keysp)[i]);
+ free(*keysp);
+ *keysp = NULL;
+ }
+ free(*providerp);
+ *providerp = NULL;
+ if (*pinp != NULL) {
+ SecureZeroMemory(*pinp, pin_len);
+ free(*pinp);
+ *pinp = NULL;
+ }
+ if (*epinp != NULL) {
+ SecureZeroMemory(*epinp, epin_len);
+ free(*epinp);
+ *epinp = NULL;
+ }
+}
+
+int
+process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
+ struct agent_connection *con)
+{
+ char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX] = { 0 };
+ char allowed_provider[PATH_MAX], **labels = NULL;
+ const char *comment;
+ int i, j, count = 0, r = 0, request_invalid = 0, success = 0;
+ int cert_only = 0, identities_stored = 0;
+ struct sshkey **keys = NULL, **certs = NULL, *cert = NULL;
+ struct pkcs11_identity_change *identity_change = NULL;
+ struct pkcs11_identity_change **identity_changes = NULL;
+ size_t k, pin_len = 0, ncerts = 0, nidentity_changes = 0;
+ HKEY user_root = NULL;
+
+ pkcs11_init(0);
+
+ if ((r = sshbuf_get_cstring(request, &provider, NULL)) != 0 ||
+ (r = sshbuf_get_cstring(request, &pin, &pin_len)) != 0 ||
+ pin_len > 256) {
+ error("add smartcard request is invalid");
+ request_invalid = 1;
+ goto done;
+ }
+ if (sshbuf_len(request) != 0 &&
+ parse_pkcs11_add_constraints(request, &cert_only, &certs,
+ &ncerts) != 0) {
+ error("add smartcard constraints are invalid");
+ request_invalid = 1;
+ goto done;
+ }
+
+ if (con->nsession_ids != 0 && !remote_add_provider) {
+ verbose("failed PKCS#11 add of \"%.100s\": remote addition of "
+ "providers is disabled", provider);
+ goto done;
+ }
+
+ if (realpath(provider, canonical_provider) == NULL) {
+ error("failed PKCS#11 add of \"%.100s\": realpath: %s",
+ provider, strerror(errno));
+ request_invalid = 1;
+ goto done;
+ }
+
+ /* Remove the leading slash from the canonical Windows drive path. */
+ if (canonical_provider[0] == '/')
+ memmove(canonical_provider, canonical_provider + 1,
+ strlen(canonical_provider));
+ strcpy_s(allowed_provider, sizeof(allowed_provider), canonical_provider);
+ for (i = 0; allowed_provider[i] != '\0'; i++) {
+ if (allowed_provider[i] == '/')
+ allowed_provider[i] = '\\';
+ }
+ to_lower_case(allowed_provider);
+ verbose("provider realpath: \"%.100s\"", canonical_provider);
+ verbose("allowed provider paths: \"%.100s\"", allowed_providers);
+ if (match_pattern_list(allowed_provider, allowed_providers, 1) != 1) {
+ verbose("refusing PKCS#11 add of \"%.100s\": "
+ "provider not allowed", canonical_provider);
+ goto done;
+ }
+
+ count = pkcs11_add_provider(canonical_provider, pin, &keys, &labels);
+ if (count <= 0) {
+ error_f("failed to load provider keys: count:%d", count);
+ goto done;
+ }
+
+ if (get_user_root(con, &user_root) != 0)
+ goto done;
+
+ for (i = 0; i < count; i++) {
+ comment = pkcs11_identity_comment(canonical_provider, labels[i]);
+ for (j = 0; j < (int)ncerts; j++) {
+ if (!sshkey_is_cert(certs[j]) ||
+ !sshkey_equal_public(keys[i], certs[j]))
+ continue;
+ if (pkcs11_make_cert(keys[i], certs[j], &cert) != 0)
+ continue;
+ if (store_pkcs11_identity(user_root, cert,
+ canonical_provider, comment, &identity_change) != 0)
+ goto done;
+ identity_changes = xrecallocarray(identity_changes,
+ nidentity_changes, nidentity_changes + 1,
+ sizeof(*identity_changes));
+ identity_changes[nidentity_changes++] = identity_change;
+ identity_change = NULL;
+ sshkey_free(cert);
+ cert = NULL;
+ identities_stored++;
+ }
+ if (!cert_only && store_pkcs11_identity(user_root, keys[i],
+ canonical_provider, comment, &identity_change) == 0) {
+ identity_changes = xrecallocarray(identity_changes,
+ nidentity_changes, nidentity_changes + 1,
+ sizeof(*identity_changes));
+ identity_changes[nidentity_changes++] = identity_change;
+ identity_change = NULL;
+ identities_stored++;
+ } else if (!cert_only)
+ goto done;
+ }
+
+ if (identities_stored == 0 || store_pkcs11_provider(user_root, con,
+ canonical_provider, pin, pin_len) != 0)
+ goto done;
+ debug("added PKCS11 provider and identities to store");
+ success = 1;
+done:
+ r = 0;
+ if (request_invalid)
+ r = -1;
+ else if (sshbuf_put_u8(response, success ? SSH_AGENT_SUCCESS : SSH_AGENT_FAILURE) != 0)
+ r = -1;
+
+ if (!success && user_root != NULL)
+ rollback_pkcs11_identities(user_root, identity_changes,
+ nidentity_changes);
+
+ sshkey_free(cert);
+ free_pkcs11_identity_change(identity_change);
+ for (k = 0; k < nidentity_changes; k++)
+ free_pkcs11_identity_change(identity_changes[k]);
+ free(identity_changes);
+ for (i = 0; i < count; i++)
+ sshkey_free(keys[i]);
+ free(keys);
+ for (i = 0; i < count; i++)
+ free(labels[i]);
+ free(labels);
+ free_pkcs11_certs(certs, ncerts);
+ pkcs11_terminate();
+ free(provider);
+ if (pin) {
+ SecureZeroMemory(pin, (DWORD)pin_len);
+ free(pin);
+ }
+ if (user_root)
+ RegCloseKey(user_root);
+ return r;
+}
+
+int process_remove_smartcard_key(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
+{
+ char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX];
+ int r = 0, request_invalid = 0, success = 0, index = 0;
+ HKEY user_root = 0;
+
+ if ((r = sshbuf_get_cstring(request, &provider, NULL)) != 0 ||
+ (r = sshbuf_get_cstring(request, &pin, NULL)) != 0) {
+ error("remove smartcard request is invalid");
+ request_invalid = 1;
+ goto done;
+ }
+
+ if (realpath(provider, canonical_provider) == NULL) {
+ error("failed PKCS#11 add of \"%.100s\": realpath: %s",
+ provider, strerror(errno));
+ request_invalid = 1;
+ goto done;
+ }
+
+ // Remove 'drive root' if exists
+ if (canonical_provider[0] == '/')
+ memmove(canonical_provider, canonical_provider + 1, strlen(canonical_provider));
+
+ if (get_user_root(con, &user_root) != 0 ||
+ !is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT, canonical_provider))
+ goto done;
+
+ if (remove_pkcs11_identities(user_root, canonical_provider) != 0 ||
+ remove_matching_subkeys_from_registry(user_root,
+ SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider) != 0) {
+ goto done;
+ }
+
+ success = 1;
+done:
+ r = 0;
+ if (request_invalid)
+ r = -1;
+ else if (sshbuf_put_u8(response, success ? SSH_AGENT_SUCCESS : SSH_AGENT_FAILURE) != 0)
+ r = -1;
+ if (provider)
+ free(provider);
+ if (pin)
+ free(pin);
+ if (user_root)
+ RegCloseKey(user_root);
+ return r;
+}
+
+#pragma warning(pop)
+
+#endif /* ENABLE_PKCS11 */
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.h b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.h
new file mode 100644
index 000000000000..82242578a294
--- /dev/null
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.h
@@ -0,0 +1,16 @@
+/*
+ * PKCS#11 provider and identity persistence of the Windows ssh-agent.
+ * Split out of keyagent-request.c.
+ */
+
+#pragma once
+
+#include
+
+#include "sshkey.h"
+
+#ifdef ENABLE_PKCS11
+void free_pkcs11_sign_provider(char **, char **, DWORD, char **, DWORD,
+ struct sshkey ***, int);
+int load_pkcs11_identities(HKEY, const char *, struct sshkey **, int);
+#endif /* ENABLE_PKCS11 */
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-registry.c b/contrib/win32/win32compat/ssh-agent/keyagent-registry.c
new file mode 100644
index 000000000000..79cb86cd7432
--- /dev/null
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-registry.c
@@ -0,0 +1,228 @@
+/*
+ * Author: Manoj Ampalam
+ * ssh-agent implementation on Windows
+ *
+ * Copyright (c) 2015 Microsoft Corp.
+ * All rights reserved
+ *
+ * Microsoft openssh win32 port
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "agent.h"
+#include "config.h"
+#include "pkcs11-cert.h"
+#include "xmalloc.h"
+#include "keyagent-registry.h"
+
+#pragma warning(push, 3)
+
+/*
+ * get registry root where keys are stored
+ * user keys are stored in user's hive
+ * while system keys (host keys) in HKLM
+ */
+int
+get_user_root(struct agent_connection* con, HKEY *root)
+{
+ int r = 0;
+ LONG ret;
+ *root = HKEY_LOCAL_MACHINE;
+
+ if (con->client_type <= ADMIN_USER) {
+ if (ImpersonateLoggedOnUser(con->client_impersonation_token) == FALSE)
+ return -1;
+ *root = NULL;
+ /*
+ * TODO - check that user profile is loaded,
+ * otherwise, this will return default profile
+ */
+ if ((ret = RegOpenCurrentUser(KEY_ALL_ACCESS, root)) != ERROR_SUCCESS) {
+ debug("unable to open user's registry hive, ERROR - %d", ret);
+ r = -1;
+ }
+
+ RevertToSelf();
+ }
+ return r;
+}
+
+int
+convert_blob(struct agent_connection* con, const char *blob, DWORD blen, char **eblob, DWORD *eblen, int encrypt) {
+ int success = 0;
+ DATA_BLOB in, out;
+ errno_t r = 0;
+
+ if (con->client_type <= ADMIN_USER)
+ if (ImpersonateLoggedOnUser(con->client_impersonation_token) == FALSE)
+ return -1;
+
+ in.cbData = blen;
+ in.pbData = (char*)blob;
+ out.cbData = 0;
+ out.pbData = NULL;
+
+ if (encrypt) {
+ if (!CryptProtectData(&in, NULL, NULL, 0, NULL, 0, &out)) {
+ debug("cannot encrypt data");
+ goto done;
+ }
+ } else {
+ if (!CryptUnprotectData(&in, NULL, NULL, 0, NULL, 0, &out)) {
+ debug("cannot decrypt data");
+ goto done;
+ }
+ }
+
+ *eblob = malloc(out.cbData);
+ if (*eblob == NULL)
+ goto done;
+
+ if((r = memcpy_s(*eblob, out.cbData, out.pbData, out.cbData)) != 0) {
+ debug("memcpy_s failed with error: %d.", r);
+ goto done;
+ }
+ *eblen = out.cbData;
+ success = 1;
+done:
+ if (out.pbData)
+ LocalFree(out.pbData);
+ if (con->client_type <= ADMIN_USER)
+ RevertToSelf();
+ return success? 0: -1;
+}
+
+int
+remove_matching_subkeys_from_registry(HKEY user_root, wchar_t const* key_name, wchar_t const* value_name_to_remove, char const* value_data_to_remove) {
+ int index = 0, success = 0;
+ DWORD data_len;
+ HKEY root = 0, sub = 0;
+ char *data = NULL;
+ wchar_t sub_name[MAX_KEY_LENGTH];
+ DWORD sub_name_len = MAX_KEY_LENGTH;
+ LSTATUS retCode;
+
+ if (RegOpenKeyExW(user_root, key_name, 0, DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY, &root) != 0) {
+ goto done;
+ }
+
+ while (1) {
+ sub_name_len = MAX_KEY_LENGTH;
+ if (sub) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ if ((retCode = RegEnumKeyExW(root, index++, sub_name, &sub_name_len, NULL, NULL, NULL, NULL)) == 0) {
+ if (RegOpenKeyExW(root, sub_name, 0, KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) == 0 &&
+ RegQueryValueExW(sub, value_name_to_remove, 0, NULL, NULL, &data_len) == 0 &&
+ data_len <= MAX_VALUE_DATA_LENGTH) {
+
+ if (data)
+ free(data);
+ data = NULL;
+
+ if ((data = malloc(data_len + 1)) == NULL ||
+ RegQueryValueExW(sub, value_name_to_remove, 0, NULL, data, &data_len) != 0)
+ goto done;
+ data[data_len] = '\0';
+ if (pkcs11_provider_equal((u_char *)data, data_len,
+ value_data_to_remove)) {
+ if (RegDeleteTreeW(root, sub_name) != 0)
+ goto done;
+ --index;
+ }
+ }
+ }
+ else {
+ if (retCode == ERROR_NO_MORE_ITEMS)
+ success = 1;
+ break;
+ }
+ }
+done:
+ if (data)
+ free(data);
+ if (root)
+ RegCloseKey(root);
+ if (sub)
+ RegCloseKey(sub);
+ return success ? 0 : -1;
+}
+
+int
+is_reg_sub_key_exists(HKEY user_root, wchar_t const* key_name, char const* sub_key_name) {
+ int rv = 0;
+ HKEY root = 0, sub = 0;
+
+ if (RegOpenKeyExW(user_root, key_name, 0, STANDARD_RIGHTS_READ | KEY_WOW64_64KEY, &root) != 0 ||
+ RegOpenKeyExA(root, sub_key_name, 0, STANDARD_RIGHTS_READ | KEY_WOW64_64KEY, &sub) != 0 || !sub) {
+ rv = 0;
+ goto done;
+ }
+
+ rv = 1;
+done:
+ if (root)
+ RegCloseKey(root);
+ return rv;
+}
+
+int
+read_optional_reg_value(HKEY key, const wchar_t *name, int *presentp,
+ DWORD *typep, u_char **datap, DWORD *lenp)
+{
+ LSTATUS status;
+
+ *presentp = 0;
+ *datap = NULL;
+ *lenp = 0;
+ status = RegQueryValueExW(key, name, NULL, typep, NULL, lenp);
+ if (status == ERROR_FILE_NOT_FOUND)
+ return 0;
+ if (status != ERROR_SUCCESS || *lenp > MAX_MESSAGE_SIZE)
+ return -1;
+ *datap = xmalloc(*lenp == 0 ? 1 : *lenp);
+ if (RegQueryValueExW(key, name, NULL, typep, *datap,
+ lenp) != ERROR_SUCCESS) {
+ free(*datap);
+ *datap = NULL;
+ return -1;
+ }
+ *presentp = 1;
+ return 0;
+}
+
+int
+restore_optional_reg_value(HKEY key, const wchar_t *name, int present,
+ DWORD type, const u_char *data, DWORD len)
+{
+ LSTATUS status;
+
+ if (present)
+ return RegSetValueExW(key, name, 0, type, data, len) ==
+ ERROR_SUCCESS ? 0 : -1;
+ status = RegDeleteValueW(key, name);
+ return status == ERROR_SUCCESS || status == ERROR_FILE_NOT_FOUND ? 0 : -1;
+}
+
+#pragma warning(pop)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-registry.h b/contrib/win32/win32compat/ssh-agent/keyagent-registry.h
new file mode 100644
index 000000000000..c05e925147bb
--- /dev/null
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-registry.h
@@ -0,0 +1,30 @@
+/*
+ * Registry and DPAPI helpers of the Windows ssh-agent key store.
+ * Split out of keyagent-request.c.
+ */
+
+#pragma once
+
+#include
+
+#include "sshbuf.h"
+
+struct agent_connection;
+
+#define MAX_KEY_LENGTH 255
+#define MAX_VALUE_NAME_LENGTH 16383
+#define MAX_VALUE_DATA_LENGTH 2048
+
+/* Registry keys are only accessible to SYSTEM and administrators. */
+#define REG_KEY_SDDL L"D:P(A;; GA;;; SY)(A;; GA;;; BA)"
+
+int get_user_root(struct agent_connection *, HKEY *);
+int convert_blob(struct agent_connection *, const char *, DWORD, char **,
+ DWORD *, int);
+int remove_matching_subkeys_from_registry(HKEY, wchar_t const *,
+ wchar_t const *, char const *);
+int is_reg_sub_key_exists(HKEY, wchar_t const *, char const *);
+int read_optional_reg_value(HKEY, const wchar_t *, int *, DWORD *,
+ u_char **, DWORD *);
+int restore_optional_reg_value(HKEY, const wchar_t *, int, DWORD,
+ const u_char *, DWORD);
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index c9e23b7e204c..26fd2e2ad8fb 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -32,194 +32,23 @@
#include "agent.h"
#include "agent-request.h"
#include "config.h"
-#include "match.h"
#include
#include "pkcs11-cert.h"
#ifdef ENABLE_PKCS11
#include "ssh-pkcs11.h"
#endif
#include "xmalloc.h"
+#include "keyagent-registry.h"
+#include "keyagent-pkcs11.h"
#pragma warning(push, 3)
-#define MAX_KEY_LENGTH 255
-#define MAX_VALUE_NAME_LENGTH 16383
-#define MAX_VALUE_DATA_LENGTH 2048
-
-extern char* allowed_providers;
-extern int remote_add_provider;
-
-/*
- * get registry root where keys are stored
- * user keys are stored in user's hive
- * while system keys (host keys) in HKLM
- */
-
extern struct sshkey *
lookup_key(const struct sshkey *k);
-extern void
-add_key(struct sshkey *k, char *name);
-
extern void
del_all_keys();
-static int
-get_user_root(struct agent_connection* con, HKEY *root)
-{
- int r = 0;
- LONG ret;
- *root = HKEY_LOCAL_MACHINE;
-
- if (con->client_type <= ADMIN_USER) {
- if (ImpersonateLoggedOnUser(con->client_impersonation_token) == FALSE)
- return -1;
- *root = NULL;
- /*
- * TODO - check that user profile is loaded,
- * otherwise, this will return default profile
- */
- if ((ret = RegOpenCurrentUser(KEY_ALL_ACCESS, root)) != ERROR_SUCCESS) {
- debug("unable to open user's registry hive, ERROR - %d", ret);
- r = -1;
- }
-
- RevertToSelf();
- }
- return r;
-}
-
-static int
-convert_blob(struct agent_connection* con, const char *blob, DWORD blen, char **eblob, DWORD *eblen, int encrypt) {
- int success = 0;
- DATA_BLOB in, out;
- errno_t r = 0;
-
- if (con->client_type <= ADMIN_USER)
- if (ImpersonateLoggedOnUser(con->client_impersonation_token) == FALSE)
- return -1;
-
- in.cbData = blen;
- in.pbData = (char*)blob;
- out.cbData = 0;
- out.pbData = NULL;
-
- if (encrypt) {
- if (!CryptProtectData(&in, NULL, NULL, 0, NULL, 0, &out)) {
- debug("cannot encrypt data");
- goto done;
- }
- } else {
- if (!CryptUnprotectData(&in, NULL, NULL, 0, NULL, 0, &out)) {
- debug("cannot decrypt data");
- goto done;
- }
- }
-
- *eblob = malloc(out.cbData);
- if (*eblob == NULL)
- goto done;
-
- if((r = memcpy_s(*eblob, out.cbData, out.pbData, out.cbData)) != 0) {
- debug("memcpy_s failed with error: %d.", r);
- goto done;
- }
- *eblen = out.cbData;
- success = 1;
-done:
- if (out.pbData)
- LocalFree(out.pbData);
- if (con->client_type <= ADMIN_USER)
- RevertToSelf();
- return success? 0: -1;
-}
-
-/*
- * in user_root sub tree under key_name key
- * remove all sub keys with value name value_name_to_remove
- * and value data value_data_to_remove
- */
-static int
-remove_matching_subkeys_from_registry(HKEY user_root, wchar_t const* key_name, wchar_t const* value_name_to_remove, char const* value_data_to_remove) {
- int index = 0, success = 0;
- DWORD data_len;
- HKEY root = 0, sub = 0;
- char *data = NULL;
- wchar_t sub_name[MAX_KEY_LENGTH];
- DWORD sub_name_len = MAX_KEY_LENGTH;
- LSTATUS retCode;
-
- if (RegOpenKeyExW(user_root, key_name, 0, DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY, &root) != 0) {
- goto done;
- }
-
- while (1) {
- sub_name_len = MAX_KEY_LENGTH;
- if (sub) {
- RegCloseKey(sub);
- sub = NULL;
- }
- if ((retCode = RegEnumKeyExW(root, index++, sub_name, &sub_name_len, NULL, NULL, NULL, NULL)) == 0) {
- if (RegOpenKeyExW(root, sub_name, 0, KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) == 0 &&
- RegQueryValueExW(sub, value_name_to_remove, 0, NULL, NULL, &data_len) == 0 &&
- data_len <= MAX_VALUE_DATA_LENGTH) {
-
- if (data)
- free(data);
- data = NULL;
-
- if ((data = malloc(data_len + 1)) == NULL ||
- RegQueryValueExW(sub, value_name_to_remove, 0, NULL, data, &data_len) != 0)
- goto done;
- data[data_len] = '\0';
- if (pkcs11_provider_equal((u_char *)data, data_len,
- value_data_to_remove)) {
- if (RegDeleteTreeW(root, sub_name) != 0)
- goto done;
- --index;
- }
- }
- }
- else {
- if (retCode == ERROR_NO_MORE_ITEMS)
- success = 1;
- break;
- }
- }
-done:
- if (data)
- free(data);
- if (root)
- RegCloseKey(root);
- if (sub)
- RegCloseKey(sub);
- return success ? 0 : -1;
-}
-
-/*
- * in user_root sub tree under key_name key
- * check whether sub_key_name sub key exists
- */
-static int
-is_reg_sub_key_exists(HKEY user_root, wchar_t const* key_name, char const* sub_key_name) {
- int rv = 0;
- HKEY root = 0, sub = 0;
-
- if (RegOpenKeyExW(user_root, key_name, 0, STANDARD_RIGHTS_READ | KEY_WOW64_64KEY, &root) != 0 ||
- RegOpenKeyExA(root, sub_key_name, 0, STANDARD_RIGHTS_READ | KEY_WOW64_64KEY, &sub) != 0 || !sub) {
- rv = 0;
- goto done;
- }
-
- rv = 1;
-done:
- if (root)
- RegCloseKey(root);
- return rv;
-}
-
-#define REG_KEY_SDDL L"D:P(A;; GA;;; SY)(A;; GA;;; BA)"
-
int
process_unsupported_request(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
{
@@ -371,519 +200,6 @@ process_add_identity(struct sshbuf* request, struct sshbuf* response, struct age
return r;
}
-#ifdef ENABLE_PKCS11
-struct pkcs11_identity_change {
- char *name;
- int created;
- int had_provider;
- DWORD provider_type;
- u_char *provider;
- DWORD provider_len;
- int had_comment;
- DWORD comment_type;
- u_char *comment;
- DWORD comment_len;
-};
-
-static void
-free_pkcs11_identity_change(struct pkcs11_identity_change *change)
-{
- if (change == NULL)
- return;
- free(change->name);
- free(change->provider);
- free(change->comment);
- free(change);
-}
-
-static int
-read_optional_reg_value(HKEY key, const wchar_t *name, int *presentp,
- DWORD *typep, u_char **datap, DWORD *lenp)
-{
- LSTATUS status;
-
- *presentp = 0;
- *datap = NULL;
- *lenp = 0;
- status = RegQueryValueExW(key, name, NULL, typep, NULL, lenp);
- if (status == ERROR_FILE_NOT_FOUND)
- return 0;
- if (status != ERROR_SUCCESS || *lenp > MAX_MESSAGE_SIZE)
- return -1;
- *datap = xmalloc(*lenp == 0 ? 1 : *lenp);
- if (RegQueryValueExW(key, name, NULL, typep, *datap,
- lenp) != ERROR_SUCCESS) {
- free(*datap);
- *datap = NULL;
- return -1;
- }
- *presentp = 1;
- return 0;
-}
-
-static int
-restore_optional_reg_value(HKEY key, const wchar_t *name, int present,
- DWORD type, const u_char *data, DWORD len)
-{
- LSTATUS status;
-
- if (present)
- return RegSetValueExW(key, name, 0, type, data, len) ==
- ERROR_SUCCESS ? 0 : -1;
- status = RegDeleteValueW(key, name);
- return status == ERROR_SUCCESS || status == ERROR_FILE_NOT_FOUND ? 0 : -1;
-}
-
-static int
-restore_pkcs11_identity_metadata(HKEY key,
- const struct pkcs11_identity_change *change)
-{
- int r1, r2;
-
- r1 = restore_optional_reg_value(key, L"provider",
- change->had_provider, change->provider_type, change->provider,
- change->provider_len);
- r2 = restore_optional_reg_value(key, L"comment", change->had_comment,
- change->comment_type, change->comment, change->comment_len);
- return r1 == 0 && r2 == 0 ? 0 : -1;
-}
-
-static int
-pkcs11_identity_reusable(HKEY sub, const struct pkcs11_identity_change *change,
- const u_char *blob, size_t blob_len, int key_type, const char *provider)
-{
- struct pkcs11_identity_entry entry;
- u_char *pub = NULL, *dflt = NULL;
- DWORD pub_type, dflt_type, pub_len, dflt_len, type, type_kind;
- DWORD type_len = sizeof(type);
- int has_pub, has_dflt, reusable = 0;
-
- memset(&entry, 0, sizeof(entry));
- if (read_optional_reg_value(sub, L"pub", &has_pub, &pub_type, &pub,
- &pub_len) != 0 ||
- read_optional_reg_value(sub, NULL, &has_dflt, &dflt_type, &dflt,
- &dflt_len) != 0)
- goto out;
- if (has_pub && pub_type == REG_BINARY) {
- entry.pub = pub;
- entry.pub_len = pub_len;
- }
- if (has_dflt && dflt_type == REG_BINARY) {
- entry.dflt = dflt;
- entry.dflt_len = dflt_len;
- }
- if (RegQueryValueExW(sub, L"type", NULL, &type_kind, (BYTE *)&type,
- &type_len) == ERROR_SUCCESS && type_kind == REG_DWORD &&
- type_len == sizeof(type)) {
- entry.has_type = 1;
- entry.type = (int)type;
- }
- if (change->had_provider) {
- entry.provider = change->provider;
- entry.provider_len = change->provider_len;
- }
- if (change->had_comment) {
- entry.comment = change->comment;
- entry.comment_len = change->comment_len;
- }
- reusable = pkcs11_identity_entry_matches(&entry, blob, blob_len,
- key_type, provider);
- out:
- free(pub);
- free(dflt);
- return reusable;
-}
-
-static int
-store_pkcs11_identity(HKEY user_root, const struct sshkey *key,
- const char *provider, const char *comment,
- struct pkcs11_identity_change **changep)
-{
- SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
- HKEY reg = NULL, sub = NULL;
- u_char *blob = NULL;
- size_t blob_len;
- char *thumbprint = NULL;
- struct pkcs11_identity_change *change = NULL;
- DWORD disposition = 0;
- ULONG sd_len = 0;
- int success = 0;
-
- if (changep == NULL || provider == NULL || comment == NULL)
- return -1;
- *changep = NULL;
- sa.nLength = sizeof(sa);
- if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
- SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sd_len) ||
- sshkey_to_blob(key, &blob, &blob_len) != 0 ||
- blob_len == 0 || blob_len > MAX_MESSAGE_SIZE ||
- (thumbprint = pkcs11_identity_name(key, blob, blob_len)) == NULL ||
- RegCreateKeyExW(user_root, SSH_KEYS_ROOT, 0, NULL, 0,
- KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
- RegCreateKeyExA(reg, thumbprint, 0, NULL, 0,
- KEY_WRITE | KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sa, &sub,
- &disposition) != ERROR_SUCCESS) {
- error_f("failed to persist PKCS11 identity");
- goto out;
- }
- change = xcalloc(1, sizeof(*change));
- change->name = xstrdup(thumbprint);
- if (disposition == REG_OPENED_EXISTING_KEY) {
- if (read_optional_reg_value(sub, L"provider",
- &change->had_provider, &change->provider_type,
- &change->provider, &change->provider_len) != 0 ||
- read_optional_reg_value(sub, L"comment",
- &change->had_comment, &change->comment_type,
- &change->comment, &change->comment_len) != 0) {
- error_f("failed to read PKCS11 identity metadata");
- goto out;
- }
- if (!pkcs11_identity_reusable(sub, change, blob, blob_len,
- key->type, provider)) {
- error_f("refusing to replace existing identity %s "
- "not created for this provider", thumbprint);
- goto out;
- }
- if (RegSetValueExW(sub, L"provider", 0, REG_BINARY,
- (const BYTE *)provider, (DWORD)strlen(provider)) !=
- ERROR_SUCCESS ||
- RegSetValueExW(sub, L"comment", 0, REG_BINARY,
- (const BYTE *)comment, (DWORD)strlen(comment)) !=
- ERROR_SUCCESS) {
- error_f("failed to update PKCS11 identity metadata");
- if (restore_pkcs11_identity_metadata(sub, change) != 0)
- error_f("failed to restore PKCS11 identity metadata");
- goto out;
- }
- } else {
- change->created = 1;
- if (RegSetValueExW(sub, NULL, 0, REG_BINARY, blob,
- (DWORD)blob_len) != ERROR_SUCCESS ||
- RegSetValueExW(sub, L"pub", 0, REG_BINARY, blob,
- (DWORD)blob_len) != ERROR_SUCCESS ||
- RegSetValueExW(sub, L"type", 0, REG_DWORD,
- (const BYTE *)&key->type, sizeof(key->type)) != ERROR_SUCCESS ||
- RegSetValueExW(sub, L"provider", 0, REG_BINARY,
- (const BYTE *)provider, (DWORD)strlen(provider)) !=
- ERROR_SUCCESS ||
- RegSetValueExW(sub, L"comment", 0, REG_BINARY,
- (const BYTE *)comment, (DWORD)strlen(comment)) !=
- ERROR_SUCCESS) {
- error_f("failed to persist PKCS11 identity");
- goto out;
- }
- }
- *changep = change;
- change = NULL;
- success = 1;
- out:
- if (sub != NULL) {
- RegCloseKey(sub);
- sub = NULL;
- }
- if (!success && disposition == REG_CREATED_NEW_KEY && reg != NULL &&
- thumbprint != NULL)
- RegDeleteTreeA(reg, thumbprint);
- if (reg != NULL)
- RegCloseKey(reg);
- if (sa.lpSecurityDescriptor != NULL)
- LocalFree(sa.lpSecurityDescriptor);
- free_pkcs11_identity_change(change);
- free(thumbprint);
- free(blob);
- return success ? 0 : -1;
-}
-
-static int
-store_pkcs11_provider(HKEY user_root, struct agent_connection *con,
- const char *provider, const char *pin, size_t pin_len)
-{
- SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
- HKEY reg = NULL, sub = NULL;
- char *epin = NULL;
- DWORD epin_len = 0;
- DWORD disposition = 0;
- ULONG sd_len = 0;
- int success = 0;
-
- sa.nLength = sizeof(sa);
- if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL,
- SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sd_len) ||
- convert_blob(con, pin, (DWORD)pin_len, &epin, &epin_len, TRUE) != 0 ||
- RegCreateKeyExW(user_root, SSH_PKCS11_PROVIDERS_ROOT, 0, NULL, 0,
- KEY_WRITE | KEY_WOW64_64KEY, &sa, ®, NULL) != ERROR_SUCCESS ||
- RegCreateKeyExA(reg, provider, 0, NULL, 0,
- KEY_WRITE | KEY_WOW64_64KEY, &sa, &sub,
- &disposition) != ERROR_SUCCESS ||
- RegSetValueExW(sub, L"provider", 0, REG_BINARY,
- (const BYTE *)provider, (DWORD)strlen(provider)) != ERROR_SUCCESS ||
- RegSetValueExW(sub, L"pin", 0, REG_BINARY, (const BYTE *)epin,
- epin_len) != ERROR_SUCCESS) {
- error_f("failed to persist PKCS11 provider");
- goto out;
- }
- success = 1;
- out:
- if (epin != NULL) {
- SecureZeroMemory(epin, epin_len);
- free(epin);
- }
- if (sub != NULL) {
- RegCloseKey(sub);
- sub = NULL;
- }
- if (!success && disposition == REG_CREATED_NEW_KEY && reg != NULL)
- RegDeleteTreeA(reg, provider);
- if (reg != NULL)
- RegCloseKey(reg);
- if (sa.lpSecurityDescriptor != NULL)
- LocalFree(sa.lpSecurityDescriptor);
- return success ? 0 : -1;
-}
-
-static void
-rollback_pkcs11_identities(HKEY user_root,
- struct pkcs11_identity_change **changes,
- size_t nidentities)
-{
- HKEY reg = NULL, sub = NULL;
- size_t i;
-
- if (nidentities == 0)
- return;
- if (RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
- DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY,
- ®) != ERROR_SUCCESS) {
- error_f("failed to open PKCS11 identities for rollback");
- return;
- }
- for (i = nidentities; i > 0; i--) {
- if (changes[i - 1]->created) {
- if (RegDeleteTreeA(reg, changes[i - 1]->name) !=
- ERROR_SUCCESS)
- error_f("failed to roll back PKCS11 identity");
- continue;
- }
- if (RegOpenKeyExA(reg, changes[i - 1]->name, 0,
- KEY_SET_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS ||
- restore_pkcs11_identity_metadata(sub, changes[i - 1]) != 0)
- error_f("failed to roll back PKCS11 identity metadata");
- if (sub != NULL) {
- RegCloseKey(sub);
- sub = NULL;
- }
- }
- RegCloseKey(reg);
-}
-
-static int
-remove_pkcs11_identities(HKEY user_root, const char *provider)
-{
- HKEY root = NULL, sub = NULL;
- wchar_t sub_name[MAX_KEY_LENGTH];
- DWORD sub_name_len, type, data_len;
- u_char *data = NULL;
- int index = 0, present, remove;
- LSTATUS status;
-
- status = RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
- DELETE | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY, &root);
- if (status == ERROR_FILE_NOT_FOUND)
- return 0;
- if (status != ERROR_SUCCESS)
- return -1;
- for (;;) {
- sub_name_len = MAX_KEY_LENGTH;
- status = RegEnumKeyExW(root, index, sub_name, &sub_name_len,
- NULL, NULL, NULL, NULL);
- if (status == ERROR_NO_MORE_ITEMS)
- break;
- if (status != ERROR_SUCCESS) {
- index++;
- continue;
- }
- if (RegOpenKeyExW(root, sub_name, 0,
- KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS) {
- index++;
- continue;
- }
- free(data);
- data = NULL;
- if (read_optional_reg_value(sub, L"provider", &present, &type,
- &data, &data_len) != 0 ||
- (!present && read_optional_reg_value(sub, L"comment",
- &present, &type, &data, &data_len) != 0)) {
- RegCloseKey(sub);
- sub = NULL;
- index++;
- continue;
- }
- remove = present && pkcs11_provider_equal(data, data_len, provider);
- RegCloseKey(sub);
- sub = NULL;
- if (remove) {
- if (RegDeleteTreeW(root, sub_name) != ERROR_SUCCESS) {
- RegCloseKey(root);
- free(data);
- return -1;
- }
- } else
- index++;
- }
- RegCloseKey(root);
- free(data);
- return 0;
-}
-
-static int
-load_pkcs11_identities(HKEY user_root, const char *provider,
- struct sshkey **token_keys, int nkeys)
-{
- HKEY root = NULL, sub = NULL;
- wchar_t sub_name[MAX_KEY_LENGTH];
- DWORD sub_name_len, blob_len, comment_len, association_len;
- u_char *blob = NULL;
- char *comment = NULL, *association = NULL;
- struct sshkey *registered = NULL, *cert = NULL;
- u_char *plain_added = NULL;
- int i, index = 0, legacy, loaded = 0;
- LSTATUS status;
-
- if (nkeys > 0)
- plain_added = xcalloc((size_t)nkeys, sizeof(*plain_added));
- status = RegOpenKeyExW(user_root, SSH_KEYS_ROOT, 0,
- KEY_ENUMERATE_SUB_KEYS | KEY_QUERY_VALUE | KEY_WOW64_64KEY, &root);
- if (status == ERROR_FILE_NOT_FOUND)
- goto out;
- if (status != ERROR_SUCCESS) {
- error_f("failed to open persisted identities: %ld", status);
- loaded = -1;
- goto out;
- }
- for (;;) {
- sub_name_len = MAX_KEY_LENGTH;
- if (sub != NULL) {
- RegCloseKey(sub);
- sub = NULL;
- }
- status = RegEnumKeyExW(root, index++, sub_name, &sub_name_len,
- NULL, NULL, NULL, NULL);
- if (status == ERROR_NO_MORE_ITEMS)
- break;
- if (status != ERROR_SUCCESS)
- continue;
- if (RegOpenKeyExW(root, sub_name, 0,
- KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) != ERROR_SUCCESS ||
- RegQueryValueExW(sub, L"pub", NULL, NULL, NULL,
- &blob_len) != ERROR_SUCCESS ||
- RegQueryValueExW(sub, L"comment", NULL, NULL, NULL,
- &comment_len) != ERROR_SUCCESS ||
- blob_len == 0 || blob_len > MAX_MESSAGE_SIZE ||
- comment_len > MAX_MESSAGE_SIZE)
- continue;
- status = RegQueryValueExW(sub, L"provider", NULL, NULL, NULL,
- &association_len);
- if (status == ERROR_FILE_NOT_FOUND) {
- legacy = 1;
- association_len = comment_len;
- } else if (status == ERROR_SUCCESS &&
- association_len <= MAX_MESSAGE_SIZE)
- legacy = 0;
- else
- continue;
- free(blob);
- free(comment);
- free(association);
- blob = xmalloc(blob_len);
- comment = xmalloc((size_t)comment_len + 1);
- association = xmalloc((size_t)association_len + 1);
- if (RegQueryValueExW(sub, L"pub", NULL, NULL, blob,
- &blob_len) != ERROR_SUCCESS ||
- RegQueryValueExW(sub, L"comment", NULL, NULL,
- (BYTE *)comment, &comment_len) != ERROR_SUCCESS ||
- (!legacy && RegQueryValueExW(sub, L"provider", NULL, NULL,
- (BYTE *)association, &association_len) != ERROR_SUCCESS))
- continue;
- comment[comment_len] = '\0';
- if (legacy)
- memcpy(association, comment, comment_len);
- association[association_len] = '\0';
- if (!pkcs11_provider_equal((u_char *)association, association_len,
- provider))
- continue;
- sshkey_free(registered);
- registered = NULL;
- if (sshkey_from_blob(blob, blob_len, ®istered) != 0)
- continue;
- for (i = 0; i < nkeys; i++) {
- if (token_keys[i] == NULL)
- continue;
- if (sshkey_is_cert(registered)) {
- if (!sshkey_equal_public(token_keys[i], registered))
- continue;
- if (pkcs11_make_cert(token_keys[i], registered,
- &cert) != 0)
- continue;
- add_key(cert, (char *)provider);
- cert = NULL;
- loaded++;
- break;
- }
- if (!plain_added[i] &&
- sshkey_equal(token_keys[i], registered)) {
- plain_added[i] = 1;
- break;
- }
- }
- }
- for (i = 0; i < nkeys; i++) {
- if (!plain_added[i] || token_keys[i] == NULL)
- continue;
- add_key(token_keys[i], (char *)provider);
- token_keys[i] = NULL;
- loaded++;
- }
- out:
- sshkey_free(cert);
- sshkey_free(registered);
- free(plain_added);
- free(association);
- free(comment);
- free(blob);
- if (sub != NULL)
- RegCloseKey(sub);
- if (root != NULL)
- RegCloseKey(root);
- return loaded;
-}
-
-static void
-free_pkcs11_sign_provider(char **providerp, char **pinp, DWORD pin_len,
- char **epinp, DWORD epin_len, struct sshkey ***keysp, int nkeys)
-{
- int i;
-
- if (*keysp != NULL) {
- for (i = 0; i < nkeys; i++)
- sshkey_free((*keysp)[i]);
- free(*keysp);
- *keysp = NULL;
- }
- free(*providerp);
- *providerp = NULL;
- if (*pinp != NULL) {
- SecureZeroMemory(*pinp, pin_len);
- free(*pinp);
- *pinp = NULL;
- }
- if (*epinp != NULL) {
- SecureZeroMemory(*epinp, epin_len);
- free(*epinp);
- *epinp = NULL;
- }
-}
-#endif /* ENABLE_PKCS11 */
-
static int sign_blob(const struct sshkey *pubkey, u_char ** sig, size_t *siglen,
const u_char *blob, size_t blen, u_int flags, struct agent_connection* con)
{
@@ -1220,201 +536,6 @@ process_remove_all(struct sshbuf* request, struct sshbuf* response, struct agent
return r;
}
-#ifdef ENABLE_PKCS11
-int
-process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
- struct agent_connection *con)
-{
- char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX] = { 0 };
- char allowed_provider[PATH_MAX], **labels = NULL;
- const char *comment;
- int i, j, count = 0, r = 0, request_invalid = 0, success = 0;
- int cert_only = 0, identities_stored = 0;
- struct sshkey **keys = NULL, **certs = NULL, *cert = NULL;
- struct pkcs11_identity_change *identity_change = NULL;
- struct pkcs11_identity_change **identity_changes = NULL;
- size_t k, pin_len = 0, ncerts = 0, nidentity_changes = 0;
- HKEY user_root = NULL;
-
- pkcs11_init(0);
-
- if ((r = sshbuf_get_cstring(request, &provider, NULL)) != 0 ||
- (r = sshbuf_get_cstring(request, &pin, &pin_len)) != 0 ||
- pin_len > 256) {
- error("add smartcard request is invalid");
- request_invalid = 1;
- goto done;
- }
- if (sshbuf_len(request) != 0 &&
- parse_pkcs11_add_constraints(request, &cert_only, &certs,
- &ncerts) != 0) {
- error("add smartcard constraints are invalid");
- request_invalid = 1;
- goto done;
- }
-
- if (con->nsession_ids != 0 && !remote_add_provider) {
- verbose("failed PKCS#11 add of \"%.100s\": remote addition of "
- "providers is disabled", provider);
- goto done;
- }
-
- if (realpath(provider, canonical_provider) == NULL) {
- error("failed PKCS#11 add of \"%.100s\": realpath: %s",
- provider, strerror(errno));
- request_invalid = 1;
- goto done;
- }
-
- /* Remove the leading slash from the canonical Windows drive path. */
- if (canonical_provider[0] == '/')
- memmove(canonical_provider, canonical_provider + 1,
- strlen(canonical_provider));
- strcpy_s(allowed_provider, sizeof(allowed_provider), canonical_provider);
- for (i = 0; allowed_provider[i] != '\0'; i++) {
- if (allowed_provider[i] == '/')
- allowed_provider[i] = '\\';
- }
- to_lower_case(allowed_provider);
- verbose("provider realpath: \"%.100s\"", canonical_provider);
- verbose("allowed provider paths: \"%.100s\"", allowed_providers);
- if (match_pattern_list(allowed_provider, allowed_providers, 1) != 1) {
- verbose("refusing PKCS#11 add of \"%.100s\": "
- "provider not allowed", canonical_provider);
- goto done;
- }
-
- count = pkcs11_add_provider(canonical_provider, pin, &keys, &labels);
- if (count <= 0) {
- error_f("failed to load provider keys: count:%d", count);
- goto done;
- }
-
- if (get_user_root(con, &user_root) != 0)
- goto done;
-
- for (i = 0; i < count; i++) {
- comment = pkcs11_identity_comment(canonical_provider, labels[i]);
- for (j = 0; j < (int)ncerts; j++) {
- if (!sshkey_is_cert(certs[j]) ||
- !sshkey_equal_public(keys[i], certs[j]))
- continue;
- if (pkcs11_make_cert(keys[i], certs[j], &cert) != 0)
- continue;
- if (store_pkcs11_identity(user_root, cert,
- canonical_provider, comment, &identity_change) != 0)
- goto done;
- identity_changes = xrecallocarray(identity_changes,
- nidentity_changes, nidentity_changes + 1,
- sizeof(*identity_changes));
- identity_changes[nidentity_changes++] = identity_change;
- identity_change = NULL;
- sshkey_free(cert);
- cert = NULL;
- identities_stored++;
- }
- if (!cert_only && store_pkcs11_identity(user_root, keys[i],
- canonical_provider, comment, &identity_change) == 0) {
- identity_changes = xrecallocarray(identity_changes,
- nidentity_changes, nidentity_changes + 1,
- sizeof(*identity_changes));
- identity_changes[nidentity_changes++] = identity_change;
- identity_change = NULL;
- identities_stored++;
- } else if (!cert_only)
- goto done;
- }
-
- if (identities_stored == 0 || store_pkcs11_provider(user_root, con,
- canonical_provider, pin, pin_len) != 0)
- goto done;
- debug("added PKCS11 provider and identities to store");
- success = 1;
-done:
- r = 0;
- if (request_invalid)
- r = -1;
- else if (sshbuf_put_u8(response, success ? SSH_AGENT_SUCCESS : SSH_AGENT_FAILURE) != 0)
- r = -1;
-
- if (!success && user_root != NULL)
- rollback_pkcs11_identities(user_root, identity_changes,
- nidentity_changes);
-
- sshkey_free(cert);
- free_pkcs11_identity_change(identity_change);
- for (k = 0; k < nidentity_changes; k++)
- free_pkcs11_identity_change(identity_changes[k]);
- free(identity_changes);
- for (i = 0; i < count; i++)
- sshkey_free(keys[i]);
- free(keys);
- for (i = 0; i < count; i++)
- free(labels[i]);
- free(labels);
- free_pkcs11_certs(certs, ncerts);
- pkcs11_terminate();
- free(provider);
- if (pin) {
- SecureZeroMemory(pin, (DWORD)pin_len);
- free(pin);
- }
- if (user_root)
- RegCloseKey(user_root);
- return r;
-}
-
-int process_remove_smartcard_key(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
-{
- char *provider = NULL, *pin = NULL, canonical_provider[PATH_MAX];
- int r = 0, request_invalid = 0, success = 0, index = 0;
- HKEY user_root = 0;
-
- if ((r = sshbuf_get_cstring(request, &provider, NULL)) != 0 ||
- (r = sshbuf_get_cstring(request, &pin, NULL)) != 0) {
- error("remove smartcard request is invalid");
- request_invalid = 1;
- goto done;
- }
-
- if (realpath(provider, canonical_provider) == NULL) {
- error("failed PKCS#11 add of \"%.100s\": realpath: %s",
- provider, strerror(errno));
- request_invalid = 1;
- goto done;
- }
-
- // Remove 'drive root' if exists
- if (canonical_provider[0] == '/')
- memmove(canonical_provider, canonical_provider + 1, strlen(canonical_provider));
-
- if (get_user_root(con, &user_root) != 0 ||
- !is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT, canonical_provider))
- goto done;
-
- if (remove_pkcs11_identities(user_root, canonical_provider) != 0 ||
- remove_matching_subkeys_from_registry(user_root,
- SSH_PKCS11_PROVIDERS_ROOT, L"provider", canonical_provider) != 0) {
- goto done;
- }
-
- success = 1;
-done:
- r = 0;
- if (request_invalid)
- r = -1;
- else if (sshbuf_put_u8(response, success ? SSH_AGENT_SUCCESS : SSH_AGENT_FAILURE) != 0)
- r = -1;
- if (provider)
- free(provider);
- if (pin)
- free(pin);
- if (user_root)
- RegCloseKey(user_root);
- return r;
-}
-#endif /* ENABLE_PKCS11 */
-
int
process_request_identities(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
{
From 1d364dd4c5f7701bd72600700151cf1191ae479b Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 18:15:57 +0200
Subject: [PATCH 15/16] Simplify keyagent PKCS#11 handling
Follow-up cleanup after splitting keyagent-request.c:
- Move the provider reload loop out of process_sign_request into
keyagent_pkcs11_reload_providers()/keyagent_pkcs11_release(). The
security descriptor allocated there is now freed and no longer shares
its size variable with SECURITY_ATTRIBUTES.nLength.
- Provide no-op stubs in keyagent-pkcs11.h so keyagent-request.c no
longer needs any ENABLE_PKCS11 conditionals.
- Move delete_matching_identity() to keyagent-registry.c and add
keyagent_pkcs11_delete_cert_identity() for the certificate fallback in
process_remove_key.
- Share the provider path canonicalization between add and remove
smartcard requests. The remove request no longer logs "add" in its
realpath error.
- Fold the store and rollback bookkeeping of a PKCS#11 identity into
store_and_track_pkcs11_identity().
- Drop the dead "#if 0" dispatcher.
Co-Authored-By: Claude Sonnet 5.5
---
.../win32compat/ssh-agent/keyagent-pkcs11.c | 210 +++++++++++++++---
.../win32compat/ssh-agent/keyagent-pkcs11.h | 49 +++-
.../win32compat/ssh-agent/keyagent-registry.c | 45 ++++
.../win32compat/ssh-agent/keyagent-registry.h | 1 +
.../win32compat/ssh-agent/keyagent-request.c | 200 +----------------
5 files changed, 273 insertions(+), 232 deletions(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
index 65ca886d0ec8..6d8b8e8c0383 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
@@ -49,9 +49,15 @@
extern char* allowed_providers;
extern int remote_add_provider;
+extern struct sshkey *
+lookup_key(const struct sshkey *k);
+
extern void
add_key(struct sshkey *k, char *name);
+extern void
+del_all_keys();
+
struct pkcs11_identity_change {
char *name;
int created;
@@ -377,7 +383,7 @@ remove_pkcs11_identities(HKEY user_root, const char *provider)
return 0;
}
-int
+static int
load_pkcs11_identities(HKEY user_root, const char *provider,
struct sshkey **token_keys, int nkeys)
{
@@ -499,7 +505,7 @@ load_pkcs11_identities(HKEY user_root, const char *provider,
return loaded;
}
-void
+static void
free_pkcs11_sign_provider(char **providerp, char **pinp, DWORD pin_len,
char **epinp, DWORD epin_len, struct sshkey ***keysp, int nkeys)
{
@@ -525,6 +531,161 @@ free_pkcs11_sign_provider(char **providerp, char **pinp, DWORD pin_len,
}
}
+struct sshkey *
+keyagent_pkcs11_lookup_key(const struct sshkey *key)
+{
+ return lookup_key(key);
+}
+
+int
+keyagent_pkcs11_reload_providers(struct agent_connection *con)
+{
+ int count = 0, index = 0, loaded = 0, ret = -1;
+ wchar_t sub_name[MAX_KEY_LENGTH];
+ DWORD sub_name_len = MAX_KEY_LENGTH;
+ DWORD pin_len = 0, epin_len = 0, provider_len = 0;
+ DWORD epin_alloc_len = 0;
+ char *pin = NULL, *npin = NULL, *epin = NULL, *provider = NULL;
+ HKEY root = 0, sub = 0, user_root = 0;
+ struct sshkey **keys = NULL;
+ SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
+ ULONG sd_len = 0;
+
+ pkcs11_init(0);
+
+ sa.nLength = sizeof(sa);
+ if ((!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL, SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sd_len)) ||
+ get_user_root(con, &user_root) != 0 ||
+ RegCreateKeyExW(user_root, SSH_PKCS11_PROVIDERS_ROOT, 0, 0, 0, KEY_WRITE | STANDARD_RIGHTS_READ | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY, &sa, &root, NULL) != 0) {
+ goto out;
+ }
+
+ while (1) {
+ sub_name_len = MAX_KEY_LENGTH;
+ pin_len = epin_len = provider_len = 0;
+ epin_alloc_len = 0;
+ if (sub) {
+ RegCloseKey(sub);
+ sub = NULL;
+ }
+ if (RegEnumKeyExW(root, index++, sub_name, &sub_name_len, NULL, NULL, NULL, NULL) == 0) {
+ if (RegOpenKeyExW(root, sub_name, 0, KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) == 0 &&
+ RegQueryValueExW(sub, L"provider", 0, NULL, NULL, &provider_len) == 0 &&
+ RegQueryValueExW(sub, L"pin", 0, NULL, NULL, &epin_len) == 0) {
+ if (provider_len == 0 || provider_len >= PATH_MAX ||
+ epin_len == 0 || epin_len > MAX_MESSAGE_SIZE)
+ continue;
+ epin_alloc_len = epin_len;
+ if ((epin = malloc(epin_alloc_len + 1)) == NULL ||
+ (provider = malloc(provider_len + 1)) == NULL ||
+ RegQueryValueExW(sub, L"provider", 0, NULL, provider, &provider_len) != 0 ||
+ RegQueryValueExW(sub, L"pin", 0, NULL, epin, &epin_len) != 0) {
+ free_pkcs11_sign_provider(&provider, &pin, pin_len,
+ &epin, epin_alloc_len, &keys, count);
+ continue;
+ }
+ provider[provider_len] = '\0';
+ epin[epin_len] = '\0';
+ if (convert_blob(con, epin, epin_len, &pin, &pin_len, 0) != 0 ||
+ (npin = realloc(pin, pin_len + 1)) == NULL) {
+ free_pkcs11_sign_provider(&provider, &pin, pin_len,
+ &epin, epin_alloc_len, &keys, count);
+ continue;
+ }
+ pin = npin;
+ pin[pin_len] = '\0';
+ count = pkcs11_add_provider(provider, pin, &keys, NULL);
+ if (count <= 0) {
+ free_pkcs11_sign_provider(&provider, &pin, pin_len,
+ &epin, epin_alloc_len, &keys, count);
+ continue;
+ }
+ loaded = load_pkcs11_identities(user_root, provider,
+ keys, count);
+ free_pkcs11_sign_provider(&provider, &pin, pin_len,
+ &epin, epin_alloc_len, &keys, count);
+ if (loaded < 0)
+ goto out;
+ }
+ }
+ else
+ break;
+ }
+ ret = 0;
+out:
+ free_pkcs11_sign_provider(&provider, &pin, pin_len, &epin, epin_alloc_len,
+ &keys, count);
+ if (sa.lpSecurityDescriptor != NULL)
+ LocalFree(sa.lpSecurityDescriptor);
+ if (user_root)
+ RegCloseKey(user_root);
+ if (root)
+ RegCloseKey(root);
+ if (sub)
+ RegCloseKey(sub);
+ return ret;
+}
+
+void
+keyagent_pkcs11_release(void)
+{
+ del_all_keys();
+ pkcs11_terminate();
+}
+
+LSTATUS
+keyagent_pkcs11_delete_cert_identity(HKEY root, const struct sshkey *key,
+ const u_char *blob, size_t blob_len)
+{
+ char *name;
+ LSTATUS status;
+
+ if ((name = pkcs11_identity_name(key, blob, blob_len)) == NULL)
+ return ERROR_INVALID_DATA;
+ status = delete_matching_identity(root, name, blob, blob_len);
+ free(name);
+ return status;
+}
+
+/*
+ * Resolve provider to the canonical path used as Registry identity, without
+ * the leading slash realpath() puts in front of a Windows drive letter.
+ * canonical must hold PATH_MAX bytes.
+ */
+static int
+canonicalize_provider_path(const char *provider, char *canonical,
+ const char *op)
+{
+ if (realpath(provider, canonical) == NULL) {
+ error("failed PKCS#11 %s of \"%.100s\": realpath: %s",
+ op, provider, strerror(errno));
+ return -1;
+ }
+ if (canonical[0] == '/')
+ memmove(canonical, canonical + 1, strlen(canonical));
+ return 0;
+}
+
+/*
+ * Persist key as identity of provider and remember how to roll it back
+ * in *changesp, which is grown by one entry on success.
+ */
+static int
+store_and_track_pkcs11_identity(HKEY user_root, const struct sshkey *key,
+ const char *provider, const char *comment,
+ struct pkcs11_identity_change ***changesp, size_t *nchangesp)
+{
+ struct pkcs11_identity_change *change = NULL;
+
+ if (store_pkcs11_identity(user_root, key, provider, comment,
+ &change) != 0)
+ return -1;
+ *changesp = xrecallocarray(*changesp, *nchangesp, *nchangesp + 1,
+ sizeof(**changesp));
+ (*changesp)[(*nchangesp)++] = change;
+ return 0;
+}
+
int
process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
struct agent_connection *con)
@@ -535,7 +696,6 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
int i, j, count = 0, r = 0, request_invalid = 0, success = 0;
int cert_only = 0, identities_stored = 0;
struct sshkey **keys = NULL, **certs = NULL, *cert = NULL;
- struct pkcs11_identity_change *identity_change = NULL;
struct pkcs11_identity_change **identity_changes = NULL;
size_t k, pin_len = 0, ncerts = 0, nidentity_changes = 0;
HKEY user_root = NULL;
@@ -563,17 +723,12 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
goto done;
}
- if (realpath(provider, canonical_provider) == NULL) {
- error("failed PKCS#11 add of \"%.100s\": realpath: %s",
- provider, strerror(errno));
+ if (canonicalize_provider_path(provider, canonical_provider,
+ "add") != 0) {
request_invalid = 1;
goto done;
}
- /* Remove the leading slash from the canonical Windows drive path. */
- if (canonical_provider[0] == '/')
- memmove(canonical_provider, canonical_provider + 1,
- strlen(canonical_provider));
strcpy_s(allowed_provider, sizeof(allowed_provider), canonical_provider);
for (i = 0; allowed_provider[i] != '\0'; i++) {
if (allowed_provider[i] == '/')
@@ -605,28 +760,21 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
continue;
if (pkcs11_make_cert(keys[i], certs[j], &cert) != 0)
continue;
- if (store_pkcs11_identity(user_root, cert,
- canonical_provider, comment, &identity_change) != 0)
+ if (store_and_track_pkcs11_identity(user_root, cert,
+ canonical_provider, comment, &identity_changes,
+ &nidentity_changes) != 0)
goto done;
- identity_changes = xrecallocarray(identity_changes,
- nidentity_changes, nidentity_changes + 1,
- sizeof(*identity_changes));
- identity_changes[nidentity_changes++] = identity_change;
- identity_change = NULL;
sshkey_free(cert);
cert = NULL;
identities_stored++;
}
- if (!cert_only && store_pkcs11_identity(user_root, keys[i],
- canonical_provider, comment, &identity_change) == 0) {
- identity_changes = xrecallocarray(identity_changes,
- nidentity_changes, nidentity_changes + 1,
- sizeof(*identity_changes));
- identity_changes[nidentity_changes++] = identity_change;
- identity_change = NULL;
- identities_stored++;
- } else if (!cert_only)
+ if (cert_only)
+ continue;
+ if (store_and_track_pkcs11_identity(user_root, keys[i],
+ canonical_provider, comment, &identity_changes,
+ &nidentity_changes) != 0)
goto done;
+ identities_stored++;
}
if (identities_stored == 0 || store_pkcs11_provider(user_root, con,
@@ -646,7 +794,6 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
nidentity_changes);
sshkey_free(cert);
- free_pkcs11_identity_change(identity_change);
for (k = 0; k < nidentity_changes; k++)
free_pkcs11_identity_change(identity_changes[k]);
free(identity_changes);
@@ -681,17 +828,12 @@ int process_remove_smartcard_key(struct sshbuf* request, struct sshbuf* response
goto done;
}
- if (realpath(provider, canonical_provider) == NULL) {
- error("failed PKCS#11 add of \"%.100s\": realpath: %s",
- provider, strerror(errno));
+ if (canonicalize_provider_path(provider, canonical_provider,
+ "remove") != 0) {
request_invalid = 1;
goto done;
}
- // Remove 'drive root' if exists
- if (canonical_provider[0] == '/')
- memmove(canonical_provider, canonical_provider + 1, strlen(canonical_provider));
-
if (get_user_root(con, &user_root) != 0 ||
!is_reg_sub_key_exists(user_root, SSH_PKCS11_PROVIDERS_ROOT, canonical_provider))
goto done;
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.h b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.h
index 82242578a294..549d8a2dae32 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.h
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.h
@@ -1,6 +1,9 @@
/*
* PKCS#11 provider and identity persistence of the Windows ssh-agent.
* Split out of keyagent-request.c.
+ *
+ * Without ENABLE_PKCS11 the functions below are no-ops, so callers do not
+ * need any conditional compilation.
*/
#pragma once
@@ -9,8 +12,48 @@
#include "sshkey.h"
+struct agent_connection;
+
#ifdef ENABLE_PKCS11
-void free_pkcs11_sign_provider(char **, char **, DWORD, char **, DWORD,
- struct sshkey ***, int);
-int load_pkcs11_identities(HKEY, const char *, struct sshkey **, int);
+
+/* Key that was loaded from a PKCS#11 provider by the current request. */
+struct sshkey *keyagent_pkcs11_lookup_key(const struct sshkey *);
+
+/*
+ * Load all persisted providers and make their identities available to
+ * signing. Must be paired with keyagent_pkcs11_release(), also on failure.
+ */
+int keyagent_pkcs11_reload_providers(struct agent_connection *);
+void keyagent_pkcs11_release(void);
+
+/* Delete the persisted PKCS#11 certificate identity matching key/blob. */
+LSTATUS keyagent_pkcs11_delete_cert_identity(HKEY, const struct sshkey *,
+ const u_char *, size_t);
+
+#else /* ENABLE_PKCS11 */
+
+static __inline struct sshkey *
+keyagent_pkcs11_lookup_key(const struct sshkey *key)
+{
+ return NULL;
+}
+
+static __inline int
+keyagent_pkcs11_reload_providers(struct agent_connection *con)
+{
+ return 0;
+}
+
+static __inline void
+keyagent_pkcs11_release(void)
+{
+}
+
+static __inline LSTATUS
+keyagent_pkcs11_delete_cert_identity(HKEY root, const struct sshkey *key,
+ const u_char *blob, size_t blob_len)
+{
+ return ERROR_FILE_NOT_FOUND;
+}
+
#endif /* ENABLE_PKCS11 */
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-registry.c b/contrib/win32/win32compat/ssh-agent/keyagent-registry.c
index 79cb86cd7432..bd8af193fe04 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-registry.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-registry.c
@@ -225,4 +225,49 @@ restore_optional_reg_value(HKEY key, const wchar_t *name, int present,
return status == ERROR_SUCCESS || status == ERROR_FILE_NOT_FOUND ? 0 : -1;
}
+/*
+ * delete the identity sub key name below root, but only if its stored
+ * public key blob matches blob
+ */
+LSTATUS
+delete_matching_identity(HKEY root, const char *name, const u_char *blob,
+ size_t blob_len)
+{
+ HKEY sub = NULL;
+ u_char *stored_blob = NULL;
+ DWORD stored_blob_len = 0;
+ LSTATUS status;
+
+ status = RegOpenKeyExA(root, name, 0,
+ KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub);
+ if (status != ERROR_SUCCESS)
+ return status;
+ status = RegQueryValueExW(sub, L"pub", NULL, NULL, NULL,
+ &stored_blob_len);
+ if (status != ERROR_SUCCESS)
+ goto out;
+ if (stored_blob_len > MAX_MESSAGE_SIZE) {
+ status = ERROR_INVALID_DATA;
+ goto out;
+ }
+ stored_blob = xmalloc(stored_blob_len == 0 ? 1 : stored_blob_len);
+ status = RegQueryValueExW(sub, L"pub", NULL, NULL, stored_blob,
+ &stored_blob_len);
+ if (status != ERROR_SUCCESS)
+ goto out;
+ if (stored_blob_len != blob_len ||
+ memcmp(stored_blob, blob, blob_len) != 0) {
+ status = ERROR_FILE_NOT_FOUND;
+ goto out;
+ }
+ RegCloseKey(sub);
+ sub = NULL;
+ status = RegDeleteTreeA(root, name);
+ out:
+ free(stored_blob);
+ if (sub != NULL)
+ RegCloseKey(sub);
+ return status;
+}
+
#pragma warning(pop)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-registry.h b/contrib/win32/win32compat/ssh-agent/keyagent-registry.h
index c05e925147bb..54df324c9219 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-registry.h
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-registry.h
@@ -28,3 +28,4 @@ int read_optional_reg_value(HKEY, const wchar_t *, int *, DWORD *,
u_char **, DWORD *);
int restore_optional_reg_value(HKEY, const wchar_t *, int, DWORD,
const u_char *, DWORD);
+LSTATUS delete_matching_identity(HKEY, const char *, const u_char *, size_t);
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-request.c b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
index 26fd2e2ad8fb..d4411ec89f4f 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-request.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-request.c
@@ -33,22 +33,12 @@
#include "agent-request.h"
#include "config.h"
#include
-#include "pkcs11-cert.h"
-#ifdef ENABLE_PKCS11
-#include "ssh-pkcs11.h"
-#endif
#include "xmalloc.h"
#include "keyagent-registry.h"
#include "keyagent-pkcs11.h"
#pragma warning(push, 3)
-extern struct sshkey *
-lookup_key(const struct sshkey *k);
-
-extern void
-del_all_keys();
-
int
process_unsupported_request(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
{
@@ -211,16 +201,12 @@ static int sign_blob(const struct sshkey *pubkey, u_char ** sig, size_t *siglen,
struct sshbuf* tmpbuf = NULL;
char *keyblob = NULL;
const char *sk_provider = NULL;
-#ifdef ENABLE_PKCS11
int is_pkcs11_key = 0;
-#endif /* ENABLE_PKCS11 */
*sig = NULL;
*siglen = 0;
-#ifdef ENABLE_PKCS11
- if ((prikey = lookup_key(pubkey)) == NULL) {
-#endif /* ENABLE_PKCS11 */
+ if ((prikey = keyagent_pkcs11_lookup_key(pubkey)) == NULL) {
if ((thumbprint = sshkey_fingerprint(pubkey, SSH_FP_HASH_DEFAULT, SSH_FP_DEFAULT)) == NULL ||
get_user_root(con, &user_root) != 0 ||
RegOpenKeyExW(user_root, SSH_KEYS_ROOT,
@@ -236,11 +222,9 @@ static int sign_blob(const struct sshkey *pubkey, u_char ** sig, size_t *siglen,
error("cannot retrieve and deserialize key from registry");
goto done;
}
-#ifdef ENABLE_PKCS11
}
else
is_pkcs11_key = 1;
-#endif /* ENABLE_PKCS11 */
if (flags & SSH_AGENT_RSA_SHA2_256)
algo = "rsa-sha2-256";
else if (flags & SSH_AGENT_RSA_SHA2_512)
@@ -262,9 +246,7 @@ static int sign_blob(const struct sshkey *pubkey, u_char ** sig, size_t *siglen,
free(regdata);
if (tmpbuf)
sshbuf_free(tmpbuf);
-#ifdef ENABLE_PKCS11
if (!is_pkcs11_key)
-#endif /* ENABLE_PKCS11 */
if (prikey)
sshkey_free(prikey);
if (thumbprint)
@@ -288,79 +270,8 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
int r, request_invalid = 0, success = 0;
struct sshkey *key = NULL;
-#ifdef ENABLE_PKCS11
- int count = 0, index = 0, loaded = 0;
- wchar_t sub_name[MAX_KEY_LENGTH];
- DWORD sub_name_len = MAX_KEY_LENGTH;
- DWORD pin_len = 0, epin_len = 0, provider_len = 0;
- DWORD epin_alloc_len = 0;
- char *pin = NULL, *npin = NULL, *epin = NULL, *provider = NULL;
- HKEY root = 0, sub = 0, user_root = 0;
- struct sshkey **keys = NULL;
- SECURITY_ATTRIBUTES sa = { 0, NULL, 0 };
-
- pkcs11_init(0);
-
- memset(&sa, 0, sizeof(SECURITY_ATTRIBUTES));
- sa.nLength = sizeof(sa);
- if ((!ConvertStringSecurityDescriptorToSecurityDescriptorW(REG_KEY_SDDL, SDDL_REVISION_1, &sa.lpSecurityDescriptor, &sa.nLength)) ||
- get_user_root(con, &user_root) != 0 ||
- RegCreateKeyExW(user_root, SSH_PKCS11_PROVIDERS_ROOT, 0, 0, 0, KEY_WRITE | STANDARD_RIGHTS_READ | KEY_ENUMERATE_SUB_KEYS | KEY_WOW64_64KEY, &sa, &root, NULL) != 0) {
+ if (keyagent_pkcs11_reload_providers(con) != 0)
goto done;
- }
-
- while (1) {
- sub_name_len = MAX_KEY_LENGTH;
- pin_len = epin_len = provider_len = 0;
- epin_alloc_len = 0;
- if (sub) {
- RegCloseKey(sub);
- sub = NULL;
- }
- if (RegEnumKeyExW(root, index++, sub_name, &sub_name_len, NULL, NULL, NULL, NULL) == 0) {
- if (RegOpenKeyExW(root, sub_name, 0, KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub) == 0 &&
- RegQueryValueExW(sub, L"provider", 0, NULL, NULL, &provider_len) == 0 &&
- RegQueryValueExW(sub, L"pin", 0, NULL, NULL, &epin_len) == 0) {
- if (provider_len == 0 || provider_len >= PATH_MAX ||
- epin_len == 0 || epin_len > MAX_MESSAGE_SIZE)
- continue;
- epin_alloc_len = epin_len;
- if ((epin = malloc(epin_alloc_len + 1)) == NULL ||
- (provider = malloc(provider_len + 1)) == NULL ||
- RegQueryValueExW(sub, L"provider", 0, NULL, provider, &provider_len) != 0 ||
- RegQueryValueExW(sub, L"pin", 0, NULL, epin, &epin_len) != 0) {
- free_pkcs11_sign_provider(&provider, &pin, pin_len,
- &epin, epin_alloc_len, &keys, count);
- continue;
- }
- provider[provider_len] = '\0';
- epin[epin_len] = '\0';
- if (convert_blob(con, epin, epin_len, &pin, &pin_len, 0) != 0 ||
- (npin = realloc(pin, pin_len + 1)) == NULL) {
- free_pkcs11_sign_provider(&provider, &pin, pin_len,
- &epin, epin_alloc_len, &keys, count);
- continue;
- }
- pin = npin;
- pin[pin_len] = '\0';
- count = pkcs11_add_provider(provider, pin, &keys, NULL);
- if (count <= 0) {
- free_pkcs11_sign_provider(&provider, &pin, pin_len,
- &epin, epin_alloc_len, &keys, count);
- continue;
- }
- loaded = load_pkcs11_identities(user_root, provider,
- keys, count);
- free_pkcs11_sign_provider(&provider, &pin, pin_len,
- &epin, epin_alloc_len, &keys, count);
- if (loaded < 0)
- goto done;
- }
- }
- else
- break;
- }
-#endif /* ENABLE_PKCS11 */
if (sshbuf_get_string_direct(request, &blob, &blen) != 0 ||
sshbuf_get_string_direct(request, &data, &dlen) != 0 ||
@@ -393,70 +304,15 @@ process_sign_request(struct sshbuf* request, struct sshbuf* response, struct age
sshkey_free(key);
if (signature)
free(signature);
-#ifdef ENABLE_PKCS11
- free_pkcs11_sign_provider(&provider, &pin, pin_len, &epin, epin_alloc_len,
- &keys, count);
- del_all_keys();
- pkcs11_terminate();
- if (user_root)
- RegCloseKey(user_root);
- if (root)
- RegCloseKey(root);
- if (sub)
- RegCloseKey(sub);
-#endif /* ENABLE_PKCS11 */
+ keyagent_pkcs11_release();
return r;
}
-static LSTATUS
-delete_matching_identity(HKEY root, const char *name, const u_char *blob,
- size_t blob_len)
-{
- HKEY sub = NULL;
- u_char *stored_blob = NULL;
- DWORD stored_blob_len = 0;
- LSTATUS status;
-
- status = RegOpenKeyExA(root, name, 0,
- KEY_QUERY_VALUE | KEY_WOW64_64KEY, &sub);
- if (status != ERROR_SUCCESS)
- return status;
- status = RegQueryValueExW(sub, L"pub", NULL, NULL, NULL,
- &stored_blob_len);
- if (status != ERROR_SUCCESS)
- goto out;
- if (stored_blob_len > MAX_MESSAGE_SIZE) {
- status = ERROR_INVALID_DATA;
- goto out;
- }
- stored_blob = xmalloc(stored_blob_len == 0 ? 1 : stored_blob_len);
- status = RegQueryValueExW(sub, L"pub", NULL, NULL, stored_blob,
- &stored_blob_len);
- if (status != ERROR_SUCCESS)
- goto out;
- if (stored_blob_len != blob_len ||
- memcmp(stored_blob, blob, blob_len) != 0) {
- status = ERROR_FILE_NOT_FOUND;
- goto out;
- }
- RegCloseKey(sub);
- sub = NULL;
- status = RegDeleteTreeA(root, name);
- out:
- free(stored_blob);
- if (sub != NULL)
- RegCloseKey(sub);
- return status;
-}
-
int
process_remove_key(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
{
HKEY user_root = 0, root = 0;
char *blob, *thumbprint = NULL;
-#ifdef ENABLE_PKCS11
- char *pkcs11_name = NULL;
-#endif
size_t blen;
int r = 0, success = 0, request_invalid = 0;
struct sshkey *key = NULL;
@@ -477,16 +333,9 @@ process_remove_key(struct sshbuf* request, struct sshbuf* response, struct agent
goto done;
status = delete_matching_identity(root, thumbprint,
(const u_char *)blob, blen);
-#ifdef ENABLE_PKCS11
- if (status == ERROR_FILE_NOT_FOUND && sshkey_is_cert(key)) {
- pkcs11_name = pkcs11_identity_name(key,
+ if (status == ERROR_FILE_NOT_FOUND && sshkey_is_cert(key))
+ status = keyagent_pkcs11_delete_cert_identity(root, key,
(const u_char *)blob, blen);
- if (pkcs11_name == NULL)
- goto done;
- status = delete_matching_identity(root, pkcs11_name,
- (const u_char *)blob, blen);
- }
-#endif
if (status != ERROR_SUCCESS)
goto done;
success = 1;
@@ -505,9 +354,6 @@ process_remove_key(struct sshbuf* request, struct sshbuf* response, struct agent
RegCloseKey(root);
if (thumbprint)
free(thumbprint);
-#ifdef ENABLE_PKCS11
- free(pkcs11_name);
-#endif
return r;
}
int
@@ -736,40 +582,4 @@ process_extension(struct sshbuf* request, struct sshbuf* response, struct agent_
return r;
}
-#if 0
-int process_keyagent_request(struct sshbuf* request, struct sshbuf* response, struct agent_connection* con)
-{
- u_char type;
-
- if (sshbuf_get_u8(request, &type) != 0)
- return -1;
- debug2("process key agent request type %d", type);
-
- switch (type) {
- case SSH2_AGENTC_ADD_IDENTITY:
- return process_add_identity(request, response, con);
- case SSH2_AGENTC_REQUEST_IDENTITIES:
- return process_request_identities(request, response, con);
- case SSH2_AGENTC_SIGN_REQUEST:
- return process_sign_request(request, response, con);
- case SSH2_AGENTC_REMOVE_IDENTITY:
- return process_remove_key(request, response, con);
- case SSH2_AGENTC_REMOVE_ALL_IDENTITIES:
- return process_remove_all(request, response, con);
-#ifdef ENABLE_PKCS11
- case SSH_AGENTC_ADD_SMARTCARD_KEY:
- return process_add_smartcard_key(request, response, con);
- case SSH_AGENTC_ADD_SMARTCARD_KEY_CONSTRAINED:
- return process_add_smartcard_key(request, response, con);
- case SSH_AGENTC_REMOVE_SMARTCARD_KEY:
- return process_remove_smartcard_key(request, response, con);
- break;
-#endif /* ENABLE_PKCS11 */
- default:
- debug("unknown key agent request %d", type);
- return -1;
- }
-}
-#endif
-
#pragma warning(pop)
From a1cd7c5e11d6c20a42bfd1608573034dedfa4417 Mon Sep 17 00:00:00 2001
From: Sebastian Ott <174621899+VSSOtt@users.noreply.github.com>
Date: Wed, 30 Sep 2026 19:00:45 +0200
Subject: [PATCH 16/16] Remove trailing whitespace from keyagent modules
---
.../win32/win32compat/ssh-agent/keyagent-pkcs11.c | 4 ++--
.../win32compat/ssh-agent/keyagent-registry.c | 14 +++++++-------
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
index 6d8b8e8c0383..bfba302b4cbd 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c
@@ -1,7 +1,7 @@
/*
* Author: Manoj Ampalam
* ssh-agent implementation on Windows
- *
+ *
* Copyright (c) 2015 Microsoft Corp.
* All rights reserved
*
@@ -722,7 +722,7 @@ process_add_smartcard_key(struct sshbuf *request, struct sshbuf *response,
"providers is disabled", provider);
goto done;
}
-
+
if (canonicalize_provider_path(provider, canonical_provider,
"add") != 0) {
request_invalid = 1;
diff --git a/contrib/win32/win32compat/ssh-agent/keyagent-registry.c b/contrib/win32/win32compat/ssh-agent/keyagent-registry.c
index bd8af193fe04..21552436afc7 100644
--- a/contrib/win32/win32compat/ssh-agent/keyagent-registry.c
+++ b/contrib/win32/win32compat/ssh-agent/keyagent-registry.c
@@ -1,7 +1,7 @@
/*
* Author: Manoj Ampalam
* ssh-agent implementation on Windows
- *
+ *
* Copyright (c) 2015 Microsoft Corp.
* All rights reserved
*
@@ -48,20 +48,20 @@ get_user_root(struct agent_connection* con, HKEY *root)
int r = 0;
LONG ret;
*root = HKEY_LOCAL_MACHINE;
-
+
if (con->client_type <= ADMIN_USER) {
if (ImpersonateLoggedOnUser(con->client_impersonation_token) == FALSE)
return -1;
*root = NULL;
- /*
- * TODO - check that user profile is loaded,
- * otherwise, this will return default profile
+ /*
+ * TODO - check that user profile is loaded,
+ * otherwise, this will return default profile
*/
if ((ret = RegOpenCurrentUser(KEY_ALL_ACCESS, root)) != ERROR_SUCCESS) {
debug("unable to open user's registry hive, ERROR - %d", ret);
r = -1;
}
-
+
RevertToSelf();
}
return r;
@@ -95,7 +95,7 @@ convert_blob(struct agent_connection* con, const char *blob, DWORD blen, char **
}
*eblob = malloc(out.cbData);
- if (*eblob == NULL)
+ if (*eblob == NULL)
goto done;
if((r = memcpy_s(*eblob, out.cbData, out.pbData, out.cbData)) != 0) {