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

#2057: Use randomly generated GREASE transport parameter. #2058

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

mstyura
Copy link
Contributor

@mstyura mstyura commented Nov 20, 2024

As a first step to make quinn be less vulnerable to fingerprinting I propose to generate GREASE transport parameter randomly.

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this seems nice to have.

quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
@mstyura mstyura force-pushed the gh-2057 branch 2 times, most recently from 811d345 to ab98f38 Compare November 20, 2024 22:06
@mstyura mstyura force-pushed the gh-2057 branch 2 times, most recently from 86a6a35 to 3c373d5 Compare November 21, 2024 13:27
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits, thanks for working on this!

quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
@mstyura
Copy link
Contributor Author

mstyura commented Nov 21, 2024

@djc thanks you very much for review! I've fixed code according to suggestions and also additionally validated comments grammar with ChatGPT and fixed several other mistakes highlighted by it.

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mstyura
Copy link
Contributor Author

mstyura commented Nov 21, 2024

Is there anything else I can do to help move this PR toward being merged?

@djc
Copy link
Member

djc commented Nov 21, 2024

Is there anything else I can do to help move this PR toward being merged?

Pay for the maintenance you're getting for free? We're volunteer maintainers so please be a little more patient.

quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
@djc
Copy link
Member

djc commented Nov 21, 2024

Thanks for working on this!

@mstyura
Copy link
Contributor Author

mstyura commented Nov 21, 2024

Pay for the maintenance you're getting for free? We're volunteer maintainers so please be a little more patient.

Sorry, I didn't intend to make a pressure on you. I understand you are volunteers with a lot of Rust projects to maintain. I just wanted to make sure I didn't miss anything required. Regarding the donations I'll talk again to company I work for to allocate some budget next year to support OSS project it rely on.

Merged via the queue into quinn-rs:main with commit 2edf192 Nov 21, 2024
16 of 17 checks passed
@djc
Copy link
Member

djc commented Nov 21, 2024

Sorry, I didn't intend to make a pressure on you. I understand you are volunteers with a lot of Rust projects to maintain. I just wanted to make sure I didn't miss anything required.

No worries, I can see how it might have been confusing that this wasn't merged yet after we both approved it -- I get a bit stingy when I get the feeling of being pressured.

Regarding the donations I'll talk again to company I work for to allocate some budget next year to support OSS project it rely on.

Any efforts in that direction are much appreciated, thanks!

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