From ca804a7a4f038973cf5d8ab8501a1c782b87ae6b Mon Sep 17 00:00:00 2001 From: Joe Birr-Pixton Date: Wed, 4 Sep 2024 14:21:46 +0100 Subject: [PATCH 1/2] Use pki-types 1.9 for PEM decoding --- Cargo.lock | 11 +- Cargo.toml | 5 +- src/lib.rs | 1 - src/pemfile.rs | 341 ++++++++++++++----------------------------------- src/tests.rs | 2 +- 5 files changed, 101 insertions(+), 259 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8b5e9a4..399cb3d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,12 +2,6 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "base64" -version = "0.22.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" - [[package]] name = "bencher" version = "0.1.5" @@ -18,13 +12,12 @@ checksum = "7dfdb4953a096c551ce9ace855a604d702e6e62d77fac690575ae347571717f5" name = "rustls-pemfile" version = "2.1.3" dependencies = [ - "base64", "bencher", "rustls-pki-types", ] [[package]] name = "rustls-pki-types" -version = "1.7.0" +version = "1.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "976295e77ce332211c0d24d92c0e83e50f5c5f046d11082cea19f3df13a3562d" +checksum = "0e696e35370c65c9c541198af4543ccd580cf17fc25d8e05c5a242b202488c55" diff --git a/Cargo.toml b/Cargo.toml index 2131acc..0c484d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,15 +10,14 @@ repository = "https://github.com/rustls/pemfile" categories = ["network-programming", "cryptography"] [dependencies] -base64 = { version = "0.22", default-features = false, features = ["alloc"] } -pki-types = { package = "rustls-pki-types", version = "1.7" } +pki-types = { package = "rustls-pki-types", version = "1.9" } [dev-dependencies] bencher = "0.1.5" [features] default = ["std"] -std = ["base64/std"] +std = ["pki-types/std"] [[bench]] name = "benchmark" diff --git a/src/lib.rs b/src/lib.rs index e19b839..4aaa579 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,7 +59,6 @@ extern crate std; #[cfg(feature = "std")] mod tests; -/// --- Main crate APIs: mod pemfile; #[cfg(feature = "std")] pub use pemfile::{read_all, read_one}; diff --git a/src/pemfile.rs b/src/pemfile.rs index 96c3c6b..245cee0 100644 --- a/src/pemfile.rs +++ b/src/pemfile.rs @@ -1,16 +1,14 @@ -use alloc::borrow::ToOwned; use alloc::format; use alloc::string::String; use alloc::vec::Vec; #[cfg(feature = "std")] use core::iter; -use core::ops::ControlFlow; #[cfg(feature = "std")] use std::io::{self, ErrorKind}; use pki_types::{ - CertificateDer, CertificateRevocationListDer, CertificateSigningRequestDer, PrivatePkcs1KeyDer, - PrivatePkcs8KeyDer, PrivateSec1KeyDer, SubjectPublicKeyInfoDer, + pem, CertificateDer, CertificateRevocationListDer, CertificateSigningRequestDer, + PrivatePkcs1KeyDer, PrivatePkcs8KeyDer, PrivateSec1KeyDer, SubjectPublicKeyInfoDer, }; /// The contents of a single recognised block in a PEM file. @@ -53,7 +51,56 @@ pub enum Item { Csr(CertificateSigningRequestDer<'static>), } +impl Item { + #[cfg(feature = "std")] + fn from_buf(rd: &mut dyn io::BufRead) -> Result, pem::Error> { + loop { + match pem::from_buf(rd)? { + Some((kind, data)) => match Self::from_kind(kind, data) { + Some(item) => return Ok(Some(item)), + None => continue, + }, + + None => return Ok(None), + } + } + } + + fn from_slice(pem: &[u8]) -> Result, pem::Error> { + let mut iter = <(pem::SectionKind, Vec) as pem::PemObject>::pem_slice_iter(pem); + + for found in iter.by_ref() { + match found { + Ok((kind, data)) => match Self::from_kind(kind, data) { + Some(item) => return Ok(Some((item, iter.remainder()))), + None => continue, + }, + Err(err) => return Err(err.into()), + } + } + + Ok(None) + } + + fn from_kind(kind: pem::SectionKind, data: Vec) -> Option { + use pem::SectionKind::*; + match kind { + Certificate => Some(Self::X509Certificate(data.into())), + PublicKey => Some(Self::SubjectPublicKeyInfo(data.into())), + RsaPrivateKey => Some(Self::Pkcs1Key(data.into())), + PrivateKey => Some(Self::Pkcs8Key(data.into())), + EcPrivateKey => Some(Self::Sec1Key(data.into())), + Crl => Some(Self::Crl(data.into())), + Csr => Some(Self::Csr(data.into())), + _ => None, + } + } +} + /// Errors that may arise when parsing the contents of a PEM file +/// +/// This differs from [`rustls_pki_types::pem::Error`] because it is `PartialEq`; +/// it is retained for compatibility. #[derive(Debug, PartialEq)] pub enum Error { /// a section is missing its "END marker" line @@ -72,30 +119,53 @@ pub enum Error { Base64Decode(String), } +#[cfg(feature = "std")] +impl From for io::Error { + fn from(error: Error) -> Self { + match error { + Error::MissingSectionEnd { end_marker } => io::Error::new( + ErrorKind::InvalidData, + format!( + "section end {:?} missing", + String::from_utf8_lossy(&end_marker) + ), + ), + + Error::IllegalSectionStart { line } => io::Error::new( + ErrorKind::InvalidData, + format!( + "illegal section start: {:?}", + String::from_utf8_lossy(&line) + ), + ), + + Error::Base64Decode(err) => io::Error::new(ErrorKind::InvalidData, err), + } + } +} + +impl From for Error { + fn from(error: pem::Error) -> Self { + match error { + pem::Error::MissingSectionEnd { end_marker } => Error::MissingSectionEnd { end_marker }, + pem::Error::IllegalSectionStart { line } => Error::IllegalSectionStart { line }, + pem::Error::Base64Decode(str) => Error::Base64Decode(str), + + // this is a necessary bodge to funnel any new errors into our existing type + // (to which we can add no new variants) + other => Error::Base64Decode(format!("{other:?}")), + } + } +} + /// Extract and decode the next PEM section from `input` /// /// - `Ok(None)` is returned if there is no PEM section to read from `input` /// - Syntax errors and decoding errors produce a `Err(...)` /// - Otherwise each decoded section is returned with a `Ok(Some((Item::..., remainder)))` where /// `remainder` is the part of the `input` that follows the returned section -pub fn read_one_from_slice(mut input: &[u8]) -> Result, Error> { - let mut b64buf = Vec::with_capacity(1024); - let mut section = None::<(Vec<_>, Vec<_>)>; - - loop { - let next_line = if let Some(index) = input.iter().position(|byte| *byte == b'\n') { - let (line, newline_plus_remainder) = input.split_at(index); - input = &newline_plus_remainder[1..]; - Some(line) - } else { - None - }; - - match read_one_impl(next_line, &mut section, &mut b64buf)? { - ControlFlow::Continue(()) => continue, - ControlFlow::Break(item) => return Ok(item.map(|item| (item, input))), - } - } +pub fn read_one_from_slice(input: &[u8]) -> Result, Error> { + Item::from_slice(input).map_err(Into::into) } /// Extract and decode the next PEM section from `rd`. @@ -108,188 +178,10 @@ pub fn read_one_from_slice(mut input: &[u8]) -> Result, Er /// `for item in iter::from_fn(|| read_one(rd).transpose()) { ... }` #[cfg(feature = "std")] pub fn read_one(rd: &mut dyn io::BufRead) -> Result, io::Error> { - let mut b64buf = Vec::with_capacity(1024); - let mut section = None::<(Vec<_>, Vec<_>)>; - let mut line = Vec::with_capacity(80); - - loop { - line.clear(); - let len = read_until_newline(rd, &mut line)?; - - let next_line = if len == 0 { - None - } else { - Some(line.as_slice()) - }; - - match read_one_impl(next_line, &mut section, &mut b64buf) { - Ok(ControlFlow::Break(opt)) => return Ok(opt), - Ok(ControlFlow::Continue(())) => continue, - Err(e) => { - return Err(match e { - Error::MissingSectionEnd { end_marker } => io::Error::new( - ErrorKind::InvalidData, - format!( - "section end {:?} missing", - String::from_utf8_lossy(&end_marker) - ), - ), - - Error::IllegalSectionStart { line } => io::Error::new( - ErrorKind::InvalidData, - format!( - "illegal section start: {:?}", - String::from_utf8_lossy(&line) - ), - ), - - Error::Base64Decode(err) => io::Error::new(ErrorKind::InvalidData, err), - }); - } - } - } -} - -fn read_one_impl( - next_line: Option<&[u8]>, - section: &mut Option<(Vec, Vec)>, - b64buf: &mut Vec, -) -> Result, ()>, Error> { - let line = if let Some(line) = next_line { - line - } else { - // EOF - return match section.take() { - Some((_, end_marker)) => Err(Error::MissingSectionEnd { end_marker }), - None => Ok(ControlFlow::Break(None)), - }; - }; - - if line.starts_with(b"-----BEGIN ") { - let (mut trailer, mut pos) = (0, line.len()); - for (i, &b) in line.iter().enumerate().rev() { - match b { - b'-' => { - trailer += 1; - pos = i; - } - b'\n' | b'\r' | b' ' => continue, - _ => break, - } - } - - if trailer != 5 { - return Err(Error::IllegalSectionStart { - line: line.to_vec(), - }); - } - - let ty = &line[11..pos]; - let mut end = Vec::with_capacity(10 + 4 + ty.len()); - end.extend_from_slice(b"-----END "); - end.extend_from_slice(ty); - end.extend_from_slice(b"-----"); - *section = Some((ty.to_owned(), end)); - return Ok(ControlFlow::Continue(())); - } - - if let Some((section_type, end_marker)) = section.as_ref() { - if line.starts_with(end_marker) { - let der = base64::ENGINE - .decode(&b64buf) - .map_err(|err| Error::Base64Decode(format!("{err:?}")))?; - - let item = match section_type.as_slice() { - b"CERTIFICATE" => Some(Item::X509Certificate(der.into())), - b"PUBLIC KEY" => Some(Item::SubjectPublicKeyInfo(der.into())), - b"RSA PRIVATE KEY" => Some(Item::Pkcs1Key(der.into())), - b"PRIVATE KEY" => Some(Item::Pkcs8Key(der.into())), - b"EC PRIVATE KEY" => Some(Item::Sec1Key(der.into())), - b"X509 CRL" => Some(Item::Crl(der.into())), - b"CERTIFICATE REQUEST" => Some(Item::Csr(der.into())), - _ => { - *section = None; - b64buf.clear(); - None - } - }; - - if item.is_some() { - return Ok(ControlFlow::Break(item)); - } - } - } - - if section.is_some() { - // Extend b64buf without leading or trailing whitespace - b64buf.extend(trim_ascii(line)); - } - - Ok(ControlFlow::Continue(())) -} - -// Ported from https://github.com/rust-lang/rust/blob/91cfcb021935853caa06698b759c293c09d1e96a/library/std/src/io/mod.rs#L1990 and -// modified to look for our accepted newlines. -#[cfg(feature = "std")] -fn read_until_newline(r: &mut R, buf: &mut Vec) -> io::Result { - let mut read = 0; - loop { - let (done, used) = { - let available = match r.fill_buf() { - Ok(n) => n, - Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, - Err(e) => return Err(e), - }; - match available - .iter() - .copied() - .position(|b| b == b'\n' || b == b'\r') - { - Some(i) => { - buf.extend_from_slice(&available[..=i]); - (true, i + 1) - } - None => { - buf.extend_from_slice(available); - (false, available.len()) - } - } - }; - r.consume(used); - read += used; - if done || used == 0 { - return Ok(read); - } - } -} - -/// Trim contiguous leading and trailing whitespace from `line`. -/// -/// We use [u8::is_ascii_whitespace] to determine what is whitespace. -// TODO(XXX): Replace with `[u8]::trim_ascii` once stabilized[0] and available in our MSRV. -// [0]: https://github.com/rust-lang/rust/issues/94035 -const fn trim_ascii(line: &[u8]) -> &[u8] { - let mut bytes = line; - - // Note: A pattern matching based approach (instead of indexing) allows - // making the function const. - while let [first, rest @ ..] = bytes { - if first.is_ascii_whitespace() { - bytes = rest; - } else { - break; - } - } - - while let [rest @ .., last] = bytes { - if last.is_ascii_whitespace() { - bytes = rest; - } else { - break; - } - } - - bytes + Item::from_buf(rd).map_err(|err| match err { + pem::Error::Io(io) => io, + other => Error::from(other).into(), + }) } /// Extract and return all PEM sections by reading `rd`. @@ -297,44 +189,3 @@ const fn trim_ascii(line: &[u8]) -> &[u8] { pub fn read_all(rd: &mut dyn io::BufRead) -> impl Iterator> + '_ { iter::from_fn(move || read_one(rd).transpose()) } - -mod base64 { - use base64::alphabet::STANDARD; - use base64::engine::general_purpose::{GeneralPurpose, GeneralPurposeConfig}; - use base64::engine::DecodePaddingMode; - pub(super) use base64::engine::Engine; - - pub(super) const ENGINE: GeneralPurpose = GeneralPurpose::new( - &STANDARD, - GeneralPurposeConfig::new().with_decode_padding_mode(DecodePaddingMode::Indifferent), - ); -} -use self::base64::Engine; - -#[cfg(test)] -mod tests { - #[test] - fn test_trim_ascii() { - let tests: &[(&[u8], &[u8])] = &[ - (b"", b""), - (b" hello world ", b"hello world"), - (b" hello\t\r\nworld ", b"hello\t\r\nworld"), - (b"\n\r \ttest\t \r\n", b"test"), - (b" \r\n ", b""), - (b"no trimming needed", b"no trimming needed"), - ( - b"\n\n content\n\n more content\n\n", - b"content\n\n more content", - ), - ]; - - for &(input, expected) in tests { - assert_eq!( - super::trim_ascii(input), - expected, - "Failed for input: {:?}", - input, - ); - } - } -} diff --git a/src/tests.rs b/src/tests.rs index 290f89b..adfc790 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -52,7 +52,7 @@ mod unit { -----END RSA PRIVATE KEY-----\n"; assert_eq!( format!("{:?}", check_io(input)), - "Err(Custom { kind: InvalidData, error: \"InvalidByte(1, 61)\" })" + "Err(Custom { kind: InvalidData, error: \"InvalidTrailingPadding\" })" ); assert!(matches!(check_slice(input), Err(Error::Base64Decode(_)))); } From 6a0f0fe6157138f16b6e5f196d6d64f4507af820 Mon Sep 17 00:00:00 2001 From: Joe Birr-Pixton Date: Wed, 25 Sep 2024 13:13:13 +0100 Subject: [PATCH 2/2] rustfmt to reformat imports (with rustls's config) --- src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4aaa579..858e996 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,6 +60,12 @@ extern crate std; mod tests; mod pemfile; +#[cfg(feature = "std")] +use core::iter; +/// --- Legacy APIs: +#[cfg(feature = "std")] +use std::io; + #[cfg(feature = "std")] pub use pemfile::{read_all, read_one}; pub use pemfile::{read_one_from_slice, Error, Item}; @@ -71,12 +77,6 @@ use pki_types::{ PrivatePkcs8KeyDer, PrivateSec1KeyDer, SubjectPublicKeyInfoDer, }; -#[cfg(feature = "std")] -use core::iter; -/// --- Legacy APIs: -#[cfg(feature = "std")] -use std::io; - /// Return an iterator over certificates from `rd`. /// /// Filters out any PEM sections that are not certificates and yields errors if a problem