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

Only permit X25519 based QUIC-TLS key exchanges #3927

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ripatel-fd
Copy link

Problem

Some versions of Agave permit a variety of key exchange algorithms.
These increase cryptographic attack surface and are slower than X25519.

Summary of Changes

Reject connection requests with key exchange algorithms other than X25519.

Fixes #

@mergify mergify bot requested a review from a team December 4, 2024 21:49
@lijunwangs lijunwangs added the CI Pull Request is ready to enter CI label Dec 5, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Dec 5, 2024
@lijunwangs lijunwangs self-requested a review December 5, 2024 00:14
@lijunwangs
Copy link

Did we do any cross version compatibility test? Like if an older version of the agave-client talks to the server with this change, does it still work?

@ripatel-fd
Copy link
Author

Did we do any cross version compatibility test? Like if an older version of the agave-client talks to the server with this change, does it still work?

I didn't do any specific version compatibility tests. But I'm certain that this change does not introduce compatibility issues.
Since Solana Labs days, solana-streamer has always defaulted to an X25519 based key exchange.
In the very unlikely event issues do arise, they will be caught in Agave's QA procedure. (invalidator, testnet deployments, etc)

@lijunwangs
Copy link

Did we do any cross version compatibility test? Like if an older version of the agave-client talks to the server with this change, does it still work?

I didn't do any specific version compatibility tests. But I'm certain that this change does not introduce compatibility issues. Since Solana Labs days, solana-streamer has always defaulted to an X25519 based key exchange. In the very unlikely event issues do arise, they will be caught in Agave's QA procedure. (invalidator, testnet deployments, etc)

I expect it work as well. But we still need to do due diligence testing. I am not sure about coverage of invalidator testing on backward compatibility on this regard. I will launch a GCE cluster and see how it goes

@lijunwangs
Copy link

Hi @ripatel-fd could you please rebase with master? Somehow GitHub does not enable to fix the conflict on this PR directly.

Sorry -- it took me some time to turn around to run the test:

I ran old code cluster and use this code as client -- worked as expected
I ran a cluster with this code and a client with old code -- worked as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants