Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
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

ExtensionPolicy changes required for Python callback support. #12417

Merged
merged 3 commits into from
Feb 8, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
ExtensionPolicy uses VerificationCertificate.
  • Loading branch information
deivse committed Feb 8, 2025
commit b4351c01b8cb12d2b5fcce52c84452bd0477b838
2 changes: 1 addition & 1 deletion src/rust/cryptography-x509-verification/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ impl<'a, 'chain, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
let leaf_extensions = leaf.certificate().extensions()?;

self.policy
.permits_ee(leaf.certificate(), &leaf_extensions)
.permits_ee(leaf, &leaf_extensions)
.map_err(|e| e.set_cert(leaf.clone()))?;

let mut chain = self.build_chain_inner(
Expand Down
85 changes: 48 additions & 37 deletions src/rust/cryptography-x509-verification/src/policy/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@

use std::sync::Arc;

use cryptography_x509::extensions::{Extension, Extensions};
use cryptography_x509::oid::{
AUTHORITY_INFORMATION_ACCESS_OID, AUTHORITY_KEY_IDENTIFIER_OID, BASIC_CONSTRAINTS_OID,
EXTENDED_KEY_USAGE_OID, KEY_USAGE_OID, NAME_CONSTRAINTS_OID, SUBJECT_ALTERNATIVE_NAME_OID,
SUBJECT_KEY_IDENTIFIER_OID,
};
use cryptography_x509::{
certificate::Certificate,
extensions::{Extension, Extensions},
};

use crate::ops::VerificationCertificate;
use crate::{
ops::CryptoOps, policy::Policy, ValidationError, ValidationErrorKind, ValidationResult,
};
Expand Down Expand Up @@ -122,7 +120,7 @@ impl<'cb, B: CryptoOps + 'cb> ExtensionPolicy<'cb, B> {
pub(crate) fn permits<'chain>(
&self,
policy: &Policy<'_, B>,
cert: &Certificate<'chain>,
cert: &VerificationCertificate<'chain, B>,
extensions: &Extensions<'_>,
) -> ValidationResult<'chain, (), B> {
let mut authority_information_access_seen = false;
Expand Down Expand Up @@ -240,7 +238,7 @@ impl Criticality {
pub type PresentExtensionValidatorCallback<'cb, B> = Arc<
dyn for<'chain> Fn(
&Policy<'_, B>,
&Certificate<'chain>,
&VerificationCertificate<'chain, B>,
&Extension<'_>,
) -> ValidationResult<'chain, (), B>
+ Send
Expand All @@ -251,7 +249,7 @@ pub type PresentExtensionValidatorCallback<'cb, B> = Arc<
pub type MaybeExtensionValidatorCallback<'cb, B> = Arc<
dyn for<'chain> Fn(
&Policy<'_, B>,
&Certificate<'chain>,
&VerificationCertificate<'chain, B>,
Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B>
+ Send
Expand Down Expand Up @@ -307,7 +305,7 @@ impl<'cb, B: CryptoOps> ExtensionValidator<'cb, B> {
pub(crate) fn permits<'chain>(
&self,
policy: &Policy<'_, B>,
cert: &Certificate<'chain>,
cert: &VerificationCertificate<'chain, B>,
extension: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
match (self, extension) {
Expand Down Expand Up @@ -367,17 +365,16 @@ impl<'cb, B: CryptoOps> ExtensionValidator<'cb, B> {
}

mod ee {
use cryptography_x509::certificate::Certificate;
use cryptography_x509::extensions::{
BasicConstraints, ExtendedKeyUsage, Extension, KeyUsage, SubjectAlternativeName,
};

use crate::ops::CryptoOps;
use crate::ops::{CryptoOps, VerificationCertificate};
use crate::policy::{Policy, ValidationError, ValidationErrorKind, ValidationResult};

pub(crate) fn basic_constraints<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
if let Some(extn) = extn {
Expand All @@ -395,10 +392,10 @@ mod ee {

pub(crate) fn subject_alternative_name<'chain, B: CryptoOps>(
policy: &Policy<'_, B>,
cert: &Certificate<'chain>,
cert: &VerificationCertificate<'chain, B>,
extn: &Extension<'_>,
) -> ValidationResult<'chain, (), B> {
match (cert.subject().is_empty(), extn.critical) {
match (cert.certificate().subject().is_empty(), extn.critical) {
// If the subject is empty, the SAN MUST be critical.
(true, false) => {
return Err(ValidationError::new(ValidationErrorKind::Other(
Expand Down Expand Up @@ -432,7 +429,7 @@ mod ee {

pub(crate) fn extended_key_usage<'chain, B: CryptoOps>(
policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
if let Some(extn) = extn {
Expand All @@ -458,7 +455,7 @@ mod ee {

pub(crate) fn key_usage<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
if let Some(extn) = extn {
Expand All @@ -476,20 +473,19 @@ mod ee {
}

mod ca {
use cryptography_x509::certificate::Certificate;
use cryptography_x509::common::Asn1Read;
use cryptography_x509::extensions::{
AuthorityKeyIdentifier, BasicConstraints, ExtendedKeyUsage, Extension, KeyUsage,
NameConstraints,
};
use cryptography_x509::oid::EKU_ANY_KEY_USAGE_OID;

use crate::ops::CryptoOps;
use crate::ops::{CryptoOps, VerificationCertificate};
use crate::policy::{Policy, ValidationError, ValidationErrorKind, ValidationResult};

pub(crate) fn authority_key_identifier<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
// CABF: AKI is required on all CA certificates *except* root CA certificates,
Expand Down Expand Up @@ -532,7 +528,7 @@ mod ca {

pub(crate) fn key_usage<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: &Extension<'_>,
) -> ValidationResult<'chain, (), B> {
let key_usage: KeyUsage<'_> = extn.value()?;
Expand All @@ -548,7 +544,7 @@ mod ca {

pub(crate) fn basic_constraints<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: &Extension<'_>,
) -> ValidationResult<'chain, (), B> {
let basic_constraints: BasicConstraints = extn.value()?;
Expand All @@ -568,7 +564,7 @@ mod ca {

pub(crate) fn name_constraints<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
if let Some(extn) = extn {
Expand Down Expand Up @@ -600,7 +596,7 @@ mod ca {

pub(crate) fn extended_key_usage<'chain, B: CryptoOps>(
policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
if let Some(extn) = extn {
Expand All @@ -622,16 +618,15 @@ mod ca {
}

mod common {
use cryptography_x509::certificate::Certificate;
use cryptography_x509::common::Asn1Read;
use cryptography_x509::extensions::{Extension, SequenceOfAccessDescriptions};

use crate::ops::CryptoOps;
use crate::ops::{CryptoOps, VerificationCertificate};
use crate::policy::{Policy, ValidationResult};

pub(crate) fn authority_information_access<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
extn: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
if let Some(extn) = extn {
Expand All @@ -649,14 +644,13 @@ mod tests {
use std::sync::Arc;

use asn1::{ObjectIdentifier, SimpleAsn1Writable};
use cryptography_x509::certificate::Certificate;
use cryptography_x509::extensions::{BasicConstraints, Extension};
use cryptography_x509::oid::BASIC_CONSTRAINTS_OID;

use super::{Criticality, ExtensionValidator};
use crate::certificate::tests::PublicKeyErrorOps;
use crate::ops::tests::{cert, v1_cert_pem};
use crate::ops::CryptoOps;
use crate::ops::{CryptoOps, VerificationCertificate};
use crate::policy::{Policy, PolicyDefinition, Subject, ValidationResult};
use crate::types::DNSName;

Expand Down Expand Up @@ -695,7 +689,7 @@ mod tests {

fn present_extension_validator<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
_ext: &Extension<'_>,
) -> ValidationResult<'chain, (), B> {
Ok(())
Expand All @@ -706,6 +700,7 @@ mod tests {
// The certificate doesn't get used for this validator, so the certificate we use isn't important.
let cert_pem = v1_cert_pem();
let cert = cert(&cert_pem);
let verification_cert = VerificationCertificate::new(&cert, ());
let ops = PublicKeyErrorOps {};
let policy_def = PolicyDefinition::server(
ops,
Expand All @@ -729,16 +724,18 @@ mod tests {
let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, true, &bc);
let raw_ext = asn1::parse_single(&der_ext).unwrap();
assert!(extension_validator
.permits(&policy, &cert, Some(&raw_ext))
.permits(&policy, &verification_cert, Some(&raw_ext))
.is_ok());

// Check the case where the extension isn't present.
assert!(extension_validator.permits(&policy, &cert, None).is_err());
assert!(extension_validator
.permits(&policy, &verification_cert, None)
.is_err());
}

fn maybe_extension_validator<'chain, B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'chain>,
_cert: &VerificationCertificate<'chain, B>,
_ext: Option<&Extension<'_>>,
) -> ValidationResult<'chain, (), B> {
Ok(())
Expand All @@ -749,6 +746,7 @@ mod tests {
// The certificate doesn't get used for this validator, so the certificate we use isn't important.
let cert_pem = v1_cert_pem();
let cert = cert(&cert_pem);
let verification_cert = VerificationCertificate::new(&cert, ());
let ops = PublicKeyErrorOps {};
let policy_def = PolicyDefinition::server(
ops,
Expand All @@ -772,18 +770,21 @@ mod tests {
let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, true, &bc);
let raw_ext = asn1::parse_single(&der_ext).unwrap();
assert!(extension_validator
.permits(&policy, &cert, Some(&raw_ext))
.permits(&policy, &verification_cert, Some(&raw_ext))
.is_ok());

// Check the case where the extension isn't present.
assert!(extension_validator.permits(&policy, &cert, None).is_ok());
assert!(extension_validator
.permits(&policy, &verification_cert, None)
.is_ok());
}

#[test]
fn test_extension_validator_not_present() {
// The certificate doesn't get used for this validator, so the certificate we use isn't important.
let cert_pem = v1_cert_pem();
let cert = cert(&cert_pem);
let verification_cert = VerificationCertificate::new(&cert, ());
let ops = PublicKeyErrorOps {};
let policy_def = PolicyDefinition::server(
ops,
Expand All @@ -804,11 +805,13 @@ mod tests {
let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, true, &bc);
let raw_ext = asn1::parse_single(&der_ext).unwrap();
assert!(extension_validator
.permits(&policy, &cert, Some(&raw_ext))
.permits(&policy, &verification_cert, Some(&raw_ext))
.is_err());

// Check the case where the extension isn't present.
assert!(extension_validator.permits(&policy, &cert, None).is_ok());
assert!(extension_validator
.permits(&policy, &verification_cert, None)
.is_ok());
}

#[test]
Expand Down Expand Up @@ -839,7 +842,11 @@ mod tests {
let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, false, &bc);
let raw_ext = asn1::parse_single(&der_ext).unwrap();
assert!(extension_validator
.permits(&policy, &cert, Some(&raw_ext))
.permits(
&policy,
&VerificationCertificate::new(&cert, ()),
Some(&raw_ext)
)
.is_err());
}

Expand Down Expand Up @@ -871,7 +878,11 @@ mod tests {
let der_ext = create_encoded_extension(BASIC_CONSTRAINTS_OID, false, &bc);
let raw_ext = asn1::parse_single(&der_ext).unwrap();
assert!(extension_validator
.permits(&policy, &cert, Some(&raw_ext))
.permits(
&policy,
&VerificationCertificate::new(&cert, ()),
Some(&raw_ext)
)
.is_err());
}
}
10 changes: 5 additions & 5 deletions src/rust/cryptography-x509-verification/src/policy/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,11 +376,11 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
/// Checks whether the given CA certificate is compatible with this policy.
pub(crate) fn permits_ca<'chain>(
&self,
cert: &Certificate<'chain>,
cert: &VerificationCertificate<'chain, B>,
current_depth: u8,
extensions: &Extensions<'_>,
) -> ValidationResult<'chain, (), B> {
self.permits_basic(cert)?;
self.permits_basic(cert.certificate())?;

// 5280 4.1.2.6: Subject
// CA certificates MUST have a subject populated with a non-empty distinguished name.
Expand Down Expand Up @@ -416,10 +416,10 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
/// Checks whether the given EE certificate is compatible with this policy.
pub(crate) fn permits_ee<'chain>(
&self,
cert: &Certificate<'chain>,
cert: &VerificationCertificate<'chain, B>,
extensions: &Extensions<'_>,
) -> ValidationResult<'chain, (), B> {
self.permits_basic(cert)?;
self.permits_basic(cert.certificate())?;

self.ee_extension_policy.permits(self, cert, extensions)?;

Expand Down Expand Up @@ -447,7 +447,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
issuer_extensions: &Extensions<'_>,
) -> ValidationResult<'chain, (), B> {
// The issuer needs to be a valid CA at the current depth.
self.permits_ca(issuer.certificate(), current_depth, issuer_extensions)
self.permits_ca(issuer, current_depth, issuer_extensions)
.map_err(|e| e.set_cert(issuer.clone()))?;

// CA/B 7.1.3.1 SubjectPublicKeyInfo
Expand Down