From 7d9c8e2522e1f331dfbc7a2dbaf200a79ffffb69 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Fri, 11 Feb 2022 19:27:42 -0700 Subject: [PATCH 1/2] [WIP] der: generic `Document` type Changes `Document` from a trait into a generic type. The previous approach necessitated a lot of repetitive boilerplate for each "document" type. Using a generic type eliminates the boilerplate. The definition was somewhat tricky due to the lifetime on `Decodable<'a>`. For more context, see #421. --- Cargo.lock | 1 + der/Cargo.toml | 1 + der/src/document.rs | 172 +++++++++++++++++++++++++++++++++----------- der/src/lib.rs | 2 +- 4 files changed, 132 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5dffe744d..ff97f8c39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -303,6 +303,7 @@ dependencies = [ "pem-rfc7468", "proptest", "time", + "zeroize", ] [[package]] diff --git a/der/Cargo.toml b/der/Cargo.toml index d832b681c..f086a53a8 100644 --- a/der/Cargo.toml +++ b/der/Cargo.toml @@ -21,6 +21,7 @@ der_derive = { version = "=0.6.0-pre.1", optional = true, path = "derive" } flagset = { version = "0.4.3", optional = true } pem-rfc7468 = { version = "=0.4.0-pre.0", optional = true, path = "../pem-rfc7468" } time = { version = "0.3", optional = true, default-features = false } +zeroize = { version = "1", optional = true, default-features = false } [dev-dependencies] hex-literal = "0.3" diff --git a/der/src/document.rs b/der/src/document.rs index c2266d3ab..51b5c26e0 100644 --- a/der/src/document.rs +++ b/der/src/document.rs @@ -1,67 +1,103 @@ //! ASN.1 DER-encoded documents stored on the heap. use crate::{Decodable, Encodable, Error, Result}; -use alloc::{boxed::Box, vec::Vec}; +use alloc::vec::Vec; +use core::marker::PhantomData; #[cfg(feature = "pem")] -use {crate::pem, alloc::string::String}; +use { + crate::pem::{self, PemLabel}, + alloc::string::String, +}; #[cfg(feature = "std")] use std::{fs, path::Path}; +/// Marker trait which identifies whether or not documents contain sensitive +/// information, such as private keys. +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] +pub trait Sensitivity { + /// Does this type contain potentially sensitive data? + /// + /// This enables hardened file permissions when persisting data to disk. + const SENSITIVE: bool; +} + /// ASN.1 DER-encoded document. /// -/// This trait is intended to impl on types which contain an ASN.1 DER-encoded -/// document which is guaranteed to encode as the associated `Message` type. +/// This type wraps an encoded ASN.1 DER message which is guaranteed to +/// infallibly decode as type `T`. /// /// It implements common functionality related to encoding/decoding such /// documents, such as PEM encapsulation as well as reading/writing documents /// from/to the filesystem. #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -pub trait Document<'a>: AsRef<[u8]> + Sized + TryFrom, Error = Error> { - /// ASN.1 message type this document decodes to. - type Message: Decodable<'a> + Encodable + Sized; +pub struct Document { + /// ASN.1 DER-encoded document guaranteed to decode to `T` infallibly. + der_bytes: Vec, - /// Does this type contain potentially sensitive data? - /// - /// This enables hardened file permissions when persisting data to disk. - const SENSITIVE: bool; + /// Rust type corresponding to the ASN.1 DER message the bytes can be + /// infallibly deserialized as. + msg_type: PhantomData, +} - /// Borrow the inner serialized bytes of this document. - fn as_der(&self) -> &[u8] { - self.as_ref() - } +impl Document where T: Sensitivity {} - /// Return an allocated ASN.1 DER serialization as a boxed slice. - fn to_der(&self) -> Box<[u8]> { - self.as_ref().to_vec().into_boxed_slice() +impl Document +where + T: Sensitivity, +{ + /// Borrow the inner serialized bytes of this document. + pub fn as_der(&self) -> &[u8] { + self.der_bytes.as_slice() } /// Decode this document as ASN.1 DER. - fn decode(&'a self) -> Self::Message { - Self::Message::from_der(self.as_ref()).expect("ASN.1 DER document malformed") + pub fn decode<'a>(&'a self) -> T + where + T: Decodable<'a> + Sized, + { + self.try_decode().expect("ASN.1 DER document malformed") } /// Create a new document from the provided ASN.1 DER bytes. - fn from_der(bytes: &[u8]) -> Result { - bytes.to_vec().try_into() + pub fn from_der(bytes: impl Into>) -> Result + where + T: for<'a> Decodable<'a> + Sized, + { + let doc = Self { + der_bytes: bytes.into(), + msg_type: PhantomData, + }; + + // Ensure document parses successfully + doc.try_decode()?; + Ok(doc) + } + + /// Return an allocated ASN.1 DER serialization as a byte vector. + pub fn to_der(&self) -> Vec { + self.der_bytes.clone() } /// Encode the provided type as ASN.1 DER. - fn from_msg(msg: &Self::Message) -> Result { + pub fn from_msg(msg: &T) -> Result + where + T: for<'a> Decodable<'a> + Encodable + Sized, + { msg.to_vec()?.try_into() } /// Decode ASN.1 DER document from PEM. #[cfg(feature = "pem")] #[cfg_attr(docsrs, doc(cfg(feature = "pem")))] - fn from_pem(s: &str) -> Result + pub fn from_pem(s: &str) -> Result where - Self: pem::PemLabel, + T: for<'a> Decodable<'a> + PemLabel + Sized, { let (label, der_bytes) = pem::decode_vec(s.as_bytes())?; - if label != Self::TYPE_LABEL { + if label != T::TYPE_LABEL { return Err(pem::Error::Label.into()); } @@ -71,49 +107,99 @@ pub trait Document<'a>: AsRef<[u8]> + Sized + TryFrom, Error = Error> { /// Encode ASN.1 DER document as a PEM string. #[cfg(feature = "pem")] #[cfg_attr(docsrs, doc(cfg(feature = "pem")))] - fn to_pem(&self, line_ending: pem::LineEnding) -> Result + pub fn to_pem(&self, line_ending: pem::LineEnding) -> Result where - Self: pem::PemLabel, + T: PemLabel, { Ok(pem::encode_string( - Self::TYPE_LABEL, + T::TYPE_LABEL, line_ending, - self.as_ref(), + self.as_der(), )?) } /// Read ASN.1 DER document from a file. #[cfg(feature = "std")] #[cfg_attr(docsrs, doc(cfg(feature = "std")))] - fn read_der_file(path: impl AsRef) -> Result { + pub fn read_der_file(path: impl AsRef) -> Result + where + T: for<'a> Decodable<'a> + Sized, + { fs::read(path)?.try_into() } + /// Write ASN.1 DER document to a file. + #[cfg(feature = "std")] + #[cfg_attr(docsrs, doc(cfg(feature = "std")))] + pub fn write_der_file(&self, path: impl AsRef) -> Result<()> { + write_file(path, self.as_der(), T::SENSITIVE) + } + /// Read PEM-encoded ASN.1 DER document from a file. #[cfg(all(feature = "pem", feature = "std"))] #[cfg_attr(docsrs, doc(cfg(all(feature = "pem", feature = "std"))))] - fn read_pem_file(path: impl AsRef) -> Result + pub fn read_pem_file(path: impl AsRef) -> Result where - Self: pem::PemLabel, + T: for<'a> Decodable<'a> + PemLabel + Sized, { Self::from_pem(&fs::read_to_string(path)?) } - /// Write ASN.1 DER document to a file. - #[cfg(feature = "std")] - #[cfg_attr(docsrs, doc(cfg(feature = "std")))] - fn write_der_file(&self, path: impl AsRef) -> Result<()> { - write_file(path, self.as_ref(), Self::SENSITIVE) - } - /// Write PEM-encoded ASN.1 DER document to a file. #[cfg(all(feature = "pem", feature = "std"))] #[cfg_attr(docsrs, doc(cfg(all(feature = "pem", feature = "std"))))] - fn write_pem_file(&self, path: impl AsRef, line_ending: pem::LineEnding) -> Result<()> + pub fn write_pem_file(&self, path: impl AsRef, line_ending: pem::LineEnding) -> Result<()> where - Self: pem::PemLabel, + T: PemLabel, { - write_file(path, self.to_pem(line_ending)?.as_bytes(), Self::SENSITIVE) + write_file(path, self.to_pem(line_ending)?.as_bytes(), T::SENSITIVE) + } + + /// Attempt to decode `self.der_bytes` as `T`. + /// + /// This method doesn't uphold the invariant that `T` always decodes + /// successfully, but is needed to make the lifetimes for the constructor + /// work. + fn try_decode<'a>(&'a self) -> Result + where + T: Decodable<'a> + Sized, + { + T::from_der(self.as_der()) + } +} + +impl AsRef<[u8]> for Document +where + T: Sensitivity, +{ + fn as_ref(&self) -> &[u8] { + self.as_der() + } +} + +// NOTE: `Drop` is defined unconditionally to ensure bounds do not change based +// on selected cargo features, which would not be a purely additive change +impl Drop for Document +where + T: Sensitivity, +{ + fn drop(&mut self) { + #[cfg(feature = "zeroize")] + if T::SENSITIVE { + use zeroize::Zeroize; + self.der_bytes.zeroize(); + } + } +} + +impl TryFrom> for Document +where + T: for<'a> Decodable<'a> + Sensitivity + Sized, +{ + type Error = Error; + + fn try_from(der_bytes: Vec) -> Result> { + Self::from_der(der_bytes) } } diff --git a/der/src/lib.rs b/der/src/lib.rs index 36fd6a5b1..365ede50c 100644 --- a/der/src/lib.rs +++ b/der/src/lib.rs @@ -367,7 +367,7 @@ pub use crate::{ }; #[cfg(feature = "alloc")] -pub use document::Document; +pub use document::{Document, Sensitivity}; #[cfg(feature = "bigint")] #[cfg_attr(docsrs, doc(cfg(feature = "bigint")))] From 3cb486dd73713ee7cb9eac1d5e57767c1b166af8 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Sat, 12 Feb 2022 07:32:47 -0700 Subject: [PATCH 2/2] Use a const generic SENSITIVE parameter --- der/src/document.rs | 86 ++++++++++++++++++++++++++++----------------- der/src/lib.rs | 2 +- 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/der/src/document.rs b/der/src/document.rs index 51b5c26e0..df6eb4aac 100644 --- a/der/src/document.rs +++ b/der/src/document.rs @@ -2,27 +2,21 @@ use crate::{Decodable, Encodable, Error, Result}; use alloc::vec::Vec; -use core::marker::PhantomData; +use core::{ + fmt::{self, Debug}, + marker::PhantomData, +}; #[cfg(feature = "pem")] use { crate::pem::{self, PemLabel}, alloc::string::String, + core::str::FromStr, }; #[cfg(feature = "std")] use std::{fs, path::Path}; -/// Marker trait which identifies whether or not documents contain sensitive -/// information, such as private keys. -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -pub trait Sensitivity { - /// Does this type contain potentially sensitive data? - /// - /// This enables hardened file permissions when persisting data to disk. - const SENSITIVE: bool; -} - /// ASN.1 DER-encoded document. /// /// This type wraps an encoded ASN.1 DER message which is guaranteed to @@ -32,7 +26,7 @@ pub trait Sensitivity { /// documents, such as PEM encapsulation as well as reading/writing documents /// from/to the filesystem. #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -pub struct Document { +pub struct Document { /// ASN.1 DER-encoded document guaranteed to decode to `T` infallibly. der_bytes: Vec, @@ -41,12 +35,7 @@ pub struct Document { msg_type: PhantomData, } -impl Document where T: Sensitivity {} - -impl Document -where - T: Sensitivity, -{ +impl Document { /// Borrow the inner serialized bytes of this document. pub fn as_der(&self) -> &[u8] { self.der_bytes.as_slice() @@ -132,7 +121,7 @@ where #[cfg(feature = "std")] #[cfg_attr(docsrs, doc(cfg(feature = "std")))] pub fn write_der_file(&self, path: impl AsRef) -> Result<()> { - write_file(path, self.as_der(), T::SENSITIVE) + write_file(path, self.as_der(), SENSITIVE) } /// Read PEM-encoded ASN.1 DER document from a file. @@ -152,7 +141,7 @@ where where T: PemLabel, { - write_file(path, self.to_pem(line_ending)?.as_bytes(), T::SENSITIVE) + write_file(path, self.to_pem(line_ending)?.as_bytes(), SENSITIVE) } /// Attempt to decode `self.der_bytes` as `T`. @@ -168,41 +157,74 @@ where } } -impl AsRef<[u8]> for Document -where - T: Sensitivity, -{ +impl AsRef<[u8]> for Document { fn as_ref(&self) -> &[u8] { self.as_der() } } -// NOTE: `Drop` is defined unconditionally to ensure bounds do not change based -// on selected cargo features, which would not be a purely additive change -impl Drop for Document +impl Debug for Document where - T: Sensitivity, + T: for<'a> Decodable<'a> + Debug + Sized, { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_tuple("Document").field(&self.decode()).finish() + } +} + +impl Debug for Document { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("Document").finish_non_exhaustive() + } +} + +// NOTE: `Drop` is defined unconditionally to ensure bounds do not change based +// on selected cargo features, which would not be a purely additive change +impl Drop for Document { fn drop(&mut self) { #[cfg(feature = "zeroize")] - if T::SENSITIVE { + if SENSITIVE { use zeroize::Zeroize; self.der_bytes.zeroize(); } } } -impl TryFrom> for Document +impl TryFrom<&[u8]> for Document where - T: for<'a> Decodable<'a> + Sensitivity + Sized, + T: for<'a> Decodable<'a> + Sized, { type Error = Error; - fn try_from(der_bytes: Vec) -> Result> { + fn try_from(der_bytes: &[u8]) -> Result { Self::from_der(der_bytes) } } +impl TryFrom> for Document +where + T: for<'a> Decodable<'a> + Sized, +{ + type Error = Error; + + fn try_from(der_bytes: Vec) -> Result { + Self::from_der(der_bytes) + } +} + +#[cfg(feature = "pem")] +#[cfg_attr(docsrs, doc(cfg(feature = "pem")))] +impl FromStr for Document +where + T: for<'a> Decodable<'a> + PemLabel + Sized, +{ + type Err = Error; + + fn from_str(s: &str) -> Result { + Self::from_pem(s) + } +} + /// Write a file to the filesystem, potentially using hardened permissions /// if the file contains secret data. #[cfg(feature = "std")] diff --git a/der/src/lib.rs b/der/src/lib.rs index 365ede50c..36fd6a5b1 100644 --- a/der/src/lib.rs +++ b/der/src/lib.rs @@ -367,7 +367,7 @@ pub use crate::{ }; #[cfg(feature = "alloc")] -pub use document::{Document, Sensitivity}; +pub use document::Document; #[cfg(feature = "bigint")] #[cfg_attr(docsrs, doc(cfg(feature = "bigint")))]