Skip to content

Commit

Permalink
webpki: avoid CRL dyn trait hurdles
Browse files Browse the repository at this point in the history
This commit adapts to the upstream webpki changes that improved the
ergonomics of working with CRLs by replacing the `CertRevocationList`
trait with an `enum` representation.

Notably this makes working with the `Vec<OwnedCertRevocationList>` that
the webpki verifier builders and verifiers hold much easier: we no long
have to do as many contortions to convert to a `&[&dyn
CertRevocationList]`.
  • Loading branch information
cpu committed Oct 24, 2023
1 parent a9a4c42 commit 264bae2
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 49 deletions.
34 changes: 12 additions & 22 deletions rustls/src/webpki/client_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,16 @@ use alloc::sync::Arc;
use alloc::vec::Vec;

use pki_types::{CertificateDer, CertificateRevocationListDer, UnixTime};
use webpki::{
BorrowedCertRevocationList, OwnedCertRevocationList, RevocationCheckDepth, UnknownStatusPolicy,
};
use webpki::{CertRevocationList, RevocationCheckDepth, UnknownStatusPolicy};

use super::{borrow_crls, crl_error, pki_error, VerifierBuilderError};
use super::{pki_error, VerifierBuilderError};
use crate::verify::{
ClientCertVerified, ClientCertVerifier, DigitallySignedStruct, HandshakeSignatureValid,
NoClientAuth,
};
use crate::webpki::parse_crls;
use crate::webpki::verify::{verify_signed_struct, verify_tls13, ParsedCertificate};
use crate::{
CertRevocationListError, DistinguishedName, Error, RootCertStore, SignatureScheme,
WebPkiSupportedAlgorithms,
};
use crate::{DistinguishedName, Error, RootCertStore, SignatureScheme, WebPkiSupportedAlgorithms};

/// A builder for configuring a `webpki` client certificate verifier.
///
Expand Down Expand Up @@ -144,14 +140,7 @@ impl ClientCertVerifierBuilder {

Ok(Arc::new(WebPkiClientVerifier::new(
self.roots,
self.crls
.into_iter()
.map(|der_crl| {
BorrowedCertRevocationList::from_der(der_crl.as_ref())
.and_then(|crl| crl.to_owned())
.map_err(crl_error)
})
.collect::<Result<Vec<_>, CertRevocationListError>>()?,
parse_crls(self.crls)?,
self.revocation_check_depth,
self.unknown_revocation_policy,
self.anon_policy,
Expand Down Expand Up @@ -215,7 +204,7 @@ impl ClientCertVerifierBuilder {
pub struct WebPkiClientVerifier {
roots: Arc<RootCertStore>,
subjects: Vec<DistinguishedName>,
crls: Vec<OwnedCertRevocationList>,
crls: Vec<CertRevocationList<'static>>,
revocation_check_depth: RevocationCheckDepth,
unknown_revocation_policy: UnknownStatusPolicy,
anonymous_policy: AnonymousClientPolicy,
Expand Down Expand Up @@ -256,7 +245,7 @@ impl WebPkiClientVerifier {
/// * `supported_algs` specifies which signature verification algorithms should be used.
pub(crate) fn new(
roots: Arc<RootCertStore>,
crls: Vec<OwnedCertRevocationList>,
crls: Vec<CertRevocationList<'static>>,
revocation_check_depth: RevocationCheckDepth,
unknown_revocation_policy: UnknownStatusPolicy,
anonymous_policy: AnonymousClientPolicy,
Expand Down Expand Up @@ -301,14 +290,15 @@ impl ClientCertVerifier for WebPkiClientVerifier {
now: UnixTime,
) -> Result<ClientCertVerified, Error> {
let cert = ParsedCertificate::try_from(end_entity)?;
let crls = borrow_crls(&self.crls);

let revocation = if crls.is_empty() {
let crl_refs = self.crls.iter().collect::<Vec<_>>();

let revocation = if self.crls.is_empty() {
None
} else {
Some(
webpki::RevocationOptionsBuilder::new(&crls)
.expect("invalid crls")
webpki::RevocationOptionsBuilder::new(&crl_refs)
.unwrap()
.with_depth(self.revocation_check_depth)
.with_status_requirement(self.unknown_revocation_policy)
.build(),
Expand Down
15 changes: 9 additions & 6 deletions rustls/src/webpki/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use alloc::sync::Arc;
use alloc::vec::Vec;
use core::fmt;

use pki_types::CertificateRevocationListDer;
use std::error::Error as StdError;
use webpki::{CertRevocationList, OwnedCertRevocationList};

use crate::error::{CertRevocationListError, CertificateError, Error};

Expand Down Expand Up @@ -104,13 +107,13 @@ fn crl_error(e: webpki::Error) -> CertRevocationListError {
}
}

fn borrow_crls(
crls: &Vec<webpki::OwnedCertRevocationList>,
) -> Vec<&dyn webpki::CertRevocationList> {
#[allow(trivial_casts)] // Cast to &dyn trait is required.
fn parse_crls(
crls: Vec<CertificateRevocationListDer>,
) -> Result<Vec<CertRevocationList>, CertRevocationListError> {
crls.iter()
.map(|crl| crl as &dyn webpki::CertRevocationList)
.collect::<Vec<_>>()
.map(|der| OwnedCertRevocationList::from_der(der.as_ref()).map(Into::into))
.collect::<Result<Vec<_>, _>>()
.map_err(crl_error)
}

mod tests {
Expand Down
31 changes: 10 additions & 21 deletions rustls/src/webpki/server_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use alloc::sync::Arc;
use alloc::vec::Vec;

use pki_types::{CertificateDer, CertificateRevocationListDer, UnixTime};
use webpki::{
BorrowedCertRevocationList, OwnedCertRevocationList, RevocationCheckDepth, UnknownStatusPolicy,
};
use webpki::{CertRevocationList, RevocationCheckDepth, UnknownStatusPolicy};

use crate::verify::{
DigitallySignedStruct, HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier,
Expand All @@ -17,11 +15,8 @@ use crate::webpki::verify::{
verify_server_cert_signed_by_trust_anchor_impl, verify_signed_struct, verify_tls13,
ParsedCertificate,
};
use crate::webpki::{borrow_crls, crl_error, verify_server_name, VerifierBuilderError};
use crate::{
CertRevocationListError, Error, RootCertStore, ServerName, SignatureScheme,
WebPkiSupportedAlgorithms,
};
use crate::webpki::{parse_crls, verify_server_name, VerifierBuilderError};
use crate::{Error, RootCertStore, ServerName, SignatureScheme, WebPkiSupportedAlgorithms};

/// A builder for configuring a `webpki` server certificate verifier.
///
Expand Down Expand Up @@ -127,14 +122,7 @@ impl ServerCertVerifierBuilder {

Ok(Arc::new(WebPkiServerVerifier::new(
self.roots,
self.crls
.into_iter()
.map(|der_crl| {
BorrowedCertRevocationList::from_der(der_crl.as_ref())
.and_then(|crl| crl.to_owned())
.map_err(crl_error)
})
.collect::<Result<Vec<_>, CertRevocationListError>>()?,
parse_crls(self.crls)?,
self.revocation_check_depth,
self.unknown_revocation_policy,
supported_algs,
Expand All @@ -146,7 +134,7 @@ impl ServerCertVerifierBuilder {
#[allow(unreachable_pub)]
pub struct WebPkiServerVerifier {
roots: Arc<RootCertStore>,
crls: Vec<OwnedCertRevocationList>,
crls: Vec<CertRevocationList<'static>>,
revocation_check_depth: RevocationCheckDepth,
unknown_revocation_policy: UnknownStatusPolicy,
supported: WebPkiSupportedAlgorithms,
Expand Down Expand Up @@ -188,7 +176,7 @@ impl WebPkiServerVerifier {
/// certificate verification and TLS handshake signature verification.
pub(crate) fn new(
roots: impl Into<Arc<RootCertStore>>,
crls: Vec<OwnedCertRevocationList>,
crls: Vec<CertRevocationList<'static>>,
revocation_check_depth: RevocationCheckDepth,
unknown_revocation_policy: UnknownStatusPolicy,
supported: WebPkiSupportedAlgorithms,
Expand Down Expand Up @@ -253,14 +241,15 @@ impl ServerCertVerifier for WebPkiServerVerifier {
) -> Result<ServerCertVerified, Error> {
let cert = ParsedCertificate::try_from(end_entity)?;

let crls = borrow_crls(&self.crls);
let revocation = if crls.is_empty() {
let crl_refs = self.crls.iter().collect::<Vec<_>>();

let revocation = if self.crls.is_empty() {
None
} else {
// Note: unwrap here is safe because RevocationOptionsBuilder only errors when given
// empty CRLs.
Some(
webpki::RevocationOptionsBuilder::new(&crls)
webpki::RevocationOptionsBuilder::new(crl_refs.as_slice())
.unwrap()
.with_depth(self.revocation_check_depth)
.with_status_requirement(self.unknown_revocation_policy)
Expand Down

0 comments on commit 264bae2

Please sign in to comment.