Skip to content

ssh-key: fix Signature::encode for sk-ecdsa-sha2-nistp256#532

Open
jkolo wants to merge 1 commit into
RustCrypto:masterfrom
jkolo:fix-sk-ecdsa-encode-master
Open

ssh-key: fix Signature::encode for sk-ecdsa-sha2-nistp256#532
jkolo wants to merge 1 commit into
RustCrypto:masterfrom
jkolo:fix-sk-ecdsa-encode-master

Conversation

@jkolo
Copy link
Copy Markdown

@jkolo jkolo commented May 22, 2026

Problem

Signature's Decode and Encode impls are asymmetric for
sk-ecdsa-sha2-nistp256@openssh.com signatures.

Decode reads the security-key trailer (byte flags || uint32 counter)
for both SK algorithms:

if algorithm == Algorithm::SkEd25519 || algorithm == Algorithm::SkEcdsaSha2NistP256 {
    let flags = u8::decode(reader)?;
    let counter = u32::decode(reader)?;
    ...
}

But Encode only special-cases SkEd25519:

if self.algorithm == Algorithm::SkEd25519 {
    // split signature body from trailer, write them separately
} else {
    self.as_bytes().encode(writer)?;   // <-- sk-ecdsa falls through here
}

For an sk-ecdsa-sha2-nistp256@openssh.com signature this writes the
whole data buffer (ECDSA signature plus the 5-byte trailer) as a
single length-prefixed string, instead of the wire layout required by
OpenSSH's PROTOCOL.u2f:

string  ecdsa_signature
byte    flags
uint32  counter

A signature produced this way fails to decode again — the trailer bytes
end up buried inside the inner string — and is rejected by OpenSSH as an
invalid signature.

Fix

Extend the Encode sk-trailer branch to also match
SkEcdsaSha2NistP256. The existing trailer-splitting logic is generic
over SK_SIGNATURE_TRAILER_SIZE and works unchanged for both algorithms.

Tests

Adds decode_sk_ecdsa_sha2_p256 and encode_sk_ecdsa_sha2_p256,
mirroring the existing sk-ed25519 tests. The test vector reuses the
ECDSA signature components from ECDSA_SHA2_P256_SIGNATURE with an
sk-ecdsa algorithm prefix and a flags/counter trailer.
encode_sk_ecdsa_sha2_p256 fails before this change (encode emits a
72-byte body as a 77-byte string) and passes after.

`Signature`'s `Decode` impl handles the SK signature trailer (flags +
counter) for both `SkEd25519` and `SkEcdsaSha2NistP256`, but the
`Encode` impl only special-cases `SkEd25519`. As a result, an
`sk-ecdsa-sha2-nistp256@openssh.com` signature falls through to the
generic branch, which writes the whole `data` buffer (ECDSA signature
+ flags + counter) as a single length-prefixed string instead of
emitting `string ecdsa_signature || byte flags || uint32 counter`.

Round-tripping such a signature through `Decode` then fails because the
trailer bytes are buried inside the string.

Extend the `Encode` sk-trailer branch to also match
`SkEcdsaSha2NistP256`; the existing trailer-splitting logic is generic
over the trailer size and works unchanged for both algorithms.

Adds `decode_sk_ecdsa_sha2_p256` and `encode_sk_ecdsa_sha2_p256` tests
mirroring the existing sk-ed25519 ones; `encode_sk_ecdsa_sha2_p256`
fails before this change and passes after.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant