From 47cf2328ff3add55c3cebba108f1b7ef965fb85e Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Wed, 9 Feb 2022 11:42:45 -0700 Subject: [PATCH] ssh-key: use `pem-rfc7468` for private key PEM parser Now that `pem-rfc7468` supports a buffered Base64 decoder (#406), it's possible to use it for parsing PEM-formatted OpenSSH private keys. This commit switches to using `pem-rfc7468` for PEM parsing. --- Cargo.lock | 1 + ssh-key/Cargo.toml | 1 + ssh-key/src/base64.rs | 16 ++++---- ssh-key/src/error.rs | 6 +++ ssh-key/src/private.rs | 45 ++++++++++++--------- ssh-key/src/private/openssh.rs | 74 ---------------------------------- 6 files changed, 43 insertions(+), 100 deletions(-) delete mode 100644 ssh-key/src/private/openssh.rs diff --git a/Cargo.lock b/Cargo.lock index f17d2fec5..f5261f236 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -961,6 +961,7 @@ version = "0.3.0-pre" dependencies = [ "base64ct", "hex-literal", + "pem-rfc7468", "sec1", "zeroize", ] diff --git a/ssh-key/Cargo.toml b/ssh-key/Cargo.toml index f6797ac5e..75d53dd49 100644 --- a/ssh-key/Cargo.toml +++ b/ssh-key/Cargo.toml @@ -17,6 +17,7 @@ rust-version = "1.57" [dependencies] base64ct = { version = "=1.4.0-pre.0", path = "../base64ct" } +pem-rfc7468 = { version = "=0.4.0-pre.0", path = "../pem-rfc7468" } zeroize = { version = "1", default-features = false } # optional dependencies diff --git a/ssh-key/src/base64.rs b/ssh-key/src/base64.rs index 30e12acd6..ef8b8f4d8 100644 --- a/ssh-key/src/base64.rs +++ b/ssh-key/src/base64.rs @@ -32,14 +32,6 @@ impl<'i> Decoder<'i> { }) } - /// Create a new decoder for a byte slice containing Base64 which - /// line wraps at the given line length. - pub fn new_wrapped(input: &'i [u8], line_width: usize) -> Result { - Ok(Self { - inner: base64ct::Decoder::new_wrapped(input, line_width)?, - }) - } - /// Decode as much Base64 as is needed to exactly fill `out`. /// /// # Returns @@ -146,6 +138,14 @@ impl<'i> Decoder<'i> { } } +impl<'i> From> for Decoder<'i> { + fn from(decoder: pem_rfc7468::Decoder<'i>) -> Decoder<'i> { + Decoder { + inner: decoder.into(), + } + } +} + /// Encoder trait. pub(crate) trait Encode: Sized { /// Get the length of this type encoded in bytes, prior to Base64 encoding. diff --git a/ssh-key/src/error.rs b/ssh-key/src/error.rs index 4ae42edce..fe3dd121d 100644 --- a/ssh-key/src/error.rs +++ b/ssh-key/src/error.rs @@ -85,6 +85,12 @@ impl From for Error { } } +impl From for Error { + fn from(_: pem_rfc7468::Error) -> Error { + Error::Pem + } +} + #[cfg(feature = "alloc")] #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl From for Error { diff --git a/ssh-key/src/private.rs b/ssh-key/src/private.rs index 922e9f077..b6b298a56 100644 --- a/ssh-key/src/private.rs +++ b/ssh-key/src/private.rs @@ -9,7 +9,6 @@ mod dsa; #[cfg(feature = "ecdsa")] mod ecdsa; mod ed25519; -mod openssh; #[cfg(feature = "alloc")] mod rsa; @@ -27,10 +26,14 @@ use crate::{ public, Algorithm, CipherAlg, Error, KdfAlg, KdfOptions, Result, }; use core::str::FromStr; +use pem_rfc7468::{self as pem, PemLabel}; #[cfg(feature = "alloc")] use alloc::string::String; +/// Line width used by the PEM encoding of OpenSSH private keys +const PEM_LINE_WIDTH: usize = 70; + /// SSH private key. #[derive(Clone, Debug)] pub struct PrivateKey { @@ -64,23 +67,25 @@ impl PrivateKey { /// -----BEGIN OPENSSH PRIVATE KEY----- /// ``` pub fn from_openssh(input: impl AsRef<[u8]>) -> Result { - let encapsulation = openssh::Encapsulation::decode(input.as_ref())?; - let mut decoder = base64::Decoder::new_wrapped( - encapsulation.base64_data, - openssh::Encapsulation::LINE_WIDTH, - )?; + let pem_decoder = pem::Decoder::new_wrapped(input.as_ref(), PEM_LINE_WIDTH)?; + + if pem_decoder.type_label() != Self::TYPE_LABEL { + return Err(Error::Pem); + } + + let mut base64_decoder = base64::Decoder::from(pem_decoder); let mut auth_magic = [0u8; Self::AUTH_MAGIC.len()]; - decoder.decode_into(&mut auth_magic)?; + base64_decoder.decode_into(&mut auth_magic)?; if auth_magic != Self::AUTH_MAGIC { return Err(Error::FormatEncoding); } - let cipher_alg = CipherAlg::decode(&mut decoder)?; - let kdf_alg = KdfAlg::decode(&mut decoder)?; - let kdf_options = KdfOptions::decode(&mut decoder)?; - let nkeys = decoder.decode_u32()? as usize; + let cipher_alg = CipherAlg::decode(&mut base64_decoder)?; + let kdf_alg = KdfAlg::decode(&mut base64_decoder)?; + let kdf_options = KdfOptions::decode(&mut base64_decoder)?; + let nkeys = base64_decoder.decode_u32()? as usize; // TODO(tarcieri): support more than one key? if nkeys != 1 { @@ -89,26 +94,26 @@ impl PrivateKey { for _ in 0..nkeys { // TODO(tarcieri): validate decoded length - let _len = decoder.decode_u32()? as usize; - let _pubkey = public::KeyData::decode(&mut decoder)?; + let _len = base64_decoder.decode_u32()? as usize; + let _pubkey = public::KeyData::decode(&mut base64_decoder)?; } // Begin decoding unencrypted list of N private keys // See OpenSSH PROTOCOL.key ยง 3 // TODO(tarcieri): validate decoded length - let _len = decoder.decode_u32()? as usize; - let checkint1 = decoder.decode_u32()?; - let checkint2 = decoder.decode_u32()?; + let _len = base64_decoder.decode_u32()? as usize; + let checkint1 = base64_decoder.decode_u32()?; + let checkint2 = base64_decoder.decode_u32()?; if checkint1 != checkint2 { // TODO(tarcieri): treat this as a cryptographic error? return Err(Error::FormatEncoding); } - let key_data = KeypairData::decode(&mut decoder)?; + let key_data = KeypairData::decode(&mut base64_decoder)?; #[cfg(feature = "alloc")] - let comment = decoder.decode_string()?; + let comment = base64_decoder.decode_string()?; // TODO(tarcieri): parse/validate padding bytes? Ok(Self { @@ -135,6 +140,10 @@ impl FromStr for PrivateKey { } } +impl PemLabel for PrivateKey { + const TYPE_LABEL: &'static str = "OPENSSH PRIVATE KEY"; +} + /// Private key data. #[derive(Clone, Debug)] #[non_exhaustive] diff --git a/ssh-key/src/private/openssh.rs b/ssh-key/src/private/openssh.rs deleted file mode 100644 index 3915e77fe..000000000 --- a/ssh-key/src/private/openssh.rs +++ /dev/null @@ -1,74 +0,0 @@ -//! Support for OpenSSH-formatted private keys. -//! -//! These keys are PEM-encoded and begin with the following: -//! -//! ```text -//! -----BEGIN OPENSSH PRIVATE KEY----- -//! ``` - -use crate::{Error, Result}; - -/// Carriage return -pub(crate) const CHAR_CR: u8 = 0x0d; - -/// Line feed -pub(crate) const CHAR_LF: u8 = 0x0a; - -/// Pre-encapsulation boundary. -const PRE_ENCAPSULATION_BOUNDARY: &[u8] = b"-----BEGIN OPENSSH PRIVATE KEY-----"; - -/// Post-encapsulation boundary. -const POST_ENCAPSULATION_BOUNDARY: &[u8] = b"-----END OPENSSH PRIVATE KEY-----"; - -/// OpenSSH private key encapsulation parser. -pub(super) struct Encapsulation<'a> { - /// Base64-encoded key data - pub(super) base64_data: &'a [u8], -} - -impl<'a> Encapsulation<'a> { - /// Width at which the Base64 is line wrapped. - pub(super) const LINE_WIDTH: usize = 70; - - /// Parse the given PEM-encapsulated data. - pub(super) fn decode(input: &'a [u8]) -> Result { - // Parse pre-encapsulation boundary (including label) - let input = strip_leading_eol(input) - .unwrap_or(input) - .strip_prefix(PRE_ENCAPSULATION_BOUNDARY) - .ok_or(Error::Pem) - .and_then(strip_leading_eol)?; - - // Parse post-encapsulation boundary and optional trailing newline - let base64_data = strip_trailing_eol(input) - .strip_suffix(POST_ENCAPSULATION_BOUNDARY) - .ok_or(Error::Pem)?; - - Ok(Self { base64_data }) - } -} - -/// Strip a newline (`eol`) from the beginning of the provided byte slice. -/// -/// The newline is considered mandatory and a decoding error will occur if it -/// is not present. -pub(crate) fn strip_leading_eol(bytes: &[u8]) -> Result<&[u8]> { - match bytes { - [CHAR_LF, rest @ ..] => Ok(rest), - [CHAR_CR, CHAR_LF, rest @ ..] => Ok(rest), - [CHAR_CR, rest @ ..] => Ok(rest), - _ => Err(Error::Pem), - } -} - -/// Strip a newline (`eol`) from the end of the provided byte slice if present. -/// -/// Returns the original slice if there is no newline. -pub(crate) fn strip_trailing_eol(bytes: &[u8]) -> &[u8] { - match bytes { - [head @ .., CHAR_CR, CHAR_LF] => head, - [head @ .., CHAR_LF] => head, - [head @ .., CHAR_CR] => head, - _ => bytes, - } -}