Repository navigation
feat(ssh): support FIDO security keys through SecurityKeyProvider - #333
Merged
Merged
Conversation
bssh did not list the sk-* key types, so the regress harness never generated sk-* host keys and the two sk-*-cert-v01@openssh.com phases of hostkey-agent could not pass. Listing them makes every regress test add sk-* IdentityFile lines, so user authentication with authenticator-hosted keys had to work too. This follows OpenSSH 10.3p1 built without its internal provider. SecurityKeyProvider becomes a runtime option (library path, internal, none, or $VAR; default $SSH_SK_PROVIDER, printed raw by -G). src/ssh/security_key holds the SecurityKeyProvider trait that #331 will plug internal into, $VAR resolution, the dlopen binding to sk-api 0x000a0000 with every provider pointer validated and the response freed as ssh-sk.c does, and the sk-* signature encoder. The provider is loaded in process instead of through an ssh-sk-helper style subprocess: bssh has no pledge sandbox to keep the middleware out of, and a re-exec helper would add a private protocol without changing what runs as the user. Provider calls run on a blocking thread, the touch notice is printed when stderr is a terminal, and a PIN is prompted once outside BatchMode. A provider that cannot be opened is reported before the identity is offered and authentication continues; a sk_sign failure after the server asked for the signature ends the transport, like LocalSignerFailed. Static musl builds compile and name the dlopen limitation. sk-* identities sign through a russh Signer for plain keys and user certificates; agent-held keys keep using the agent. -Q and the default HostKeyAlgorithms and PubkeyAcceptedAlgorithms match the reference order except for the webauthn-sk-* pair, which bssh cannot verify. A regression test keeps a certificate-only HostKeyAlgorithms list from being widened. Validation: hostkey-agent, keytype, limit-keytype and agent pass locally with the regress sk-dummy provider; connect, proxy-connect and hostkey-rotate still pass. Unit tests cover $VAR expansion, the internal diagnostic, load failures, signature encoding, the algorithm lists and the signer, and each fails when its behavior is reverted. Refs #328
5 of 6 tasks
SecurityKeyProvider now has a runtime consumer, so the compat-output test can no longer use it as the example of a parsed-but-unimplemented option. The fixture now uses VisualHostKey at the same line for the once-only unsupported diagnostic, keeps a SecurityKeyProvider line, and asserts that it produces no unsupported diagnostic. Validated with cargo test --test ssh_compat_output_test and cargo test --test ssh_config_dump_test. Refs #328
A failed sk_sign (device absent, PIN required in BatchMode, provider error) after the server accepted an sk-* key ended the whole connection, while OpenSSH's sign_and_send_pubkey failure path moves on to the next identity. russh 0.64.1 blocks its session loop on Msg::Signed after an Err from the signer, but it sends nothing when the signer returns the request unchanged. The sk signer now declines that way, records the error, and authenticate_declinable drops the handle-side exchange that would otherwise wait for a reply forever. SecurityKeySignFailed is now a retryable rejection. Provider calls are serialized process-wide inside the blocking task: sk-api makes no thread-safety promise and OpenSSH never calls a provider concurrently, while bssh authenticates to many hosts at once. sk-* identity files are checked against the provider from their unencrypted public half before any passphrase prompt, and ~/.ssh/id_ed25519_sk and ~/.ssh/id_ecdsa_sk join the runtime default identities that -G already printed. New tests cover each behavior and fail when it is reverted: the next identity authenticates on the same connection after a declined signature, provider calls never overlap, an encrypted sk key without a provider is skipped before decryption, and the defaults include the FIDO key files. Refs #328
Member
Author
Implementation Review SummaryIntent
Findings Addressed
Remaining Items
Verification
|
PROVIDER_CALL_LOCK was a std mutex taken inside each spawn_blocking task. In multi-host mode every host queued behind a touch pinned its own blocking thread, so with --parallel above tokio's blocking-pool limit the pool filled up and tokio::fs and DNS work for every connection, including ones without security keys, stalled until the touches drained. A connection that hit ConnectTimeout while queued also kept its blocking task, which later called sk_sign anyway and asked the user to touch the key for a connection that no longer existed, holding the lock and pushing the remaining hosts toward their own timeouts. The lock is now a FIFO tokio mutex acquired before spawn_blocking; the owned guard moves into the blocking task, so a dropped future still cannot release it while the provider runs. A queued waiter that is cancelled leaves the queue without reaching the provider. The touch notice is printed once the identity holds the provider, on every attempt as identity_sign does, so it matches the authenticator that is actually waiting. Two new tests fail with the previous lock: a cancelled queued signer must never reach the provider, and queued signers must leave a two-thread blocking pool free for unrelated spawn_blocking work. Refs #328
Member
Author
Security and Performance ReviewFixed
Remaining
VerifiedFFI struct layout and prototypes match |
russh 0.64.1 verifies the exchange signature with whatever key the server sends, and check_server_key never compared it with the negotiated host-key algorithm, so a HostKeyAlgorithms restriction (including one that excludes sk-*) did not limit which trusted key type a server could use. OpenSSH's kex_verify_host_key rejects this with SSH_ERR_KEY_TYPE_MISMATCH. kex_done now records Names::key, and check_server_key refuses a key whose type differs before any known_hosts or StrictHostKeyChecking decision, with a new HostKeyTypeMismatch error. Signature names map to their key type (rsa-sha2-256/512 select RSA), ECDSA curves must match, and sk types match only themselves. russh records only the plain algorithm and keeps the certificate flag private, but it parses the blob as a Certificate exactly when a certificate was negotiated, so the certificate half comes from that variant and certificate key data in the plain-key slot is refused. A key received before kex_done fails closed. The existing policy body moved to verify_server_key, which the policy tests now call directly. Tests cover RSA under every rsa-sha2 name, Ed25519 under ECDSA, curve mismatch, certificate versus plain in both directions, sk types, and the handler path; each fails with the check reverted. Refs #328
Library::new opened the provider with RTLD_LAZY, so a library with an unresolved symbol loaded fine and then aborted bssh at the first sk_sign. It is now opened with RTLD_NOW | RTLD_LOCAL, as sshsk_open does with RTLD_NOW (Library::new stays on non-Unix targets). Each validated library is cached per canonical path for the process lifetime instead of being dlopened and dlclosed per identity per connection; failed loads are not cached. Canonicalizing first also means a bare file name refers to a file in the working directory, as OpenSSH's lib_contains_symbol does, rather than one on the loader search path. An empty application is refused before the provider is called, as sshsk_sign refuses it, both in the signer (before the touch notice) and in the external provider. An Ed25519 response must carry exactly 64 bytes in sig_r and nothing in sig_s. The PIN is copied into a wiped buffer only after the NUL check, so a rejected PIN no longer leaves an unwiped copy inside CString's error, SecurityKeyIdentity's Debug prints only the key handle length, and the free_response call has its SAFETY comment. The module doc and docs/openssh-compatibility-status.md record that the provider runs in process: a started sk_sign cannot be cancelled, a hung provider holds the process-wide lock, and the provider can read bssh's memory. New tests compile small C providers at test time on Linux to cover RTLD_NOW, the cache, and the FFI sign path; each new test fails with its fix reverted. Moved external.rs tests to external_tests.rs. Refs #328
ARCHITECTURE.md did not mention FIDO sk-* client identities, the SecurityKeyProvider flow, the default id_ed25519_sk and id_ecdsa_sk identities, or the negotiated host-key type check. Add them to the Authentication and host certificate sections, and list host_key_type.rs in the ssh-client module structure. Docs only, no code changed. Other listed docs were already consistent with the code. Refs #328
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FIDO security keys (
sk-ssh-ed25519@openssh.com,sk-ecdsa-sha2-nistp256@openssh.com) now work for host keys, host certificates, user keys and user certificates, matching OpenSSH 10.3p1 built without its internal provider. The built-ininternalprovider is #331; it plugs into the provider seam added here, and until thenSecurityKeyProvider internalprints a diagnostic and authentication continues with the next identity.SecurityKeyProvideris a runtime option: library path,internal,none, or$VAR(default$SSH_SK_PROVIDER, printed raw by-Glike the reference). An unset variable ornoneskipssk-*identities at debug level, assshdoes.id_ed25519_skandid_ecdsa_skare default identities, and ansk-*file is checked against the provider before any passphrase prompt.0x000a0000) library is loaded in process withdlopen(RTLD_NOW | RTLD_LOCAL), validated (version,sk_enroll/sk_sign/sk_load_resident_keys) and cached per canonical path for the process lifetime. OpenSSH isolates the provider inssh-sk-helperto keep it out of itspledged client; bssh has no such sandbox, so a helper would add a private protocol without changing what runs as the user. The trade-off, documented in the compatibility status: a started provider call cannot be cancelled, and the provider can read bssh's memory.spawn_blocking, so queued hosts hold no blocking thread and a host that hitsConnectTimeoutwhile queued never reaches the provider. Every response pointer and length is checked (Ed25519 requires a 64-byte signature), an empty application is rejected, the response is zeroed and freed assshsk_free_sign_responsedoes, and PINs are zeroed and never logged.sk_signfailure after the server accepted the key printssecurity key signing failed for <path>: ...and authentication continues with the next identity on the same connection, as in OpenSSH: the signer returns the request unsigned, russh 0.64.1 sends nothing for it, and bssh drops the exchange that would otherwise wait for a reply.-Q key-plain,key,key-cert,key-sigand the defaultHostKeyAlgorithmsandPubkeyAcceptedAlgorithmsmatch the referencessh -Qandssh -Gexcept for thewebauthn-sk-ecdsa-sha2-nistp256@openssh.compair, which ssh-key cannot verify and bssh therefore does not advertise.sk-*host keys and certificates verify through ssh-key'sskverifiers (SHA256(application) || flags || counter || SHA256(message)); like OpenSSH's client, no flag policy applies to host keys.kex_verify_host_key,SSH_ERR_KEY_TYPE_MISMATCH), so aHostKeyAlgorithmsrestriction also limits which trusted key type a server may present.Test plan
cargo fmt --check,cargo clippy -- -D warnings,cargo clippy --lib --tests -- -D warnings,cargo test --libandcargo test --tests -- --skip integration_testpass locally on 20d38c4 (the head before the docs-only commit).hostkey-agent(bothsk-*certificate phases),keytype,limit-keytype,agent,connect,proxy-connect,sshcfgparse,hostkey-rotate,rekeypass.agent-restrict(feat(agent): bind destination-constrained agent keys with session-bind@openssh.com #327).$VARexpansion, theinternaldiagnostic, load and validation failures including lazy symbols, the library cache, signature encoding, the decline-and-continue path against a scripted server, signer queueing and cancellation, the algorithm lists, the certificate-only host-key list, and host-key type matching; each was checked to fail when its behavior is reverted.cargo check --target aarch64-unknown-linux-musl -p libloadingpasses, and a faileddlopenon a static build says so. A full musl build of bssh needs a musl cross compiler foraws-lc-sys, which this host lacks, so the next release workflow run is the first full build check of the musl targets.Follow-up candidates: bssh-server verification of
sk-*user keys,bssh-keygenenrollment and resident keys,webauthn-sk-*verification, and host-key checks on re-key (russh does not pass the re-key host key to the handler).Closes #328