Skip to content

Commit

Permalink
tls: Avoid circular dependencies (#1334)
Browse files Browse the repository at this point in the history
The `linkerd-tls` crate has a test dependency on `linkerd-tls-rustls`,
which itself depends on `linkerd-tls`. This is an avoidable circular
dependency.

This change moves all of the TLS testdata from the `linkerd-tls-rustls`
crate to the new `linkerd-tls-test-util` crate. The `tls_accept` test is
moved from `linkerd-tls` to `linkerd-tls-rustls` so that `linkerd-tls`
no longer depends on `linkerd-tls-rustls`.
  • Loading branch information
olix0r authored Oct 26, 2021
1 parent 1a762f5 commit 968361d
Show file tree
Hide file tree
Showing 33 changed files with 115 additions and 92 deletions.
12 changes: 10 additions & 2 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,7 @@ dependencies = [
"linkerd-stack",
"linkerd-tls",
"linkerd-tls-rustls",
"linkerd-tls-test-util",
"linkerd2-proxy-api",
"pin-project",
"thiserror",
Expand Down Expand Up @@ -1376,9 +1377,7 @@ dependencies = [
"linkerd-error",
"linkerd-identity",
"linkerd-io",
"linkerd-proxy-transport",
"linkerd-stack",
"linkerd-tls-rustls",
"linkerd-tracing",
"pin-project",
"thiserror",
Expand All @@ -1393,17 +1392,26 @@ name = "linkerd-tls-rustls"
version = "0.1.0"
dependencies = [
"futures",
"linkerd-conditional",
"linkerd-identity",
"linkerd-io",
"linkerd-proxy-transport",
"linkerd-stack",
"linkerd-tls",
"linkerd-tls-test-util",
"linkerd-tracing",
"ring",
"thiserror",
"tokio",
"tokio-rustls",
"tracing",
"webpki",
]

[[package]]
name = "linkerd-tls-test-util"
version = "0.1.0"

[[package]]
name = "linkerd-tonic-watch"
version = "0.1.0"
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ members = [
"linkerd/tonic-watch",
"linkerd/tls",
"linkerd/tls/rustls",
"linkerd/tls/test-util",
"linkerd/tracing",
"linkerd/transport-header",
"linkerd/transport-metrics",
Expand Down
3 changes: 2 additions & 1 deletion linkerd/proxy/identity/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ publish = false

[features]
rustfmt = ["linkerd2-proxy-api/rustfmt"]
test-util = ["linkerd-tls-rustls/test-util"]
test-util = ["linkerd-tls-test-util", "linkerd-tls-rustls/test-util"]

[dependencies]
futures = { version = "0.3", default-features = false }
Expand All @@ -19,6 +19,7 @@ linkerd-metrics = { path = "../../metrics" }
linkerd-stack = { path = "../../stack" }
linkerd-tls = { path = "../../tls" }
linkerd-tls-rustls = { path = "../../tls/rustls" }
linkerd-tls-test-util = { path = "../../tls/test-util", optional = true }
thiserror = "1"
tokio = { version = "1", features = ["time", "sync"] }
tonic = { version = "0.5", default-features = false }
Expand Down
11 changes: 6 additions & 5 deletions linkerd/proxy/identity/src/certify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,24 +196,25 @@ impl LocalCrtKey {
}

#[cfg(feature = "test-util")]
pub fn for_test(id: &rustls::test_util::Identity) -> Self {
let crt_key = id.validate().expect("Identity must be valid");
pub fn for_test(id: &linkerd_tls_test_util::Entity) -> Self {
let (trust_anchors, crt_key) = CrtKey::for_test(id);
let id = crt_key.id().clone();
let (tx, rx) = watch::channel(Some(crt_key));
// Prevent the receiver stream from ending.
tokio::spawn(async move {
tx.closed().await;
});
Self {
id: id.id(),
trust_anchors: id.trust_anchors(),
id,
trust_anchors,
crt_key: rx,
refreshes: Arc::new(Counter::new()),
}
}

#[cfg(feature = "test-util")]
pub fn default_for_test() -> Self {
Self::for_test(&rustls::test_util::DEFAULT_DEFAULT)
Self::for_test(&linkerd_tls_test_util::DEFAULT_DEFAULT)
}

pub async fn await_crt(mut self) -> Result<Self, LostDaemon> {
Expand Down
1 change: 1 addition & 0 deletions linkerd/stack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub use self::{
unwrap_or::UnwrapOr,
};
pub use tower::{
service_fn,
util::{future_service, FutureService, Oneshot, ServiceExt},
Service,
};
Expand Down
3 changes: 0 additions & 3 deletions linkerd/tls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,5 @@ tracing = "0.1.29"
untrusted = "0.7"

[dev-dependencies]
linkerd-tls-rustls = { path = "rustls", features = ["test-util"] }
linkerd-proxy-transport = { path = "../proxy/transport" }
linkerd-tracing = { path = "../tracing", features = ["ansi"] }
tokio = { version = "1", features = ["rt-multi-thread"] }
tower = { version = "0.4.10", default-features = false, features = ["util"] }
10 changes: 9 additions & 1 deletion linkerd/tls/rustls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,24 @@ publish = false

[features]
default = []
test-util = []
test-util = ["linkerd-tls-test-util"]

[dependencies]
futures = { version = "0.3", default-features = false }
linkerd-identity = { path = "../../identity" }
linkerd-io = { path = "../../io" }
linkerd-stack = { path = "../../stack" }
linkerd-tls = { path = ".." }
linkerd-tls-test-util = { path = "../test-util", optional = true }
ring = "0.16.19"
thiserror = "1"
tokio-rustls = "0.22"
tracing = "0.1"
webpki = "0.21"

[dev-dependencies]
linkerd-conditional = { path = "../../conditional" }
linkerd-tls-test-util = { path = "../test-util" }
linkerd-proxy-transport = { path = "../../proxy/transport" }
linkerd-tracing = { path = "../../tracing", features = ["ansi"] }
tokio = { version = "1", features = ["rt-multi-thread"] }
25 changes: 21 additions & 4 deletions linkerd/tls/rustls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

mod client;
mod server;
#[cfg(feature = "test-util")]
pub mod test_util;

pub use self::{
client::{ClientIo, Connect, ConnectFuture},
Expand Down Expand Up @@ -96,8 +94,7 @@ impl sign::Signer for Signer {
// === impl TrustAnchors ===

impl TrustAnchors {
#[cfg(feature = "test-util")]
fn empty() -> Self {
pub fn empty() -> Self {
TrustAnchors(Arc::new(ClientConfig::new()))
}

Expand Down Expand Up @@ -224,6 +221,26 @@ impl From<&'_ Crt> for id::LocalId {
// === CrtKey ===

impl CrtKey {
#[cfg(feature = "test-util")]
pub fn for_test(id: &linkerd_tls_test_util::Entity) -> (TrustAnchors, Self) {
let key = Key::from_pkcs8(id.key).expect("key must be valid");

let crt = {
let n = id.name.parse::<id::Name>().expect("name must be valid");
let der = id.crt.iter().copied().collect();
let expiry = std::time::SystemTime::now() + std::time::Duration::from_secs(60 * 60);
Crt::new(id::LocalId(n), der, vec![], expiry)
};

let anchors = {
let pem = std::str::from_utf8(id.trust_anchors).expect("utf-8");
TrustAnchors::from_pem(pem).unwrap_or_else(TrustAnchors::empty)
};

let ck = anchors.certify(key, crt).expect("Identity must be valid");
(anchors, ck)
}

pub fn name(&self) -> &id::Name {
&self.id.0
}
Expand Down
60 changes: 0 additions & 60 deletions linkerd/tls/rustls/src/test_util.rs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,51 @@

use futures::prelude::*;
use linkerd_conditional::Conditional;
use linkerd_error::Infallible;
use linkerd_identity as id;
use linkerd_io::{self as io, AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
use linkerd_proxy_transport::{
addrs::*,
listen::{Addrs, Bind, BindTcp},
ConnectTcp, Keepalive, ListenAddr,
};
use linkerd_stack::{ExtractParam, InsertParam, NewService, Param, Service};
use linkerd_stack::{
layer::Layer, service_fn, ExtractParam, InsertParam, NewService, Param, Service, ServiceExt,
};
use linkerd_tls as tls;
use linkerd_tls_rustls as rustls;
use std::{future::Future, net::SocketAddr, sync::mpsc, task, time::Duration};
use linkerd_tls_test_util as test_util;
use std::{convert::Infallible, future::Future, net::SocketAddr, sync::mpsc, task, time::Duration};
use tokio::net::TcpStream;
use tower::{
layer::Layer,
util::{service_fn, ServiceExt},
};
use tracing::instrument::Instrument;

type ServerConn<T, I> = (
(tls::ConditionalServerTls, T),
io::EitherIo<rustls::ServerIo<tls::server::DetectIo<I>>, tls::server::DetectIo<I>>,
);

fn load(id: &linkerd_tls_test_util::Entity) -> (rustls::TrustAnchors, rustls::CrtKey) {
let key = rustls::Key::from_pkcs8(id.key).expect("key must be valid");

let crt = {
let n = id.name.parse::<id::Name>().expect("name must be valid");
let der = id.crt.iter().copied().collect();
let expiry = std::time::SystemTime::now() + std::time::Duration::from_secs(60 * 60);
rustls::Crt::new(id::LocalId(n), der, vec![], expiry)
};

let anchors = {
let pem = std::str::from_utf8(id.trust_anchors).expect("utf-8");
rustls::TrustAnchors::from_pem(pem).unwrap_or_else(rustls::TrustAnchors::empty)
};

let ck = anchors.certify(key, crt).expect("Identity must be valid");
(anchors, ck)
}

#[tokio::test(flavor = "current_thread")]
async fn plaintext() {
let server_tls = rustls::test_util::FOO_NS1.validate().unwrap();
let client_tls = rustls::test_util::BAR_NS1.validate().unwrap();
let (_, server_tls) = load(&test_util::FOO_NS1);
let (_, client_tls) = load(&test_util::BAR_NS1);
let (client_result, server_result) = run_test(
client_tls,
Conditional::None(tls::NoClientTls::NotProvidedByServiceDiscovery),
Expand All @@ -58,8 +76,8 @@ async fn plaintext() {

#[tokio::test(flavor = "current_thread")]
async fn proxy_to_proxy_tls_works() {
let server_tls = rustls::test_util::FOO_NS1.validate().unwrap();
let client_tls = rustls::test_util::BAR_NS1.validate().unwrap();
let (_, server_tls) = load(&test_util::FOO_NS1);
let (_, client_tls) = load(&test_util::BAR_NS1);
let server_id = tls::ServerId(server_tls.name().clone());
let (client_result, server_result) = run_test(
client_tls.clone(),
Expand Down Expand Up @@ -89,14 +107,12 @@ async fn proxy_to_proxy_tls_works() {

#[tokio::test(flavor = "current_thread")]
async fn proxy_to_proxy_tls_pass_through_when_identity_does_not_match() {
let server_tls = rustls::test_util::FOO_NS1.validate().unwrap();
let (_, server_tls) = load(&test_util::FOO_NS1);

// Misuse the client's identity instead of the server's identity. Any
// identity other than `server_tls.server_identity` would work.
let client_tls = rustls::test_util::BAR_NS1
.validate()
.expect("valid client cert");
let sni = rustls::test_util::BAR_NS1.crt().name().clone();
let (_, client_tls) = load(&test_util::BAR_NS1);
let sni = (**client_tls.id()).clone();

let (client_result, server_result) = run_test(
client_tls,
Expand Down
6 changes: 6 additions & 0 deletions linkerd/tls/test-util/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "linkerd-tls-test-util"
version = "0.1.0"
license = "Apache-2.0"
edition = "2018"
publish = false
27 changes: 27 additions & 0 deletions linkerd/tls/test-util/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
pub struct Entity {
pub name: &'static str,
pub trust_anchors: &'static [u8],
pub crt: &'static [u8],
pub key: &'static [u8],
}

pub static DEFAULT_DEFAULT: Entity = Entity {
name: "default.default.serviceaccount.identity.linkerd.cluster.local",
trust_anchors: include_bytes!("testdata/ca1.pem"),
crt: include_bytes!("testdata/default-default-ca1/crt.der"),
key: include_bytes!("testdata/default-default-ca1/key.p8"),
};

pub static FOO_NS1: Entity = Entity {
name: "foo.ns1.serviceaccount.identity.linkerd.cluster.local",
trust_anchors: include_bytes!("testdata/ca1.pem"),
crt: include_bytes!("testdata/foo-ns1-ca1/crt.der"),
key: include_bytes!("testdata/foo-ns1-ca1/key.p8"),
};

pub static BAR_NS1: Entity = Entity {
name: "bar.ns1.serviceaccount.identity.linkerd.cluster.local",
trust_anchors: include_bytes!("testdata/ca1.pem"),
crt: include_bytes!("testdata/bar-ns1-ca1/crt.der"),
key: include_bytes!("testdata/bar-ns1-ca1/key.p8"),
};
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.

0 comments on commit 968361d

Please sign in to comment.