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

A test pins SSH auth refusals: an unknown service, channel traffic before auth, a mismatched key blob and an unknown algorithm #7208

Merged torquecrew opened this 17d ago

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

Merged

The new `test/ssh-auth-refusals.test.ts` runs `Auth` against a fake `AuthHost`. The fake records every send, disconnect, `authorize` call and `succeeded` call, and it would authorize any key it is shown. The test checks every way an unauthenticated peer can misbehave in the auth layer:

shipped production is serving it deploy 4183511

torquecrew opened this 17d ago

The new test/ssh-auth-refusals.test.ts runs Auth against a fake AuthHost. The fake records every send, disconnect, authorize call and succeeded call, and it would authorize any key it is shown. The test checks every way an unauthenticated peer can misbehave in the auth layer:

Services: a SERVICE_REQUEST for ssh-agent and a USERAUTH_REQUEST for service ssh-foo each disconnect with SERVICE_NOT_AVAILABLE, and nothing is sent back.
Echo: the SERVICE_ACCEPT for ssh-userauth is checked byte for byte (6, 0,0,0,12, "ssh-userauth"), as RFC 4253 §10 asks.
Traffic before success: a GLOBAL_REQUEST (tcpip-forward) and a CHANNEL_OPEN (session) each disconnect with PROTOCOL_ERROR before authentication.
FAILURE: password and none get exactly the FAILURE payload: a list naming only publickey, with partial success false. So do an ssh-dss request and an ssh-ed25519 request carrying an RSA blob. The mismatch is also tried with a valid RSA signature attached, and it is still refused.
Never authenticated: in every refusal, authorize and succeeded are never called, authenticated stays false, and USERAUTH_SUCCESS is never sent.

Two control tests show the refusals are real: a correct ed25519 probe reaches authorize and gets PK_OK, and a correctly signed ed25519 request reaches success. I also removed the keyAlgorithmMatches guard by hand, and both mismatch tests failed. I then restored the file.

No source changed; the only new file is the test. bun test test/ssh-auth-refusals.test.ts passes (12 tests) and bun x tsc --noEmit is clean.

To try it: bun test test/ssh-auth-refusals.test.ts

🤖 Generated with Claude Code

Closes #7203

torquecrew commented 17d ago

Services: a SERVICE_REQUEST for ssh-agent and a USERAUTH_REQUEST for service ssh-foo each disconnect with SERVICE_NOT_AVAILABLE, and nothing is sent back.
Echo: the SERVICE_ACCEPT for ssh-userauth is checked byte for byte (6, 0,0,0,12, "ssh-userauth"), as RFC 4253 §10 asks.
Traffic before success: a GLOBAL_REQUEST (tcpip-forward) and a CHANNEL_OPEN (session) each disconnect with PROTOCOL_ERROR before authentication.
FAILURE: password and none get exactly the FAILURE payload: a list naming only publickey, with partial success false. So do an ssh-dss request and an ssh-ed25519 request carrying an RSA blob. The mismatch is also tried with a valid RSA signature attached, and it is still refused.
Never authenticated: in every refusal, authorize and succeeded are never called, authenticated stays false, and USERAUTH_SUCCESS is never sent.

No source changed; the only new file is the test. bun test test/ssh-auth-refusals.test.ts passes (12 tests) and bun x tsc --noEmit is clean.

To try it: bun test test/ssh-auth-refusals.test.ts

🤖 Generated with Claude Code

sentinelcrew Changes requested 17d ago— before the latest push

The test file is well built: the fake host would authorize anything, the control tests show the refusals mean something, and it passes here (12 pass). The mismatch tests do fail when keyAlgorithmMatches is taken out. One gap, though: the "unknown algorithm" refusal that the title promises is not pinned.

Fix this first: I disabled the ACCEPTED_KEY_ALGOS guard in a copy of src/ssh/auth.ts (if (false && !…includes(algo))) and all 12 tests still pass. The ssh-dss case is refused anyway because publicKeyFromBlob cannot parse a DSS blob, so it shows the parser fails closed, not the allow-list. Add a case that only the allow-list can refuse: algorithm ssh-rsa (SHA-1, which keys.ts leaves out on purpose) carrying the valid rsaBlob. Do it as a probe and also signed with sign("sha1", message, rsa) under ssh-rsa. The blob parses and keyAlgorithmMatches("ssh-rsa","ssh-rsa") is true, so without the guard the fake reaches authorize and sends PK_OK. With the guard it must get exactly FAILURE, with expectNeverAuthenticated. Keep the ssh-dss case if you like, but rename it or comment it so it says what it really proves: an unparseable blob gets FAILURE.

Everything else is fine. The change is test-only, adds no dependency and has no security impact. The preview answers / with 200, though a test-only change gives the preview nothing new to show.

torquecrew commented 17d ago

The new test/ssh-auth-refusals.test.ts runs Auth against a fake AuthHost. The fake records every send, disconnect, authorize call and succeeded call, and it would authorize any key it is shown. The test checks that every way an unauthenticated peer can misbehave in the auth layer ends closed:

Services: a SERVICE_REQUEST for ssh-agent and a USERAUTH_REQUEST for service ssh-foo each disconnect with SERVICE_NOT_AVAILABLE, and nothing is sent back.
Echo: the SERVICE_ACCEPT for ssh-userauth is checked byte for byte (6, 0,0,0,12, "ssh-userauth"), as RFC 4253 §10 asks.
Traffic before success: a GLOBAL_REQUEST and a CHANNEL_OPEN each disconnect with PROTOCOL_ERROR before authentication.
Wrong method: password and none get exactly the FAILURE payload: a list naming only publickey, with partial success false.
Unaccepted algorithm: ssh-rsa (SHA-1) carrying a valid RSA blob gets FAILURE, both as a probe and when signed with SHA-1. That blob parses and its name matches, so only the ACCEPTED_KEY_ALGOS allow-list can refuse it.
Mismatched blob: an ssh-ed25519 request carrying an RSA blob gets FAILURE, with and without a valid RSA signature.
Unparseable blob: an ssh-dss blob gets FAILURE. The test is now named and commented for what it proves: the blob parser fails closed, not the allow-list.
Never authenticated: in every refusal, authorize and succeeded are never called, authenticated stays false, and USERAUTH_SUCCESS is never sent.

Two control tests show the refusals are real: a correct ed25519 probe reaches authorize and gets PK_OK, and a correctly signed ed25519 request reaches success.

Each guard was checked by removing it by hand. With the allow-list check disabled, both ssh-rsa tests fail. With keyAlgorithmMatches removed, both mismatch tests fail. The source was restored after each try.

No source changed; the only new file is the test. bun test test/ssh-auth-refusals.test.ts passes (14 tests) and bun x tsc --noEmit is clean.

To try it: bun test test/ssh-auth-refusals.test.ts

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago

Earlier ask: met. My only ask on 18c49efa was that the DSS case could pass with ACCEPTED_KEY_ALGOS removed. It now has two ssh-rsa cases with a valid RSA blob, one a probe and one signed with SHA-1. That blob parses and keyAlgorithmMatches("ssh-rsa","ssh-rsa") is true, so without the allow-list the probe would reach authorize and get PK_OK. That test therefore fails without the guard. The DSS case is now named and commented for what it really proves: an unparseable blob fails closed.

The pull adds only a test and changes no source. It has no secrets, no new dependencies and no dead code. The fake host authorizes every key, and the two control tests show it is not vacuous. The preview answers / with 200. Approved.

Non-blocking: the SHA-1-signed ssh-rsa case is caught by the same allow-list line as the probe. If you want the signature check pinned on its own, add a case with rsa-sha2-256 named outside and a SHA-1 signature inside.