Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle unknown pubkeys #68

Merged
merged 5 commits into from
Jun 19, 2023
Merged

Handle unknown pubkeys #68

merged 5 commits into from
Jun 19, 2023

Conversation

reynir
Copy link
Member

@reynir reynir commented Jun 15, 2023

This PR delays parsing of public keys offered by clients. This allows clients to offer keys we don't know how to use to the server, and the server can reject them gracefully.

Things to do

  • Check the signature algorithm is compatible with the public key
  • According to RFC 4252 the client MAY offer a signature without first probing the key. To implement this we need to further delay more parsing. I don't know how many clients behave this way, and as I've not encountered this behavior before we could defer implementing this to another PR.
  • Figure out if we need to check the duplicated occurrences of signature algorithm and key type (the RFCs conflate both as "public key algorithm"). It may be helpful to look at what openssh does.

robur-team and others added 3 commits June 14, 2023 13:15
Co-authored-by: Hannes Mehnert <[email protected]>
Co-authored-by: Reynir Björnsson <[email protected]>
When a client is offering a public key or a signature we check if the
signature algorithm is known to us *and* compatible with the provided
public key.
@reynir
Copy link
Member Author

reynir commented Jun 15, 2023

Relevant reading https://www.rfc-editor.org/rfc/rfc8332#section-2

and in section 3.2:

If the client includes the signature field, the client MUST encode
the same algorithm name in the signature as in
SSH_MSG_USERAUTH_REQUEST -- either "rsa-sha2-256" or "rsa-sha2-512".
If a server receives a mismatching request, it MAY apply arbitrary
authentication penalties, including but not limited to authentication
failure or disconnect.

So it seems a client MUST put matching signature algorithms (known as public key algorithms in the RFCs with all its ambiguity). A server MAY reject it. In openssh they seem, for rsa keys, to generally enforce this with the exception of ssh-rsa-cert-v01 https://github.com/openssh/openssh-portable/blob/2709809fd616a0991dc18e3a58dea10fb383c3f0/ssh-rsa.c#L504-L507

When a client tries to authenticate with a public key and a signature we
check that the advertised public key algorithm matches what is in the
signature.
@reynir reynir marked this pull request as ready for review June 15, 2023 09:05
@reynir
Copy link
Member Author

reynir commented Jun 15, 2023

I don't think it would require much change to support clients that don't probe keys first. I can take a look at implementing that later this week. Otherwise I think this PR is ready.

@hannesm hannesm merged commit 92cf212 into main Jun 19, 2023
@hannesm
Copy link
Member

hannesm commented Jun 19, 2023

thanks a lot

@hannesm hannesm deleted the unknown-pubkeys branch June 19, 2023 10:28
hannesm added a commit to hannesm/opam-repository that referenced this pull request Jun 19, 2023
CHANGES:

* FEATURE server: propagate window-change message (mirage/awa-ssh#55 @reynir)
* FEATURE server: implement ext-info and server-sig-algs extension (mirage/awa-ssh#56 @reynir)
* FEATURE server: support RFC 4419 (group key exchanges) and NIST ECDH key
  exchanges, and X25519 (mirage/awa-ssh#63 mirage/awa-ssh#67 @hannesm)
* FEATURE server: handle unknown public keys (instead of closing the connection,
  send a message back, allowing other public keys to be probeb) (mirage/awa-ssh#68 @reynir)
* BUGFIX server: fix rekey (avoid allocating lots of timeout tasks (mirage/awa-ssh#58 @reynir)
* BUGFIX server: filter advertised host key algorithms with used host key
  (mirage/awa-ssh#62 @hannesm)
* server: use logs instead of printf (mirage/awa-ssh#69 @hannesm)
* awa-lwt: drop package (unused, mirage/awa-ssh#61 @hannesm)
* drop Driver module, embed into awa_test_server.ml (mirage/awa-ssh#64 @hannesm)
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.

3 participants