Implement PKCS#11 certificate support in ssh-agent by VSSOtt · Pull Request #865 · PowerShell/openssh-portable

VSSOtt · 2026-07-17T16:32:37Z

PR Summary

Implement Windows ssh-agent support for OpenSSH certificates backed by
PKCS#11 token keys via associated-certs-v00@openssh.com.

The implementation covers RSA and ECDSA token keys and includes:

certificate-only loading with ssh-add -s provider -C cert.pub
matching supplied certificates to token keys
persistent certificate identities across agent service restarts
individual certificate deletion and provider-wide removal
additive, idempotent repeated provider adds, so certificates can be added in
separate requests without dropping existing persisted identities
PKCS#11 token labels as identity comments, with provider-path fallback
signing that continues to work when unrelated persisted providers are stale
legacy PKCS#11 registry entries that remain readable and removable

PR Context

The Windows ssh-add help text already advertised the PKCS#11 certificate
flow, but the Windows agent did not handle certificate identities
from associated-certs-v00@openssh.com. As a result, adding certificates
appeared to succeed without making them usable as agent identities.

Upstream OpenSSH already defines how certificates are associated with
PKCS#11-backed keys. The main Windows-specific difference is persistence:
unlike the usual Unix agent model, the Windows agent stores PKCS#11 providers
and identities in the registry and reloads them after the service restarts.

Because of that persistent model, repeated provider adds merge with existing
stored identities instead of replacing them. This allows certificates for the
same token provider to be added in separate requests while preserving
certificates and plain token-key identities that were already stored.

Windows PKCS#11 additions with lifetime, confirmation, or destination
constraints are explicitly rejected with SSH_AGENT_FAILURE. Persisted
Windows PKCS#11 identities cannot currently enforce these restrictions,
including after a service restart, so accepting such requests would falsely
report that the requested security conditions had been applied. This is an
intentional security-related behavior change: ssh-add -c -s ...,
ssh-add -t ... -s ..., and destination-constrained provider additions fail
until full enforcement is available. Unconstrained additions, including
certificate-only adds, remain supported. Existing persisted identities are
not migrated or removed by this change.

This implementation keeps these existing security boundaries intact:

provider allowlist enforcement remains unchanged
remote-provider restrictions remain unchanged
PIN storage keeps the existing encrypted registry model
no private key material is stored by the agent

Validation

Windows x64 Debug/Release and x86 Release builds, Windows unit tests, Pester
tests, bash regressions, and non-Windows regression coverage pass.

Real-token validation was performed with a YubiKey 5 using RSA 2048 and ECDSA
NIST P-256. It covered:

add, list, and sign
certificate-only mode
unmatched certificates
repeated provider adds
token-label identity comments
individual certificate deletion
software certificate deletion
stale unrelated provider records
provider removal
service restart and reload

azure-pipelines · 2026-07-17T16:32:46Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

VSSOtt · 2026-07-17T16:35:14Z

@microsoft-github-policy-service agree

VSSOtt · 2026-07-17T16:35:54Z

/azp run

azure-pipelines · 2026-07-17T16:36:00Z

Commenter does not have sufficient privileges for PR 865 in repo PowerShell/openssh-portable

Copilot

Pull request overview

This PR adds Windows ssh-agent support for OpenSSH certificates backed by PKCS#11 token keys (via associated-certs-v00@openssh.com), aligning behavior with upstream semantics while preserving the Windows agent’s registry-backed persistence model.

Changes:

Add Windows support code for constructing PKCS#11-backed certificate identities and persisting/loading them across agent restarts.
Extend the Windows agent request handling to support PKCS#11 certificate add/list/delete flows and more robust signing behavior with stale providers.
Add Windows unit tests + Pester E2E coverage and wire new Win32 PKCS#11 cert helper code into the build.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`ssh-pkcs11-client.c`	Fix provider lifecycle memory handling; add Windows implementation for PKCS#11-backed cert private keys.
`ssh-add.c`	Enable `-C` parsing on Windows to support certificate-only PKCS#11 adds.
`regress/unittests/win32compat/tests.h`	Declare new PKCS#11 cert test entry point.
`regress/unittests/win32compat/tests.c`	Register PKCS#11 cert unit tests in the Windows test runner.
`regress/unittests/win32compat/pkcs11_cert_tests.c`	Add unit tests for PKCS#11 cert constraint parsing and identity naming/comment behavior.
`regress/pesterTests/README.md`	Document required environment variables for PKCS#11 certificate E2E tests.
`regress/pesterTests/KeyUtils.Tests.ps1`	Add Pester E2E scenarios for PKCS#11 certificate add/sign/restart/delete flows and stale-provider isolation.
`regress/pesterTests/CommonUtils.psm1`	Make askpass environment setup more robust across PowerShell/Windows environments.
`contrib/win32/win32compat/ssh-agent/keyagent-request.c`	Implement persisted PKCS#11 cert identities + provider/identity registry management and signing behavior updates.
`contrib/win32/win32compat/pkcs11-cert.h`	Add Win32 helper API for cert constraints and identity naming/comment logic.
`contrib/win32/win32compat/pkcs11-cert.c`	Implement constraint parsing and PKCS#11 cert identity naming/comment helpers.
`contrib/win32/openssh/unittest-win32compat.vcxproj`	Add the new helper + unit test compilation units to the Win32 test project.
`contrib/win32/openssh/ssh-agent.vcxproj`	Add the new helper compilation unit/include to the Windows ssh-agent project.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Tess Gauthier (tgauth) · 2026-08-27T15:49:54Z

/azp run

azure-pipelines · 2026-08-27T15:50:07Z

Azure Pipelines: Successfully started running 1 pipeline(s).

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.

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.

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.

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.

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.

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.

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.

VSSOtt · 2026-09-29T16:44:20Z

Hi, I’ve rebased this onto the latest latestw_all and resolved the conflicts from the OpenSSH 10.3p1 update.
The PR is cleanly mergeable again. I also rebuilt and reran the Windows compatibility tests (89 tests ok).
I’d appreciate another look when you have time. Please let me know if anything else is needed before this can be merged.

Copilot

Copilot review overview

🟡 Changes recommended

Case-sensitive provider associations and service-invisible SoftHSM configuration can break persisted identities and Windows regression coverage.

Review effort: Balanced
Findings: 4

Open (4)

Defer OpenSSL requirement to optional SoftHSM setup · New
Configure SOFTHSM2_CONF for the LocalSystem service · New
Use case-insensitive provider path matching when removing identities · New
Compare Windows provider paths case-insensitively · New

Resolved since last review (1)

In the Windows-specific pkcs11_make_cert(), wrap_key(ret) already sets SSHKEY_FLAG_EXT on the key,…

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The security-sensitive agent and registry persistence changes warrant final human review despite comprehensive validation.

Review effort: Balanced
Findings: None

Resolved since last review (4)

Compare Windows provider paths case-insensitively
Use case-insensitive provider path matching when removing identities
Configure SOFTHSM2_CONF for the LocalSystem service
Defer OpenSSL requirement to optional SoftHSM setup

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

Use a pinned SoftHSM fixture locally and in the Azure core job. Fail required runs on missing prerequisites or incomplete results, and publish actual skips for optional hardware. Keep certificate lifecycle coverage in a dedicated integration suite. Bind local recovery to the original user SID, agent executable and run marker. Journal restoration completion, restore running disabled services safely, and retain core E2E coverage on ARM and ARM64. Runner regression checks were temporary only. Validation: x64 Debug/Release and x86 Release builds, all three native unit suites, RSA/ECDSA integration on x64/x86, disabled/running service restoration and 38 temporary checks passed. Full local Pester/Bash runs still report environment and legacy-suite failures plus a cfginclude timeout. Remote Azure CI is unverified.

Resolve Administrators and SYSTEM permissions and test-user group membership by SID so the E2E harness can run on localized Windows installations. Validation: PowerShell syntax check and local core E2E/Bash setup exercised successfully; full legacy suites retain the separately reported failures.

Copilot

Copilot review overview

🟡 Changes recommended

Constraint compatibility, default SoftHSM service configuration, and software-certificate collision handling remain unresolved.

Review effort: Balanced
Findings: 3

Open (3)

Configure SoftHSM service environment for default DLL discovery · New
Preserve established acceptance semantics for ignored constraints · New
Prevent duplicate PKCS#11 identities for existing software certificates · New

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 12

Open (13)

Add-LocalGroupMember typically does not support a -SID parameter (it expects -Group,… · New
If parsing fails after some certificates have been appended to *certsp, the function returns… · New
If parsing fails after some certificates have been appended to *certsp, the function returns… · New
If parsing fails after some certificates have been appended to *certsp, the function returns… · New
If parsing fails after some certificates have been appended to *certsp, the function returns… · New
*neededp is not initialized at function entry, and the early-return path for result = 1… · New
*neededp is not initialized at function entry, and the early-return path for result = 1… · New
*neededp is not initialized at function entry, and the early-return path for result = 1… · New
On memcpy_s failure, *eblob remains allocated and *eblen is not set, leaving outputs in a… · New
On memcpy_s failure, *eblob remains allocated and *eblen is not set, leaving outputs in a… · New
Prevent duplicate PKCS#11 identities for existing software certificates
Preserve established acceptance semantics for ignored constraints
Configure SoftHSM service environment for default DLL discovery

Ignore only MemberExistsException when adding the E2E admin user to the Administrators group. The module runs with ErrorActionPreference Stop, so a repeated setup run would otherwise abort where "net localgroup" did not. Document that parse_pkcs11_add_constraints leaves ownership of the partial certificate list with the caller on every return path, and that store_pkcs11_identity only sets *neededp because the caller accumulates it over all identities of a provider. Validation: PowerShell syntax parse and git diff --check only; no builds or tests were run. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 3 · 1

Open (5)

RegDeleteKeyExA is attempting to delete a Registry subkey while a handle to that same subkey… · New
These new calls pass unquoted paths to p11_make_public. If $SSH_SOFTHSM_DIR (or the build/test… · New
These new calls pass unquoted paths to p11_make_public. If $SSH_SOFTHSM_DIR (or the build/test… · New
These new calls pass unquoted paths to p11_make_public. If $SSH_SOFTHSM_DIR (or the build/test… · New
The Restart-Pkcs11Agent function declares a BinaryDirectory parameter that is not used anywhere… · New

Resolved since last review (13)

Add-LocalGroupMember typically does not support a -SID parameter (it expects -Group,…
On memcpy_s failure, *eblob remains allocated and *eblen is not set, leaving outputs in a…
On memcpy_s failure, *eblob remains allocated and *eblen is not set, leaving outputs in a…
*neededp is not initialized at function entry, and the early-return path for result = 1…
*neededp is not initialized at function entry, and the early-return path for result = 1…
*neededp is not initialized at function entry, and the early-return path for result = 1…
If parsing fails after some certificates have been appended to *certsp, the function returns…
If parsing fails after some certificates have been appended to *certsp, the function returns…
If parsing fails after some certificates have been appended to *certsp, the function returns…
If parsing fails after some certificates have been appended to *certsp, the function returns…
Prevent duplicate PKCS#11 identities for existing software certificates
Preserve established acceptance semantics for ignored constraints
Configure SoftHSM service environment for default DLL discovery

Copilot AI review requested due to automatic review settings July 17, 2026 16:32

Copilot started reviewing on behalf of VSSOtt July 17, 2026 16:33

View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Comment thread ssh-pkcs11-client.c Outdated

Comment thread contrib/win32/win32compat/ssh-agent/keyagent-request.c Outdated

Comment thread contrib/win32/win32compat/ssh-agent/keyagent-request.c Outdated

Comment thread contrib/win32/win32compat/ssh-agent/keyagent-request.c Outdated

VSSOtt requested a review from Copilot July 17, 2026 19:02

Copilot started reviewing on behalf of VSSOtt July 17, 2026 19:02

View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

VSSOtt added 7 commits September 29, 2026 18:23

Harden Windows PKCS#11 registry helpers

1cb677e

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.

VSSOtt force-pushed the agent-pkcs11-certificates branch from 46b57e8 to 1cb677e Compare September 29, 2026 16:31

VSSOtt added 2 commits September 30, 2026 15:37

Fix PKCS#11 helper cleanup ordering

0583824

Enable PKCS#11 certificate regressions on Windows

678cc42

VSSOtt requested a balanced review from Copilot September 30, 2026 13:41

Copilot started reviewing on behalf of VSSOtt September 30, 2026 13:42

View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

Comment thread contrib/win32/openssh/bash_tests_iterator.ps1 Outdated

Comment thread contrib/win32/openssh/bash_tests_iterator.ps1 Outdated

Comment thread contrib/win32/win32compat/ssh-agent/keyagent-request.c Outdated

Comment thread contrib/win32/win32compat/ssh-agent/keyagent-request.c Outdated

VSSOtt and others added 4 commits September 30, 2026 16:28

Fix Windows SoftHSM regression prerequisites

471eb48

VSSOtt requested a balanced review from Copilot September 30, 2026 15:56

Copilot started reviewing on behalf of VSSOtt September 30, 2026 15:57

View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

VSSOtt and others added 6 commits September 30, 2026 18:11

Remove trailing whitespace from keyagent modules

a1cd7c5

Reject unsupported Windows PKCS#11 constraints

f23e7a5

VSSOtt requested a balanced review from Copilot October 2, 2026 10:13

Copilot started reviewing on behalf of VSSOtt October 2, 2026 10:14

View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Comment thread contrib/win32/openssh/bash_tests_iterator.ps1 Outdated

Comment thread contrib/win32/win32compat/pkcs11-cert.c

Comment thread contrib/win32/win32compat/ssh-agent/keyagent-pkcs11.c

Make Windows PKCS11 certificate adds idempotent

588866d

VSSOtt requested a balanced review from Copilot October 2, 2026 16:14

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Copilot started reviewing on behalf of VSSOtt October 2, 2026 16:56

View session

VSSOtt requested a balanced review from Copilot October 2, 2026 17:10

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Comment thread contrib/win32/win32compat/ssh-agent/keyagent-request.c

Comment thread regress/test-exec.sh Outdated

Comment thread regress/test-exec.sh Outdated

Comment thread regress/test-exec.sh Outdated

Comment thread .github/tools/PKCS11TestHelpers.psm1 Outdated

Copilot started reviewing on behalf of VSSOtt October 2, 2026 17:42

View session

Quote PKCS11 test paths and simplify agent restart helper

af34a36

Conversation

VSSOtt commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

Validation

Uh oh!

azure-pipelines Bot commented Jul 17, 2026

Uh oh!

VSSOtt commented Jul 17, 2026

Uh oh!

VSSOtt commented Jul 17, 2026

Uh oh!

azure-pipelines Bot commented Jul 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Tess Gauthier (tgauth) commented Aug 27, 2026

Uh oh!

azure-pipelines Bot commented Aug 27, 2026

Uh oh!

VSSOtt commented Sep 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

VSSOtt commented Jul 17, 2026 •

edited

Loading