From 217cf3ce50c61fbba9511517183c96c5da3d9f07 Mon Sep 17 00:00:00 2001 From: Daniel James Date: Sun, 12 Sep 2021 17:06:08 -0700 Subject: [PATCH 1/3] Add Error case for PEM with headers fields PEM from RFC 1421 defines header fields https://datatracker.ietf.org/doc/html/rfc1421.html#section-9 this change detects header fields as an initial line that contains the colon char. PEM from RFC 7468 disallows https://datatracker.ietf.org/doc/html/rfc7468#section-2 > Unlike legacy PEM encoding [RFC1421], OpenPGP ASCII armor, and the > OpenSSH key file format, textual encoding does *not* define or permit > headers to be encoded alongside the data. --- pem-rfc7468/src/decoder.rs | 45 +++++++++++++------ pem-rfc7468/src/error.rs | 4 ++ pem-rfc7468/src/grammar.rs | 8 ++++ pem-rfc7468/tests/decode.rs | 10 +++++ .../tests/examples/ssh_rsa_pem_password.pem | 30 +++++++++++++ 5 files changed, 83 insertions(+), 14 deletions(-) create mode 100644 pem-rfc7468/tests/examples/ssh_rsa_pem_password.pem diff --git a/pem-rfc7468/src/decoder.rs b/pem-rfc7468/src/decoder.rs index 44101e07..69cd5ef1 100644 --- a/pem-rfc7468/src/decoder.rs +++ b/pem-rfc7468/src/decoder.rs @@ -154,6 +154,7 @@ impl<'a> Encapsulation<'a> { /// encapsulated text. pub fn encapsulated_text(self) -> Lines<'a> { Lines { + is_start: true, bytes: self.encapsulated_text, } } @@ -169,6 +170,8 @@ impl<'a> TryFrom<&'a [u8]> for Encapsulation<'a> { /// Iterator over the lines in the encapsulated text. struct Lines<'a> { + /// true if no lines have been read + is_start: bool, /// Remaining data being iterated over. bytes: &'a [u8], } @@ -177,21 +180,35 @@ impl<'a> Iterator for Lines<'a> { type Item = Result<&'a [u8]>; fn next(&mut self) -> Option { - if self.bytes.len() > BASE64_WRAP_WIDTH { - let (line, rest) = self.bytes.split_at(BASE64_WRAP_WIDTH); - let result = grammar::strip_leading_eol(rest) - .ok_or(Error::EncapsulatedText) - .map(|rest| { - self.bytes = rest; - line - }); - Some(result) - } else if !self.bytes.is_empty() { - let line = self.bytes; - self.bytes = &[]; - Some(Ok(line)) + if self.is_start + && self + .bytes + .iter() + .take_while(|&&b| !grammar::is_vwsp(b)) + .any(|&b| b == grammar::CHAR_COLON) + { + Some(Err(Error::HeaderDetected)) } else { - None + if self.is_start { + self.is_start = false; + } + + if self.bytes.len() > BASE64_WRAP_WIDTH { + let (line, rest) = self.bytes.split_at(BASE64_WRAP_WIDTH); + let result = grammar::strip_leading_eol(rest) + .ok_or(Error::EncapsulatedText) + .map(|rest| { + self.bytes = rest; + line + }); + Some(result) + } else if !self.bytes.is_empty() { + let line = self.bytes; + self.bytes = &[]; + Some(Ok(line)) + } else { + None + } } } } diff --git a/pem-rfc7468/src/error.rs b/pem-rfc7468/src/error.rs index bbf2fb6b..4486ca6a 100644 --- a/pem-rfc7468/src/error.rs +++ b/pem-rfc7468/src/error.rs @@ -18,6 +18,9 @@ pub enum Error { /// Errors in the encapsulated text (which aren't specifically Base64-related). EncapsulatedText, + /// Header detected in the encapsulated text + HeaderDetected, + /// Invalid label. Label, @@ -37,6 +40,7 @@ impl fmt::Display for Error { Error::Base64 => "PEM Base64 error", Error::CharacterEncoding => "PEM character encoding error", Error::EncapsulatedText => "PEM error in encapsulated text", + Error::HeaderDetected => "PEM header (disallowed) detected in encapsulated text", Error::Label => "PEM type label invalid", Error::Length => "PEM length invalid", Error::PreEncapsulationBoundary => "PEM error in pre-encapsulation boundary", diff --git a/pem-rfc7468/src/grammar.rs b/pem-rfc7468/src/grammar.rs index 662f2ac9..4ac82cfc 100644 --- a/pem-rfc7468/src/grammar.rs +++ b/pem-rfc7468/src/grammar.rs @@ -19,6 +19,9 @@ pub(crate) const CHAR_CR: u8 = 0x0d; /// Line feed pub(crate) const CHAR_LF: u8 = 0x0a; +/// Colon ':' +pub(crate) const CHAR_COLON: u8 = 0x3A; + /// Does the provided byte match a character allowed in a label? // TODO(tarcieri): relax this to match the RFC 7468 ABNF pub(crate) fn is_labelchar(char: u8) -> bool { @@ -33,6 +36,11 @@ pub(crate) fn is_wsp(char: u8) -> bool { matches!(char, CHAR_HT | CHAR_SP) } +/// Does the provided byte match either of vertical whitespace chars CR / LF +pub(crate) fn is_vwsp(char: u8) -> bool { + matches!(char, CHAR_CR | CHAR_LF) +} + /// Split a slice beginning with a type label as located in an encapsulation /// boundary. Returns the label as a `&str`, and slice beginning with the /// encapsulated text with leading `-----` and newline removed. diff --git a/pem-rfc7468/tests/decode.rs b/pem-rfc7468/tests/decode.rs index 7c42e471..34a0fd3d 100644 --- a/pem-rfc7468/tests/decode.rs +++ b/pem-rfc7468/tests/decode.rs @@ -9,6 +9,16 @@ fn pkcs1_example() { assert_eq!(decoded, include_bytes!("examples/pkcs1.der")); } +#[test] +fn pkcs1_enc_example() { + let pem = include_bytes!("examples/ssh_rsa_pem_password.pem"); + let mut buf = [0u8; 2048]; + match pem_rfc7468::decode(pem, &mut buf) { + Err(pem_rfc7468::Error::HeaderDetected) => (), + _ => panic!("Expected HeaderDetected error"), + } +} + #[test] fn pkcs8_example() { let pem = include_bytes!("examples/pkcs8.pem"); diff --git a/pem-rfc7468/tests/examples/ssh_rsa_pem_password.pem b/pem-rfc7468/tests/examples/ssh_rsa_pem_password.pem new file mode 100644 index 00000000..92084bd4 --- /dev/null +++ b/pem-rfc7468/tests/examples/ssh_rsa_pem_password.pem @@ -0,0 +1,30 @@ +-----BEGIN RSA PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-128-CBC,15670D76FD184D46C40C971733E0543F + +/lVaZdZ2a6x5d3b96F6XpFzcnP35pUrwpKxEB07nF15Jc81jwEAg72OpFTp5QRMu +WXbbZ/dKF7ucGHvLQ/VvCNkbl6oqowSme94fzFsa/xuKRHAGDHVi/TQylIOBBJFv +vru/3EZkO8mAQRDTNfuSl0Y5Ir7uqAQy0E/xKfOdY73BO4//KEDEIshRwBxbOm3K +D2sU1Kp8RnnBgSNydG8AH/LrtBnFs9HWrb9JD0Nj5bIxZDzil5CYmTB8PRgb2Qy7 +bckVc+0Y/h8Ai+NjSc/rJVw0smKJbmNSoPyJH1WjDPW8wWtngCFQWaVCTubm08N1 +nqrzIclT3fnq8YFSbFJYZVPaADxjv2HW7dLH7grYqXx/5FTE34ixXTcwQ1KR0ZQX +uaGZhkiDVWf/q82JPREqH5hwbeGL9QwZHF4/74vKIsddEuVFp8EW7jn9INRoVBtK +/OBiVXmVELFhmVBqvQU7GSci7+fCntXIx6W3hGiJL2WyXfuP16u9BhF5kd+c3pAm +tOZ3Lc5XsceBIYq0rKhy7rDhEg0V8wF1jHeeiW0VKDt2cFePSAd4CIbHiRWbvwh+ +zIoNAB34k4cYShmjOHKem9FMHVHSwfRE39Vrwssj0HWVOp7KdXYv64w4Ywmn6wvA +r6p8IZWg7KqA5UApPpiBVs0BAx1KtZk3o1dvXAazklw23icnnZF6XqaH6EmnVsf9 +gbyK1NcH3lIalTYhs+hMwizkw/XDb1uU8G7Rz1QFKBiL56J8ePIA2NWRUwvdMEAv +rZXSq4Icwy566GIqdtMRNLcz6LthNEg9qg+fD5aGLrtTk8ACSQpb/ELMMzqDVTkI +07dB1Nhzx9nd9mUlIuA030I5w7f//5pS6/lGmmPZblygY1PBludl+p/P9OKJ+Jr0 +HTAI4SVxoYdp6YHDBJ9J7Wt6UnIe+/3WarY9d9X1XNGOE4K+nRFihSShtKHDtMY6 +eBEV1sBTXJ1KANG683CU+uDx2XpOVAwDGl5hyRdzOovNC1iWjSu+CvppDvZLuIMj +zIllu5E8PR2Zd1wIT1gnU/7HiVdM0m4jf6ptkGSWNSCLA0ipii0YYarXoyu0kbMY +BKKpp5QRXv6OwmSDMwQTPuRIWyk839X1ABE1XeTKt43Ns+Wtdboi8Cu/aO/Z5AoA +gbJ+CdyKJIJxDXA11cPq9SF2daYmqHV3agrrKmAwWBRwpCKvotv0Hxw2M1+91ZoU +NY52RraoNVQPOAEfhYNS0ltVPzxcDU5bA2WczO6QzmMl7So6dysw+fxtxaEUGt4m +Fj+p+rE64Okq4wWDlEQya/xu4KMZwzyDncgJHHyYahs+vCv9KbQLW8R0iHTbxQzX +Vhomq++Cm8kg5aA/UsLas/l6ZyfNIcA99U8shFFA5urOKMl/jSRd9v1c7H3nOPZ7 ++eN10E7hcRruwOkoBlpd2It3Y2M+1qBDWXLVSHSXmIuzdE+MZ8CZfvxe+FcfpvJU +BFsZbSEF2PQC+zhd1HjV6DUe3jCz88/rjUnXQCvEJ7z7Tuz3C7kKdR3OYYYLwuLW +LTy2VS0p3QuUeMnNRl0HxpB16BZax9mzFr0UvFKp2QQYzOkIghg2sLNEbtaJvHNh +-----END RSA PRIVATE KEY----- From e378e7c7d4b21ade548f918bac5a7a71a85da257 Mon Sep 17 00:00:00 2001 From: Daniel James Date: Mon, 13 Sep 2021 16:55:21 -0700 Subject: [PATCH 2/3] rewrite error case for header to avoid violating the constant time parsing review feedback https://github.com/RustCrypto/utils/pull/630#issuecomment-918201276 --- pem-rfc7468/src/decoder.rs | 62 ++++++++++++++++++++++---------------- pem-rfc7468/src/grammar.rs | 5 --- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/pem-rfc7468/src/decoder.rs b/pem-rfc7468/src/decoder.rs index 69cd5ef1..9ce8a07b 100644 --- a/pem-rfc7468/src/decoder.rs +++ b/pem-rfc7468/src/decoder.rs @@ -31,7 +31,20 @@ pub fn decode<'i, 'o>(pem: &'i [u8], buf: &'o mut [u8]) -> Result<(&'i str, &'o let mut out_len = 0; for line in encapsulation.encapsulated_text() { - out_len += Base64::decode(line?, &mut buf[out_len..])?.len(); + let line = line?; + match Base64::decode(line, &mut buf[out_len..]) { + Err(error) => { + // in the case that we are decoding the first line + // and we error, then attribute the error to an unsupported header + // if a colon char is present in the line + if out_len == 0 && line.iter().any(|&b| b == grammar::CHAR_COLON) { + return Err(Error::HeaderDetected); + } else { + return Err(error.into()); + } + } + Ok(out) => out_len += out.len(), + } } Ok((label, &buf[..out_len])) @@ -180,35 +193,32 @@ impl<'a> Iterator for Lines<'a> { type Item = Result<&'a [u8]>; fn next(&mut self) -> Option { - if self.is_start - && self - .bytes - .iter() - .take_while(|&&b| !grammar::is_vwsp(b)) - .any(|&b| b == grammar::CHAR_COLON) - { - Some(Err(Error::HeaderDetected)) - } else { - if self.is_start { + if self.bytes.len() > BASE64_WRAP_WIDTH { + let (line, rest) = self.bytes.split_at(BASE64_WRAP_WIDTH); + if let Some(rest) = grammar::strip_leading_eol(rest) { self.is_start = false; - } - - if self.bytes.len() > BASE64_WRAP_WIDTH { - let (line, rest) = self.bytes.split_at(BASE64_WRAP_WIDTH); - let result = grammar::strip_leading_eol(rest) - .ok_or(Error::EncapsulatedText) - .map(|rest| { - self.bytes = rest; - line - }); - Some(result) - } else if !self.bytes.is_empty() { - let line = self.bytes; - self.bytes = &[]; + self.bytes = rest; Some(Ok(line)) } else { - None + // if bytes remaining does not split at BASE64_WRAP_WIDTH such + // that the next char(s) in the rest is vertical whitespace + // then attribute the error generically as `EncapsulatedText` + // unless we are at the first line and the line contains a colon + // then it may be a unsupported header + Some(Err( + if self.is_start && line.iter().any(|&b| b == grammar::CHAR_COLON) { + Error::HeaderDetected + } else { + Error::EncapsulatedText + }, + )) } + } else if !self.bytes.is_empty() { + let line = self.bytes; + self.bytes = &[]; + Some(Ok(line)) + } else { + None } } } diff --git a/pem-rfc7468/src/grammar.rs b/pem-rfc7468/src/grammar.rs index 4ac82cfc..80c2c224 100644 --- a/pem-rfc7468/src/grammar.rs +++ b/pem-rfc7468/src/grammar.rs @@ -36,11 +36,6 @@ pub(crate) fn is_wsp(char: u8) -> bool { matches!(char, CHAR_HT | CHAR_SP) } -/// Does the provided byte match either of vertical whitespace chars CR / LF -pub(crate) fn is_vwsp(char: u8) -> bool { - matches!(char, CHAR_CR | CHAR_LF) -} - /// Split a slice beginning with a type label as located in an encapsulation /// boundary. Returns the label as a `&str`, and slice beginning with the /// encapsulated text with leading `-----` and newline removed. From 50e8557f8e048843c727eb1ae3f1cee4ffdb3969 Mon Sep 17 00:00:00 2001 From: Daniel James Date: Mon, 13 Sep 2021 17:03:42 -0700 Subject: [PATCH 3/3] add test for case when disallowed header is 64 chars long --- pem-rfc7468/tests/decode.rs | 10 +++++++ pem-rfc7468/tests/examples/chosen_header.pem | 31 ++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 pem-rfc7468/tests/examples/chosen_header.pem diff --git a/pem-rfc7468/tests/decode.rs b/pem-rfc7468/tests/decode.rs index 34a0fd3d..7e2e47a2 100644 --- a/pem-rfc7468/tests/decode.rs +++ b/pem-rfc7468/tests/decode.rs @@ -19,6 +19,16 @@ fn pkcs1_enc_example() { } } +#[test] +fn header_of_length_64() { + let pem = include_bytes!("examples/chosen_header.pem"); + let mut buf = [0u8; 2048]; + match pem_rfc7468::decode(pem, &mut buf) { + Err(pem_rfc7468::Error::HeaderDetected) => (), + _ => panic!("Expected HeaderDetected error"), + } +} + #[test] fn pkcs8_example() { let pem = include_bytes!("examples/pkcs8.pem"); diff --git a/pem-rfc7468/tests/examples/chosen_header.pem b/pem-rfc7468/tests/examples/chosen_header.pem new file mode 100644 index 00000000..f4930744 --- /dev/null +++ b/pem-rfc7468/tests/examples/chosen_header.pem @@ -0,0 +1,31 @@ +-----BEGIN RSA PRIVATE KEY----- +A-Header-That-Happens-To-Be-Exactly: 64 characters long......... +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-128-CBC,15670D76FD184D46C40C971733E0543F + +/lVaZdZ2a6x5d3b96F6XpFzcnP35pUrwpKxEB07nF15Jc81jwEAg72OpFTp5QRMu +WXbbZ/dKF7ucGHvLQ/VvCNkbl6oqowSme94fzFsa/xuKRHAGDHVi/TQylIOBBJFv +vru/3EZkO8mAQRDTNfuSl0Y5Ir7uqAQy0E/xKfOdY73BO4//KEDEIshRwBxbOm3K +D2sU1Kp8RnnBgSNydG8AH/LrtBnFs9HWrb9JD0Nj5bIxZDzil5CYmTB8PRgb2Qy7 +bckVc+0Y/h8Ai+NjSc/rJVw0smKJbmNSoPyJH1WjDPW8wWtngCFQWaVCTubm08N1 +nqrzIclT3fnq8YFSbFJYZVPaADxjv2HW7dLH7grYqXx/5FTE34ixXTcwQ1KR0ZQX +uaGZhkiDVWf/q82JPREqH5hwbeGL9QwZHF4/74vKIsddEuVFp8EW7jn9INRoVBtK +/OBiVXmVELFhmVBqvQU7GSci7+fCntXIx6W3hGiJL2WyXfuP16u9BhF5kd+c3pAm +tOZ3Lc5XsceBIYq0rKhy7rDhEg0V8wF1jHeeiW0VKDt2cFePSAd4CIbHiRWbvwh+ +zIoNAB34k4cYShmjOHKem9FMHVHSwfRE39Vrwssj0HWVOp7KdXYv64w4Ywmn6wvA +r6p8IZWg7KqA5UApPpiBVs0BAx1KtZk3o1dvXAazklw23icnnZF6XqaH6EmnVsf9 +gbyK1NcH3lIalTYhs+hMwizkw/XDb1uU8G7Rz1QFKBiL56J8ePIA2NWRUwvdMEAv +rZXSq4Icwy566GIqdtMRNLcz6LthNEg9qg+fD5aGLrtTk8ACSQpb/ELMMzqDVTkI +07dB1Nhzx9nd9mUlIuA030I5w7f//5pS6/lGmmPZblygY1PBludl+p/P9OKJ+Jr0 +HTAI4SVxoYdp6YHDBJ9J7Wt6UnIe+/3WarY9d9X1XNGOE4K+nRFihSShtKHDtMY6 +eBEV1sBTXJ1KANG683CU+uDx2XpOVAwDGl5hyRdzOovNC1iWjSu+CvppDvZLuIMj +zIllu5E8PR2Zd1wIT1gnU/7HiVdM0m4jf6ptkGSWNSCLA0ipii0YYarXoyu0kbMY +BKKpp5QRXv6OwmSDMwQTPuRIWyk839X1ABE1XeTKt43Ns+Wtdboi8Cu/aO/Z5AoA +gbJ+CdyKJIJxDXA11cPq9SF2daYmqHV3agrrKmAwWBRwpCKvotv0Hxw2M1+91ZoU +NY52RraoNVQPOAEfhYNS0ltVPzxcDU5bA2WczO6QzmMl7So6dysw+fxtxaEUGt4m +Fj+p+rE64Okq4wWDlEQya/xu4KMZwzyDncgJHHyYahs+vCv9KbQLW8R0iHTbxQzX +Vhomq++Cm8kg5aA/UsLas/l6ZyfNIcA99U8shFFA5urOKMl/jSRd9v1c7H3nOPZ7 ++eN10E7hcRruwOkoBlpd2It3Y2M+1qBDWXLVSHSXmIuzdE+MZ8CZfvxe+FcfpvJU +BFsZbSEF2PQC+zhd1HjV6DUe3jCz88/rjUnXQCvEJ7z7Tuz3C7kKdR3OYYYLwuLW +LTy2VS0p3QuUeMnNRl0HxpB16BZax9mzFr0UvFKp2QQYzOkIghg2sLNEbtaJvHNh +-----END RSA PRIVATE KEY-----