Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x501+x509: CertificateDocument, X509Version, and more additions #393

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f2ab456
add Display impl for Time
carl-wallace Feb 5, 2022
213e9a2
refactor Display to use new to_date_time
carl-wallace Feb 5, 2022
945bfe9
move algorithm OID definitions to x509 from certval
carl-wallace Feb 7, 2022
c2d3b40
move OID lookup from certval (and yubikey), refactor to have a &'stat…
carl-wallace Feb 7, 2022
40cf4aa
add version enum and add constraints to govern decode/encode relative…
carl-wallace Feb 7, 2022
7504101
clippy suggestions
carl-wallace Feb 7, 2022
30c77d0
address unused import on alloc feature
carl-wallace Feb 7, 2022
66e6b9a
add links to doc comment
carl-wallace Feb 7, 2022
89dc750
use const length of 1 vs calculated
carl-wallace Feb 7, 2022
b34f48b
Merge branch 'master' into yubikey_mods
carl-wallace Feb 7, 2022
bc391a7
address cargo-hack findings
carl-wallace Feb 7, 2022
c2f9377
Merge branch 'yubikey_mods' of github.com:carl-wallace/formats into y…
carl-wallace Feb 7, 2022
1e44e55
rename new version enum, add a default, delete an unused const
carl-wallace Feb 7, 2022
fb203b6
reorder derive attributes on version
carl-wallace Feb 7, 2022
ee9f1a3
use EncodeValue/DecodeValue instead of Encodable/Decodable for Version
carl-wallace Feb 8, 2022
c226d37
fix bug in value_len (was still implemented as if encoded_len). drop …
carl-wallace Feb 8, 2022
b600b99
refactor version handling (including adding proper support for v1), a…
carl-wallace Feb 8, 2022
6fe650f
fix visibility on version default function
carl-wallace Feb 8, 2022
a9dab3e
drop traits for certificate as unnecessary. move PEM support to impls…
carl-wallace Feb 8, 2022
b7a19de
remove redundant impls
carl-wallace Feb 9, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x501/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl Time {
}
}

/// Get Time as DateTime
/// Get [`Time`] as [`DateTime`]
pub fn to_date_time(&self) -> DateTime {
match self {
Time::UtcTime(t) => t.to_date_time(),
Expand Down
2 changes: 2 additions & 0 deletions x509/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ x501 = { version = "=0.1.0-pre.0", path = "../x501" }
hex-literal = "0.3"

[features]
alloc = ["der/alloc"]
std = ["der/std"]
pem = ["alloc", "der/pem"]

[package.metadata.docs.rs]
all-features = true
Expand Down
144 changes: 118 additions & 26 deletions x509/src/certificate.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
//! Certificate [`Certificate`] and TBSCertificate [`TBSCertificate`] as defined in RFC 5280

use der::asn1::{BitString, ContextSpecific, ObjectIdentifier, UIntBytes};
use der::{Sequence, TagMode, TagNumber};
use der::{
DecodeValue, Decoder, EncodeValue, Encoder, Error, FixedTag, Header, Length, Sequence, Tag,
TagMode, TagNumber,
};
use spki::{AlgorithmIdentifier, SubjectPublicKeyInfo};
use x501::name::Name;
use x501::time::Validity;

/// returns false in support of integer DEFAULT fields set to 0
pub fn default_zero_u8() -> u8 {
0
fn default_v1() -> Version {
Version::V1
}

/// returns false in support of boolean DEFAULT fields
Expand All @@ -21,9 +24,65 @@ pub fn default_zero() -> u32 {
0
}

/// only support v3 certificates
/// only supporting v3 certificates, but all versions are listed here
/// Version ::= INTEGER { v1(0), v2(1), v3(2) }
pub const X509_CERT_VERSION: u8 = 2;

/// Version identifier for X.509 certificates. In practice, only v3 is used.
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd)]
pub enum Version {
/// Denotes X.509 v1
V1 = 0,

/// Denotes X.509 v2 - added issuerUniqueID and subjectUniqueID (both of which have been deprecated)
V2 = 1,

/// Denotes X.509 v3 - add extensions
V3 = 2,
}

impl Default for Version {
fn default() -> Self {
Version::V3
}
}

impl DecodeValue<'_> for Version {
fn decode_value(decoder: &mut Decoder<'_>, header: Header) -> der::Result<Self> {
Version::try_from(u8::decode_value(decoder, header)?).map_err(|_| Self::TAG.value_error())
}
}

impl EncodeValue for Version {
fn value_len(&self) -> der::Result<der::Length> {
Ok(Length::ONE)
}

fn encode_value(&self, encoder: &mut Encoder<'_>) -> der::Result<()> {
u8::from(*self).encode_value(encoder)
}
}

impl FixedTag for Version {
const TAG: Tag = Tag::Integer;
}

impl From<Version> for u8 {
fn from(version: Version) -> Self {
version as u8
}
}

impl TryFrom<u8> for Version {
type Error = Error;
fn try_from(byte: u8) -> Result<Version, Error> {
match byte {
0 => Ok(Version::V1),
1 => Ok(Version::V2),
2 => Ok(Version::V3),
_ => Err(Self::TAG.value_error()),
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carl-wallace All the changes in this file are obsolete if we merge #410 instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Further review of this until after alignment with changes to dependencies is likely not worthwhile.


/// X.509 `TBSCertificate` as defined in [RFC 5280 Section 4.1.2.5]
///
Expand Down Expand Up @@ -53,8 +112,8 @@ pub const X509_CERT_VERSION: u8 = 2;
#[derive(Clone, Eq, PartialEq)]
pub struct TBSCertificate<'a> {
/// version [0] Version DEFAULT v1,
//#[asn1(context_specific = "0", default = "default_zero_u8")]
pub version: u8,
//#[asn1(context_specific = "0", default = "default_v1")]
pub version: Version,
/// serialNumber CertificateSerialNumber,
pub serial_number: UIntBytes<'a>,
/// signature AlgorithmIdentifier{SIGNATURE-ALGORITHM, {SignatureAlgorithms}},
Expand Down Expand Up @@ -83,22 +142,33 @@ impl<'a> ::der::Decodable<'a> for TBSCertificate<'a> {
let version =
::der::asn1::ContextSpecific::decode_explicit(decoder, ::der::TagNumber::N0)?
.map(|cs| cs.value)
.unwrap_or_else(default_zero_u8);
.unwrap_or_else(default_v1);
let serial_number = decoder.decode()?;
let signature = decoder.decode()?;
let issuer = decoder.decode()?;
let validity = decoder.decode()?;
let subject = decoder.decode()?;
let subject_public_key_info = decoder.decode()?;
let issuer_unique_id =

let issuer_unique_id = if Version::V2 <= version {
::der::asn1::ContextSpecific::decode_implicit(decoder, ::der::TagNumber::N1)?
.map(|cs| cs.value);
let subject_unique_id =
.map(|cs| cs.value)
} else {
None
};
let subject_unique_id = if Version::V2 <= version {
::der::asn1::ContextSpecific::decode_implicit(decoder, ::der::TagNumber::N2)?
.map(|cs| cs.value);
let extensions =
.map(|cs| cs.value)
} else {
None
};
let extensions = if Version::V3 <= version {
::der::asn1::ContextSpecific::decode_explicit(decoder, ::der::TagNumber::N3)?
.map(|cs| cs.value);
.map(|cs| cs.value)
} else {
None
};

Ok(Self {
version,
serial_number,
Expand All @@ -121,27 +191,49 @@ impl<'a> ::der::Sequence<'a> for TBSCertificate<'a> {
where
F: FnOnce(&[&dyn der::Encodable]) -> ::der::Result<T>,
{
let version = &ContextSpecific {
tag_number: VERSION_TAG,
tag_mode: TagMode::Explicit,
value: self.version,
};

let issuer_unique_id = if Version::V2 <= self.version {
&self.issuer_unique_id
} else {
&None
};
let subject_unique_id = if Version::V2 <= self.version {
&self.subject_unique_id
} else {
&None
};
let extensions = if Version::V3 <= self.version {
self.extensions.as_ref().map(|exts| ContextSpecific {
tag_number: EXTENSIONS_TAG,
tag_mode: TagMode::Explicit,
value: exts.clone(),
})
} else {
None
};

#[allow(unused_imports)]
use core::convert::TryFrom;
f(&[
&ContextSpecific {
tag_number: VERSION_TAG,
tag_mode: TagMode::Explicit,
value: self.version,
},
&::der::asn1::OptionalRef(if self.version == Version::V1 {
None
} else {
Some(version)
}),
&self.serial_number,
&self.signature,
&self.issuer,
&self.validity,
&self.subject,
&self.subject_public_key_info,
&self.issuer_unique_id,
&self.subject_unique_id,
&self.extensions.as_ref().map(|exts| ContextSpecific {
tag_number: EXTENSIONS_TAG,
tag_mode: TagMode::Explicit,
value: exts.clone(),
}),
issuer_unique_id,
subject_unique_id,
&extensions,
])
}
}
Expand Down
97 changes: 97 additions & 0 deletions x509/src/certificate_document.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
//! CertificateDocument implementation

use crate::Certificate;
use der::{Error, Result};

use alloc::vec::Vec;
use core::fmt;
use der::{Decodable, Document};

#[cfg(feature = "pem")]
use {core::str::FromStr, der::pem};

/// Certificate document.
///
/// This type provides storage for [`Certificate`] encoded as ASN.1
/// DER with the invariant that the contained-document is "well-formed", i.e.
/// it will parse successfully according to this crate's parsing rules.
#[derive(Clone)]
#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
pub struct CertificateDocument(Vec<u8>);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tarcieri I know I've implemented one of these document types for pkcs10. However:

  1. What is the purpose of them? I don't understand how they are intended to be used. It seems that PEM decoding is implemented through the document types. However, in order to instantiate a document, you have to fully parse the DER. So why not just have the fully parsed type?

  2. If there is a good reason for the document types, surely these can be genericized. They are all exactly the same code and differ only by the type of the value decoded therein. So Document<T: Decodable + Encodable> seems like an obvious win here.

Copy link
Member

@tarcieri tarcieri Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a complicated and important enough issue I did a long-form write up on it, so as not to lose the answers to these questions in a PR comment: #421

Some short answers though:

  1. The purpose of these types is discussed at length in der: *Document type improvements #421, but in regard to PEM specifically, since Decode<'a> is zero-copy and needs to borrow from a slice containing DER, parsing PEM needs to decode DER into a buffer that Decode<'a> can borrow from. If we had something like DecodeOwned which didn't need to borrow, documents could be decoded directly from PEM. As it were, this is how the ssh-key crate works, and I just added support for incremental/buffered PEM decoding to the pem-rfc7468 crate (pem-rfc7468: buffered Base64 decoder #406) and changed the ssh-key crate to use that (ssh-key: use pem-rfc7468 for private key PEM parser #407). But that only works because the ssh-key crate is built entirely around owned types (and doesn't use ASN.1 DER).

  2. The problem with Document<T: Decodable + Encodable> is you omitted the lifetime on Decodable<'a>. Since it's a 'self borrow and Rust doesn't actually have a way to notate a 'self lifetime you can't notate that on a struct definition, however as I noted in der: *Document type improvements #421 it can probably be done by moving the bounds to individual methods so Decodable<'a> can hang off pub fn decode<'a>(&'a self) -> T where T: Decode<'a>


impl<'a> TryFrom<&'a [u8]> for Certificate<'a> {
type Error = Error;

fn try_from(bytes: &'a [u8]) -> Result<Self> {
Self::from_der(bytes)
}
}

impl<'a> Document<'a> for CertificateDocument {
type Message = Certificate<'a>;
const SENSITIVE: bool = false;
}

impl AsRef<[u8]> for CertificateDocument {
fn as_ref(&self) -> &[u8] {
self.0.as_ref()
}
}

impl TryFrom<&[u8]> for CertificateDocument {
type Error = Error;

fn try_from(bytes: &[u8]) -> Result<Self> {
Self::from_der(bytes)
}
}

impl TryFrom<Certificate<'_>> for CertificateDocument {
type Error = Error;

fn try_from(cert: Certificate<'_>) -> Result<CertificateDocument> {
Self::try_from(&cert)
}
}

impl TryFrom<&Certificate<'_>> for CertificateDocument {
type Error = Error;

fn try_from(cert: &Certificate<'_>) -> Result<CertificateDocument> {
Self::from_msg(cert)
}
}

impl TryFrom<Vec<u8>> for CertificateDocument {
type Error = der::Error;

fn try_from(bytes: Vec<u8>) -> der::Result<Self> {
// Ensure document is well-formed
Certificate::from_der(bytes.as_slice())?;
Ok(Self(bytes))
}
}

impl fmt::Debug for CertificateDocument {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt.debug_tuple("CertificateDocument")
.field(&self.decode())
.finish()
}
}

#[cfg(feature = "pem")]
#[cfg_attr(docsrs, doc(cfg(feature = "pem")))]
impl FromStr for CertificateDocument {
type Err = Error;

fn from_str(s: &str) -> Result<Self> {
Self::from_pem(s)
}
}

#[cfg(feature = "pem")]
#[cfg_attr(docsrs, doc(cfg(feature = "pem")))]
impl pem::PemLabel for CertificateDocument {
const TYPE_LABEL: &'static str = "CERTIFICATE";
}
5 changes: 3 additions & 2 deletions x509/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ extern crate alloc;
#[cfg(feature = "std")]
extern crate std;

mod certificate;
pub mod certificate;
pub mod certificate_document;
pub mod extensions_utils;
mod general_name;
pub mod general_name;
pub mod pkix_extensions;
pub mod pkix_oids;
pub mod trust_anchor_format;
Expand Down
Loading