Skip to content

Commit

Permalink
trust_anchor: clarify and rename extract_trust_anchor
Browse files Browse the repository at this point in the history
This function has caused some confusion w.r.t self-signed end-entity
certificates and their support within rustls/webpki. This commit takes
two actions:

1. It renames `extract_trust_anchor` to
   `extract_prevalidated_trust_anchor` to emphasize that the input
   **must** be validated out-of-band as suitable to use as a trust
   anchor.
2. It adds more detail to the rustdoc comment for the function, in
   particular emphasizing why no additional validations are performed,
   and that it is not appropriate to use with an end-entity certificate
   in an attempt to support self-signed certificate use-cases.
  • Loading branch information
cpu committed Oct 16, 2023
1 parent 5d17ef0 commit 03dd414
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub use {
AddrParseError, DnsNameRef, InvalidDnsNameError, InvalidSubjectNameError, IpAddrRef,
SubjectNameRef,
},
trust_anchor::extract_trust_anchor,
trust_anchor::extract_prevalidated_trust_anchor,
verify_cert::KeyUsage,
};

Expand Down
24 changes: 19 additions & 5 deletions src/trust_anchor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,25 @@ use crate::cert::{lenient_certificate_serial_number, Cert};
use crate::der;
use crate::error::{DerTypeId, Error};

/// Interprets the given DER-encoded certificate as a `TrustAnchor`. The
/// certificate is not validated. In particular, there is no check that the
/// certificate is self-signed or even that the certificate has the cA basic
/// constraint.
pub fn extract_trust_anchor<'a>(cert: &'a CertificateDer<'a>) -> Result<TrustAnchor<'a>, Error> {
/// Interprets the given pre-validated DER-encoded certificate as a `TrustAnchor`.
///
/// This function extracts the components of a trust anchor (see [RFC 5280 6.1.1]) from
/// an X.509 certificate that has **already been validated as trustworthy** out-of-band.
///
/// All callers **must** verify the subject name, public key and name constraints
/// are trustworthy and match expected values before using this function. No additional checks
/// on the content of the certificate, including whether it is self-signed, or has a basic
/// constraints extension indicating the `cA` boolean is true, will be performed.
///
/// This function is intended for users constructing `TrustAnchor`'s from existing trust stores
/// that express trust anchors as X.509 certificates. It should **not** be used to treat an
/// end-entity certificate as a `TrustAnchor` in an effort to validate the same end-entity
/// certificate during path building. Webpki has no support for self-signed certificates.
///
/// [RFC 5280 6.1.1]: <https://datatracker.ietf.org/doc/html/rfc5280#section-6.1.1>
pub fn extract_prevalidated_trust_anchor<'a>(
cert: &'a CertificateDer<'a>,
) -> Result<TrustAnchor<'a>, Error> {
let cert_der = untrusted::Input::from(cert.as_ref());

// XXX: `EndEntityOrCA::EndEntity` is used instead of `EndEntityOrCA::CA`
Expand Down
10 changes: 5 additions & 5 deletions src/verify_cert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ pub(crate) enum Role {
mod tests {
use super::*;
use crate::test_utils::{issuer_params, make_end_entity, make_issuer};
use crate::trust_anchor::extract_trust_anchor;
use crate::trust_anchor::extract_prevalidated_trust_anchor;

#[test]
fn eku_key_purpose_id() {
Expand Down Expand Up @@ -746,7 +746,7 @@ mod tests {
let ee_der = make_end_entity(&issuer);
let ee_cert = EndEntityCert::try_from(&ee_der).unwrap();
verify_chain(
&[extract_trust_anchor(&trust_anchor).unwrap()],
&[extract_prevalidated_trust_anchor(&trust_anchor).unwrap()],
&intermediates,
&ee_cert,
None,
Expand Down Expand Up @@ -775,7 +775,7 @@ mod tests {
fn build_linear_chain(chain_length: usize) -> Result<(), ControlFlow<Error, Error>> {
let ca_cert = make_issuer(format!("Bogus Subject {chain_length}"));
let ca_cert_der = CertificateDer::from(ca_cert.serialize_der().unwrap());
let anchor = extract_trust_anchor(&ca_cert_der).unwrap();
let anchor = extract_prevalidated_trust_anchor(&ca_cert_der).unwrap();
let anchors = &[anchor.clone()];

let mut intermediates = Vec::with_capacity(chain_length);
Expand Down Expand Up @@ -859,7 +859,7 @@ mod tests {
});
let ca_cert = rcgen::Certificate::from_params(ca_cert_params).unwrap();
let ca_cert_der = CertificateDer::from(ca_cert.serialize_der().unwrap());
let anchors = &[extract_trust_anchor(&ca_cert_der).unwrap()];
let anchors = &[extract_prevalidated_trust_anchor(&ca_cert_der).unwrap()];

// Create a series of intermediate issuers. We'll only use one in the actual built path,
// helping demonstrate that the name constraint budget is not expended checking certificates
Expand Down Expand Up @@ -973,7 +973,7 @@ mod tests {
let trust_anchor_der = CertificateDer::from(trust_anchor.serialize_der().unwrap());
let trust_anchor_cert =
Cert::from_der(untrusted::Input::from(trust_anchor_der.as_ref())).unwrap();
let trust_anchors = &[extract_trust_anchor(&trust_anchor_der).unwrap()];
let trust_anchors = &[extract_prevalidated_trust_anchor(&trust_anchor_der).unwrap()];

let intermediate_a = make_issuer("Intermediate A");
let intermediate_a_der = CertificateDer::from(
Expand Down
6 changes: 3 additions & 3 deletions tests/better_tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use pki_types::UnixTime;
use serde::Deserialize;

use webpki::types::{CertificateDer, SignatureVerificationAlgorithm, TrustAnchor};
use webpki::{extract_trust_anchor, KeyUsage, SubjectNameRef};
use webpki::{extract_prevalidated_trust_anchor, KeyUsage, SubjectNameRef};

// All of the BetterTLS testcases use P256 keys.
static ALGS: &[&dyn SignatureVerificationAlgorithm] = &[
Expand All @@ -26,7 +26,7 @@ fn path_building() {
let better_tls = testdata();
let root_der = &better_tls.root_der();
let root_der = CertificateDer::from(root_der.as_slice());
let roots = &[extract_trust_anchor(&root_der).expect("invalid trust anchor")];
let roots = &[extract_prevalidated_trust_anchor(&root_der).expect("invalid trust anchor")];

let suite = "pathbuilding";
run_testsuite(
Expand All @@ -45,7 +45,7 @@ fn name_constraints() {
let better_tls = testdata();
let root_der = &better_tls.root_der();
let root_der = CertificateDer::from(root_der.as_slice());
let roots = &[extract_trust_anchor(&root_der).expect("invalid trust anchor")];
let roots = &[extract_prevalidated_trust_anchor(&root_der).expect("invalid trust anchor")];

let suite = "nameconstraints";
run_testsuite(
Expand Down
4 changes: 2 additions & 2 deletions tests/client_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@

use core::time::Duration;
use pki_types::{CertificateDer, UnixTime};
use webpki::{extract_trust_anchor, KeyUsage};
use webpki::{extract_prevalidated_trust_anchor, KeyUsage};

fn check_cert(ee: &[u8], ca: &[u8]) -> Result<(), webpki::Error> {
let ca = CertificateDer::from(ca);
let anchors = &[extract_trust_anchor(&ca).unwrap()];
let anchors = &[extract_prevalidated_trust_anchor(&ca).unwrap()];

let time = UnixTime::since_unix_epoch(Duration::from_secs(0x1fed_f00d));
let ee = CertificateDer::from(ee);
Expand Down
4 changes: 2 additions & 2 deletions tests/client_auth_revocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use core::time::Duration;

use pki_types::{CertificateDer, SignatureVerificationAlgorithm, UnixTime};
use webpki::{
extract_trust_anchor, KeyUsage, RevocationCheckDepth, RevocationOptions,
extract_prevalidated_trust_anchor, KeyUsage, RevocationCheckDepth, RevocationOptions,
RevocationOptionsBuilder,
};

Expand All @@ -36,7 +36,7 @@ fn check_cert(
revocation: Option<RevocationOptions>,
) -> Result<(), webpki::Error> {
let ca = CertificateDer::from(ca);
let anchors = &[extract_trust_anchor(&ca).unwrap()];
let anchors = &[extract_prevalidated_trust_anchor(&ca).unwrap()];
let ee = CertificateDer::from(ee);
let cert = webpki::EndEntityCert::try_from(&ee).unwrap();
let time = UnixTime::since_unix_epoch(Duration::from_secs(0x1fed_f00d));
Expand Down
4 changes: 2 additions & 2 deletions tests/custom_ekus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use core::time::Duration;

use pki_types::{CertificateDer, UnixTime};
use webpki::{extract_trust_anchor, KeyUsage};
use webpki::{extract_prevalidated_trust_anchor, KeyUsage};

fn check_cert(
ee: &[u8],
Expand All @@ -13,7 +13,7 @@ fn check_cert(
result: Result<(), webpki::Error>,
) {
let ca = CertificateDer::from(ca);
let anchors = [extract_trust_anchor(&ca).unwrap()];
let anchors = [extract_prevalidated_trust_anchor(&ca).unwrap()];

let ee = CertificateDer::from(ee);
let cert = webpki::EndEntityCert::try_from(&ee).unwrap();
Expand Down
18 changes: 9 additions & 9 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
use core::time::Duration;

use pki_types::{CertificateDer, UnixTime};
use webpki::{extract_trust_anchor, KeyUsage};
use webpki::{extract_prevalidated_trust_anchor, KeyUsage};

/* Checks we can verify netflix's cert chain. This is notable
* because they're rooted at a Verisign v1 root. */
Expand All @@ -28,7 +28,7 @@ fn netflix() {
let inter = CertificateDer::from(&include_bytes!("netflix/inter.der")[..]);
let ca = CertificateDer::from(&include_bytes!("netflix/ca.der")[..]);

let anchors = [extract_trust_anchor(&ca).unwrap()];
let anchors = [extract_prevalidated_trust_anchor(&ca).unwrap()];

let time = UnixTime::since_unix_epoch(Duration::from_secs(1_492_441_716)); // 2017-04-17T15:08:36Z

Expand Down Expand Up @@ -56,7 +56,7 @@ fn cloudflare_dns() {
let ca = CertificateDer::from(&include_bytes!("cloudflare_dns/ca.der")[..]);

let ca_cert = CertificateDer::from(&ca[..]);
let anchors = [extract_trust_anchor(&ca_cert).unwrap()];
let anchors = [extract_prevalidated_trust_anchor(&ca_cert).unwrap()];

let time = UnixTime::since_unix_epoch(Duration::from_secs(1_663_495_771));

Expand Down Expand Up @@ -111,7 +111,7 @@ fn wpt() {
let ee = CertificateDer::from(&include_bytes!("wpt/ee.der")[..]);
let ca = CertificateDer::from(&include_bytes!("wpt/ca.der")[..]);

let anchors = [extract_trust_anchor(&ca).unwrap()];
let anchors = [extract_prevalidated_trust_anchor(&ca).unwrap()];

let time = UnixTime::since_unix_epoch(Duration::from_secs(1_619_256_684)); // 2021-04-24T09:31:24Z
let cert = webpki::EndEntityCert::try_from(&ee).unwrap();
Expand All @@ -133,7 +133,7 @@ fn ed25519() {
let ee = CertificateDer::from(&include_bytes!("ed25519/ee.der")[..]);
let ca = CertificateDer::from(&include_bytes!("ed25519/ca.der")[..]);

let anchors = [extract_trust_anchor(&ca).unwrap()];
let anchors = [extract_prevalidated_trust_anchor(&ca).unwrap()];

let time = UnixTime::since_unix_epoch(Duration::from_secs(1_547_363_522)); // 2019-01-13T07:12:02Z

Expand All @@ -158,7 +158,7 @@ fn critical_extensions() {
let ca = CertificateDer::from(&include_bytes!("critical_extensions/ca-cert.der")[..]);

let time = UnixTime::since_unix_epoch(Duration::from_secs(1_670_779_098));
let anchors = [extract_trust_anchor(&root).unwrap()];
let anchors = [extract_prevalidated_trust_anchor(&root).unwrap()];
let intermediates = [ca];

let ee = CertificateDer::from(
Expand Down Expand Up @@ -195,13 +195,13 @@ fn critical_extensions() {
#[test]
fn read_root_with_zero_serial() {
let ca = CertificateDer::from(&include_bytes!("misc/serial_zero.der")[..]);
extract_trust_anchor(&ca).expect("godaddy cert should parse as anchor");
extract_prevalidated_trust_anchor(&ca).expect("godaddy cert should parse as anchor");
}

#[test]
fn read_root_with_neg_serial() {
let ca = CertificateDer::from(&include_bytes!("misc/serial_neg.der")[..]);
extract_trust_anchor(&ca).expect("idcat cert should parse as anchor");
extract_prevalidated_trust_anchor(&ca).expect("idcat cert should parse as anchor");
}

#[test]
Expand All @@ -210,7 +210,7 @@ fn read_ee_with_neg_serial() {
let ca = CertificateDer::from(&include_bytes!("misc/serial_neg_ca.der")[..]);
let ee = CertificateDer::from(&include_bytes!("misc/serial_neg_ee.der")[..]);

let anchors = [extract_trust_anchor(&ca).unwrap()];
let anchors = [extract_prevalidated_trust_anchor(&ca).unwrap()];

let time = UnixTime::since_unix_epoch(Duration::from_secs(1_667_401_500)); // 2022-11-02T15:05:00Z

Expand Down
4 changes: 2 additions & 2 deletions tests/tls_server_certs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use core::time::Duration;

use pki_types::{CertificateDer, UnixTime};
use webpki::{extract_trust_anchor, KeyUsage};
use webpki::{extract_prevalidated_trust_anchor, KeyUsage};

fn check_cert(
ee: &[u8],
Expand All @@ -25,7 +25,7 @@ fn check_cert(
invalid_names: &[&str],
) -> Result<(), webpki::Error> {
let ca_cert_der = CertificateDer::from(ca);
let anchors = [extract_trust_anchor(&ca_cert_der).unwrap()];
let anchors = [extract_prevalidated_trust_anchor(&ca_cert_der).unwrap()];

let ee_der = CertificateDer::from(ee);
let time = UnixTime::since_unix_epoch(Duration::from_secs(0x1fed_f00d));
Expand Down

0 comments on commit 03dd414

Please sign in to comment.