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

Support configuring a CA certificates bundle #290

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support configuring a CA certificates bundle #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 25 additions & 8 deletions src/client/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub struct Config {
#[derive(Clone, Debug)]
pub(crate) enum TrustConfig {
CaCertificateLocation(PathBuf),
CaCertificateBundle(Vec<u8>),
TrustAll,
Default,
}
Expand Down Expand Up @@ -127,12 +128,12 @@ impl Config {
/// storage (or use `trust_cert_ca` instead), using this setting is potentially dangerous.
///
/// # Panics
/// Will panic in case `trust_cert_ca` was called before.
/// Will panic in case `trust_cert_ca` or `trust_cert_ca_bundle` was called before.
///
/// - Defaults to `default`, meaning server certificate is validated against system-truststore.
/// - Defaults to validating the server certificate is validated against system's certificate storage.
pub fn trust_cert(&mut self) {
if let TrustConfig::CaCertificateLocation(_) = &self.trust {
panic!("'trust_cert' and 'trust_cert_ca' are mutual exclusive! Only use one.")
if !matches!(&self.trust, TrustConfig::Default) {
panic!("'trust_cert'/'trust_cert_ca'/'trust_cert_ca_bundle' are mutual exclusive! Only use one.")
}
self.trust = TrustConfig::TrustAll;
}
Expand All @@ -143,14 +144,30 @@ impl Config {
/// trust-chain.
///
/// # Panics
/// Will panic in case `trust_cert` was called before.
/// Will panic in case `trust_cert` or `trust_cert_ca_bundle` was called before.
///
/// - Defaults to validating the server certificate is validated against system's certificate storage.
pub fn trust_cert_ca(&mut self, path: impl ToString) {
if let TrustConfig::TrustAll = &self.trust {
panic!("'trust_cert' and 'trust_cert_ca' are mutual exclusive! Only use one.")
if !matches!(&self.trust, TrustConfig::Default) {
panic!("'trust_cert'/'trust_cert_ca'/'trust_cert_ca_bundle' are mutual exclusive! Only use one.")
} else {
self.trust = TrustConfig::CaCertificateLocation(PathBuf::from(path.to_string()));
}
}

/// If set, the server certificate will be validated against the given bundle of PEM-encoded CA certificates.
/// Useful when using self-signed certificates on the server without having to disable the
/// trust-chain.
///
/// # Panics
/// Will panic in case `trust_cert` or `trust_cert_ca` was called before.
///
/// - Defaults to validating the server certificate is validated against system's certificate storage.
pub fn trust_cert_ca_bundle(&mut self, bundle: Vec<u8>) {
if !matches!(&self.trust, TrustConfig::Default) {
panic!("'trust_cert'/'trust_cert_ca'/'trust_cert_ca_bundle' are mutual exclusive! Only use one.")
} else {
self.trust = TrustConfig::CaCertificateLocation(PathBuf::from(path.to_string()))
self.trust = TrustConfig::CaCertificateBundle(bundle);
}
}

Expand Down
27 changes: 27 additions & 0 deletions src/client/tls_stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,30 @@ pub(crate) async fn create_tls_stream<S: AsyncRead + AsyncWrite + Unpin + Send>(
) -> crate::Result<TlsStream<S>> {
opentls_tls_stream::create_tls_stream(config, stream).await
}

#[cfg(any(feature = "native-tls", feature = "vendored-openssl"))]
mod iter_certs {
const BEGIN: &[u8] = b"-----BEGIN";
pub struct IterCertBundle<'a> {
bundle: &'a [u8],
current_begin: Option<usize>,
}
impl<'a> IterCertBundle<'a> {
pub fn new(bundle: &'a [u8]) -> Self {
Self {
bundle,
current_begin: bundle.windows(BEGIN.len()).position(|x| x == BEGIN),
}
}
}
impl<'a> Iterator for IterCertBundle<'a> {
type Item = &'a [u8];
fn next(&mut self) -> Option<&'a [u8]> {
self.current_begin.map(|begin| {
let next_begin = self.bundle.windows(BEGIN.len()).skip(begin + 1).position(|x| x == BEGIN).map(|x| x + begin + 1);
self.current_begin = next_begin;
&self.bundle[begin..next_begin.unwrap_or(self.bundle.len() - 1)]
})
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good, but I think it ignores the markers for the end of the certificate. Have you tested this outside of this repo? Do you think a test case in this repo would be doable, to confirm that this doesn't break?

Copy link
Author

Choose a reason for hiding this comment

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

I think it ignores the markers for the end of the certificate

I'm not sure I understand. In any valid PEM bundle, each certificate is terminated before the next one begins. The only thing this code does is splitting a bundle of concatenated PEM structures into individual PEM structures. The PEM format explicitly allows arbitrary data (which parsers must skip over) before the BEGIN and after the END marker, so cleanly terminating after the END marker is not a requirement.

Have you tested this outside of this repo

We're using this code in production.

Do you think a test case in this repo would be doable

Sure. Do you think it would be sufficient to just have a small unit test that loads an arbitrary bundle using include_str!() and verifies that the expected number of certificates is produced? Or did you have something else in mind?

}
7 changes: 7 additions & 0 deletions src/client/tls_stream/native_tls_stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use futures_util::io::{AsyncRead, AsyncWrite};
use std::fs;
use tracing::{event, Level};

use super::iter_certs::IterCertBundle;

pub(crate) async fn create_tls_stream<S: AsyncRead + AsyncWrite + Unpin + Send>(
config: &Config,
stream: S,
Expand Down Expand Up @@ -41,6 +43,11 @@ pub(crate) async fn create_tls_stream<S: AsyncRead + AsyncWrite + Unpin + Send>(
});
}
}
TrustConfig::CaCertificateBundle(bundle) => {
for cert in IterCertBundle::new(bundle) {
builder = builder.add_root_certificate(Certificate::from_pem(cert)?);
}
}
TrustConfig::TrustAll => {
event!(
Level::WARN,
Expand Down
7 changes: 7 additions & 0 deletions src/client/tls_stream/opentls_tls_stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use opentls::Certificate;
use std::fs;
use tracing::{event, Level};

use super::iter_certs::IterCertBundle;

pub(crate) async fn create_tls_stream<S: AsyncRead + AsyncWrite + Unpin + Send>(
config: &Config,
stream: S,
Expand Down Expand Up @@ -41,6 +43,11 @@ pub(crate) async fn create_tls_stream<S: AsyncRead + AsyncWrite + Unpin + Send>(
});
}
}
TrustConfig::CaCertificateBundle(bundle) => {
for cert in IterCertBundle::new(bundle) {
builder = builder.add_root_certificate(Certificate::from_pem(cert)?);
}
}
TrustConfig::TrustAll => {
event!(
Level::WARN,
Expand Down
10 changes: 10 additions & 0 deletions src/client/tls_stream/rustls_tls_stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,16 @@ impl<S: AsyncRead + AsyncWrite + Unpin + Send> TlsStream<S> {
});
}
}
TrustConfig::CaCertificateBundle(bundle) => {
let mut cert_store = RootCertStore::empty();
let certs = rustls_pemfile::certs(&mut bundle.as_slice())?;
for cert in certs {
cert_store.add(&Certificate(cert))?;
}
builder
.with_root_certificates(cert_store)
.with_no_client_auth()
}
TrustConfig::TrustAll => {
event!(
Level::WARN,
Expand Down