Skip to content

Commit

Permalink
Move more from_der() methods to trait impls
Browse files Browse the repository at this point in the history
  • Loading branch information
@djc
djc committed Jul 26, 2023
1 parent ec911ac commit 10506cd
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 127 deletions.
243 changes: 127 additions & 116 deletions src/crl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@

use crate::cert::lenient_certificate_serial_number;
use crate::der::Tag;
use crate::der::{self, DerIterator, FromDer};
use crate::signed_data::{self, SignedData};
use crate::x509::{remember_extension, set_extension_once, Extension};
use crate::der::{self, FromDer, DerIterator};
use crate::{Error, SignatureAlgorithm, Time};

#[cfg(feature = "alloc")]
Expand Down Expand Up @@ -122,112 +122,8 @@ impl<'a> BorrowedCertRevocationList<'a> {
///
/// [^1]: <https://www.rfc-editor.org/rfc/rfc5280#section-5>
pub fn from_der(crl_der: &'a [u8]) -> Result<Self, Error> {
// Try to parse the CRL.
let reader = untrusted::Input::from(crl_der);
let (tbs_cert_list, signed_data) = reader.read_all(Error::BadDer, |crl_der| {
der::nested_limited(
crl_der,
Tag::Sequence,
Error::BadDer,
|signed_der| SignedData::from_der(signed_der, der::MAX_DER_SIZE),
der::MAX_DER_SIZE,
)
})?;

let crl = tbs_cert_list.read_all(Error::BadDer, |tbs_cert_list| {
// RFC 5280 §5.1.2.1:
// This optional field describes the version of the encoded CRL. When
// extensions are used, as required by this profile, this field MUST be
// present and MUST specify version 2 (the integer value is 1).
// RFC 5280 §5.2:
// Conforming CRL issuers are REQUIRED to include the authority key
// identifier (Section 5.2.1) and the CRL number (Section 5.2.3)
// extensions in all CRLs issued.
// As a result of the above we parse this as a required section, not OPTIONAL.
// NOTE: Encoded value of version 2 is 1.
if der::small_nonnegative_integer(tbs_cert_list)? != 1 {
return Err(Error::UnsupportedCrlVersion);
}

// RFC 5280 §5.1.2.2:
// This field MUST contain the same algorithm identifier as the
// signatureAlgorithm field in the sequence CertificateList
let signature = der::expect_tag_and_get_value(tbs_cert_list, Tag::Sequence)?;
if signature != signed_data.algorithm {
return Err(Error::SignatureAlgorithmMismatch);
}

// RFC 5280 §5.1.2.3:
// The issuer field MUST contain a non-empty X.500 distinguished name (DN).
let issuer = der::expect_tag_and_get_value(tbs_cert_list, Tag::Sequence)?;

// RFC 5280 §5.1.2.4:
// This field indicates the issue date of this CRL. thisUpdate may be
// encoded as UTCTime or GeneralizedTime.
// We do not presently enforce the correct choice of UTCTime or GeneralizedTime based on
// whether the date is post 2050.
der::time_choice(tbs_cert_list)?;

// While OPTIONAL in the ASN.1 module, RFC 5280 §5.1.2.5 says:
// Conforming CRL issuers MUST include the nextUpdate field in all CRLs.
// We do not presently enforce the correct choice of UTCTime or GeneralizedTime based on
// whether the date is post 2050.
der::time_choice(tbs_cert_list)?;

// RFC 5280 §5.1.2.6:
// When there are no revoked certificates, the revoked certificates list
// MUST be absent
// TODO(@cpu): Do we care to support empty CRLs if we don't support delta CRLs?
let revoked_certs = if tbs_cert_list.peek(Tag::Sequence.into()) {
der::expect_tag_and_get_value_limited(
tbs_cert_list,
Tag::Sequence,
der::MAX_DER_SIZE,
)?
} else {
untrusted::Input::from(&[])
};

let mut crl = BorrowedCertRevocationList {
signed_data,
issuer,
revoked_certs,
};

// RFC 5280 §5.1.2.7:
// This field may only appear if the version is 2 (Section 5.1.2.1). If
// present, this field is a sequence of one or more CRL extensions.
// RFC 5280 §5.2:
// Conforming CRL issuers are REQUIRED to include the authority key
// identifier (Section 5.2.1) and the CRL number (Section 5.2.3)
// extensions in all CRLs issued.
// As a result of the above we parse this as a required section, not OPTIONAL.
der::nested(
tbs_cert_list,
Tag::ContextSpecificConstructed0,
Error::MalformedExtensions,
|tagged| {
der::nested_of_mut(
tagged,
Tag::Sequence,
Tag::Sequence,
Error::BadDer,
|extension| {
// RFC 5280 §5.2:
// If a CRL contains a critical extension
// that the application cannot process, then the application MUST NOT
// use that CRL to determine the status of certificates. However,
// applications may ignore unrecognized non-critical extensions.
crl.remember_extension(&Extension::parse(extension)?)
},
)
},
)?;

Ok(crl)
})?;

Ok(crl)
let input = untrusted::Input::from(crl_der);
input.read_all(Error::BadDer, <Self as FromDer>::from_der)
}

/// Convert the CRL to an [`OwnedCertRevocationList`]. This may error if any of the revoked
Expand Down Expand Up @@ -338,6 +234,123 @@ impl CertRevocationList for BorrowedCertRevocationList<'_> {
}
}

impl<'a> FromDer<'a> for BorrowedCertRevocationList<'a> {
/// Try to parse the given bytes as a RFC 5280[^1] profile Certificate Revocation List (CRL).
///
/// Webpki does not support:
/// * CRL versions other than version 2.
/// * CRLs missing the next update field.
/// * CRLs missing certificate revocation list extensions.
/// * Delta CRLs.
/// * CRLs larger than (2^32)-1 bytes in size.
///
/// [^1]: <https://www.rfc-editor.org/rfc/rfc5280#section-5>
fn from_der(reader: &mut untrusted::Reader<'a>) -> Result<Self, Error> {
let (tbs_cert_list, signed_data) = der::nested_limited(
reader,
Tag::Sequence,
Error::BadDer,
|signed_der| SignedData::from_der(signed_der, der::MAX_DER_SIZE),
der::MAX_DER_SIZE,
)?;

let crl = tbs_cert_list.read_all(Error::BadDer, |tbs_cert_list| {
// RFC 5280 §5.1.2.1:
// This optional field describes the version of the encoded CRL. When
// extensions are used, as required by this profile, this field MUST be
// present and MUST specify version 2 (the integer value is 1).
// RFC 5280 §5.2:
// Conforming CRL issuers are REQUIRED to include the authority key
// identifier (Section 5.2.1) and the CRL number (Section 5.2.3)
// extensions in all CRLs issued.
// As a result of the above we parse this as a required section, not OPTIONAL.
// NOTE: Encoded value of version 2 is 1.
if der::small_nonnegative_integer(tbs_cert_list)? != 1 {
return Err(Error::UnsupportedCrlVersion);
}

// RFC 5280 §5.1.2.2:
// This field MUST contain the same algorithm identifier as the
// signatureAlgorithm field in the sequence CertificateList
let signature = der::expect_tag_and_get_value(tbs_cert_list, Tag::Sequence)?;
if signature != signed_data.algorithm {
return Err(Error::SignatureAlgorithmMismatch);
}

// RFC 5280 §5.1.2.3:
// The issuer field MUST contain a non-empty X.500 distinguished name (DN).
let issuer = der::expect_tag_and_get_value(tbs_cert_list, Tag::Sequence)?;

// RFC 5280 §5.1.2.4:
// This field indicates the issue date of this CRL. thisUpdate may be
// encoded as UTCTime or GeneralizedTime.
// We do not presently enforce the correct choice of UTCTime or GeneralizedTime based on
// whether the date is post 2050.
der::time_choice(tbs_cert_list)?;

// While OPTIONAL in the ASN.1 module, RFC 5280 §5.1.2.5 says:
// Conforming CRL issuers MUST include the nextUpdate field in all CRLs.
// We do not presently enforce the correct choice of UTCTime or GeneralizedTime based on
// whether the date is post 2050.
der::time_choice(tbs_cert_list)?;

// RFC 5280 §5.1.2.6:
// When there are no revoked certificates, the revoked certificates list
// MUST be absent
// TODO(@cpu): Do we care to support empty CRLs if we don't support delta CRLs?
let revoked_certs = if tbs_cert_list.peek(Tag::Sequence.into()) {
der::expect_tag_and_get_value_limited(
tbs_cert_list,
Tag::Sequence,
der::MAX_DER_SIZE,
)?
} else {
untrusted::Input::from(&[])
};

let mut crl = BorrowedCertRevocationList {
signed_data,
issuer,
revoked_certs,
};

// RFC 5280 §5.1.2.7:
// This field may only appear if the version is 2 (Section 5.1.2.1). If
// present, this field is a sequence of one or more CRL extensions.
// RFC 5280 §5.2:
// Conforming CRL issuers are REQUIRED to include the authority key
// identifier (Section 5.2.1) and the CRL number (Section 5.2.3)
// extensions in all CRLs issued.
// As a result of the above we parse this as a required section, not OPTIONAL.
der::nested(
tbs_cert_list,
Tag::ContextSpecificConstructed0,
Error::MalformedExtensions,
|tagged| {
der::nested_of_mut(
tagged,
Tag::Sequence,
Tag::Sequence,
Error::BadDer,
|extension| {
// RFC 5280 §5.2:
// If a CRL contains a critical extension
// that the application cannot process, then the application MUST NOT
// use that CRL to determine the status of certificates. However,
// applications may ignore unrecognized non-critical extensions.
crl.remember_extension(&Extension::parse(extension)?)
},
)
},
)?;

Ok(crl)
})?;

Ok(crl)
}
}

impl<'a> IntoIterator for &'a BorrowedCertRevocationList<'a> {
type Item = Result<BorrowedRevokedCert<'a>, Error>;
type IntoIter = DerIterator<'a, BorrowedRevokedCert<'a>>;
Expand Down Expand Up @@ -429,7 +442,7 @@ impl<'a> BorrowedRevokedCert<'a> {
match id {
// id-ce-cRLReasons 2.5.29.21 - RFC 5280 §5.3.1.
21 => set_extension_once(&mut self.reason_code, || {
RevocationReason::from_der(extension.value)
RevocationReason::from_der(&mut untrusted::Reader::new(extension.value))
}),

// id-ce-invalidityDate 2.5.29.24 - RFC 5280 §5.3.2.
Expand Down Expand Up @@ -538,15 +551,13 @@ pub enum RevocationReason {
AaCompromise = 10,
}

impl RevocationReason {
impl<'a> FromDer<'a> for RevocationReason {
// RFC 5280 §5.3.1.
fn from_der(value: untrusted::Input<'_>) -> Result<Self, Error> {
value.read_all(Error::BadDer, |enumerated_reason| {
let value = der::expect_tag(enumerated_reason, Tag::Enum)?;
Self::try_from(value.value().read_all(Error::BadDer, |reason| {
reason.read_byte().map_err(|_| Error::BadDer)
})?)
})
fn from_der(reader: &mut untrusted::Reader<'a>) -> Result<Self, Error> {
let value = der::expect_tag(reader, Tag::Enum)?;
Self::try_from(value.value().read_all(Error::BadDer, |reason| {
reason.read_byte().map_err(|_| Error::BadDer)
})?)
}
}

Expand Down
22 changes: 11 additions & 11 deletions src/signed_data.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

use crate::{der, Error};
use ring::signature;

use crate::der::{self, FromDer};
use crate::error::Error;

/// X.509 certificates and related items that are signed are almost always
/// encoded in the format "tbs||signatureAlgorithm||signature". This structure
/// captures this pattern as an owned data type.
Expand Down Expand Up @@ -206,7 +208,7 @@ pub(crate) fn verify_signature(
msg: untrusted::Input,
signature: untrusted::Input,
) -> Result<(), Error> {
let spki = SubjectPublicKeyInfo::from_der(spki_value)?;
let spki = spki_value.read_all(Error::BadDer, SubjectPublicKeyInfo::from_der)?;
if !signature_alg
.public_key_alg_id
.matches_algorithm_id_value(spki.algorithm_id_value)
Expand All @@ -226,19 +228,17 @@ struct SubjectPublicKeyInfo<'a> {
key_value: untrusted::Input<'a>,
}

impl<'a> SubjectPublicKeyInfo<'a> {
impl<'a> FromDer<'a> for SubjectPublicKeyInfo<'a> {
// Parse the public key into an algorithm OID, an optional curve OID, and the
// key value. The caller needs to check whether these match the
// `PublicKeyAlgorithm` for the `SignatureAlgorithm` that is matched when
// parsing the signature.
fn from_der(input: untrusted::Input<'a>) -> Result<Self, Error> {
input.read_all(Error::BadDer, |input| {
let algorithm_id_value = der::expect_tag_and_get_value(input, der::Tag::Sequence)?;
let key_value = der::bit_string_with_no_unused_bits(input)?;
Ok(SubjectPublicKeyInfo {
algorithm_id_value,
key_value,
})
fn from_der(reader: &mut untrusted::Reader<'a>) -> Result<Self, Error> {
let algorithm_id_value = der::expect_tag_and_get_value(reader, der::Tag::Sequence)?;
let key_value = der::bit_string_with_no_unused_bits(reader)?;
Ok(SubjectPublicKeyInfo {
algorithm_id_value,
key_value,
})
}
}
Expand Down

0 comments on commit 10506cd

Please sign in to comment.