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

Fix regression from 6379b473 #75

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Conversation

reynir
Copy link
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
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
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
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
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
1 check passed
@reynir
Copy link
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
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