From a35d70b66bf8bf6362f859e0d31e802387efcbbb Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 28 Jun 2021 15:56:32 +0200 Subject: [PATCH 1/4] Remove dependency on rustls --- Cargo.toml | 10 ++-------- examples/google.rs | 8 ++++++-- src/lib.rs | 18 ++++-------------- src/macos.rs | 27 +++++++++------------------ src/unix.rs | 25 +++++++++++++++---------- src/windows.rs | 23 +++++++---------------- tests/compare_mozilla.rs | 16 ++++++++-------- tests/smoketests.rs | 10 ++++++---- 8 files changed, 57 insertions(+), 80 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6e3e51c..1e869b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,20 +11,14 @@ repository = "https://github.com/ctz/rustls-native-certs" categories = ["network-programming", "cryptography"] [dependencies] -rustls = { version = "0.19.0", optional = true } +rustls-pemfile = "0.2.1" [dev-dependencies] webpki = "0.21" webpki-roots = "0.21.1" ring = "0.16.5" untrusted = "0.7.0" - -[features] -default = ["rustls"] - -[[example]] -name = "google" -required-features = ["rustls"] +rustls = "0.19.1" [target.'cfg(windows)'.dependencies] schannel = "0.1.15" diff --git a/examples/google.rs b/examples/google.rs index e341f57..c543957 100644 --- a/examples/google.rs +++ b/examples/google.rs @@ -10,9 +10,13 @@ use webpki; use rustls::Session; fn main() { + let mut roots = rustls::RootCertStore::empty(); + for cert in rustls_native_certs::load_native_certs().expect("could not load platform certs") { + roots.add(&rustls::Certificate(cert.0)).unwrap(); + } + let mut config = rustls::ClientConfig::new(); - config.root_store = rustls_native_certs::load_native_certs() - .expect("could not load platform certs"); + config.root_store = roots; let dns_name = webpki::DNSNameRef::try_from_ascii_str("google.com") .unwrap(); diff --git a/src/lib.rs b/src/lib.rs index bf6c86d..3b439d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,19 +25,7 @@ mod macos; #[cfg(target_os = "macos")] use macos as platform; -#[cfg(feature = "rustls")] -mod rustls; - use std::io::Error; -use std::io::BufRead; - -#[cfg(feature = "rustls")] -pub use crate::rustls::{load_native_certs, PartialResult}; - -pub trait RootStoreBuilder { - fn load_der(&mut self, der: Vec) -> Result<(), Error>; - fn load_pem_file(&mut self, rd: &mut dyn BufRead) -> Result<(), Error>; -} /// Loads root certificates found in the platform's native certificate /// store, executing callbacks on the provided builder. @@ -47,6 +35,8 @@ pub trait RootStoreBuilder { /// This function can be expensive: on some platforms it involves loading /// and parsing a ~300KB disk file. It's therefore prudent to call /// this sparingly. -pub fn build_native_certs(builder: &mut B) -> Result<(), Error> { - platform::build_native_certs(builder) +pub fn load_native_certs() -> Result, Error> { + platform::load_native_certs() } + +pub struct Certificate(pub Vec); diff --git a/src/macos.rs b/src/macos.rs index 62867e9..f52f6eb 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -1,14 +1,15 @@ +use crate::Certificate; + use security_framework::trust_settings::{ Domain, TrustSettings, TrustSettingsForCertificate }; + use std::io::{Error, ErrorKind}; use std::collections::HashMap; -use crate::RootStoreBuilder; - -pub fn build_native_certs(builder: &mut B) -> Result<(), Error> { +pub fn load_native_certs() -> Result, Error> { // The various domains are designed to interact like this: // // "Per-user Trust Settings override locally administered @@ -45,26 +46,16 @@ pub fn build_native_certs(builder: &mut B) -> Result<(), Er } } - let mut first_error = None; + let mut certs = Vec::new(); // Now we have all the certificates and an idea of whether // to use them. for (der, trusted) in all_certs.drain() { - match trusted { - TrustSettingsForCertificate::TrustRoot | - TrustSettingsForCertificate::TrustAsRoot => { - if let Err(err) = builder.load_der(der) { - first_error = first_error - .or_else(|| Some(Error::new(ErrorKind::InvalidData, err))); - } - }, - _ => {} // discard + use TrustSettingsForCertificate::*; + if let TrustRoot | TrustAsRoot = trusted { + certs.push(Certificate(der)); } } - if let Some(err) = first_error { - Err(err) - } else { - Ok(()) - } + Ok(certs) } diff --git a/src/unix.rs b/src/unix.rs index 165dc2c..1a0ec46 100644 --- a/src/unix.rs +++ b/src/unix.rs @@ -1,27 +1,32 @@ -use crate::RootStoreBuilder; +use crate::Certificate; use std::io::{Error, ErrorKind}; use std::io::BufReader; use std::fs::File; use std::path::Path; -fn load_file(builder: &mut impl RootStoreBuilder, path: &Path) -> Result<(), Error> { +fn load_file(certs: &mut Vec, path: &Path) -> Result<(), Error> { let f = File::open(&path)?; let mut f = BufReader::new(f); - if builder.load_pem_file(&mut f).is_err() { - Err(Error::new(ErrorKind::InvalidData, - format!("Could not load PEM file {:?}", path))) - } else { - Ok(()) + match rustls_pemfile::certs(&mut f) { + Ok(contents) => { + certs.extend(contents.into_iter().map(Certificate)); + Ok(()) + } + Err(_) => Err(Error::new( + ErrorKind::InvalidData, + format!("Could not load PEM file {:?}", path), + )), } } -pub fn build_native_certs(builder: &mut B) -> Result<(), Error> { +pub fn load_native_certs() -> Result, Error> { let likely_locations = openssl_probe::probe(); let mut first_error = None; + let mut certs = Vec::new(); if let Some(file) = likely_locations.cert_file { - if let Err(err) = load_file(builder, &file) { + if let Err(err) = load_file(&mut certs, &file) { first_error = first_error.or(Some(err)); } } @@ -29,6 +34,6 @@ pub fn build_native_certs(builder: &mut B) -> Result<(), Er if let Some(err) = first_error { Err(err) } else { - Ok(()) + Ok(certs) } } diff --git a/src/windows.rs b/src/windows.rs index 50351a5..2360a89 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -1,6 +1,6 @@ -use crate::RootStoreBuilder; +use crate::Certificate; -use std::io::{Error, ErrorKind}; +use std::io::Error; static PKIX_SERVER_AUTH: &str = "1.3.6.1.5.5.7.3.1"; @@ -13,25 +13,16 @@ fn usable_for_rustls(uses: schannel::cert_context::ValidUses) -> bool { } } -pub fn build_native_certs(builder: &mut B) -> Result<(), Error> { - let mut first_error = None; +pub fn load_native_certs() -> Result, Error> { + let mut certs = Vec::new(); let current_user_store = schannel::cert_store::CertStore::open_current_user("ROOT")?; for cert in current_user_store.certs() { - if !usable_for_rustls(cert.valid_uses().unwrap()) { - continue; - } - - if let Err(err) = builder.load_der(cert.to_der().to_vec()) { - first_error = first_error - .or_else(|| Some(Error::new(ErrorKind::InvalidData, err))); + if usable_for_rustls(cert.valid_uses().unwrap()) { + certs.push(Certificate(cert.to_der().to_vec())); } } - if let Some(err) = first_error { - Err(err) - } else { - Ok(()) - } + Ok(certs) } diff --git a/tests/compare_mozilla.rs b/tests/compare_mozilla.rs index c3b4198..3a5cc35 100644 --- a/tests/compare_mozilla.rs +++ b/tests/compare_mozilla.rs @@ -4,8 +4,6 @@ //! as expressed by the `webpki-roots` crate. //! //! This is, obviously, quite a heuristic test. -#![cfg(feature = "rustls")] - use std::collections::HashMap; use ring::io::der; use untrusted; @@ -75,8 +73,8 @@ fn test_does_not_have_many_roots_unknown_by_mozilla() { let mut missing_in_moz_roots = 0; - for cert in &native.roots { - let cert = cert.to_trust_anchor(); + for cert in &native { + let cert = webpki::trust_anchor_util::cert_der_as_trust_anchor(&cert.0).unwrap(); if let Some(moz) = mozilla.get(cert.spki) { assert_eq!(cert.subject, moz.subject, "subjects differ for public key"); @@ -106,8 +104,9 @@ fn test_contains_most_roots_known_by_mozilla() { .unwrap(); let mut native_map = HashMap::new(); - for anchor in &native.roots { - native_map.insert(anchor.to_trust_anchor().spki.to_vec(), anchor); + for anchor in &native { + let cert = webpki::trust_anchor_util::cert_der_as_trust_anchor(&anchor.0).unwrap(); + native_map.insert(cert.spki.to_vec(), anchor); } let mut missing_in_native_roots = 0; @@ -139,7 +138,8 @@ fn util_list_certs() { let native = rustls_native_certs::load_native_certs() .unwrap(); - for (i, cert) in native.roots.iter().enumerate() { - println!("cert[{}] = {}", i, stringify_x500name(cert.to_trust_anchor().subject)); + for (i, cert) in native.iter().enumerate() { + let cert = webpki::trust_anchor_util::cert_der_as_trust_anchor(&cert.0).unwrap(); + println!("cert[{}] = {}", i, stringify_x500name(cert.subject)); } } diff --git a/tests/smoketests.rs b/tests/smoketests.rs index 3bc5124..715c346 100644 --- a/tests/smoketests.rs +++ b/tests/smoketests.rs @@ -1,5 +1,3 @@ -#![cfg(feature = "rustls")] - use std::sync::Arc; use std::net::TcpStream; @@ -10,9 +8,13 @@ use webpki; use rustls_native_certs; fn check_site(domain: &str) { + let mut roots = rustls::RootCertStore::empty(); + for cert in rustls_native_certs::load_native_certs().unwrap() { + roots.add(&rustls::Certificate(cert.0)).unwrap(); + } + let mut config = rustls::ClientConfig::new(); - config.root_store = rustls_native_certs::load_native_certs() - .unwrap(); + config.root_store = roots; let dns_name = webpki::DNSNameRef::try_from_ascii_str(domain) .unwrap(); From ec42550e1a4b1a76eaeb3d142e5e6c8ca2a0ba48 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 27 Sep 2021 10:54:32 +0200 Subject: [PATCH 2/4] Upgrade to latest webpki and rustls releases --- Cargo.toml | 6 ++--- examples/google.rs | 58 ++++++++++++++++++++++------------------ tests/compare_mozilla.rs | 8 +++--- tests/smoketests.rs | 30 ++++++++++++--------- 4 files changed, 58 insertions(+), 44 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1e869b2..067427b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,11 +14,11 @@ categories = ["network-programming", "cryptography"] rustls-pemfile = "0.2.1" [dev-dependencies] -webpki = "0.21" -webpki-roots = "0.21.1" +webpki = "0.22" +webpki-roots = "0.22" ring = "0.16.5" untrusted = "0.7.0" -rustls = "0.19.1" +rustls = "0.20" [target.'cfg(windows)'.dependencies] schannel = "0.1.15" diff --git a/examples/google.rs b/examples/google.rs index c543957..60104b4 100644 --- a/examples/google.rs +++ b/examples/google.rs @@ -1,13 +1,11 @@ +use std::convert::TryInto; use std::sync::Arc; +use std::io::{stdout, Read, Write}; use std::net::TcpStream; -use std::io::{Read, Write, stdout}; use rustls; use rustls_native_certs; -use webpki; - -use rustls::Session; fn main() { let mut roots = rustls::RootCertStore::empty(); @@ -15,29 +13,37 @@ fn main() { roots.add(&rustls::Certificate(cert.0)).unwrap(); } - let mut config = rustls::ClientConfig::new(); - config.root_store = roots; + let config = rustls::ClientConfig::builder() + .with_safe_defaults() + .with_root_certificates(roots) + .with_no_client_auth(); - let dns_name = webpki::DNSNameRef::try_from_ascii_str("google.com") - .unwrap(); - let mut sess = rustls::ClientSession::new(&Arc::new(config), dns_name); - let mut sock = TcpStream::connect("google.com:443") - .expect("cannot connect"); - let mut tls = rustls::Stream::new(&mut sess, &mut sock); - tls.write(concat!("GET / HTTP/1.1\r\n", - "Host: google.com\r\n", - "Connection: close\r\n", - "Accept-Encoding: identity\r\n", - "\r\n") - .as_bytes()) - .expect("write failed"); - let ciphersuite = tls.sess.get_negotiated_ciphersuite() + let mut conn = + rustls::ClientConnection::new(Arc::new(config), "google.com".try_into().unwrap()).unwrap(); + let mut sock = TcpStream::connect("google.com:443").expect("cannot connect"); + let mut tls = rustls::Stream::new(&mut conn, &mut sock); + tls.write( + concat!( + "GET / HTTP/1.1\r\n", + "Host: google.com\r\n", + "Connection: close\r\n", + "Accept-Encoding: identity\r\n", + "\r\n" + ) + .as_bytes(), + ) + .expect("write failed"); + let ciphersuite = tls + .conn + .negotiated_cipher_suite() .expect("tls handshake failed"); - writeln!(&mut std::io::stderr(), "Current ciphersuite: {:?}", ciphersuite.suite) - .unwrap(); + writeln!( + &mut std::io::stderr(), + "Current ciphersuite: {:?}", + ciphersuite.suite() + ) + .unwrap(); let mut plaintext = Vec::new(); - tls.read_to_end(&mut plaintext) - .unwrap(); - stdout().write_all(&plaintext) - .unwrap(); + tls.read_to_end(&mut plaintext).unwrap(); + stdout().write_all(&plaintext).unwrap(); } diff --git a/tests/compare_mozilla.rs b/tests/compare_mozilla.rs index 3a5cc35..31d599e 100644 --- a/tests/compare_mozilla.rs +++ b/tests/compare_mozilla.rs @@ -5,8 +5,10 @@ //! //! This is, obviously, quite a heuristic test. use std::collections::HashMap; + use ring::io::der; use untrusted; +use webpki::TrustAnchor; fn stringify_x500name(subject: &[u8]) -> String { let mut parts = vec![]; @@ -74,7 +76,7 @@ fn test_does_not_have_many_roots_unknown_by_mozilla() { let mut missing_in_moz_roots = 0; for cert in &native { - let cert = webpki::trust_anchor_util::cert_der_as_trust_anchor(&cert.0).unwrap(); + let cert = TrustAnchor::try_from_cert_der(&cert.0).unwrap(); if let Some(moz) = mozilla.get(cert.spki) { assert_eq!(cert.subject, moz.subject, "subjects differ for public key"); @@ -105,7 +107,7 @@ fn test_contains_most_roots_known_by_mozilla() { let mut native_map = HashMap::new(); for anchor in &native { - let cert = webpki::trust_anchor_util::cert_der_as_trust_anchor(&anchor.0).unwrap(); + let cert = TrustAnchor::try_from_cert_der(&anchor.0).unwrap(); native_map.insert(cert.spki.to_vec(), anchor); } @@ -139,7 +141,7 @@ fn util_list_certs() { .unwrap(); for (i, cert) in native.iter().enumerate() { - let cert = webpki::trust_anchor_util::cert_der_as_trust_anchor(&cert.0).unwrap(); + let cert = TrustAnchor::try_from_cert_der(&cert.0).unwrap(); println!("cert[{}] = {}", i, stringify_x500name(cert.subject)); } } diff --git a/tests/smoketests.rs b/tests/smoketests.rs index 715c346..f08e596 100644 --- a/tests/smoketests.rs +++ b/tests/smoketests.rs @@ -1,10 +1,10 @@ +use std::convert::TryInto; use std::sync::Arc; -use std::net::TcpStream; use std::io::{Read, Write}; +use std::net::TcpStream; use rustls; -use webpki; use rustls_native_certs; fn check_site(domain: &str) { @@ -13,21 +13,27 @@ fn check_site(domain: &str) { roots.add(&rustls::Certificate(cert.0)).unwrap(); } - let mut config = rustls::ClientConfig::new(); - config.root_store = roots; + let config = rustls::ClientConfig::builder() + .with_safe_defaults() + .with_root_certificates(roots) + .with_no_client_auth(); - let dns_name = webpki::DNSNameRef::try_from_ascii_str(domain) - .unwrap(); - let mut sess = rustls::ClientSession::new(&Arc::new(config), dns_name); + let mut conn = + rustls::ClientConnection::new(Arc::new(config), domain.try_into().unwrap()).unwrap(); let mut sock = TcpStream::connect(format!("{}:443", domain)).unwrap(); - let mut tls = rustls::Stream::new(&mut sess, &mut sock); - tls.write(format!("GET / HTTP/1.1\r\n\ + let mut tls = rustls::Stream::new(&mut conn, &mut sock); + tls.write( + format!( + "GET / HTTP/1.1\r\n\ Host: {}\r\n\ Connection: close\r\n\ Accept-Encoding: identity\r\n\ - \r\n", domain) - .as_bytes()) - .unwrap(); + \r\n", + domain + ) + .as_bytes(), + ) + .unwrap(); let mut plaintext = [0u8; 1024]; let len = tls.read(&mut plaintext).unwrap(); assert!(plaintext[..len].starts_with(b"HTTP/1.1 ")); // or whatever From 6bd0a9892bc23eb42aa2b9bb832daa10b0d70740 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 27 Sep 2021 10:56:42 +0200 Subject: [PATCH 3/4] Bump MSRV to 1.50.0 --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 4e5609c..1cfcfa1 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -17,7 +17,7 @@ jobs: - stable - beta - nightly - - 1.39.0 + - 1.50.0 os: [ubuntu-18.04] # but only stable on macos/windows (slower platforms) include: From e6a855168bc7a70ee6c9088d434183785c8904e8 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 6 Oct 2021 16:23:07 +0200 Subject: [PATCH 4/4] Apply clippy suggestions in test code --- examples/google.rs | 5 +---- tests/compare_mozilla.rs | 31 +++++++++++++++---------------- tests/smoketests.rs | 5 +---- 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/examples/google.rs b/examples/google.rs index 60104b4..dac0b03 100644 --- a/examples/google.rs +++ b/examples/google.rs @@ -4,9 +4,6 @@ use std::sync::Arc; use std::io::{stdout, Read, Write}; use std::net::TcpStream; -use rustls; -use rustls_native_certs; - fn main() { let mut roots = rustls::RootCertStore::empty(); for cert in rustls_native_certs::load_native_certs().expect("could not load platform certs") { @@ -22,7 +19,7 @@ fn main() { rustls::ClientConnection::new(Arc::new(config), "google.com".try_into().unwrap()).unwrap(); let mut sock = TcpStream::connect("google.com:443").expect("cannot connect"); let mut tls = rustls::Stream::new(&mut conn, &mut sock); - tls.write( + tls.write_all( concat!( "GET / HTTP/1.1\r\n", "Host: google.com\r\n", diff --git a/tests/compare_mozilla.rs b/tests/compare_mozilla.rs index 31d599e..66b4068 100644 --- a/tests/compare_mozilla.rs +++ b/tests/compare_mozilla.rs @@ -7,7 +7,6 @@ use std::collections::HashMap; use ring::io::der; -use untrusted; use webpki::TrustAnchor; fn stringify_x500name(subject: &[u8]) -> String { @@ -19,30 +18,30 @@ fn stringify_x500name(subject: &[u8]) -> String { .unwrap(); assert!(tag == 0x31); // sequence, constructed, context=1 - let mut inner = untrusted::Reader::new(contents.into()); + let mut inner = untrusted::Reader::new(contents); let pair = der::expect_tag_and_get_value(&mut inner, der::Tag::Sequence) .unwrap(); - let mut pair = untrusted::Reader::new(pair.into()); + let mut pair = untrusted::Reader::new(pair); let oid = der::expect_tag_and_get_value(&mut pair, der::Tag::OID) .unwrap(); let (valuety, value) = der::read_tag_and_get_value(&mut pair) .unwrap(); let name = match oid.as_slice_less_safe() { - &[0x55, 0x04, 0x03] => "CN", - &[0x55, 0x04, 0x05] => "serialNumber", - &[0x55, 0x04, 0x06] => "C", - &[0x55, 0x04, 0x07] => "L", - &[0x55, 0x04, 0x08] => "ST", - &[0x55, 0x04, 0x09] => "STREET", - &[0x55, 0x04, 0x0a] => "O", - &[0x55, 0x04, 0x0b] => "OU", - &[0x55, 0x04, 0x11] => "postalCode", - &[0x55, 0x04, 0x61] => "organizationIdentifier", - &[0x09, 0x92, 0x26, 0x89, 0x93, 0xf2, 0x2c, 0x64, 0x01, 0x19] => "domainComponent", - &[0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01] => "emailAddress", - _ => panic!("unhandled x500 attr {:?}", oid) + [0x55, 0x04, 0x03] => "CN", + [0x55, 0x04, 0x05] => "serialNumber", + [0x55, 0x04, 0x06] => "C", + [0x55, 0x04, 0x07] => "L", + [0x55, 0x04, 0x08] => "ST", + [0x55, 0x04, 0x09] => "STREET", + [0x55, 0x04, 0x0a] => "O", + [0x55, 0x04, 0x0b] => "OU", + [0x55, 0x04, 0x11] => "postalCode", + [0x55, 0x04, 0x61] => "organizationIdentifier", + [0x09, 0x92, 0x26, 0x89, 0x93, 0xf2, 0x2c, 0x64, 0x01, 0x19] => "domainComponent", + [0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01] => "emailAddress", + _ => panic!("unhandled x500 attr {:?}", oid), }; let str_value = match valuety { diff --git a/tests/smoketests.rs b/tests/smoketests.rs index f08e596..e31da86 100644 --- a/tests/smoketests.rs +++ b/tests/smoketests.rs @@ -4,9 +4,6 @@ use std::sync::Arc; use std::io::{Read, Write}; use std::net::TcpStream; -use rustls; -use rustls_native_certs; - fn check_site(domain: &str) { let mut roots = rustls::RootCertStore::empty(); for cert in rustls_native_certs::load_native_certs().unwrap() { @@ -22,7 +19,7 @@ fn check_site(domain: &str) { rustls::ClientConnection::new(Arc::new(config), domain.try_into().unwrap()).unwrap(); let mut sock = TcpStream::connect(format!("{}:443", domain)).unwrap(); let mut tls = rustls::Stream::new(&mut conn, &mut sock); - tls.write( + tls.write_all( format!( "GET / HTTP/1.1\r\n\ Host: {}\r\n\