Skip to content

Commit

Permalink
Misc. improvements for x509-certs (#841)
Browse files Browse the repository at this point in the history
#806

The above PR made most types in this crate owned. However, some types were
missed. For consistency, complete the migration.

Removed alloc feature: this crate unconditionally requires alloc.

Signed-off-by: Nathaniel McCallum <[email protected]>
  • Loading branch information
npmccallum authored Jan 10, 2023
1 parent 89d7d90 commit 7f91388
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 90 deletions.
2 changes: 1 addition & 1 deletion pkcs7/src/revocation_info_choices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use x509_cert::crl::CertificateList;
#[allow(clippy::large_enum_variant)]
pub enum RevocationInfoChoice<'a> {
/// The CertificateList type gives a certificate revocation list (CRL).
Crl(CertificateList<'a>),
Crl(CertificateList),

/// The OtherRevocationInfoFormat alternative is provided to support any
/// other revocation information format without further modifications to
Expand Down
3 changes: 1 addition & 2 deletions x509-cert/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ hex-literal = "0.3"
rstest = "0.16"

[features]
alloc = ["der/alloc"]
arbitrary = ["std", "dep:arbitrary", "const-oid/arbitrary", "der/arbitrary", "spki/arbitrary"]
pem = ["alloc", "der/pem"]
pem = ["der/pem"]
std = ["der/std", "spki/std"]

[package.metadata.docs.rs]
Expand Down
3 changes: 2 additions & 1 deletion x509-cert/fuzz/src/bin/certreq.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#![no_main]

use libfuzzer_sys::fuzz_target;
use x509_cert::der::Decode;
use x509_cert::request::CertReq;

fuzz_target!(|input: &[u8]| {
let _ = CertReq::try_from(input);
let _ = CertReq::from_der(input);
});
3 changes: 2 additions & 1 deletion x509-cert/fuzz/src/bin/certreqinfo.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#![no_main]

use libfuzzer_sys::fuzz_target;
use x509_cert::der::Decode;
use x509_cert::request::CertReqInfo;

fuzz_target!(|input: &[u8]| {
let _ = CertReqInfo::try_from(input);
let _ = CertReqInfo::from_der(input);
});
8 changes: 0 additions & 8 deletions x509-cert/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,6 @@ pub struct Attribute {
pub values: SetOfVec<AttributeValue>,
}

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

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

/// X.501 `Attributes` as defined in [RFC 2986 Section 4].
///
/// ```text
Expand Down
12 changes: 6 additions & 6 deletions x509-cert/src/crl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use alloc::vec::Vec;

use der::asn1::BitString;
use der::{Sequence, ValueOrd};
use spki::AlgorithmIdentifierRef;
use spki::AlgorithmIdentifierOwned;

/// `CertificateList` as defined in [RFC 5280 Section 5.1].
///
Expand All @@ -25,9 +25,9 @@ use spki::AlgorithmIdentifierRef;
/// [RFC 5280 Section 5.1]: https://datatracker.ietf.org/doc/html/rfc5280#section-5.1
#[derive(Clone, Debug, Eq, PartialEq, Sequence, ValueOrd)]
#[allow(missing_docs)]
pub struct CertificateList<'a> {
pub tbs_cert_list: TbsCertList<'a>,
pub signature_algorithm: AlgorithmIdentifierRef<'a>,
pub struct CertificateList {
pub tbs_cert_list: TbsCertList,
pub signature_algorithm: AlgorithmIdentifierOwned,
pub signature: BitString,
}

Expand Down Expand Up @@ -74,9 +74,9 @@ pub struct RevokedCert {
/// [RFC 5280 Section 5.1]: https://datatracker.ietf.org/doc/html/rfc5280#section-5.1
#[derive(Clone, Debug, Eq, PartialEq, Sequence, ValueOrd)]
#[allow(missing_docs)]
pub struct TbsCertList<'a> {
pub struct TbsCertList {
pub version: Version,
pub signature: AlgorithmIdentifierRef<'a>,
pub signature: AlgorithmIdentifierOwned,
pub issuer: Name,
pub this_update: Time,
pub next_update: Option<Time>,
Expand Down
24 changes: 8 additions & 16 deletions x509-cert/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use const_oid::db::rfc5912::ID_EXTENSION_REQ;
use const_oid::{AssociatedOid, ObjectIdentifier};
use der::asn1::BitString;
use der::{Decode, Enumerated, Sequence};
use spki::{AlgorithmIdentifierRef, SubjectPublicKeyInfoRef};
use spki::{AlgorithmIdentifierOwned, SubjectPublicKeyInfoOwned};

#[cfg(feature = "pem")]
use der::pem::PemLabel;
Expand Down Expand Up @@ -38,29 +38,21 @@ pub enum Version {
///
/// [RFC 2986 Section 4]: https://datatracker.ietf.org/doc/html/rfc2986#section-4
#[derive(Clone, Debug, PartialEq, Eq, Sequence)]
pub struct CertReqInfo<'a> {
pub struct CertReqInfo {
/// Certification request version.
pub version: Version,

/// Subject name.
pub subject: Name,

/// Subject public key info.
pub public_key: SubjectPublicKeyInfoRef<'a>,
pub public_key: SubjectPublicKeyInfoOwned,

/// Request attributes.
#[asn1(context_specific = "0", tag_mode = "IMPLICIT")]
pub attributes: Attributes,
}

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

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

/// PKCS#10 `CertificationRequest` as defined in [RFC 2986 Section 4].
///
/// ```text
Expand All @@ -73,24 +65,24 @@ impl<'a> TryFrom<&'a [u8]> for CertReqInfo<'a> {
///
/// [RFC 2986 Section 4]: https://datatracker.ietf.org/doc/html/rfc2986#section-4
#[derive(Clone, Debug, PartialEq, Eq, Sequence)]
pub struct CertReq<'a> {
pub struct CertReq {
/// Certification request information.
pub info: CertReqInfo<'a>,
pub info: CertReqInfo,

/// Signature algorithm identifier.
pub algorithm: AlgorithmIdentifierRef<'a>,
pub algorithm: AlgorithmIdentifierOwned,

/// Signature.
pub signature: BitString,
}

#[cfg(feature = "pem")]
#[cfg_attr(docsrs, doc(cfg(feature = "pem")))]
impl PemLabel for CertReq<'_> {
impl PemLabel for CertReq {
const PEM_LABEL: &'static str = "CERTIFICATE REQUEST";
}

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

fn try_from(bytes: &'a [u8]) -> Result<Self, Self::Error> {
Expand Down
3 changes: 2 additions & 1 deletion x509-cert/src/serial_number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ mod tests {

#[test]
fn serial_number_display() {
let sn = SerialNumber::new(&[0xAA, 0xBB, 0xCC, 0x01, 0x10, 0x00, 0x11]).unwrap();
let sn = SerialNumber::new(&[0xAA, 0xBB, 0xCC, 0x01, 0x10, 0x00, 0x11])
.expect("unexpected error");

assert_eq!(sn.to_string(), "AA:BB:CC:01:10:00:11")
}
Expand Down
18 changes: 5 additions & 13 deletions x509-cert/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use core::fmt;
use core::time::Duration;
use der::asn1::{GeneralizedTime, UtcTime};
use der::{Choice, DateTime, Decode, Error, Result, Sequence, ValueOrd};
use der::{Choice, DateTime, Sequence, ValueOrd};

#[cfg(feature = "std")]
use std::time::SystemTime;
Expand Down Expand Up @@ -62,7 +62,7 @@ impl Time {
}

impl fmt::Display for Time {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> core::result::Result<(), fmt::Error> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.to_date_time())
}
}
Expand Down Expand Up @@ -98,9 +98,9 @@ impl From<&Time> for SystemTime {
#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
impl TryFrom<SystemTime> for Time {
type Error = Error;
type Error = der::Error;

fn try_from(time: SystemTime) -> Result<Time> {
fn try_from(time: SystemTime) -> der::Result<Time> {
Ok(GeneralizedTime::try_from(time)?.into())
}
}
Expand Down Expand Up @@ -128,7 +128,7 @@ impl Validity {
/// Creates a `Validity` which starts now and lasts for `duration`.
#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn from_now(duration: Duration) -> Result<Self> {
pub fn from_now(duration: Duration) -> der::Result<Self> {
let now = SystemTime::now();
let then = now + duration;

Expand All @@ -138,11 +138,3 @@ impl Validity {
})
}
}

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

fn try_from(bytes: &'a [u8]) -> Result<Self> {
Self::from_der(bytes)
}
}
48 changes: 21 additions & 27 deletions x509-cert/tests/certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,13 @@ fn decode_cert() {
.tag(),
Tag::Null
);
assert_eq!(
cert.tbs_certificate
.signature
.parameters
.as_ref()
.unwrap()
.is_null(),
true
);
assert!(cert
.tbs_certificate
.signature
.parameters
.as_ref()
.unwrap()
.is_null());

let mut counter = 0;
let i = cert.tbs_certificate.issuer.0.iter();
Expand Down Expand Up @@ -352,16 +350,14 @@ fn decode_cert() {
.tag(),
Tag::Null
);
assert_eq!(
cert.tbs_certificate
.subject_public_key_info
.algorithm
.parameters
.as_ref()
.unwrap()
.is_null(),
true
);
assert!(cert
.tbs_certificate
.subject_public_key_info
.algorithm
.parameters
.as_ref()
.unwrap()
.is_null());

// TODO - parse and compare public key

Expand All @@ -379,14 +375,12 @@ fn decode_cert() {
cert.signature_algorithm.parameters.as_ref().unwrap().tag(),
Tag::Null
);
assert_eq!(
cert.signature_algorithm
.parameters
.as_ref()
.unwrap()
.is_null(),
true
);
assert!(cert
.signature_algorithm
.parameters
.as_ref()
.unwrap()
.is_null());

assert_eq!(
&hex!("2A892F357BF3EF19E1211986106803FA18E66237802F1B1B0C6756CE678DB01D72CD0A4EB7171C2CDDF110ACD38AA65C35699E869C219AD7550AA4F287BB784F72EF8C9EA0E3DD103EFE5BF182EA36FFBCB45AAE65840263680534789C4F3215AF5454AD48CBC4B7A881E0135401A0BD5A849C11101DD1C66178E762C00DF59DD50F8DE9ED46FC6A0D742AE5697D87DD08DAC5291A75FB13C82FF2865C9E36799EA726137E1814E6A878C9532E8FC3D0A2A942D1CCC668FFCEAC255E6002FDE5ACDF2CE47556BB141C3A797A4BFDB673F6F1C229D7914FFEEF1505EE36F8038137D1B8F90106994BAB3E6FF0F60360A2E32F7A30B7ECEC1502DF3CC725BD6E436BA8F96A1847C9CEBB3F5A5906472292501D59BE1A98475BB1F30B677FAA8A45E351640C85B1B22661D33BD23EC6C0CA33DDD79E1120C7FC869EC4D0175ADB4A258AEAC5E8D2F0F578B8BF4B2C5DCC3269768AAA5B9E26D0592C5BB09C702C72E0A60F66D3EEB2B4983279634D59B0A2011B0E26AE796CC95D3243DF49615434E5CC06C374C3F936C005D360CAE6101F3AE7E97E29A157F5020770D4648D7877EBF8248CF3F3E68F9957A36F92D50616F2C60D3842327EF9BC0312CFF03A48C78E97254C2ADEADCA05069168443D833831FF66295A2EED685F164F1DBE01F8C897E1F63D42851682CBEE7B5A64D7BA2923D33644DBF1F7B3EDCE996F9928F043"),
Expand Down
10 changes: 4 additions & 6 deletions x509-cert/tests/certreq.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
//! Certification request (`CertReq`) tests
use der::{
asn1::{PrintableStringRef, Utf8StringRef},
Encode, Tag, Tagged,
};
use der::asn1::{PrintableStringRef, Utf8StringRef};
use der::{Decode, Encode, Tag, Tagged};
use hex_literal::hex;
use x509_cert::request::{CertReq, Version};

Expand Down Expand Up @@ -31,7 +29,7 @@ const EXTENSIONS: &[(&str, &[u8])] = &[

#[test]
fn decode_rsa_2048_der() {
let cr = CertReq::try_from(RSA_2048_DER_EXAMPLE).unwrap();
let cr = CertReq::from_der(RSA_2048_DER_EXAMPLE).unwrap();

// Check the version.
assert_eq!(cr.info.version, Version::V1);
Expand Down Expand Up @@ -83,7 +81,7 @@ fn decode_rsa_2048_der() {

#[test]
fn encode_rsa_2048_der() {
let cr = CertReq::try_from(RSA_2048_DER_EXAMPLE).unwrap();
let cr = CertReq::from_der(RSA_2048_DER_EXAMPLE).unwrap();
let cr_encoded = cr.to_vec().unwrap();
assert_eq!(RSA_2048_DER_EXAMPLE, cr_encoded.as_slice());
}
16 changes: 8 additions & 8 deletions x509-cert/tests/validity.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Validity tests
use der::Encode;
use der::{Decode, Encode};
use hex_literal::hex;
use x509_cert::time::Validity;

Expand All @@ -11,7 +11,7 @@ fn decode_validity() {
// 104 13: UTCTime 01/01/2010 08:30:00 GMT
// 119 13: UTCTime 31/12/2030 08:30:00 GMT
// : }
let val1 = Validity::try_from(
let val1 = Validity::from_der(
&hex!("301E170D3130303130313038333030305A170D3330313233313038333030305A")[..],
)
.unwrap();
Expand All @@ -21,7 +21,7 @@ fn decode_validity() {
// 99 13: UTCTime 01/01/2010 08:30:00 GMT
// 114 13: UTCTime 01/01/2011 08:30:00 GMT
// : }
let val2 = Validity::try_from(
let val2 = Validity::from_der(
&hex!("301E170D3130303130313038333030305A170D3131303130313038333030305A")[..],
)
.unwrap();
Expand Down Expand Up @@ -51,7 +51,7 @@ fn decode_validity() {
// 99 13: UTCTime 01/01/2010 08:30:00 GMT
// 114 15: GeneralizedTime 01/01/2050 12:01:00 GMT
// : }
let val3 = Validity::try_from(
let val3 = Validity::from_der(
&hex!("3020170D3130303130313038333030305A180F32303530303130313132303130305A")[..],
)
.unwrap();
Expand All @@ -71,7 +71,7 @@ fn decode_validity() {
// 99 15: GeneralizedTime 01/01/2002 12:01:00 GMT
// 116 13: UTCTime 31/12/2030 08:30:00 GMT
// : }
let val4 = Validity::try_from(
let val4 = Validity::from_der(
&hex!("3020180F32303032303130313132303130305A170D3330313233313038333030305A")[..],
)
.unwrap();
Expand All @@ -94,7 +94,7 @@ fn encode_validity() {
// 104 13: UTCTime 01/01/2010 08:30:00 GMT
// 119 13: UTCTime 31/12/2030 08:30:00 GMT
// : }
let val1 = Validity::try_from(
let val1 = Validity::from_der(
&hex!("301E170D3130303130313038333030305A170D3330313233313038333030305A")[..],
)
.unwrap();
Expand All @@ -109,7 +109,7 @@ fn encode_validity() {
// 99 13: UTCTime 01/01/2010 08:30:00 GMT
// 114 15: GeneralizedTime 01/01/2050 12:01:00 GMT
// : }
let val3 = Validity::try_from(
let val3 = Validity::from_der(
&hex!("3020170D3130303130313038333030305A180F32303530303130313132303130305A")[..],
)
.unwrap();
Expand All @@ -124,7 +124,7 @@ fn encode_validity() {
// 99 15: GeneralizedTime 01/01/2002 12:01:00 GMT
// 116 13: UTCTime 31/12/2030 08:30:00 GMT
// : }
let val4 = Validity::try_from(
let val4 = Validity::from_der(
&hex!("3020180F32303032303130313132303130305A170D3330313233313038333030305A")[..],
)
.unwrap();
Expand Down

0 comments on commit 7f91388

Please sign in to comment.