Skip to content

Commit

Permalink
pemfile: also trim leading contiguous content whitespace
Browse files Browse the repository at this point in the history
RFC 7468 §2[0] says:
> parsers SHOULD ignore whitespace

Previously we only stripped the trailing whitespace from the base64
content lines within the PEM section boundaries. This branch updates our
processing to additionally trim leading contiguous whitespace. This felt
more sensible than outright ignoring whitespace (e.g. from the middle of
content) and will be sufficient to resolve the real world PEM files
we've seen with leading whitespace.

We base our implementation on the stdlib's unstable `[u8]::trim_ascii`
fn, which we can't (yet) use directly due to our MSRV/stable rust
requirements. A TODO is left for future cleanup.

[0]: https://www.rfc-editor.org/rfc/rfc7468#section-2
  • Loading branch information
cpu committed Mar 1, 2024
1 parent fc3414f commit 95152b4
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 8 deletions.
67 changes: 59 additions & 8 deletions src/pemfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,8 @@ fn read_one_impl(
}

if section.is_some() {
let mut trim = 0;
for &b in line.iter().rev() {
match b {
b'\n' | b'\r' | b' ' => trim += 1,
_ => break,
}
}
b64buf.extend(&line[..line.len() - trim]);
// Extend b64buf without leading or trailing whitespace
b64buf.extend(trim_ascii(line));
}

Ok(ControlFlow::Continue(()))
Expand Down Expand Up @@ -266,6 +260,35 @@ fn read_until_newline<R: io::BufRead + ?Sized>(
}
}

/// 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
}

/// Extract and return all PEM sections by reading `rd`.
#[cfg(feature = "std")]
pub fn read_all(rd: &mut dyn io::BufRead) -> impl Iterator<Item = Result<Item, io::Error>> + '_ {
Expand All @@ -284,3 +307,31 @@ mod base64 {
);
}
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,
);
}
}
}
21 changes: 21 additions & 0 deletions tests/data/whitespace-prefix.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
-----BEGIN CERTIFICATE-----
MIIDaTCCAlGgAwIBAgIJAOq/zL+84IswMA0GCSqGSIb3DQEBCwUAMFoxCzAJBgNV
BAYTAlVTMQswCQYDVQQIDAJOQzEMMAoGA1UEBwwDUlRQMQ8wDQYDVQQKDAZOZXRB
cHAxDTALBgNVBAsMBEVTSVMxEDAOBgNVBAMMB1NTRk1DQ0EwHhcNMTcxMTAxMjEw
OTQyWhcNMjcxMDMwMjEwOTQyWjBaMQswCQYDVQQGEwJVUzELMAkGA1UECAwCTkMx
DDAKBgNVBAcMA1JUUDEPMA0GA1UECgwGTmV0QXBwMQ0wCwYDVQQLDARFU0lTMRAw
DgYDVQQDDAdTU0ZNQ0NBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA
iaD9Ee0Yrdka0I+9GTJBIW/Fp5JU6kyjaxfOldW/R9lEubegXQFhDD2Xi1HZ+fTM
f224glB9xLJXAHhipRK01C2MgC4kSH75WL1iAiYeOBloExqmK6OCX+sdyO7RXm/H
Ra9tN2INWdvyO2pnmxsSnq56mCMsUZLtrRKp89FWgcxLg5r8QxH7xwfh5k54rxjE
144TD9yrIiQOgRSIRHUrVJ9l/F/gnwzP8wcNABeXwN71Mzl7mliPA703kONQIAyU
0E0tLpmy/U8dZdMmTBZGB7jI9f95Hl1RunfwhR371a6z38kgkvwrLzl4qflfsPjw
K9n4omNk9rCH9H9tWkxxjwIDAQABozIwMDAdBgNVHQ4EFgQU/bFyCCnqdDFKlQBJ
ExtV6wcMYkEwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAOQMs
Pz2iBD1+3RcSOsahB36WAwPCjgPiXXXpU+Zri11+m6I0Lq+OWtf+YgaQ8ylLmCQd
0p1wHlYA4qo896SycrhTQfy9GlS/aQqN192k3oBGoJcMIUnGUBGuEvyZ2aDUfkzy
JUqBe+0KaT7pkvvbRL7VUz34I7ouq9fQIRZ26vUDLTY3KM1n/DXBj3e30GHGMV3K
NN2twuLXPNjnryfgpliHU1rwV7r1WvrCVn4StjimP2bO5HGqD/SbiYUL2M9LOuLK
6mqY4OHumYXq3k7CHrvt0FepsN0L14LYEt1LvpPDFWP3SdN4z4KqT9AGqBaJnhhl
Qiq8GWnAChspdBLxCg==
-----END CERTIFICATE-----
12 changes: 12 additions & 0 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,15 @@ fn different_line_endings() {
assert!(matches!(cert, rustls_pemfile::Item::X509Certificate(_)));
}
}

#[test]
fn whitespace_prefix() {
let items = rustls_pemfile::read_all(&mut BufReader::new(
&include_bytes!("data/whitespace-prefix.crt")[..],
))
.collect::<Result<Vec<_>, _>>()
.unwrap();

assert_eq!(items.len(), 1);
assert!(matches!(items[0], rustls_pemfile::Item::X509Certificate(_)));
}

0 comments on commit 95152b4

Please sign in to comment.