Skip to content

Commit

Permalink
Use a type alias for ValidationResult (#11866)
Browse files Browse the repository at this point in the history 
refs #11160
  • Loading branch information
@alex
alex authored Oct 30, 2024
1 parent 73f5758 commit e2fce25
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 34 deletions.
16 changes: 9 additions & 7 deletions src/rust/cryptography-x509-verification/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ pub enum ValidationError {
Other(String),
}

pub type ValidationResult<T> = Result<T, ValidationError>;

impl From<asn1::ParseError> for ValidationError {
fn from(value: asn1::ParseError) -> Self {
Self::Malformed(value)
Expand Down Expand Up @@ -89,7 +91,7 @@ impl Budget {
}
}

fn name_constraint_check(&mut self) -> Result<(), ValidationError> {
fn name_constraint_check(&mut self) -> ValidationResult<()> {
self.name_constraint_checks =
self.name_constraint_checks
.checked_sub(1)
Expand All @@ -110,7 +112,7 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
child: Option<&'a NameChain<'a, 'chain>>,
extensions: &Extensions<'chain>,
self_issued_intermediate: bool,
) -> Result<Self, ValidationError> {
) -> ValidationResult<Self> {
let sans = match (
self_issued_intermediate,
extensions.get_extension(&SUBJECT_ALTERNATIVE_NAME_OID),
Expand All @@ -129,7 +131,7 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
constraint: &GeneralName<'chain>,
san: &GeneralName<'chain>,
budget: &mut Budget,
) -> Result<ApplyNameConstraintStatus, ValidationError> {
) -> ValidationResult<ApplyNameConstraintStatus> {
budget.name_constraint_check()?;

match (constraint, san) {
Expand Down Expand Up @@ -195,7 +197,7 @@ impl<'a, 'chain> NameChain<'a, 'chain> {
&self,
constraints: &NameConstraints<'chain>,
budget: &mut Budget,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
if let Some(child) = self.child {
child.evaluate_constraints(constraints, budget)?;
}
Expand Down Expand Up @@ -244,7 +246,7 @@ pub fn verify<'a, 'chain: 'a, B: CryptoOps>(
intermediates: &'a [&'a VerificationCertificate<'chain, B>],
policy: &'a Policy<'_, B>,
store: &'a Store<'chain, B>,
) -> Result<Chain<'a, 'chain, B>, ValidationError> {
) -> ValidationResult<Chain<'a, 'chain, B>> {
let builder = ChainBuilder::new(intermediates, policy, store);

let mut budget = Budget::new();
Expand Down Expand Up @@ -310,7 +312,7 @@ impl<'a, 'chain: 'a, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
working_cert_extensions: &Extensions<'chain>,
name_chain: NameChain<'_, 'chain>,
budget: &mut Budget,
) -> Result<Chain<'a, 'chain, B>, ValidationError> {
) -> ValidationResult<Chain<'a, 'chain, B>> {
if let Some(nc) = working_cert_extensions.get_extension(&NAME_CONSTRAINTS_OID) {
name_chain.evaluate_constraints(&nc.value()?, budget)?;
}
Expand Down Expand Up @@ -413,7 +415,7 @@ impl<'a, 'chain: 'a, B: CryptoOps> ChainBuilder<'a, 'chain, B> {
&self,
leaf: &'a VerificationCertificate<'chain, B>,
budget: &mut Budget,
) -> Result<Chain<'a, 'chain, B>, ValidationError> {
) -> ValidationResult<Chain<'a, 'chain, B>> {
// Before anything else, check whether the given leaf cert
// is well-formed according to our policy (and its underlying
// certificate profile).
Expand Down
42 changes: 21 additions & 21 deletions src/rust/cryptography-x509-verification/src/policy/extension.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use cryptography_x509::{
extensions::{Extension, Extensions},
};

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

pub(crate) struct ExtensionPolicy<B: CryptoOps> {
pub(crate) authority_information_access: ExtensionValidator<B>,
Expand All @@ -31,7 +31,7 @@ impl<B: CryptoOps> ExtensionPolicy<B> {
policy: &Policy<'_, B>,
cert: &Certificate<'_>,
extensions: &Extensions<'_>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
let mut authority_information_access_seen = false;
let mut authority_key_identifier_seen = false;
let mut subject_key_identifier_seen = false;
Expand Down Expand Up @@ -145,10 +145,10 @@ impl Criticality {
}

type PresentExtensionValidatorCallback<B> =
fn(&Policy<'_, B>, &Certificate<'_>, &Extension<'_>) -> Result<(), ValidationError>;
fn(&Policy<'_, B>, &Certificate<'_>, &Extension<'_>) -> ValidationResult<()>;

type MaybeExtensionValidatorCallback<B> =
fn(&Policy<'_, B>, &Certificate<'_>, Option<&Extension<'_>>) -> Result<(), ValidationError>;
fn(&Policy<'_, B>, &Certificate<'_>, Option<&Extension<'_>>) -> ValidationResult<()>;

/// Represents different validation states for an extension.
pub(crate) enum ExtensionValidator<B: CryptoOps> {
Expand Down Expand Up @@ -200,7 +200,7 @@ impl<B: CryptoOps> ExtensionValidator<B> {
policy: &Policy<'_, B>,
cert: &Certificate<'_>,
extension: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
match (self, extension) {
// Extension MUST NOT be present and isn't; OK.
(ExtensionValidator::NotPresent, None) => Ok(()),
Expand Down Expand Up @@ -265,14 +265,14 @@ pub(crate) mod ee {

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

pub(crate) fn basic_constraints<B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
if let Some(extn) = extn {
let basic_constraints: BasicConstraints = extn.value()?;

Expand All @@ -290,7 +290,7 @@ pub(crate) mod ee {
policy: &Policy<'_, B>,
cert: &Certificate<'_>,
extn: &Extension<'_>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
match (cert.subject().is_empty(), extn.critical) {
// If the subject is empty, the SAN MUST be critical.
(true, false) => {
Expand Down Expand Up @@ -327,7 +327,7 @@ pub(crate) mod ee {
policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
if let Some(extn) = extn {
let mut ekus: ExtendedKeyUsage<'_> = extn.value()?;

Expand All @@ -351,7 +351,7 @@ pub(crate) mod ee {
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
if let Some(extn) = extn {
let key_usage: KeyUsage<'_> = extn.value()?;

Expand All @@ -378,14 +378,14 @@ pub(crate) mod ca {

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

pub(crate) fn authority_key_identifier<B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
// CABF: AKI is required on all CA certificates *except* root CA certificates,
// where is it merely recommended. This is slightly different from RFC 5280,
// which requires AKI on all CA certificates *except* self-signed root CA certificates.
Expand Down Expand Up @@ -428,7 +428,7 @@ pub(crate) mod ca {
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: &Extension<'_>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
let key_usage: KeyUsage<'_> = extn.value()?;

if !key_usage.key_cert_sign() {
Expand All @@ -444,7 +444,7 @@ pub(crate) mod ca {
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: &Extension<'_>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
let basic_constraints: BasicConstraints = extn.value()?;

if !basic_constraints.ca {
Expand All @@ -464,7 +464,7 @@ pub(crate) mod ca {
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
if let Some(extn) = extn {
let name_constraints: NameConstraints<'_> = extn.value()?;

Expand Down Expand Up @@ -496,7 +496,7 @@ pub(crate) mod ca {
policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
if let Some(extn) = extn {
let mut ekus: ExtendedKeyUsage<'_> = extn.value()?;

Expand All @@ -521,14 +521,14 @@ pub(crate) mod common {

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

pub(crate) fn authority_information_access<B: CryptoOps>(
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
extn: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
if let Some(extn) = extn {
// We don't currently do anything useful with these, but we
// do check that they're well-formed.
Expand All @@ -550,7 +550,7 @@ mod tests {
use crate::certificate::tests::PublicKeyErrorOps;
use crate::ops::tests::{cert, v1_cert_pem};
use crate::ops::CryptoOps;
use crate::policy::{Policy, Subject, ValidationError};
use crate::policy::{Policy, Subject, ValidationResult};
use crate::types::DNSName;

#[test]
Expand Down Expand Up @@ -590,7 +590,7 @@ mod tests {
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
_ext: &Extension<'_>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
Ok(())
}

Expand Down Expand Up @@ -630,7 +630,7 @@ mod tests {
_policy: &Policy<'_, B>,
_cert: &Certificate<'_>,
_ext: Option<&Extension<'_>>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
Ok(())
}

Expand Down
12 changes: 6 additions & 6 deletions src/rust/cryptography-x509-verification/src/policy/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use once_cell::sync::Lazy;
use crate::ops::CryptoOps;
use crate::policy::extension::{ca, common, ee, Criticality, ExtensionPolicy, ExtensionValidator};
use crate::types::{DNSName, DNSPattern, IPAddress};
use crate::{ValidationError, VerificationCertificate};
use crate::{ValidationError, ValidationResult, VerificationCertificate};

// RSA key constraints, as defined in CA/B 6.1.5.
static WEBPKI_MINIMUM_RSA_MODULUS: usize = 2048;
Expand Down Expand Up @@ -373,7 +373,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
)
}

fn permits_basic(&self, cert: &Certificate<'_>) -> Result<(), ValidationError> {
fn permits_basic(&self, cert: &Certificate<'_>) -> ValidationResult<()> {
// CA/B 7.1.1:
// Certificates MUST be of type X.509 v3.
if cert.tbs_cert.version != 2 {
Expand Down Expand Up @@ -441,7 +441,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
cert: &Certificate<'_>,
current_depth: u8,
extensions: &Extensions<'_>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
self.permits_basic(cert)?;

// 5280 4.1.2.6: Subject
Expand Down Expand Up @@ -480,7 +480,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
&self,
cert: &Certificate<'_>,
extensions: &Extensions<'_>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
self.permits_basic(cert)?;

self.ee_extension_policy.permits(self, cert, extensions)?;
Expand All @@ -507,7 +507,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
child: &VerificationCertificate<'_, B>,
current_depth: u8,
issuer_extensions: &Extensions<'_>,
) -> Result<(), ValidationError> {
) -> ValidationResult<()> {
// The issuer needs to be a valid CA at the current depth.
self.permits_ca(issuer.certificate(), current_depth, issuer_extensions)?;

Expand Down Expand Up @@ -569,7 +569,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
}
}

fn permits_validity_date(validity_date: &Time) -> Result<(), ValidationError> {
fn permits_validity_date(validity_date: &Time) -> ValidationResult<()> {
const GENERALIZED_DATE_INVALIDITY_RANGE: Range<u16> = 1950..2050;

// NOTE: The inverse check on `asn1::UtcTime` is already done for us
Expand Down

0 comments on commit e2fce25

Please sign in to comment.