From 85587359fc28a836ec5a053957c98208b084570e Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 1 Sep 2023 11:24:28 -0700 Subject: [PATCH 1/6] use `rustls-webpki` instead of `linkerd/webpki` This commit changes the `linkerd-meshtls-rustls` crate to use the upstream `rustls-webpki` crate, maintained by Rustls, rather than our fork of `briansmith/webpki` from GitHub. Since `rustls-webpki` includes the change which was the initial motivation for the `linkerd/webpki` fork (rustls/webpki#42), we can now depend on upstream. --- Cargo.lock | 17 ++++++++++++++--- Cargo.toml | 3 --- linkerd/meshtls/rustls/Cargo.toml | 2 +- linkerd/meshtls/rustls/src/creds/store.rs | 10 ++++++---- linkerd/meshtls/rustls/src/server.rs | 17 +++++++---------- 5 files changed, 28 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e8b689e3f7..bb42bcb045 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1408,11 +1408,11 @@ dependencies = [ "linkerd-tls-test-util", "ring", "rustls-pemfile", + "rustls-webpki", "thiserror", "tokio", "tokio-rustls", "tracing", - "webpki", ] [[package]] @@ -2462,6 +2462,16 @@ dependencies = [ "base64", ] +[[package]] +name = "rustls-webpki" +version = "0.101.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d93931baf2d282fff8d3a532bbfd7653f734643161b87e3e01e59a04439bf0d" +dependencies = [ + "ring", + "untrusted", +] + [[package]] name = "rustversion" version = "1.0.11" @@ -3149,8 +3159,9 @@ dependencies = [ [[package]] name = "webpki" -version = "0.22.0" -source = "git+https://github.com/linkerd/webpki?branch=cert-dns-names-0.22#a26def03ec88d3b69542ccd2f0073369ecedc4f9" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0e74f82d49d545ad128049b7e88f6576df2da6b02e9ce565c6f533be576957e" dependencies = [ "ring", "untrusted", diff --git a/Cargo.toml b/Cargo.toml index dd2bdac671..b96f0e90ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,6 +80,3 @@ debug = false [profile.release] lto = true - -[patch.crates-io] -webpki = { git = "https://github.com/linkerd/webpki", branch = "cert-dns-names-0.22" } diff --git a/linkerd/meshtls/rustls/Cargo.toml b/linkerd/meshtls/rustls/Cargo.toml index 977f4cf899..0c012ed875 100644 --- a/linkerd/meshtls/rustls/Cargo.toml +++ b/linkerd/meshtls/rustls/Cargo.toml @@ -19,11 +19,11 @@ linkerd-tls = { path = "../../tls" } linkerd-tls-test-util = { path = "../../tls/test-util", optional = true } ring = { version = "0.16", features = ["std"] } rustls-pemfile = "1.0" +rustls-webpki = { version = "0.101.4", features = [ "std"] } thiserror = "1" tokio = { version = "1", features = ["macros", "rt", "sync"] } tokio-rustls = { version = "0.23", features = ["dangerous_configuration"] } tracing = "0.1" -webpki = "0.22" [dev-dependencies] linkerd-tls-test-util = { path = "../../tls/test-util" } diff --git a/linkerd/meshtls/rustls/src/creds/store.rs b/linkerd/meshtls/rustls/src/creds/store.rs index d744bbf20e..864222732b 100644 --- a/linkerd/meshtls/rustls/src/creds/store.rs +++ b/linkerd/meshtls/rustls/src/creds/store.rs @@ -239,9 +239,11 @@ impl rustls::server::ResolvesServerCert for CertResolver { hello: rustls::server::ClientHello<'_>, ) -> Option> { let server_name = match hello.server_name() { - Some(name) => webpki::DnsNameRef::try_from_ascii_str(name) - .expect("server name must be a valid server name"), - + Some(name) => { + let name = webpki::DnsNameRef::try_from_ascii_str(name) + .expect("server name must be a valid server name"); + webpki::SubjectNameRef::DnsName(name) + } None => { debug!("no SNI -> no certificate"); return None; @@ -251,7 +253,7 @@ impl rustls::server::ResolvesServerCert for CertResolver { // Verify that our certificate is valid for the given SNI name. let c = self.0.cert.first()?; if let Err(error) = webpki::EndEntityCert::try_from(c.as_ref()) - .and_then(|c| c.verify_is_valid_for_dns_name(server_name)) + .and_then(|c| c.verify_is_valid_for_subject_name(server_name)) { debug!(%error, "Local certificate is not valid for SNI"); return None; diff --git a/linkerd/meshtls/rustls/src/server.rs b/linkerd/meshtls/rustls/src/server.rs index 2cdcb6a381..df2b20b5f7 100644 --- a/linkerd/meshtls/rustls/src/server.rs +++ b/linkerd/meshtls/rustls/src/server.rs @@ -130,18 +130,15 @@ fn client_identity(tls: &tokio_rustls::server::TlsStream) -> Option { - let s: &str = (*n).into(); - s.parse().ok().map(ClientId) - } - webpki::GeneralDnsNameRef::Wildcard(_) => { - // Wildcards can perhaps be handled in a future path... - None - } + let name: &str = dns_names.next()?.into(); + if name == "*" { + // Wildcards can perhaps be handled in a future path... + return None; } + + name.parse().ok().map(ClientId) } // === impl ServerIo === From df23b6a6d2feb352f3bfd8db0d34a08b23a23999 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Fri, 8 Sep 2023 09:39:47 -0700 Subject: [PATCH 2/6] update to rustls-webpki v0.101.5 This picks up the upstream fix for rustls/webpki#167 --- Cargo.lock | 5 ++--- Cargo.toml | 3 +++ linkerd/meshtls/rustls/Cargo.toml | 2 +- linkerd/meshtls/rustls/src/server.rs | 7 +++++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bb42bcb045..0261074dbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2464,9 +2464,8 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.101.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d93931baf2d282fff8d3a532bbfd7653f734643161b87e3e01e59a04439bf0d" +version = "0.101.5" +source = "git+https://github.com/cpu/webpki?branch=cpu-0.101.5-prep#702d57f444e3f7d743277524e832a2363290ec4d" dependencies = [ "ring", "untrusted", diff --git a/Cargo.toml b/Cargo.toml index b96f0e90ed..ecdc85da37 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,3 +80,6 @@ debug = false [profile.release] lto = true + +[patch.crates-io] +rustls-webpki = { git = "https://github.com/cpu/webpki", branch = "cpu-0.101.5-prep" } diff --git a/linkerd/meshtls/rustls/Cargo.toml b/linkerd/meshtls/rustls/Cargo.toml index 0c012ed875..7a01e3e5ba 100644 --- a/linkerd/meshtls/rustls/Cargo.toml +++ b/linkerd/meshtls/rustls/Cargo.toml @@ -19,7 +19,7 @@ linkerd-tls = { path = "../../tls" } linkerd-tls-test-util = { path = "../../tls/test-util", optional = true } ring = { version = "0.16", features = ["std"] } rustls-pemfile = "1.0" -rustls-webpki = { version = "0.101.4", features = [ "std"] } +rustls-webpki = { version = "0.101.5", features = [ "std"] } thiserror = "1" tokio = { version = "1", features = ["macros", "rt", "sync"] } tokio-rustls = { version = "0.23", features = ["dangerous_configuration"] } diff --git a/linkerd/meshtls/rustls/src/server.rs b/linkerd/meshtls/rustls/src/server.rs index df2b20b5f7..2f54f47f86 100644 --- a/linkerd/meshtls/rustls/src/server.rs +++ b/linkerd/meshtls/rustls/src/server.rs @@ -130,9 +130,12 @@ fn client_identity(tls: &tokio_rustls::server::TlsStream) -> Option Date: Mon, 11 Sep 2023 09:22:01 -0700 Subject: [PATCH 3/6] pin git dep to revision --- Cargo.lock | 2 +- Cargo.toml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0261074dbe..d9d519ba7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2465,7 +2465,7 @@ dependencies = [ [[package]] name = "rustls-webpki" version = "0.101.5" -source = "git+https://github.com/cpu/webpki?branch=cpu-0.101.5-prep#702d57f444e3f7d743277524e832a2363290ec4d" +source = "git+https://github.com/cpu/webpki?rev=702d57f444e3f7d743277524e832a2363290ec4d#702d57f444e3f7d743277524e832a2363290ec4d" dependencies = [ "ring", "untrusted", diff --git a/Cargo.toml b/Cargo.toml index ecdc85da37..54562b79d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -82,4 +82,5 @@ debug = false lto = true [patch.crates-io] -rustls-webpki = { git = "https://github.com/cpu/webpki", branch = "cpu-0.101.5-prep" } +# remove this patch when https://github.com/rustls/webpki/pull/170 is published! +rustls-webpki = { git = "https://github.com/cpu/webpki", rev = "702d57f444e3f7d743277524e832a2363290ec4d" } From 2f1f99ccefd6ab029b5536c6811840c500f752bb Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 11 Sep 2023 09:22:10 -0700 Subject: [PATCH 4/6] update deny.toml to allow git dep --- deny.toml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/deny.toml b/deny.toml index 940b98e48a..893de038a9 100644 --- a/deny.toml +++ b/deny.toml @@ -72,8 +72,7 @@ skip-tree = [ unknown-registry = "deny" unknown-git = "deny" allow-registry = ["https://github.com/rust-lang/crates.io-index"] - -[sources.allow-org] -github = [ - "linkerd", +allow-git = [ + # remove this when https://github.com/rustls/webpki/pull/170 is published! + "https://github.com/cpu/webpki", ] From d9af08828c58ce4d5ae531e04027b154cdd43b03 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 11 Sep 2023 10:23:45 -0700 Subject: [PATCH 5/6] remove debugging printlns --- my bad! --- linkerd/meshtls/rustls/src/server.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/linkerd/meshtls/rustls/src/server.rs b/linkerd/meshtls/rustls/src/server.rs index 2f54f47f86..43b0e63652 100644 --- a/linkerd/meshtls/rustls/src/server.rs +++ b/linkerd/meshtls/rustls/src/server.rs @@ -130,12 +130,7 @@ fn client_identity(tls: &tokio_rustls::server::TlsStream) -> Option Date: Mon, 11 Sep 2023 10:30:17 -0700 Subject: [PATCH 6/6] meshtls: log errors parsing client certs Currently, if errors occur while parsing a client identity from a TLS certificate, the `client_identity` function in `linkerd-meshtls-rustls` will simply discard the error and return `None`. This means that we cannot easily determine *why* a connection has no client identity --- there may have been no client cert, but we may also have failed to parse a client cert that was present. In order to make debugging these issues a little easier, I've changed this function to log any errors returned by `rustls-webpki` while parsing client certs. --- linkerd/meshtls/rustls/src/server.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/linkerd/meshtls/rustls/src/server.rs b/linkerd/meshtls/rustls/src/server.rs index 43b0e63652..1b98216634 100644 --- a/linkerd/meshtls/rustls/src/server.rs +++ b/linkerd/meshtls/rustls/src/server.rs @@ -129,14 +129,26 @@ fn client_identity(tls: &tokio_rustls::server::TlsStream) -> Option