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.

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

v0.101.5 preparation #170

Merged
merged 28 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f475596
Pin specific rcgen commit
ctz Aug 23, 2023
b9ec1f1
misc: clippy fixes
cpu Jul 28, 2023
a65cc14
Fix `expect_fun_call` clippy lints
ctz Aug 23, 2023
72b07cc
Track signature limit using `Budget` type
ctz Sep 5, 2023
387afe2
Add comment indicating source of signature budget
ctz Sep 5, 2023
cece639
Apply budget to number of calls to `build_chain_inner`
ctz Sep 5, 2023
575ab1f
Make error ranks more sparse
ctz Sep 5, 2023
2b2d5d6
Introduce and test for `MaximumPathDepthExceeded` error
ctz Sep 5, 2023
92ad878
Import Default trait
djc Sep 5, 2023
667370a
Force all signature verification operations to consume budget
djc Sep 5, 2023
f50e54a
Improve readability of build chain calls budget
djc Sep 5, 2023
e0729cf
subject_name: clarify var name in check_name_constraints
cpu Aug 31, 2023
3a07985
verify_cert: check_signatures -> check_signed_chain
cpu Sep 1, 2023
0ccf679
error: alpha sort
cpu Sep 5, 2023
2c555d9
verify_cert: pull out `make_issuer` test helper
cpu Sep 6, 2023
315d816
verify_cert: pull out `make_end_entity` test helper
cpu Sep 6, 2023
e1423de
verify_cert: pull out `verify_chain` test helper
cpu Sep 6, 2023
3401dd1
verify_cert: budget for name constraint comparisons
cpu Sep 5, 2023
caa516f
verify_cert: name constraint checking on verified chain
cpu Sep 5, 2023
0e100e2
Add tests for PrintableString, empty SEQUENCE CNs
hawkw Sep 6, 2023
9fe852e
Remove tests for common name handling
hawkw Sep 7, 2023
65eb6a0
Remove common name parsing from `NameIterator`
hawkw Sep 7, 2023
6d86211
verify_cert: take references in verify_chain helper
cpu Sep 7, 2023
7ba21bf
verify_cert: optional `Budget` arg for `verify_chain` helper
cpu Sep 7, 2023
b86d82e
error: add is_fatal helper, use in verify_cert
cpu Sep 7, 2023
1bb7ce0
verify_cert: correct handling of fatal errors
cpu Sep 7, 2023
4969bf6
verify_cert: use enum for build chain error
cpu Sep 7, 2023
702d57f
Cargo: bump version 0.101.4 -> 0.101.5
cpu Sep 8, 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
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ license = "ISC"
name = "rustls-webpki"
readme = "README.md"
repository = "https://github.com/rustls/webpki"
version = "0.101.4"
version = "0.101.5"

include = [
"Cargo.toml",
Expand Down Expand Up @@ -101,4 +101,4 @@ harness = false

[patch.crates-io]
# TODO(XXX): Remove this once rcgen has cut a release w/ CRL support included.
rcgen = { git = 'https://github.com/est31/rcgen.git' }
rcgen = { git = 'https://github.com/est31/rcgen.git', rev = '83e548a06848d923eada1ac66d1a912735b67e79' }
8 changes: 8 additions & 0 deletions src/crl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use crate::cert::lenient_certificate_serial_number;
use crate::der::Tag;
use crate::signed_data::{self, SignedData};
use crate::verify_cert::Budget;
use crate::x509::{remember_extension, set_extension_once, Extension};
use crate::{der, Error, SignatureAlgorithm, Time};

Expand Down Expand Up @@ -42,6 +43,7 @@ pub trait CertRevocationList: Sealed {
&self,
supported_sig_algs: &[&SignatureAlgorithm],
issuer_spki: &[u8],
budget: &mut Budget,
) -> Result<(), Error>;
}

Expand Down Expand Up @@ -85,11 +87,13 @@ impl CertRevocationList for OwnedCertRevocationList {
&self,
supported_sig_algs: &[&SignatureAlgorithm],
issuer_spki: &[u8],
budget: &mut Budget,
) -> Result<(), Error> {
signed_data::verify_signed_data(
supported_sig_algs,
untrusted::Input::from(issuer_spki),
&self.signed_data.borrow(),
budget,
)
}
}
Expand Down Expand Up @@ -329,11 +333,13 @@ impl CertRevocationList for BorrowedCertRevocationList<'_> {
&self,
supported_sig_algs: &[&SignatureAlgorithm],
issuer_spki: &[u8],
budget: &mut Budget,
) -> Result<(), Error> {
signed_data::verify_signed_data(
supported_sig_algs,
untrusted::Input::from(issuer_spki),
&self.signed_data,
budget,
)
}
}
Expand Down Expand Up @@ -631,6 +637,8 @@ mod tests {

#[test]
#[cfg(feature = "alloc")]
// redundant clone, clone_on_copy allowed to verify derived traits.
#[allow(clippy::redundant_clone, clippy::clone_on_copy)]
fn test_derived_traits() {
let crl = crate::crl::BorrowedCertRevocationList::from_der(include_bytes!(
"../tests/crls/crl.valid.der"
Expand Down
2 changes: 0 additions & 2 deletions src/der.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ pub(crate) enum Tag {
OctetString = 0x04,
OID = 0x06,
Enum = 0x0A,
UTF8String = 0x0C,
Sequence = CONSTRUCTED | 0x10, // 0x30
Set = CONSTRUCTED | 0x11, // 0x31
UTCTime = 0x17,
GeneralizedTime = 0x18,

Expand Down
56 changes: 56 additions & 0 deletions src/end_entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,3 +257,59 @@ impl<'a> EndEntityCert<'a> {
subject_name::list_cert_dns_names(self)
}
}

#[cfg(feature = "alloc")]
#[cfg(test)]
mod tests {
use super::*;
use crate::test_utils;

// This test reproduces https://github.com/rustls/webpki/issues/167 --- an
// end-entity cert where the common name is a `PrintableString` rather than
// a `UTF8String` cannot iterate over its subject alternative names.
#[test]
fn printable_string_common_name() {
const DNS_NAME: &str = "test.example.com";

let issuer = test_utils::make_issuer("Test", None);

let ee_cert_der = {
let mut params = rcgen::CertificateParams::new(vec![DNS_NAME.to_string()]);
// construct a certificate that uses `PrintableString` as the
// common name value, rather than `UTF8String`.
params.distinguished_name.push(
rcgen::DnType::CommonName,
rcgen::DnValue::PrintableString("example.com".to_string()),
);
params.is_ca = rcgen::IsCa::ExplicitNoCa;
params.alg = test_utils::RCGEN_SIGNATURE_ALG;
let cert = rcgen::Certificate::from_params(params)
.expect("failed to make ee cert (this is a test bug)");
cert.serialize_der_with_signer(&issuer)
.expect("failed to serialize signed ee cert (this is a test bug)")
};

expect_dns_name(&ee_cert_der, DNS_NAME);
}

// This test reproduces https://github.com/rustls/webpki/issues/167 --- an
// end-entity cert where the common name is an empty SEQUENCE.
#[test]
fn empty_sequence_common_name() {
// handcrafted cert DER produced using `ascii2der`, since `rcgen` is
// unwilling to generate this particular weird cert.
let ee_cert_der = include_bytes!("../tests/misc/empty_sequence_common_name.der").as_slice();
expect_dns_name(ee_cert_der, "example.com");
}

fn expect_dns_name(der: &[u8], name: &str) {
let cert =
EndEntityCert::try_from(der).expect("should parse end entity certificate correctly");

let mut names = cert
.dns_names()
.expect("should get all DNS names correctly for end entity cert");
assert_eq!(names.next().map(<&str>::from), Some(name));
assert_eq!(names.next().map(<&str>::from), None);
}
}
97 changes: 68 additions & 29 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

use core::fmt;
use core::ops::ControlFlow;

/// An error that occurs during certificate validation or name validation.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -93,6 +94,15 @@ pub enum Error {
/// invalid labels.
MalformedNameConstraint,

/// The maximum number of name constraint comparisons has been reached.
MaximumNameConstraintComparisonsExceeded,

/// The maximum number of internal path building calls has been reached. Path complexity is too great.
MaximumPathBuildCallsExceeded,

/// The path search was terminated because it became too deep.
MaximumPathDepthExceeded,

/// The maximum number of signature checks has been reached. Path complexity is too great.
MaximumSignatureChecksExceeded,

Expand Down Expand Up @@ -185,51 +195,80 @@ impl Error {
pub(crate) fn rank(&self) -> u32 {
match &self {
// Errors related to certificate validity
Error::CertNotValidYet | Error::CertExpired => 27,
Error::CertNotValidForName => 26,
Error::CertRevoked => 25,
Error::InvalidCrlSignatureForPublicKey | Error::InvalidSignatureForPublicKey => 24,
Error::SignatureAlgorithmMismatch => 23,
Error::RequiredEkuNotFound => 22,
Error::NameConstraintViolation => 21,
Error::PathLenConstraintViolated => 20,
Error::CaUsedAsEndEntity | Error::EndEntityUsedAsCa => 19,
Error::IssuerNotCrlSigner => 18,
Error::CertNotValidYet | Error::CertExpired => 290,
Error::CertNotValidForName => 280,
Error::CertRevoked => 270,
Error::InvalidCrlSignatureForPublicKey | Error::InvalidSignatureForPublicKey => 260,
Error::SignatureAlgorithmMismatch => 250,
Error::RequiredEkuNotFound => 240,
Error::NameConstraintViolation => 230,
Error::PathLenConstraintViolated => 220,
Error::CaUsedAsEndEntity | Error::EndEntityUsedAsCa => 210,
Error::IssuerNotCrlSigner => 200,

// Errors related to supported features used in an invalid way.
Error::InvalidCertValidity => 17,
Error::InvalidNetworkMaskConstraint => 16,
Error::InvalidSerialNumber => 15,
Error::InvalidCrlNumber => 14,
Error::InvalidCertValidity => 190,
Error::InvalidNetworkMaskConstraint => 180,
Error::InvalidSerialNumber => 170,
Error::InvalidCrlNumber => 160,

// Errors related to unsupported features.
Error::UnsupportedCrlSignatureAlgorithmForPublicKey
| Error::UnsupportedSignatureAlgorithmForPublicKey => 13,
Error::UnsupportedCrlSignatureAlgorithm | Error::UnsupportedSignatureAlgorithm => 12,
Error::UnsupportedCriticalExtension => 11,
Error::UnsupportedCertVersion => 11,
Error::UnsupportedCrlVersion => 10,
Error::UnsupportedDeltaCrl => 9,
Error::UnsupportedIndirectCrl => 8,
Error::UnsupportedRevocationReason => 7,
| Error::UnsupportedSignatureAlgorithmForPublicKey => 150,
Error::UnsupportedCrlSignatureAlgorithm | Error::UnsupportedSignatureAlgorithm => 140,
Error::UnsupportedCriticalExtension => 130,
Error::UnsupportedCertVersion => 130,
Error::UnsupportedCrlVersion => 120,
Error::UnsupportedDeltaCrl => 110,
Error::UnsupportedIndirectCrl => 100,
Error::UnsupportedRevocationReason => 90,
// Reserved for webpki 0.102.0+ usages:
// Error::UnsupportedRevocationReasonsPartitioning => 80,
// Error::UnsupportedCrlIssuingDistributionPoint => 70,
Error::MaximumPathDepthExceeded => 61,

// Errors related to malformed data.
Error::MalformedDnsIdentifier => 6,
Error::MalformedNameConstraint => 5,
Error::MalformedExtensions => 4,
Error::ExtensionValueInvalid => 3,
Error::MalformedDnsIdentifier => 60,
Error::MalformedNameConstraint => 50,
Error::MalformedExtensions => 40,
Error::ExtensionValueInvalid => 30,

// Generic DER errors.
Error::BadDerTime => 2,
Error::BadDer => 1,
Error::BadDerTime => 20,
Error::BadDer => 10,

// Special case error - not subject to ranking.
// Special case errors - not subject to ranking.
Error::MaximumSignatureChecksExceeded => 0,
Error::MaximumPathBuildCallsExceeded => 0,
Error::MaximumNameConstraintComparisonsExceeded => 0,

// Default catch all error - should be renamed in the future.
Error::UnknownIssuer => 0,
}
}

/// Returns true for errors that should be considered fatal during path building. Errors of
/// this class should halt any further path building and be returned immediately.
#[inline]
pub(crate) fn is_fatal(&self) -> bool {
matches!(
self,
Error::MaximumSignatureChecksExceeded
| Error::MaximumPathBuildCallsExceeded
| Error::MaximumNameConstraintComparisonsExceeded
)
}
}

impl From<Error> for ControlFlow<Error, Error> {
fn from(value: Error) -> Self {
match value {
// If an error is fatal, we've exhausted the potential for continued search.
err if err.is_fatal() => Self::Break(err),
// Otherwise we've rejected one candidate chain, but may continue to search for others.
err => Self::Continue(err),
}
}
}

impl fmt::Display for Error {
Expand Down
4 changes: 4 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ mod x509;

#[allow(deprecated)]
pub use trust_anchor::{TlsClientTrustAnchors, TlsServerTrustAnchors};

#[cfg(test)]
pub(crate) mod test_utils;

pub use {
cert::{Cert, EndEntityOrCa},
crl::{BorrowedCertRevocationList, BorrowedRevokedCert, CertRevocationList, RevocationReason},
Expand Down
11 changes: 10 additions & 1 deletion src/signed_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

use crate::verify_cert::Budget;
use crate::{der, Error};
use ring::signature;

#[cfg(feature = "alloc")]
use alloc::vec::Vec;

/// X.509 certificates and related items that are signed are almost always
/// encoded in the format "tbs||signatureAlgorithm||signature". This structure
/// captures this pattern as an owned data type.
Expand Down Expand Up @@ -152,7 +156,10 @@ pub(crate) fn verify_signed_data(
supported_algorithms: &[&SignatureAlgorithm],
spki_value: untrusted::Input,
signed_data: &SignedData,
budget: &mut Budget,
) -> Result<(), Error> {
budget.consume_signature()?;

// We need to verify the signature in `signed_data` using the public key
// in `public_key`. In order to know which *ring* signature verification
// algorithm to use, we need to know the public key algorithm (ECDSA,
Expand Down Expand Up @@ -498,7 +505,8 @@ mod tests {
signed_data::verify_signed_data(
SUPPORTED_ALGORITHMS_IN_TESTS,
spki_value,
&signed_data
&signed_data,
&mut Budget::default()
)
);
}
Expand Down Expand Up @@ -795,6 +803,7 @@ mod tests {
}
}

use crate::verify_cert::Budget;
use alloc::str::Lines;

fn read_pem_section(lines: &mut Lines, section_name: &str) -> Vec<u8> {
Expand Down
4 changes: 1 addition & 3 deletions src/subject_name/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,4 @@ pub use ip_address::IpAddr;
mod verify;
#[cfg(feature = "alloc")]
pub(super) use verify::list_cert_dns_names;
pub(super) use verify::{
check_name_constraints, verify_cert_subject_name, SubjectCommonNameContents,
};
pub(super) use verify::{check_name_constraints, verify_cert_subject_name};
Loading