Skip to content

Commit

Permalink
Bump rustls version to 0.23
Browse files Browse the repository at this point in the history
We were running into symbol issues with older rustls versions, see briansmith/ring#1808.

Note that the new rustls version requires nasm to be installed on windows, so we install it in CI.

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
  • Loading branch information
bwoebi committed Jul 5, 2024
1 parent 6f91123 commit c229c13
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 34 deletions.
7 changes: 7 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ jobs:
rust_version: "${RUST_VERSION}"
fail-fast: false
steps:
- name: "(Windows) Install nasm"
if: matrix.platform == 'windows-latest'
uses: ilammy/setup-nasm@v1

- name: Checkout sources
uses: actions/checkout@v4

Expand Down Expand Up @@ -79,6 +83,9 @@ jobs:
- platform: "ubuntu-latest"
rust_version: "${RUST_VERSION}"
steps:
- name: "(Windows) Install nasm"
if: matrix.platform == 'windows-latest'
uses: ilammy/setup-nasm@v1

- name: Checkout sources
uses: actions/checkout@v4
Expand Down
7 changes: 4 additions & 3 deletions ddcommon/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ hyper = { version = "0.14", features = [
"tcp",
"stream",
], default-features = false }
hyper-rustls = { version = "0.23", default-features = false, features = [
hyper-util = "0.1.3"
hyper-rustls = { version = "0.27", default-features = false, features = [
"native-tokio",
"http1",
"tls12",
Expand All @@ -33,10 +34,10 @@ lazy_static = "1.4"
log = { version = "0.4" }
pin-project = "1"
regex = "1.5"
rustls = { version = "0.20.4", default-features = false }
rustls = { version = "0.23", default-features = false }
rustls-native-certs = { version = "0.6" }
tokio = { version = "1.23", features = ["rt", "macros"] }
tokio-rustls = { version = "0.23" }
tokio-rustls = { version = "0.26" }
serde = { version = "1.0", features = ["derive"] }
static_assertions = "1.1.0"

Expand Down
39 changes: 22 additions & 17 deletions ddcommon/src/connector/conn_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub enum ConnStream {
},
Tls {
#[pin]
transport: Box<tokio_rustls::client::TlsStream<tokio::net::TcpStream>>,
transport: Box<tokio_rustls::client::TlsStream<TokioIo<TokioIo<tokio::net::TcpStream>>>>,
},
#[cfg(unix)]
Udp {
Expand All @@ -37,6 +37,8 @@ pub enum ConnStream {
pub type ConnStreamError = Box<dyn std::error::Error + Send + Sync>;

use hyper::{client::HttpConnector, service::Service};
use hyper_util::rt::TokioIo;

impl ConnStream {
pub async fn from_uds_uri(uri: hyper::Uri) -> Result<ConnStream, ConnStreamError> {
#[cfg(unix)]
Expand Down Expand Up @@ -79,25 +81,28 @@ impl ConnStream {
}

pub fn from_https_connector_with_uri(
c: &mut HttpsConnector<HttpConnector>,
c: &mut HttpsConnector<hyper_util::client::legacy::connect::HttpConnector>,
uri: hyper::Uri,
require_tls: bool,
) -> impl Future<Output = Result<ConnStream, ConnStreamError>> {
c.call(uri).and_then(move |stream| match stream {
// move only require_tls
hyper_rustls::MaybeHttpsStream::Http(t) => {
if require_tls {
future::ready(Err(
super::errors::Error::CannotEstablishTlsConnection.into()
))
} else {
future::ready(Ok(ConnStream::Tcp { transport: t }))
c.call(uri.to_string().parse().unwrap())
.and_then(move |stream| match stream {
// move only require_tls
hyper_rustls::MaybeHttpsStream::Http(t) => {
if require_tls {
future::ready(Err(
super::errors::Error::CannotEstablishTlsConnection.into()
))
} else {
future::ready(Ok(ConnStream::Tcp {
transport: t.into_inner(),
}))
}
}
}
hyper_rustls::MaybeHttpsStream::Https(t) => future::ready(Ok(ConnStream::Tls {
transport: Box::from(t),
})),
})
hyper_rustls::MaybeHttpsStream::Https(t) => future::ready(Ok(ConnStream::Tls {
transport: Box::from(t.into_inner()),
})),
})
}
}

Expand All @@ -124,7 +129,7 @@ impl hyper::client::connect::Connection for ConnStream {
Self::Tcp { transport } => transport.connected(),
Self::Tls { transport } => {
let (tcp, _) = transport.get_ref();
tcp.connected()
tcp.inner().inner().connected()
}
#[cfg(unix)]
Self::Udp { transport: _ } => hyper::client::connect::Connected::new(),
Expand Down
11 changes: 6 additions & 5 deletions ddcommon/src/connector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use futures::{future, FutureExt};
use hyper::client::HttpConnector;

use lazy_static::lazy_static;
use rustls::pki_types::CertificateDer;
use rustls::ClientConfig;
use std::future::Future;
use std::pin::Pin;
Expand All @@ -24,7 +25,7 @@ use conn_stream::{ConnStream, ConnStreamError};
#[derive(Clone)]
pub enum Connector {
Http(hyper::client::HttpConnector),
Https(hyper_rustls::HttpsConnector<hyper::client::HttpConnector>),
Https(hyper_rustls::HttpsConnector<hyper_util::client::legacy::connect::HttpConnector>),
}

lazy_static! {
Expand Down Expand Up @@ -69,10 +70,10 @@ impl Connector {
}

fn build_https_connector(
) -> anyhow::Result<hyper_rustls::HttpsConnector<hyper::client::HttpConnector>> {
) -> anyhow::Result<hyper_rustls::HttpsConnector<hyper_util::client::legacy::connect::HttpConnector>>
{
let certs = load_root_certs()?;
let client_config = ClientConfig::builder()
.with_safe_defaults()
.with_root_certificates(certs)
.with_no_client_auth();
Ok(hyper_rustls::HttpsConnectorBuilder::new()
Expand All @@ -86,10 +87,10 @@ fn load_root_certs() -> anyhow::Result<rustls::RootCertStore> {
let mut roots = rustls::RootCertStore::empty();

for cert in rustls_native_certs::load_native_certs()? {
let cert = rustls::Certificate(cert.0);
let cert = CertificateDer::from(cert.0);

//TODO: log when invalid cert is loaded
roots.add(&cert).ok();
roots.add(cert).ok();
}
if roots.is_empty() {
return Err(errors::Error::NoValidCertifacteRootsFound.into());
Expand Down
4 changes: 3 additions & 1 deletion tools/docker/Dockerfile.build
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ RUN apk update \
pkgconf \
unzip \
bash \
clang16-libclang \
&& mkdir /usr/local/src

# Tell docker to use bash as the default
Expand Down Expand Up @@ -59,10 +60,11 @@ ENV PATH="/root/.cargo/bin:$PATH"
ARG CARGO_BUILD_INCREMENTAL
ARG CARGO_NET_RETRY
ENV CARGO_NET_RETRY="${CARGO_NET_RETRY}"
RUN cargo install cbindgen && rm -rf /root/.cargo/registry /root/.cargo/git
RUN cargo install cbindgen && cargo install bindgen-cli --locked && rm -rf /root/.cargo/registry /root/.cargo/git

FROM alpine_aws_cli as alpine_builder
COPY --from=alpine_cbindgen /root/.cargo/bin/cbindgen /usr/local/bin/cbindgen
COPY --from=alpine_cbindgen /root/.cargo/bin/bindgen /usr/local/bin/bindgen


### Cache cargo metadata between builds
Expand Down
2 changes: 1 addition & 1 deletion trace-utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ license.workspace = true
[dependencies]
anyhow = "1.0"
hyper = { version = "0.14", features = ["client", "server"] }
hyper-rustls = {version = "0.23", default-features = false, features = ["native-tokio", "http1", "tls12"]}
hyper-rustls = {version = "0.27", default-features = false, features = ["native-tokio", "http1", "tls12"]}
serde = { version = "1.0.145", features = ["derive"] }
prost = "0.11.6"
rmp-serde = "1.1.1"
Expand Down
9 changes: 2 additions & 7 deletions trace-utils/src/stats_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@

use flate2::{write::GzEncoder, Compression};
use hyper::{body::Buf, Body, Client, Method, Request, StatusCode};
use hyper_rustls::HttpsConnectorBuilder;
use log::debug;
use std::io::Write;

use datadog_trace_protobuf::pb;
use ddcommon::connector::Connector;
use ddcommon::Endpoint;

pub async fn get_stats_from_request_body(body: Body) -> anyhow::Result<pb::ClientStatsPayload> {
Expand Down Expand Up @@ -61,12 +61,7 @@ pub async fn send_stats_payload(
.header("DD-API-KEY", api_key)
.body(Body::from(data.clone()))?;

let https = HttpsConnectorBuilder::new()
.with_native_roots()
.https_only()
.enable_http1()
.build();
let client: Client<_, hyper::Body> = Client::builder().build(https);
let client: Client<_, hyper::Body> = Client::builder().build(Connector::default());
match client.request(req).await {
Ok(response) => {
if response.status() != StatusCode::ACCEPTED {
Expand Down

0 comments on commit c229c13

Please sign in to comment.