Skip to content

Commit

Permalink
crl: make verify_signature crate-internal, use budget
Browse files Browse the repository at this point in the history
Previously the `verify_signature` fn was part of the
`CertRevocationList` trait, and so part of the public API. This meant we
couldn't accept an `untrusted::Input` argument, or more importantly,
a `Budget` to use to indicate we've consumed a signature validation
operation.

Now that the `CertRevocationList` type is an enum we can easily make
this fn crate-internal. It doesn't seem especially valuable to external
users, especially given that it's somewhat nuanced/cumbersome to build
the right SPKI representation to use for validation outside of webpki.

This commit makes the fn crate internal, adds the `Budget` argument,
reworks the SPKI to be an `untrusted::Input` and folds-in the
`signed_data` crate-internal fn since it wasn't being used anywhere
except in `verify_signature`.

The net result is that it's not impossible to verify a CRL signature
without providing a budget to use. In the path building context in
`verify_cert.rs` we pass through the budget we use for the overall
accounting during path building.
  • Loading branch information
cpu committed Oct 27, 2023
1 parent 4859a87 commit d45305b
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 34 deletions.
6 changes: 1 addition & 5 deletions src/crl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,7 @@ impl<'a> RevocationOptions<'a> {
// TODO(XXX): consider whether we can refactor so this happens once up-front, instead
// of per-lookup.
// https:/rustls/webpki/issues/81
// Note: The `verify_signature` method is part of a public trait in the exported API.
// We can't add a budget argument to that fn in a semver compatible way and so must
// consume signature budget here before calling verify_signature.
budget.consume_signature()?;
crl.verify_signature(supported_sig_algs, issuer_spki.as_slice_less_safe())
crl.verify_signature(supported_sig_algs, issuer_spki, budget)
.map_err(crl_signature_err)?;

// Verify that if the issuer has a KeyUsage bitstring it asserts cRLSign.
Expand Down
51 changes: 22 additions & 29 deletions src/crl/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,26 +74,6 @@ impl<'a> CertRevocationList<'a> {
}
}

/// Verify the CRL signature using the issuer certificate and a list of supported signature
/// verification algorithms.
///
/// This is appropriate for one-off validation of CRL signatures and shouldn't be used in
/// a context where the overall number of signature validation operations is budgeted (e.g.
/// during path building).
pub fn verify_signature(
&self,
supported_sig_algs: &[&dyn SignatureVerificationAlgorithm],
issuer_spki: &[u8],
) -> Result<(), Error> {
signed_data::verify_signed_data(
supported_sig_algs,
untrusted::Input::from(issuer_spki),
&self.signed_data(),
&mut Budget::default(),
)
.map_err(crl_signature_err)
}

/// Returns true if the CRL can be considered authoritative for the given certificate.
///
/// A CRL is considered authoritative for a certificate when:
Expand Down Expand Up @@ -136,16 +116,29 @@ impl<'a> CertRevocationList<'a> {
crl_idp.authoritative_for(path)
}

pub(crate) fn signed_data(&self) -> SignedData {
match self {
#[cfg(feature = "alloc")]
CertRevocationList::Owned(crl) => crl.signed_data.borrow(),
CertRevocationList::Borrowed(crl) => SignedData {
data: crl.signed_data.data,
algorithm: crl.signed_data.algorithm,
signature: crl.signed_data.signature,
/// Verify the CRL signature using the issuer certificate and a list of supported signature
/// verification algorithms, consuming signature operations from the [`Budget`].
pub(crate) fn verify_signature(
&self,
supported_sig_algs: &[&dyn SignatureVerificationAlgorithm],
issuer_spki: untrusted::Input,
budget: &mut Budget,
) -> Result<(), Error> {
signed_data::verify_signed_data(
supported_sig_algs,
issuer_spki,
&match self {
#[cfg(feature = "alloc")]
CertRevocationList::Owned(crl) => crl.signed_data.borrow(),
CertRevocationList::Borrowed(crl) => SignedData {
data: crl.signed_data.data,
algorithm: crl.signed_data.algorithm,
signature: crl.signed_data.signature,
},
},
}
budget,
)
.map_err(crl_signature_err)
}
}

Expand Down

0 comments on commit d45305b

Please sign in to comment.