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

Track the latest changes from upstream rustls and tokio-rustls #222

Closed
wants to merge 27 commits into from
+119 −66
Closed

Track the latest changes from upstream rustls and tokio-rustls #222

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
671d9d1
preliminary hack to use dynamic dispatch for crypto provider
stevefan1999-personal Sep 13, 2023
aad1c2b
Update for upstream changes
stevefan1999-personal Sep 14, 2023
c180f50
update to use upstream main branch
stevefan1999-personal Sep 21, 2023
2a4e759
Merge branch 'rustls:main' into main
stevefan1999-personal Sep 21, 2023
f16bef1
use patch instead
stevefan1999-personal Sep 21, 2023
488b28c
add pki-types back
stevefan1999-personal Sep 21, 2023
a735de0
use upstream tokio-rustls as patch target
stevefan1999-personal Sep 21, 2023
ebf8a54
upgrade rustls to 0.22.0-alpha.3
stevefan1999-personal Sep 21, 2023
7a8db93
Merge remote-tracking branch 'origin/patch-tokio-rustls'
stevefan1999-personal Sep 23, 2023
47dc485
fix checks for examples and tests
stevefan1999-personal Sep 23, 2023
24398a6
upgrade webpki-roots and mandate pki-types usage
stevefan1999-personal Sep 23, 2023
0d1c44a
match original lines in config as much as possible
stevefan1999-personal Sep 23, 2023
f23517e
use alpha version of rustls family crates
stevefan1999-personal Sep 23, 2023
b4f2668
assert exact version for rustls
stevefan1999-personal Sep 23, 2023
805af65
fold pki_types CertificateDer, PrivateKeyDer
stevefan1999-personal Sep 23, 2023
b4e5ee5
don't ignore errors on parsing private keys
stevefan1999-personal Sep 23, 2023
2da8671
Merge remote-tracking branch 'upstream/main'
stevefan1999-personal Nov 16, 2023
9a03a61
use upstream pemfile
stevefan1999-personal Nov 16, 2023
7475b76
use exact version match for alpha packages
stevefan1999-personal Nov 16, 2023
09c1b2a
bump rustls version and rewrite crate until alpha 4 release on crates.io
stevefan1999-personal Nov 16, 2023
6766642
breaking abi change, exclude ring as a necessary default
stevefan1999-personal Nov 16, 2023
b78f598
allow builder extension to be used but futher distinguish away from ring
stevefan1999-personal Nov 16, 2023
b5ed5bf
bump version as 0.25.0-alpha.1
stevefan1999-personal Nov 16, 2023
ea1aff5
revert ring renaming but keep custom provider
stevefan1999-personal Nov 17, 2023
37eaf18
track upstream tokio-rustls
stevefan1999-personal Nov 17, 2023
847817a
add back the missing comment
stevefan1999-personal Nov 17, 2023
f0bb165
guard more import items with cfg features
stevefan1999-personal Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,21 @@ http = "0.2"
hyper = { version = "0.14", default-features = false, features = ["client"] }
log = { version = "0.4.4", optional = true }
rustls-native-certs = { version = "0.6", optional = true }
rustls = { version = "0.21.6", default-features = false }
rustls = { version = "0.22.0-alpha.3", default-features = false }
stevefan1999-personal marked this conversation as resolved.
Show resolved Hide resolved
tokio = "1.0"
tokio-rustls = { version = "0.24.0", default-features = false }
webpki-roots = { version = "0.25", optional = true }
tokio-rustls = { version = "0.24.1", default-features = false }
webpki-roots = { version = "0.26.0-alpha.1", optional = true }
futures-util = { version = "0.3", default-features = false }
pki-types = { package = "rustls-pki-types", version = "0.2.1" }

[dev-dependencies]
hyper = { version = "0.14", features = ["full"] }
rustls = { version = "0.21.0", default-features = false, features = ["tls12"] }
rustls = { version = "0.22.0-alpha.3", default-features = false, features = ["tls12"] }
rustls-pemfile = "1.0.0"
tokio = { version = "1.0", features = ["io-std", "macros", "net", "rt-multi-thread"] }

[features]
default = ["native-tokio", "http1", "tls12", "logging", "acceptor"]
default = ["native-tokio", "http1", "tls12", "logging", "acceptor", "ring"]
acceptor = ["hyper/server", "tokio-runtime"]
http1 = ["hyper/http1"]
http2 = ["hyper/http2"]
Expand All @@ -37,6 +38,7 @@ native-tokio = ["tokio-runtime", "rustls-native-certs"]
tokio-runtime = ["hyper/runtime"]
tls12 = ["tokio-rustls/tls12", "rustls/tls12"]
logging = ["log", "tokio-rustls/logging", "rustls/logging"]
ring = ["rustls/ring"]

[[example]]
name = "client"
Expand All @@ -51,3 +53,6 @@ required-features = ["tokio-runtime", "acceptor"]
[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]

[patch.crates-io]
tokio-rustls = { git = 'https://github.com/rustls/tokio-rustls' }
6 changes: 4 additions & 2 deletions examples/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ async fn run_client() -> io::Result<()> {
Some(ref mut rd) => {
// Read trust roots
let certs = rustls_pemfile::certs(rd)
.map_err(|_| error("failed to load custom CA store".into()))?;
.map_err(|_| error("failed to load custom CA store".into()))?
.into_iter()
.map(Into::into);
let mut roots = RootCertStore::empty();
roots.add_parsable_certificates(&certs);
roots.add_parsable_certificates(certs);
// TLS client config using the custom CA store for lookups
rustls::ClientConfig::builder()
.with_safe_defaults()
Expand Down
11 changes: 7 additions & 4 deletions examples/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use hyper::server::conn::AddrIncoming;
use hyper::service::{make_service_fn, service_fn};
use hyper::{Body, Method, Request, Response, Server, StatusCode};
use hyper_rustls::TlsAcceptor;
use pki_types::CertificateDer;
stevefan1999-personal marked this conversation as resolved.
Show resolved Hide resolved
use pki_types::PrivateKeyDer;

fn main() {
// Serve an echo service over HTTPS, with proper error handling.
Expand Down Expand Up @@ -80,7 +82,7 @@ async fn echo(req: Request<Body>) -> Result<Response<Body>, hyper::Error> {
}

// Load public certificate from file.
fn load_certs(filename: &str) -> io::Result<Vec<rustls::Certificate>> {
fn load_certs(filename: &str) -> io::Result<Vec<CertificateDer>> {
// Open certificate file.
let certfile = fs::File::open(filename)
.map_err(|e| error(format!("failed to open {}: {}", filename, e)))?;
Expand All @@ -91,12 +93,12 @@ fn load_certs(filename: &str) -> io::Result<Vec<rustls::Certificate>> {
.map_err(|_| error("failed to load certificate".into()))?;
Ok(certs
.into_iter()
.map(rustls::Certificate)
.map(Into::into)
stevefan1999-personal marked this conversation as resolved.
Show resolved Hide resolved
.collect())
}

// Load private key from file.
fn load_private_key(filename: &str) -> io::Result<rustls::PrivateKey> {
fn load_private_key(filename: &str) -> io::Result<PrivateKeyDer> {
// Open keyfile.
let keyfile = fs::File::open(filename)
.map_err(|e| error(format!("failed to open {}: {}", filename, e)))?;
Expand All @@ -109,5 +111,6 @@ fn load_private_key(filename: &str) -> io::Result<rustls::PrivateKey> {
return Err(error("expected a single private key".into()));
}

Ok(rustls::PrivateKey(keys[0].clone()))
// TODO: should PKCS#8 be supported?
Ok(PrivateKeyDer::Pkcs1(keys[0].clone().into()))
}
8 changes: 6 additions & 2 deletions src/acceptor/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ use std::sync::Arc;
use hyper::server::conn::AddrIncoming;
use rustls::ServerConfig;

#[cfg(feature = "ring")]
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these ring guards make sense.

Copy link
Author

Choose a reason for hiding this comment

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

warning: unused imports: `CertificateDer`, `PrivateKeyDer`
 --> src\acceptor\builder.rs:4:17
  |
4 | use pki_types::{CertificateDer, PrivateKeyDer};
  |                 ^^^^^^^^^^^^^^  ^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

This makes sense if ring is not part of the features

Copy link
Author

Choose a reason for hiding this comment

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

Maybe I should just directly reference the type using full crate path.

Copy link
Member

Choose a reason for hiding this comment

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

IMO all of the ring guards in this module don't make sense. What errors do you get if you leave them out?

Copy link
Author

Choose a reason for hiding this comment

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

IMO all of the ring guards in this module don't make sense. What errors do you get if you leave them out?

A list of errors:

error[E0412]: cannot find type `CertificateDer` in this scope
  --> F:\.cache\cargo\registry\src\index.crates.io-6f17d22bba15001f\rustls-0.22.0-alpha.3\src\server\builder.rs:71:25
   |
71 |         cert_chain: Vec<CertificateDer<'static>>,
   |                         ^^^^^^^^^^^^^^ not found in this scope
   |
help: consider importing this struct
   |
1  + use pki_types::CertificateDer;
   |

error[E0412]: cannot find type `PrivateKeyDer` in this scope
  --> F:\.cache\cargo\registry\src\index.crates.io-6f17d22bba15001f\rustls-0.22.0-alpha.3\src\server\builder.rs:72:18
   |
72 |         key_der: PrivateKeyDer<'static>,
   |                  ^^^^^^^^^^^^^ not found in this scope
   |
help: consider importing this enum
   |
1  + use pki_types::PrivateKeyDer;
   |

error[E0412]: cannot find type `Error` in this scope
  --> F:\.cache\cargo\registry\src\index.crates.io-6f17d22bba15001f\rustls-0.22.0-alpha.3\src\server\builder.rs:73:31
   |
73 |     ) -> Result<ServerConfig, Error> {
   |                               ^^^^^ not found in this scope
   |
help: consider importing one of these items
   |
1  + use alloc::fmt::Error;
   |
1  + use core::error::Error;
   |
1  + use core::fmt::Error;
   |
1  + use crate::Error;
   |
     and 4 other candidates

error[E0412]: cannot find type `CertificateDer` in this scope
  --> F:\.cache\cargo\registry\src\index.crates.io-6f17d22bba15001f\rustls-0.22.0-alpha.3\src\server\builder.rs:89:25
   |
89 |         cert_chain: Vec<CertificateDer<'static>>,
   |                         ^^^^^^^^^^^^^^ not found in this scope
   |
help: consider importing this struct
   |
1  + use pki_types::CertificateDer;
   |

error[E0412]: cannot find type `PrivateKeyDer` in this scope
  --> F:\.cache\cargo\registry\src\index.crates.io-6f17d22bba15001f\rustls-0.22.0-alpha.3\src\server\builder.rs:90:18
   |
90 |         key_der: PrivateKeyDer<'static>,
   |                  ^^^^^^^^^^^^^ not found in this scope
   |
help: consider importing this enum
   |
1  + use pki_types::PrivateKeyDer;
   |

error[E0412]: cannot find type `Error` in this scope
  --> F:\.cache\cargo\registry\src\index.crates.io-6f17d22bba15001f\rustls-0.22.0-alpha.3\src\server\builder.rs:92:31
   |
92 |     ) -> Result<ServerConfig, Error> {
   |                               ^^^^^ not found in this scope
   |
help: consider importing one of these items
   |
1  + use alloc::fmt::Error;
   |
1  + use core::error::Error;
   |
1  + use core::fmt::Error;
   |
1  + use crate::Error;
   |
     and 4 other candidates

Copy link
Author

@stevefan1999-personal stevefan1999-personal Nov 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh I reckon, this is the problem:

    #[cfg(feature = "ring")]
    /// Create a builder for a client configuration with the default
    /// [`CryptoProvider`]: [`crate::crypto::ring::RING`].
    ///
    /// For more information, see the [`ConfigBuilder`] documentation.
    pub fn builder() -> ConfigBuilder<Self, WantsCipherSuites> {
        Self::builder_with_provider(crate::crypto::ring::RING)
    }

Should we break the ABI instead (i.e. rename this to builder_with_ring)

Copy link
Author

Choose a reason for hiding this comment

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

Yep, guess that would do. But I think the bigger problem is that rustls::ClientConfig was not liberal enough as a builder...

Copy link
Author

@stevefan1999-personal stevefan1999-personal Nov 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe we should further remove ring as part of the default features...in exchange for a chaos on the crates that depends on hyper-rustls, but this would definitely promote a good use for custom crypto suites provider.

Copy link
Member

@djc djc Nov 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No, we should mirror the API as it's setup in rustls alpha.4, which uses builder() with ring as a default and offers an alternative option to select a different provider.

(And note that most of those errors have nothing to do with ring, which is why they shouldn't be guarded with it.)

Copy link
Author

Choose a reason for hiding this comment

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

Acknowledged. Maybe this is the best we could do for now.

use pki_types::{CertificateDer, PrivateKeyDer};

use super::TlsAcceptor;
/// Builder for [`TlsAcceptor`]
pub struct AcceptorBuilder<State>(State);
Expand All @@ -25,10 +28,11 @@ impl AcceptorBuilder<WantsTlsConfig> {
///
/// [with_safe_defaults]: rustls::ConfigBuilder::with_safe_defaults
/// [with_no_client_auth]: rustls::ConfigBuilder::with_no_client_auth
#[cfg(feature = "ring")]
pub fn with_single_cert(
self,
cert_chain: Vec<rustls::Certificate>,
key_der: rustls::PrivateKey,
cert_chain: Vec<CertificateDer<'static>>,
key_der: PrivateKeyDer<'static>,
) -> Result<AcceptorBuilder<WantsAlpn>, rustls::Error> {
Ok(AcceptorBuilder(WantsAlpn(
ServerConfig::builder()
Expand Down
31 changes: 12 additions & 19 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustls::client::WantsTransparencyPolicyOrClientCert;
use rustls::client::WantsClientCert;
use rustls::{ClientConfig, ConfigBuilder, WantsVerifier};

/// Methods for configuring roots
Expand All @@ -8,30 +8,29 @@ use rustls::{ClientConfig, ConfigBuilder, WantsVerifier};
pub trait ConfigBuilderExt {
/// This configures the platform's trusted certs, as implemented by
/// rustls-native-certs
#[cfg(feature = "rustls-native-certs")]
#[cfg(all(feature = "rustls-native-certs", feature = "ring"))]
#[cfg_attr(docsrs, doc(cfg(feature = "rustls-native-certs")))]
fn with_native_roots(self) -> ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert>;
fn with_native_roots(self) -> ConfigBuilder<ClientConfig, WantsClientCert>;

/// This configures the webpki roots, which are Mozilla's set of
/// trusted roots as packaged by webpki-roots.
#[cfg(feature = "webpki-roots")]
#[cfg(all(feature = "webpki-roots", feature = "ring"))]
#[cfg_attr(docsrs, doc(cfg(feature = "webpki-roots")))]
fn with_webpki_roots(self) -> ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert>;
fn with_webpki_roots(self) -> ConfigBuilder<ClientConfig, WantsClientCert>;
}

impl ConfigBuilderExt for ConfigBuilder<ClientConfig, WantsVerifier> {
#[cfg(feature = "rustls-native-certs")]
#[cfg(all(feature = "rustls-native-certs", feature = "ring"))]
#[cfg_attr(docsrs, doc(cfg(feature = "rustls-native-certs")))]
#[cfg_attr(not(feature = "logging"), allow(unused_variables))]
fn with_native_roots(self) -> ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert> {
fn with_native_roots(self) -> ConfigBuilder<ClientConfig, WantsClientCert> {
let mut roots = rustls::RootCertStore::empty();
let mut valid_count = 0;
let mut invalid_count = 0;

for cert in rustls_native_certs::load_native_certs().expect("could not load platform certs")
{
let cert = rustls::Certificate(cert.0);
match roots.add(&cert) {
match roots.add(cert.0.clone().into()) {
Ok(_) => valid_count += 1,
Err(err) => {
crate::log::trace!("invalid cert der {:?}", cert.0);
Expand All @@ -50,20 +49,14 @@ impl ConfigBuilderExt for ConfigBuilder<ClientConfig, WantsVerifier> {
self.with_root_certificates(roots)
}

#[cfg(feature = "webpki-roots")]
#[cfg(all(feature = "webpki-roots", feature = "ring"))]
#[cfg_attr(docsrs, doc(cfg(feature = "webpki-roots")))]
fn with_webpki_roots(self) -> ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert> {
fn with_webpki_roots(self) -> ConfigBuilder<ClientConfig, WantsClientCert> {
let mut roots = rustls::RootCertStore::empty();
roots.add_trust_anchors(
roots.extend(
webpki_roots::TLS_SERVER_ROOTS
.iter()
.map(|ta| {
rustls::OwnedTrustAnchor::from_subject_spki_name_constraints(
ta.subject,
ta.spki,
ta.name_constraints,
)
}),
.cloned(),
);
self.with_root_certificates(roots)
}
Expand Down
7 changes: 5 additions & 2 deletions src/connector/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use rustls::ClientConfig;

use super::HttpsConnector;
#[cfg(any(feature = "rustls-native-certs", feature = "webpki-roots"))]
#[cfg(all(
any(feature = "rustls-native-certs", feature = "webpki-roots"),
feature = "ring"
))]
use crate::config::ConfigBuilderExt;

#[cfg(feature = "tokio-runtime")]
Expand Down Expand Up @@ -57,7 +60,7 @@ impl ConnectorBuilder<WantsTlsConfig> {
/// See [`ConfigBuilderExt::with_native_roots`]
///
/// [with_safe_defaults]: rustls::ConfigBuilder::with_safe_defaults
#[cfg(feature = "rustls-native-certs")]
#[cfg(all(feature = "rustls-native-certs", feature = "ring"))]
#[cfg_attr(docsrs, doc(cfg(feature = "rustls-native-certs")))]
pub fn with_native_roots(self) -> ConnectorBuilder<WantsSchemes> {
self.with_tls_config(
Expand Down
6 changes: 4 additions & 2 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@
//!
//! // Load and return certificate.
//! let certs = rustls_pemfile::certs(&mut reader).unwrap();
//! let certs = certs.into_iter().map(rustls::Certificate).collect();
//! let certs = certs.into_iter().map(Into::into).collect();
//!
//! // Load private key. (see `examples/server.rs`)
//! let keyfile = File::open("examples/sample.rsa").unwrap();
//! let mut reader = io::BufReader::new(keyfile);
//!
//! // Load and return a single private key.
//! let keys = rustls_pemfile::rsa_private_keys(&mut reader).unwrap();
//! let key = rustls::PrivateKey(keys[0].clone());
//! let key = pki_types::PrivateKeyDer::Pkcs1(keys[0].clone().into());
//! let https = hyper_rustls::HttpsConnectorBuilder::new()
//! .with_native_roots()
//! .https_only()
Expand All @@ -82,6 +82,7 @@
#[cfg(feature = "acceptor")]
/// TLS acceptor implementing hyper's `Accept` trait.
pub mod acceptor;
#[cfg(feature = "ring")]
mod config;
mod connector;
mod stream;
Expand All @@ -100,6 +101,7 @@ mod log {

#[cfg(feature = "acceptor")]
pub use crate::acceptor::{AcceptorBuilder, TlsAcceptor};
#[cfg(feature = "ring")]
pub use crate::config::ConfigBuilderExt;
pub use crate::connector::builder::ConnectorBuilder as HttpsConnectorBuilder;
pub use crate::connector::HttpsConnector;
Expand Down