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

fix(771): Adding mTLS support in QUIC #899

Merged
merged 6 commits into from
Apr 24, 2024
Merged

fix(771): Adding mTLS support in QUIC #899

merged 6 commits into from
Apr 24, 2024

Conversation

gabrik
Copy link
Contributor

@gabrik gabrik commented Apr 4, 2024 •
edited
Loading

This PR contains

  • Some refactoring for shared method between TLS and QUIC transport
  • mTLS support for QUIC
  • tests

Closes #771

@eclipse-zenoh-bot
Copy link
Contributor

@gabrik If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

1 similar comment
@eclipse-zenoh-bot
Copy link
Contributor

@gabrik If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

@gabrik
Copy link
Contributor Author

gabrik commented Apr 4, 2024 •
edited
Loading

This is may be blocked by: quinn-rs/quinn#1737

@Mallets Mallets changed the base branch from main_old to main April 11, 2024 08:49
@gabrik gabrik force-pushed the fix/771-quic-mtls branch from 0a72149 to 6682595 Compare April 24, 2024 06:31
@gabrik
Copy link
Contributor Author

gabrik commented Apr 24, 2024 •
edited
Loading

@Mallets you can have a first look, I need to add the tests.
There is "duplicated" code between TLS and QUIC transports as two different version of rustls are being used, this "duplicated" code is the one in utils.rs of both crates.

@gabrik
test(quic-mlts): added tests for QUIC with mTLS, using rustls 0.22 to…
8c0c619 
… workaround the default CryptoProvider panic

Signed-off-by: gabrik <[email protected]>
@gabrik gabrik marked this pull request as ready for review April 24, 2024 09:40
@gabrik gabrik requested a review from Mallets April 24, 2024 09:40
@gabrik
Copy link
Contributor Author

gabrik commented Apr 24, 2024

I was using rustls 0.23 that introduced this: CryptoProvider that must be initialized early in any process using TLS, thus tests where failing.

I did a rollback to 0.22 (the same version used in main) to make the test pass and check if the logic is correct.

Mallets
Mallets reviewed Apr 24, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
Mallets
Mallets reviewed Apr 24, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
Mallets
Mallets reviewed Apr 24, 2024
View reviewed changes
io/zenoh-links/zenoh-link-quic/src/lib.rs
use secrecy::ExposeSecret;
use std::net::SocketAddr;
use zenoh_config::Config;

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

@Mallets Mallets merged commit 9a9832a into main Apr 24, 2024
21 checks passed
@Mallets Mallets deleted the fix/771-quic-mtls branch April 24, 2024 14:23
@gabrik gabrik mentioned this pull request Jul 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mallets Mallets Mallets left review comments

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

Successfully merging this pull request may close these issues.

Mutual authentication (mTLS) does not work with quic transport
3 participants
@gabrik @eclipse-zenoh-bot @Mallets