Skip to content

Commit

Permalink
Return all errors to caller
Browse files Browse the repository at this point in the history
  • Loading branch information
djc committed Aug 27, 2024
1 parent 6d3504f commit 5b9908e
Show file tree
Hide file tree
Showing 10 changed files with 209 additions and 114 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rustls-native-certs"
version = "0.7.2"
version = "0.8.0"
edition = "2021"
rust-version = "1.60"
license = "Apache-2.0 OR ISC OR MIT"
Expand Down
2 changes: 1 addition & 1 deletion examples/google.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::sync::Arc;

fn main() {
let mut roots = rustls::RootCertStore::empty();
for cert in rustls_native_certs::load_native_certs().expect("could not load platform certs") {
for cert in rustls_native_certs::load_native_certs().certs {
roots.add(cert).unwrap();
}

Expand Down
2 changes: 1 addition & 1 deletion examples/print-trust-anchors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::error::Error;
use x509_parser::prelude::*;

fn main() -> Result<(), Box<dyn Error>> {
for cert in rustls_native_certs::load_native_certs()? {
for cert in rustls_native_certs::load_native_certs().certs {
match parse_x509_certificate(cert.as_ref()) {
Ok((_, cert)) => println!("{}", cert.tbs_certificate.subject),
Err(e) => eprintln!("error parsing certificate: {}", e),
Expand Down
236 changes: 161 additions & 75 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@
// Enable documentation for all features on docs.rs
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]

use std::borrow::Cow;
use std::env;
use std::error::Error as StdError;
use std::ffi::OsStr;
use std::fs::{self, File};
use std::io::BufReader;
use std::io::{Error, ErrorKind};
use std::io::{self, BufReader};
use std::path::{Path, PathBuf};

use pki_types::CertificateDer;
Expand Down Expand Up @@ -117,10 +118,56 @@ use macos as platform;
/// this sparingly.
///
/// [c_rehash]: https://www.openssl.org/docs/manmaster/man1/c_rehash.html
pub fn load_native_certs() -> Result<Vec<CertificateDer<'static>>, Error> {
match CertPaths::from_env().load()? {
Some(certs) => Ok(certs),
None => platform::load_native_certs(),
pub fn load_native_certs() -> CertificateResult {
match CertPaths::from_env().load() {
out if !out.certs.is_empty() => out,
_ => platform::load_native_certs(),
}
}

#[derive(Debug, Default)]
pub struct CertificateResult {
pub certs: Vec<CertificateDer<'static>>,
pub errors: Vec<Error>,
}

impl CertificateResult {
pub fn expect(self, msg: &str) -> Vec<CertificateDer<'static>> {
match self.errors.is_empty() {
true => self.certs,
false => panic!("{msg}: {:?}", self.errors),
}
}

pub fn unwrap(self) -> Vec<CertificateDer<'static>> {
match self.errors.is_empty() {
true => self.certs,
false => panic!(
"errors occurred while loading certificates: {:?}",
self.errors
),
}
}

fn io_error(&mut self, err: io::Error, path: &Path, cx: impl Into<Cow<'static, str>>) {
self.errors.push(Error {
context: cx.into(),
kind: ErrorKind::Io {
inner: err,
path: path.to_owned(),
},
});
}

fn os_error(
&mut self,
err: Box<dyn StdError + Send + Sync + 'static>,
cx: impl Into<Cow<'static, str>>,
) {
self.errors.push(Error {
context: cx.into(),
kind: ErrorKind::Os(err),
});
}
}

Expand Down Expand Up @@ -152,40 +199,24 @@ impl CertPaths {
/// [hash files](`is_hash_file_name()`) contained in it must be loaded successfully,
/// subject to the rules outlined above for `self.file`. The directory is not
/// scanned recursively and may be empty.
fn load(&self) -> Result<Option<Vec<CertificateDer<'static>>>, Error> {
fn load(&self) -> CertificateResult {
let mut out = CertificateResult::default();
if self.file.is_none() && self.dir.is_none() {
return Ok(None);
return out;
}

let mut certs = match &self.file {
Some(cert_file) => {
load_pem_certs(cert_file).map_err(|err| Self::load_err(cert_file, err))?
}
None => Vec::new(),
};
if let Some(cert_file) = &self.file {
load_pem_certs(cert_file, &mut out);
}

if let Some(cert_dir) = &self.dir {
certs.append(
&mut load_pem_certs_from_dir(cert_dir)
.map_err(|err| Self::load_err(cert_dir, err))?,
);
load_pem_certs_from_dir(cert_dir, &mut out);
}

certs.sort_unstable_by(|a, b| a.cmp(b));
certs.dedup();

Ok(Some(certs))
}

fn load_err(path: &Path, err: Error) -> Error {
Error::new(
err.kind(),
format!(
"could not load certs from {} {}: {err}",
if path.is_file() { "file" } else { "dir" },
path.display()
),
)
out.certs
.sort_unstable_by(|a, b| a.cmp(b));
out.certs.dedup();
out
}
}

Expand All @@ -196,11 +227,24 @@ impl CertPaths {
/// isn't a valid certificate, we limit ourselves to loading those files
/// that have a hash-based file name matching the pattern used by OpenSSL.
/// The hash is not verified, however.
fn load_pem_certs_from_dir(dir: &Path) -> Result<Vec<CertificateDer<'static>>, Error> {
let dir_reader = fs::read_dir(dir)?;
let mut certs = Vec::new();
fn load_pem_certs_from_dir(dir: &Path, out: &mut CertificateResult) {
let dir_reader = match fs::read_dir(dir) {
Ok(reader) => reader,
Err(err) => {
out.io_error(err, dir, "opening directory");
return;
}
};

for entry in dir_reader {
let entry = entry?;
let entry = match entry {
Ok(entry) => entry,
Err(err) => {
out.io_error(err, dir, "reading directory entries");
continue;
}
};

let path = entry.path();
let file_name = path
.file_name()
Expand All @@ -213,21 +257,37 @@ fn load_pem_certs_from_dir(dir: &Path) -> Result<Vec<CertificateDer<'static>>, E
// make sure we resolve them.
let metadata = match fs::metadata(&path) {
Ok(metadata) => metadata,
Err(e) if e.kind() == ErrorKind::NotFound => {
Err(e) if e.kind() == io::ErrorKind::NotFound => {
// Dangling symlink
continue;
}
Err(e) => return Err(e),
Err(e) => {
out.io_error(e, &path, "failed to open file");
continue;
}
};

if metadata.is_file() && is_hash_file_name(file_name) {
certs.append(&mut load_pem_certs(&path)?);
load_pem_certs(&path, out);
}
}
Ok(certs)
}

fn load_pem_certs(path: &Path) -> Result<Vec<CertificateDer<'static>>, Error> {
rustls_pemfile::certs(&mut BufReader::new(File::open(path)?)).collect()
fn load_pem_certs(path: &Path, out: &mut CertificateResult) {
let reader = match File::open(path) {
Ok(file) => BufReader::new(file),
Err(err) => {
out.io_error(err, path, "failed to open file");
return;
}
};

for result in rustls_pemfile::certs(&mut BufReader::new(reader)) {
match result {
Ok(cert) => out.certs.push(cert),
Err(err) => out.io_error(err, path, "failed to parse PEM"),
}
}
}

/// Check if this is a hash-based file name for a certificate
Expand Down Expand Up @@ -259,6 +319,19 @@ fn is_hash_file_name(file_name: &OsStr) -> bool {
&& matches!(iter.next(), Some(c) if c.is_ascii_digit())
}

#[derive(Debug)]
pub struct Error {
pub context: Cow<'static, str>,
pub kind: ErrorKind,
}

#[non_exhaustive]
#[derive(Debug)]
pub enum ErrorKind {
Io { inner: io::Error, path: PathBuf },
Os(Box<dyn StdError + Send + Sync + 'static>),
}

const ENV_CERT_FILE: &str = "SSL_CERT_FILE";
const ENV_CERT_DIR: &str = "SSL_CERT_DIR";

Expand Down Expand Up @@ -328,64 +401,65 @@ mod tests {
write!(file, "{}", &cert2).unwrap();
}

let certs_from_file = CertPaths {
let result = CertPaths {
file: Some(file_path.clone()),
dir: None,
}
.load()
.unwrap();
assert_eq!(certs_from_file.unwrap().len(), 2);
.load();
assert_eq!(result.certs.len(), 2);

let certs_from_dir = CertPaths {
let result = CertPaths {
file: None,
dir: Some(dir_path.clone()),
}
.load()
.unwrap();
assert_eq!(certs_from_dir.unwrap().len(), 2);
.load();
assert_eq!(result.certs.len(), 2);

let certs_from_both = CertPaths {
let result = CertPaths {
file: Some(file_path),
dir: Some(dir_path),
}
.load()
.unwrap();
assert_eq!(certs_from_both.unwrap().len(), 2);
.load();
assert_eq!(result.certs.len(), 2);
}

#[test]
fn malformed_file_from_env() {
// Certificate parser tries to extract certs from file ignoring
// invalid sections.
let certs = load_pem_certs(Path::new(file!())).unwrap();
assert_eq!(certs.len(), 0);
let mut result = CertificateResult::default();
load_pem_certs(Path::new(file!()), &mut result);
assert_eq!(result.certs.len(), 0);
assert!(result.errors.is_empty());
}

#[test]
fn from_env_missing_file() {
assert_eq!(
load_pem_certs(Path::new("no/such/file"))
.unwrap_err()
.kind(),
ErrorKind::NotFound
);
let mut result = CertificateResult::default();
load_pem_certs(Path::new("no/such/file"), &mut result);
match &first_error(&result).kind {
ErrorKind::Io { inner, .. } => assert_eq!(inner.kind(), io::ErrorKind::NotFound),
_ => panic!("unexpected error {:?}", result.errors),
}
}

#[test]
fn from_env_missing_dir() {
assert_eq!(
load_pem_certs_from_dir(Path::new("no/such/directory"))
.unwrap_err()
.kind(),
ErrorKind::NotFound
);
let mut result = CertificateResult::default();
load_pem_certs_from_dir(Path::new("no/such/directory"), &mut result);
match &first_error(&result).kind {
ErrorKind::Io { inner, .. } => assert_eq!(inner.kind(), io::ErrorKind::NotFound),
_ => panic!("unexpected error {:?}", result.errors),
}
}

#[test]
#[cfg(unix)]
fn from_env_with_non_regular_and_empty_file() {
let certs = load_pem_certs(Path::new("/dev/null")).unwrap();
assert_eq!(certs.len(), 0);
let mut result = CertificateResult::default();
load_pem_certs(Path::new("/dev/null"), &mut result);
assert_eq!(result.certs.len(), 0);
assert!(result.errors.is_empty());
}

#[test]
Expand Down Expand Up @@ -420,7 +494,7 @@ mod tests {

#[cfg(unix)]
fn test_cert_paths_bad_perms(cert_paths: CertPaths) {
let err = cert_paths.load().unwrap_err();
let result = cert_paths.load();

let affected_path = match (cert_paths.file, cert_paths.dir) {
(Some(file), None) => file,
Expand All @@ -432,12 +506,24 @@ mod tests {
false => "dir",
};

assert_eq!(err.kind(), ErrorKind::PermissionDenied);
assert!(err
let error = first_error(&result);
let io = match &error.kind {
ErrorKind::Io { inner, .. } => {
assert_eq!(inner.kind(), io::ErrorKind::PermissionDenied);
inner
}
_ => panic!("unexpected error {:?}", result.errors),
};

assert!(io
.to_string()
.contains(&format!("certs from {type}")));
assert!(err
assert!(io
.to_string()
.contains(&affected_path.display().to_string()));
}

fn first_error(result: &CertificateResult) -> &Error {
result.errors.first().unwrap()
}
}
Loading

0 comments on commit 5b9908e

Please sign in to comment.