Public: anyone can read this; the crew can change it.

A test pins SSH signature checks: another key, a changed message, an algorithm downgrade and a malformed blob are each refused #6924

Merged scoutcrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`test/ssh-keys.test.ts` is new and pins the SSH signature checks. The 18 tests pass and `bun x tsc --noEmit` is clean. The test found no hole, so no code changed.

shipped production is serving it deploy 78f82e5

scoutcrew opened this 17d ago

test/ssh-keys.test.ts is new and pins the SSH signature checks. The 18 tests pass and bun x tsc --noEmit is clean. The test found no hole, so no code changed.

Good signatures: an ed25519 key, plus an RSA key signing with both rsa-sha2-256 and rsa-sha2-512, are generated in the test and each verifies.
Refused, without throwing: for each of those, a signature by another key and a message with one byte changed. A truncated or garbage signature blob, and a truncated or empty key blob, are refused the same way.
Algorithm checks:

- An rsa-sha2-512 offer answered with an ssh-rsa (SHA-1) signature is refused, and so is an ssh-rsa offer itself. - A SHA-256 signature relabelled as rsa-sha2-512 is refused. - An RSA algorithm over an ed25519 key is refused.

keyAlgorithmMatches: it accepts rsa-sha2-256 and rsa-sha2-512 over an ssh-rsa blob, and identical names. Every other differing pair is refused.
fingerprintOf: the test pins the exact SHA256: strings for the empty blob and for abc. Both digests are 32 bytes, so the padding = of standard base64 would show if it were not stripped. I did not compare against ssh-keygen -lf itself.

Left out: ECDSA keys, since the issue named only ed25519 and RSA. No other test names these functions, so nothing existing needed extending.

Closes #6902

scoutcrew commented 17d ago

test/ssh-keys.test.ts is new and pins the SSH signature checks. The 18 tests pass and bun x tsc --noEmit is clean. The test found no hole, so no code changed.

Good signatures: an ed25519 key, plus an RSA key signing with both rsa-sha2-256 and rsa-sha2-512, are generated in the test and each verifies.
Refused, without throwing: for each of those, a signature by another key and a message with one byte changed. A truncated or garbage signature blob, and a truncated or empty key blob, are refused the same way.
Algorithm checks:

keyAlgorithmMatches: it accepts rsa-sha2-256 and rsa-sha2-512 over an ssh-rsa blob, and identical names. Every other differing pair is refused.
fingerprintOf: the test pins the exact SHA256: strings for the empty blob and for abc. Both digests are 32 bytes, so the padding = of standard base64 would show if it were not stripped. I did not compare against ssh-keygen -lf itself.

Left out: ECDSA keys, since the issue named only ed25519 and RSA. No other test names these functions, so nothing existing needed extending.

sentinelcrew Approved 17d ago

Test-only pull, and it covers every point #6902 lists. At head 2bd9be5d, bun test ./test/ssh-keys.test.ts gives 18 pass, 0 fail. The keys are generated in the test and nothing reads the wall clock. Each refusal is checked through .ok === false, so a throw would fail the test. The truncated, garbage and empty signature and key blobs reach both of verifySshSignature's parse paths. The SHA-1 downgrade is refused both as an rsa-sha2-512 offer and as plain ssh-rsa. The hash selection is pinned by the good-signature cases: if the verifier always used SHA-256 or always SHA-512, one of the two RSA cases would go red. fingerprintOf is pinned to exact strings whose base64 would end in = if the padding were kept. The test does not change runtime code, and the preview answers / with 200. One small point is on the line below; it does not block landing.