Skip to content

Fix regression from 6379b473#75

Merged
reynir merged 1 commit into
mirage:mainfrom
reynir:fix-server-sig-algs
Sep 19, 2024
Merged

Fix regression from 6379b473#75
reynir merged 1 commit into
mirage:mainfrom
reynir:fix-server-sig-algs

Conversation

@reynir
Copy link
Copy Markdown
Member

@reynir reynir commented Sep 18, 2024

The server-sig-algs should be Hostkey.preferred_algs, not just the algorithm for the servers host key. That way we can have a server that uses a ed25519 host key and allow authentication with an RSA key.

Found while testing #74 using awa_test_server.

The server-sig-algs should be Hostkey.preferred_algs, not just the
algorithm for the servers host key. That way we can have a server that
uses a ed25519 host key and allow authentication with an RSA key.
@hannesm
Copy link
Copy Markdown
Member

hannesm commented Sep 18, 2024

this is a partial revert of 6379b47 - I believe your code is correct, but I wonder whether the other part (in let rekey...) should be reverted as well? I don't know exactly how I ended up with 6379b47...

@reynir
Copy link
Copy Markdown
Member Author

reynir commented Sep 19, 2024

I commented in the commit also. The other usages are for server_host_key_algs in kexinit. It's used there for a different purpose: what algorithms are available for the server host key(s) (of which we currently only have one). So in that case it's desirable to only send algorithms which match our server host key.

@reynir
Copy link
Copy Markdown
Member Author

reynir commented Sep 19, 2024

On the other hand it seems we had an error in rekeying before that commit as we allow (almost) any server host key sig algs when rekeying?! I don't remember how rekeying works, but that seems odd if not wrong.

@reynir
Copy link
Copy Markdown
Member Author

reynir commented Sep 19, 2024

Reading rfc4253 pp. 14, 22-23 it seems an error. It's possible to change the host keys and the algorithms in a rekey.

@reynir reynir merged commit 5b5fe34 into mirage:main Sep 19, 2024
@reynir
Copy link
Copy Markdown
Member Author

reynir commented Sep 19, 2024

It seems I am to blame for suggesting this regression :D #62 (comment)

@reynir reynir deleted the fix-server-sig-algs branch September 19, 2024 07:45
hannesm added a commit to hannesm/opam-repository that referenced this pull request Feb 5, 2025
CHANGES:

* Fix regression from 6379b473 (server-sig-algs should be Hostkey.preferred_algs)
  (mirage/awa-ssh#75, @reynir)
* Avoid functors over Mirage_time.S and Mirage_clock.MCLOCK (mirage/awa-ssh#76 @hannesm)
* Adapt to mirage-crypto-rng 1.2.0 (mirage/awa-ssh#76 @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.

2 participants