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.

Sign up for GitHub

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

Jump to bottom

Refactor DER parsing #142

Merged
merged 6 commits into from
Aug 2, 2023
Merged

Refactor DER parsing #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
00df82c
der: add read_all() abstraction
djc Jul 31, 2023
0a7536d
der: remove unnecessary Value wrapper type
djc Jul 31, 2023
ada1168
der: remove layer of abstraction
djc Jul 31, 2023
be6a7b0
der: remove expect_tag_and_get_value()
djc Jul 31, 2023
056c003
der: no need for abstract error type arguments
djc Jul 31, 2023
69441d6
Pass more specific errors to read_all()
djc Jul 31, 2023
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
210 changes: 114 additions & 96 deletions src/cert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@
// 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::Tag;
use crate::der::{self, DerIterator, FromDer, CONSTRUCTED, CONTEXT_SPECIFIC};
use crate::der::{self, DerIterator, FromDer, Tag, CONSTRUCTED, CONTEXT_SPECIFIC};
use crate::error::{DerTypeId, Error};
use crate::signed_data::SignedData;
use crate::x509::{remember_extension, set_extension_once, DistributionPointName, Extension};
use crate::Error;

/// An enumeration indicating whether a [`Cert`] is a leaf end-entity cert, or a linked
/// list node from the CA `Cert` to a child `Cert` it issued.
Expand All @@ -37,7 +36,7 @@
pub(crate) issuer: untrusted::Input<'a>,
pub(crate) validity: untrusted::Input<'a>,
pub(crate) subject: untrusted::Input<'a>,
pub(crate) spki: der::Value<'a>,
pub(crate) spki: untrusted::Input<'a>,

pub(crate) basic_constraints: Option<untrusted::Input<'a>>,
// key usage (KU) extension (if any). When validating certificate revocation lists (CRLs) this
Expand All @@ -56,75 +55,87 @@
cert_der: untrusted::Input<'a>,
ee_or_ca: EndEntityOrCa<'a>,
) -> Result<Self, Error> {
let (tbs, signed_data) = cert_der.read_all(Error::BadDer, |cert_der| {
der::nested(cert_der, der::Tag::Sequence, Error::BadDer, |der| {
// limited to SEQUENCEs of size 2^16 or less.
SignedData::from_der(der, der::TWO_BYTE_DER_SIZE)
})
})?;

tbs.read_all(Error::BadDer, |tbs| {
version3(tbs)?;

let serial = lenient_certificate_serial_number(tbs)?;

let signature = der::expect_tag_and_get_value(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 spki = der::expect_tag(tbs, der::Tag::Sequence)?;

// In theory there could be fields [1] issuerUniqueID and [2]
// subjectUniqueID, but in practice there never are, and to keep the
// code small and simple we don't accept any certificates that do
// contain them.

let mut cert = Cert {
ee_or_ca,

signed_data,
serial,
issuer,
validity,
subject,
spki,

basic_constraints: None,
key_usage: None,
eku: None,
name_constraints: None,
subject_alt_name: None,
crl_distribution_points: None,
};

if !tbs.at_end() {
let (tbs, signed_data) =
cert_der.read_all(Error::TrailingData(DerTypeId::Certificate), |cert_der| {
der::nested(
tbs,
der::Tag::ContextSpecificConstructed3,
Error::MalformedExtensions,
|tagged| {
der::nested_of_mut(
tagged,
der::Tag::Sequence,
der::Tag::Sequence,
Error::BadDer,
|extension| {
remember_cert_extension(&mut cert, &Extension::from_der(extension)?)
},
)
cert_der,
der::Tag::Sequence,
Error::TrailingData(DerTypeId::SignedData),
|der| {
// limited to SEQUENCEs of size 2^16 or less.
SignedData::from_der(der, der::TWO_BYTE_DER_SIZE)
},
)?;
}
)
})?;

tbs.read_all(
Error::TrailingData(DerTypeId::CertificateTbsCertificate),
|tbs| {
version3(tbs)?;

let serial = lenient_certificate_serial_number(tbs)?;

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);

Check warning on line 83 in src/cert.rs

View check run for this annotation

Codecov / codecov/patch

src/cert.rs#L83 

Added line #L83 was not covered by tests
}

Ok(cert)
})
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]
// subjectUniqueID, but in practice there never are, and to keep the
// code small and simple we don't accept any certificates that do
// contain them.

let mut cert = Cert {
ee_or_ca,

signed_data,
serial,
issuer,
validity,
subject,
spki,

basic_constraints: None,
key_usage: None,
eku: None,
name_constraints: None,
subject_alt_name: None,
crl_distribution_points: None,
};

if !tbs.at_end() {
der::nested(
tbs,
der::Tag::ContextSpecificConstructed3,
Error::TrailingData(DerTypeId::CertificateExtensions),
|tagged| {
der::nested_of_mut(
tagged,
der::Tag::Sequence,
der::Tag::Sequence,
Error::TrailingData(DerTypeId::Extension),
|extension| {
remember_cert_extension(
&mut cert,
&Extension::from_der(extension)?,
)
},
)
},
)?;
}

Ok(cert)
},
)
}

/// Raw DER encoded certificate serial number.
Expand Down Expand Up @@ -188,7 +199,7 @@
// 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 +240,7 @@
// 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 Expand Up @@ -273,34 +284,41 @@
crl_issuer: None,
};

der::nested(reader, Tag::Sequence, Error::BadDer, |der| {
const DISTRIBUTION_POINT_TAG: u8 = CONTEXT_SPECIFIC | CONSTRUCTED;
const REASONS_TAG: u8 = CONTEXT_SPECIFIC | 1;
const CRL_ISSUER_TAG: u8 = CONTEXT_SPECIFIC | CONSTRUCTED | 2;

while !der.at_end() {
let (tag, value) = der::read_tag_and_get_value(der)?;
match tag {
DISTRIBUTION_POINT_TAG => {
set_extension_once(&mut result.distribution_point, || Ok(value))?
der::nested(
reader,
Tag::Sequence,
Error::TrailingData(Self::TYPE_ID),
|der| {
const DISTRIBUTION_POINT_TAG: u8 = CONTEXT_SPECIFIC | CONSTRUCTED;
const REASONS_TAG: u8 = CONTEXT_SPECIFIC | 1;
const CRL_ISSUER_TAG: u8 = CONTEXT_SPECIFIC | CONSTRUCTED | 2;

while !der.at_end() {
let (tag, value) = der::read_tag_and_get_value(der)?;
match tag {
DISTRIBUTION_POINT_TAG => {
set_extension_once(&mut result.distribution_point, || Ok(value))?
}
REASONS_TAG => set_extension_once(&mut result.reasons, || {
der::bit_string_flags(value)
})?,
CRL_ISSUER_TAG => set_extension_once(&mut result.crl_issuer, || Ok(value))?,
_ => return Err(Error::BadDer),
}
REASONS_TAG => {
set_extension_once(&mut result.reasons, || der::bit_string_flags(value))?
}
CRL_ISSUER_TAG => set_extension_once(&mut result.crl_issuer, || Ok(value))?,
_ => return Err(Error::BadDer),
}
}

// RFC 5280 section §4.2.1.13:
// a DistributionPoint MUST NOT consist of only the reasons field; either distributionPoint or
// cRLIssuer MUST be present.
match (result.distribution_point, result.crl_issuer) {
(None, None) => Err(Error::MalformedExtensions),
_ => Ok(result),
}
})
// RFC 5280 section §4.2.1.13:
// a DistributionPoint MUST NOT consist of only the reasons field; either distributionPoint or
// cRLIssuer MUST be present.
match (result.distribution_point, result.crl_issuer) {
(None, None) => Err(Error::MalformedExtensions),
_ => Ok(result),
}
},
)
}

const TYPE_ID: DerTypeId = DerTypeId::CrlDistributionPoint;
}

#[cfg(test)]
Expand Down
Loading