Skip to content

Commit

Permalink
der: remove expect_tag_and_get_value()
Browse files Browse the repository at this point in the history
Use the equivalent expect_tag() instead.
  • Loading branch information
djc committed Jul 31, 2023
1 parent 95c16b9 commit d163706
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 41 deletions.
12 changes: 6 additions & 6 deletions src/cert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,17 @@ impl<'a> Cert<'a> {

let serial = lenient_certificate_serial_number(tbs)?;

let signature = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
let signature = der::expect_tag(tbs, der::Tag::Sequence)?;
// TODO: In mozilla::pkix, the comparison is done based on the
// normalized value (ignoring whether or not there is an optional NULL
// parameter for RSA-based algorithms), so this may be too strict.
if signature != signed_data.algorithm {
return Err(Error::SignatureAlgorithmMismatch);
}

let issuer = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
let validity = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
let subject = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
let issuer = der::expect_tag(tbs, der::Tag::Sequence)?;
let validity = der::expect_tag(tbs, der::Tag::Sequence)?;
let subject = der::expect_tag(tbs, der::Tag::Sequence)?;
let spki = der::expect_tag(tbs, der::Tag::Sequence)?;

// In theory there could be fields [1] issuerUniqueID and [2]
Expand Down Expand Up @@ -188,7 +188,7 @@ pub(crate) fn lenient_certificate_serial_number<'a>(
// Note: Non-conforming CAs may issue certificates with serial numbers
// that are negative or zero. Certificate users SHOULD be prepared to
// gracefully handle such certificates.
der::expect_tag_and_get_value(input, Tag::Integer)
der::expect_tag(input, Tag::Integer)
}

fn remember_cert_extension<'a>(
Expand Down Expand Up @@ -229,7 +229,7 @@ fn remember_cert_extension<'a>(
// read the raw bytes here and parse at the time of use.
15 => Ok(value.read_bytes_to_end()),
// All other remembered certificate extensions are wrapped in a Sequence.
_ => der::expect_tag_and_get_value(value, Tag::Sequence),
_ => der::expect_tag(value, Tag::Sequence),
})
})
})
Expand Down
6 changes: 3 additions & 3 deletions src/crl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,14 +283,14 @@ impl<'a> FromDer<'a> for BorrowedCertRevocationList<'a> {
// 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)?;
let signature = der::expect_tag(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)?;
let issuer = der::expect_tag(tbs_cert_list, Tag::Sequence)?;

// RFC 5280 §5.1.2.4:
// This field indicates the issue date of this CRL. thisUpdate may be
Expand Down Expand Up @@ -519,7 +519,7 @@ impl<'a> FromDer<'a> for BorrowedRevokedCert<'a> {
// It would be convenient to use der::nested_of_mut here to unpack a SEQUENCE of one or
// more SEQUENCEs, however CAs have been mis-encoding the absence of extensions as an
// empty SEQUENCE so we must be tolerant of that.
let ext_seq = der::expect_tag_and_get_value(der, Tag::Sequence)?;
let ext_seq = der::expect_tag(der, Tag::Sequence)?;
if ext_seq.is_empty() {
return Ok(revoked_cert);
}
Expand Down
14 changes: 1 addition & 13 deletions src/der.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,6 @@ impl From<Tag> for u8 {
} // XXX: narrowing conversion.
}

#[inline(always)]
pub(crate) fn expect_tag_and_get_value<'a>(
input: &mut untrusted::Reader<'a>,
tag: Tag,
) -> Result<untrusted::Input<'a>, Error> {
let (actual_tag, inner) = read_tag_and_get_value_limited(input, TWO_BYTE_DER_SIZE)?;
if usize::from(tag) != usize::from(actual_tag) {
return Err(Error::BadDer);
}
Ok(inner)
}

#[inline(always)]
pub(crate) fn expect_tag_and_get_value_limited<'a>(
input: &mut untrusted::Reader<'a>,
Expand Down Expand Up @@ -370,7 +358,7 @@ impl<'a> FromDer<'a> for u8 {
pub(crate) fn nonnegative_integer<'a>(
input: &mut untrusted::Reader<'a>,
) -> Result<untrusted::Input<'a>, Error> {
let value = expect_tag_and_get_value(input, Tag::Integer)?;
let value = expect_tag(input, Tag::Integer)?;
match value
.as_slice_less_safe()
.split_first()
Expand Down
6 changes: 3 additions & 3 deletions src/ring_algs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ mod tests {
let spki_value = untrusted::Input::from(&tsd.spki);
let spki_value = spki_value
.read_all(Error::BadDer, |input| {
der::expect_tag_and_get_value(input, der::Tag::Sequence)
der::expect_tag(input, der::Tag::Sequence)
})
.unwrap();

Expand All @@ -205,7 +205,7 @@ mod tests {
let algorithm = untrusted::Input::from(&tsd.algorithm);
let algorithm = algorithm
.read_all(Error::BadDer, |input| {
der::expect_tag_and_get_value(input, der::Tag::Sequence)
der::expect_tag(input, der::Tag::Sequence)
})
.unwrap();

Expand Down Expand Up @@ -272,7 +272,7 @@ mod tests {
assert_eq!(
Err(expected_error),
spki.read_all(Error::BadDer, |input| {
der::expect_tag_and_get_value(input, der::Tag::Sequence)
der::expect_tag(input, der::Tag::Sequence)
})
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/signed_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl<'a> SignedData<'a> {
let (data, tbs) = der.read_partial(|input| {
der::expect_tag_and_get_value_limited(input, der::Tag::Sequence, size_limit)
})?;
let algorithm = der::expect_tag_and_get_value(der, der::Tag::Sequence)?;
let algorithm = der::expect_tag(der, der::Tag::Sequence)?;
let signature = der::bit_string_with_no_unused_bits(der)?;

Ok((
Expand Down Expand Up @@ -234,7 +234,7 @@ impl<'a> FromDer<'a> for SubjectPublicKeyInfo<'a> {
// `PublicKeyAlgorithm` for the `SignatureVerificationAlgorithm` that is matched when
// parsing the signature.
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 algorithm_id_value = der::expect_tag(reader, der::Tag::Sequence)?;
let key_value = der::bit_string_with_no_unused_bits(reader)?;
Ok(SubjectPublicKeyInfo {
algorithm_id_value,
Expand Down
8 changes: 4 additions & 4 deletions src/subject_name/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ pub(crate) fn check_name_constraints(
if !inner.peek(subtrees_tag.into()) {
return Ok(None);
}
der::expect_tag_and_get_value(inner, subtrees_tag).map(Some)
der::expect_tag(inner, subtrees_tag).map(Some)
}

let permitted_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed0)?;
Expand Down Expand Up @@ -161,7 +161,7 @@ fn check_presented_id_conforms_to_constraints(
];

fn general_subtree<'b>(input: &mut untrusted::Reader<'b>) -> Result<GeneralName<'b>, Error> {
der::expect_tag_and_get_value(input, der::Tag::Sequence)?
der::expect_tag(input, der::Tag::Sequence)?
.read_all(Error::BadDer, GeneralName::from_der)
}

Expand Down Expand Up @@ -452,9 +452,9 @@ fn common_name(input: untrusted::Input) -> Result<Option<untrusted::Input>, Erro
der::nested(inner, der::Tag::Set, Error::BadDer, |tagged| {
der::nested(tagged, der::Tag::Sequence, Error::BadDer, |tagged| {
while !tagged.at_end() {
let name_oid = der::expect_tag_and_get_value(tagged, der::Tag::OID)?;
let name_oid = der::expect_tag(tagged, der::Tag::OID)?;
if name_oid == COMMON_NAME {
return der::expect_tag_and_get_value(tagged, der::Tag::UTF8String).map(Some);
return der::expect_tag(tagged, der::Tag::UTF8String).map(Some);
} else {
// discard unused name value
der::read_tag_and_get_value(tagged)?;
Expand Down
6 changes: 3 additions & 3 deletions src/trust_anchor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ impl<'a> TrustAnchor<'a> {
skip(tbs, der::Tag::Sequence)?; // signature.
skip(tbs, der::Tag::Sequence)?; // issuer.
skip(tbs, der::Tag::Sequence)?; // validity.
let subject = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
let spki = der::expect_tag_and_get_value(tbs, der::Tag::Sequence)?;
let subject = der::expect_tag(tbs, der::Tag::Sequence)?;
let spki = der::expect_tag(tbs, der::Tag::Sequence)?;

Ok(TrustAnchor {
subject: subject.as_slice_less_safe(),
Expand Down Expand Up @@ -92,5 +92,5 @@ impl<'a> From<Cert<'a>> for TrustAnchor<'a> {
}

fn skip(input: &mut untrusted::Reader, tag: der::Tag) -> Result<(), Error> {
der::expect_tag_and_get_value(input, tag).map(|_| ())
der::expect_tag(input, tag).map(|_| ())
}
9 changes: 4 additions & 5 deletions src/verify_cert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ impl ExtendedKeyUsage {
};

loop {
let value = der::expect_tag_and_get_value(input, der::Tag::OID)?;
let value = der::expect_tag(input, der::Tag::OID)?;
if self.key_purpose_id_equals(value) {
input.skip_to_end();
break;
Expand Down Expand Up @@ -464,10 +464,9 @@ impl KeyUsageMode {
// https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.3
fn check(self, input: Option<untrusted::Input>) -> Result<(), Error> {
let bit_string = match input {
Some(input) => der::expect_tag_and_get_value(
&mut untrusted::Reader::new(input),
der::Tag::BitString,
)?,
Some(input) => {
der::expect_tag(&mut untrusted::Reader::new(input), der::Tag::BitString)?
}
// While RFC 5280 requires KeyUsage be present, historically the absence of a KeyUsage
// has been treated as "Any Usage". We follow that convention here and assume the absence
// of KeyUsage implies the required_ku_bit_if_present we're checking for.
Expand Down
4 changes: 2 additions & 2 deletions src/x509.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ impl<'a> Extension<'a> {

impl<'a> FromDer<'a> for Extension<'a> {
fn from_der(reader: &mut untrusted::Reader<'a>) -> Result<Self, Error> {
let id = der::expect_tag_and_get_value(reader, der::Tag::OID)?;
let id = der::expect_tag(reader, der::Tag::OID)?;
let critical = bool::from_der(reader)?;
let value = der::expect_tag_and_get_value(reader, der::Tag::OctetString)?;
let value = der::expect_tag(reader, der::Tag::OctetString)?;
Ok(Extension {
id,
critical,
Expand Down

0 comments on commit d163706

Please sign in to comment.