Closed reyk closed 4 years ago
Thanks for opening @reyk. What OS are you running?
There seem to be differences in how native-tls works (or maybe more accurately how schannel and openssl is configured) across Windows, macOS and Linux (Ubuntu) which are captured in cert.rs.
#[cfg(feature = "native-tls-crate")]
pub(crate) fn add_to_native_tls(self, tls: &mut native_tls_crate::TlsConnectorBuilder) {
tls.add_root_certificate(self.native);
}
#[cfg(feature = "rustls-tls")]
pub(crate) fn add_to_rustls(self, tls: &mut rustls::ClientConfig) -> crate::Result<()> {
use rustls::internal::pemfile;
use std::io::Cursor;
match self.original {
Cert::Der(buf) => tls
.root_store
.add(&::rustls::Certificate(buf))
.map_err(|e| crate::error::builder(TLSError::WebPKIError(e)))?,
Cert::Pem(buf) => {
let mut pem = Cursor::new(buf);
let certs = pemfile::certs(&mut pem).map_err(|_| {
crate::error::builder(TLSError::General(String::from(
"No valid certificate was found",
)))
})?;
for c in certs {
tls.root_store
.add(&c)
.map_err(|e| crate::error::builder(TLSError::WebPKIError(e)))?;
}
}
}
Ok(())
}
I'm wondering if this might be an issue to fix upstream?
I‘m using native-tls with OpenSSL (or LibreSSL) under Linux and OpenBSD.
It currently doesn’t split the CA bundle and just loads a single cert from it. Maybe this could eventually fix it one day: https://github.com/sfackler/rust-native-tls/pull/168/
You can also call reqwest’s add_root_certificate() multiple times as it pushes each new cert to a Vec internally. I’m using a workaround of CertificateValidation::Full(Vec<Certificate>)
right now which allows me to split the PEM in advance and to call that method for each cert in it.
I wish there would be a better option but I didn’t see one without patching elasticsearch
or native-tls
(which is far more intrusive, so I opted for the latter).
It currently doesn’t split the CA bundle and just loads a single cert from it. Maybe this could eventually fix it one day: sfackler/rust-native-tls#168
That looks promising, thanks for the link!
In the meantime though, I don't see harm in splitting a PEM formatted file and calling add_root_certificate()
multiple times internally when native-tls feature is used. Is this something you'd be interested in contributing, @reyk? If not, it'd probably be later next week that I can get round to looking at it.
The problem is that you currently embed a single reqwest Certificate in the CertificateValidation. We could fix that by turning it into a Vec as described or by turning it into a newtype. Would you be interested in a change that breaks the current API or should it be done with a new, additional variant?
ah, yes. Hmm.
I'm fine with breaking the API at this stage as it's still an alpha release. I'm thinking that we should introduce our own type that accepts &[u8]
in either PEM or DER format and handle splitting PEM inside of that, internally holding a Vec<Certificate>
. I think it's fairly trivial for the library to take care of this and not force users to have to do the splitting themselves. What do you think?
See PR #101
Describe the bug Using a CA pem cert
CertificateValidation::Full
only supports a single certificate and breaks with CAs that require an intermediate CA.To Reproduce Steps to reproduce the behavior:
elasticsearch
client to itExpected behavior Either accept an array of certs or split the PEM file into individual certs and call the underlying reqwest method multiple times.
Environment (please complete the following information): native-tls