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.

Sign up for GitHub

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

Jump to bottom

Support external crypto implementations. #1496

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Support external crypto implementations. #1496

merged 1 commit into from
Mar 22, 2023

Conversation

nmittler
Copy link
Contributor

@nmittler nmittler commented Feb 22, 2023 •
edited
Loading

These changes are needed for the BoringSSL crypto provider (#1488), which will reside in a separate repository.

@nmittler
Copy link
Contributor Author

@djc I think the visibility changes here make sense, since they're effectively part of the crypto provider API. WDYT?

djc
djc reviewed Feb 23, 2023
View reviewed changes
quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
perf/src/noprotection.rs Outdated Show resolved Hide resolved
@nmittler nmittler force-pushed the conformance_simple branch 2 times, most recently from be1d69c to 2a45cd0 Compare February 24, 2023 19:45
@nmittler
Copy link
Contributor Author

@djc I believe I've addressed your concerns. PTAL

nmittler
nmittler commented Feb 24, 2023
View reviewed changes
quinn/src/endpoint.rs Outdated Show resolved Hide resolved
@nmittler nmittler force-pushed the conformance_simple branch 2 times, most recently from 2e71fdb to f1cce12 Compare February 27, 2023 13:29
@nmittler
Copy link
Contributor Author

nmittler commented Feb 27, 2023 •
edited
Loading

@djc any other comments here? I'd like to get this in quickly, since there will be 2+ follow-on PRs that depend on this. First one is #1501.

nmittler added a commit to nmittler/quinn that referenced this pull request Feb 27, 2023
@nmittler
Provider-based crypto tests
3dac6f0 
This allows the provider integration tests to be run against any crypto provider. For now, only rustls is supported. This will be updated in the future once the boringssl provider has landed.

Requires quinn-rs#1496.
@nmittler nmittler mentioned this pull request Feb 27, 2023
Closed
@nmittler
Copy link
Contributor Author

@Ralith did you want to take a look as well?

nmittler added a commit to nmittler/quinn that referenced this pull request Mar 1, 2023
@nmittler
Provider-based crypto tests
ebc716c 
This allows the provider integration tests to be run against any crypto provider. For now, only rustls is supported. This will be updated in the future once the boringssl provider has landed.

Requires quinn-rs#1496.
djc
djc reviewed Mar 2, 2023
View reviewed changes
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.

@Ralith would be great to get your feedback on these changes, too.

quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
Ralith
Ralith requested changes Mar 4, 2023
View reviewed changes
quinn-proto/src/transport_parameters.rs Outdated Show resolved Hide resolved
quinn/src/endpoint.rs Outdated Show resolved Hide resolved
Ralith
Ralith reviewed Mar 4, 2023
View reviewed changes
quinn-proto/src/token.rs Outdated Show resolved Hide resolved
nmittler added a commit to nmittler/quinn that referenced this pull request Mar 5, 2023
@nmittler
Provider-based crypto tests
78b29b8 
This allows the provider integration tests to be run against any crypto provider. For now, only rustls is supported. This will be updated in the future once the boringssl provider has landed.

Requires quinn-rs#1496.
nmittler added a commit to nmittler/quinn that referenced this pull request Mar 5, 2023
@nmittler
Provider-based crypto tests
43abca3 
This allows the provider integration tests to be run against any crypto provider. For now, only rustls is supported. This will be updated in the future once the boringssl provider has landed.

Requires quinn-rs#1496.
nmittler added a commit to nmittler/quinn that referenced this pull request Mar 5, 2023
@nmittler
Provider-based crypto tests
ce962f3 
This allows the provider integration tests to be run against any crypto provider. For now, only rustls is supported. This will be updated in the future once the boringssl provider has landed.

Requires quinn-rs#1496.
@nmittler
Copy link
Contributor Author

nmittler commented Mar 5, 2023

@Ralith PTAL

nmittler added a commit to nmittler/quinn that referenced this pull request Mar 8, 2023
@nmittler
Provider-based crypto tests
9839437 
This allows the provider integration tests to be run against any crypto provider. For now, only rustls is supported. This will be updated in the future once the boringssl provider has landed.

Requires quinn-rs#1496.
nmittler added a commit to nmittler/quinn that referenced this pull request Mar 9, 2023
@nmittler
Provider-based crypto tests
cce6a25 
This allows the provider integration tests to be run against any crypto provider. For now, only rustls is supported. This will be updated in the future once the boringssl provider has landed.

Requires quinn-rs#1496.
@nmittler nmittler mentioned this pull request Mar 9, 2023
Merged
@nmittler
Copy link
Contributor Author

nmittler commented Mar 9, 2023

@djc if I'm reading the lint error correctly, I think it's an existing issue with main. Can you take a look?

@djc
Copy link
Member

djc commented Mar 10, 2023

Yup, new clippy lints from Rust 1.68. Addressed in #1506.

@nmittler
Copy link
Contributor Author

@djc @Ralith this is blocking quinn-rs/quinn-boring#2. Any chance we can get this in soon?

@djc
Copy link
Member

djc commented Mar 10, 2023

We'll get to it when we get to it -- we're maintaining Quinn on a voluntary basis and you're adding code paths here that are somewhat far out of the core use case. Some patience will be required, just have your downstream code depend on a Git commit for now?

@nmittler
Copy link
Contributor Author

@djc understood. Thanks!

@Ralith
Copy link
Collaborator

Ralith commented Mar 16, 2023

Thanks for your patience and persistence here; I've been preoccupied, but I do want to land this.

@nmittler
Copy link
Contributor Author

@Ralith @djc transport parameters have been reverted. PTAL

Ralith
Ralith requested changes Mar 20, 2023
View reviewed changes
quinn/src/endpoint.rs Outdated Show resolved Hide resolved
quinn-proto/src/crypto.rs Show resolved Hide resolved
@nmittler
Support external crypto implementations.
0123241 
These changes are needed for the BoringSSL crypto provider (quinn-rs#1488),
which will reside in a separate repository.
@djc djc merged commit 88897e7 into quinn-rs:main Mar 22, 2023
@nmittler
Copy link
Contributor Author

@Ralith @djc thanks for the review! I guess this will be included in the next patch release?

@djc
Copy link
Member

djc commented Mar 27, 2023

Yes -- if you would like one sooner rather than later, can you send a PR to bump the quinn-proto version number?

@nmittler nmittler mentioned this pull request Mar 27, 2023
Closed
@nmittler
Copy link
Contributor Author

@djc thanks ... I've opened #1520.

@djc djc mentioned this pull request Apr 14, 2023
Merged
@djc djc mentioned this pull request May 8, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djc djc djc left review comments

@Ralith Ralith Ralith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nmittler @djc @Ralith