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.

Sign up for GitHub

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

validation: make subject non-optional #7

Merged
merged 1 commit into from
Nov 2, 2023
Merged

validation: make subject non-optional #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 2 additions & 6 deletions src/rust/cryptography-x509-validation/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,7 @@ emyPxgcYxn/eR44/KJ4EBs+lVDR3veyJm+kXQ99b21/+jh5Xos1AnX5iItreGCc=
let time = asn1::DateTime::new(2023, 7, 10, 0, 0, 0).unwrap();
let policy: Policy<'_, _> = Policy::new(
ops,
Some(policy::Subject::DNS(
DNSName::new("cryptography.io").unwrap(),
)),
policy::Subject::DNS(DNSName::new("cryptography.io").unwrap()),
time,
);

Expand Down Expand Up @@ -532,9 +530,7 @@ nLRbwHOoq7hHwg==
let time = asn1::DateTime::new(2023, 7, 10, 0, 0, 0).unwrap();
let policy: Policy<'_, _> = Policy::new(
ops,
Some(policy::Subject::DNS(
DNSName::new("cryptography.io").unwrap(),
)),
policy::Subject::DNS(DNSName::new("cryptography.io").unwrap()),
time,
);
assert_eq!(
Expand Down
33 changes: 27 additions & 6 deletions src/rust/cryptography-x509-validation/src/policy/extension.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ mod tests {
use super::{Criticality, ExtensionPolicy};
use crate::ops::tests::{cert, v1_cert_pem, NullOps};
use crate::ops::CryptoOps;
use crate::policy::{Policy, PolicyError};
use crate::policy::{Policy, PolicyError, Subject};
use crate::types::DNSName;
use asn1::{ObjectIdentifier, SimpleAsn1Writable};
use cryptography_x509::certificate::Certificate;
use cryptography_x509::extensions::{BasicConstraints, Extension, Extensions};
Expand Down Expand Up @@ -394,7 +395,11 @@ mod tests {
let cert_pem = v1_cert_pem();
let cert = cert(&cert_pem);
let ops = NullOps {};
let policy = Policy::new(ops, None, epoch());
let policy = Policy::new(
ops,
Subject::DNS(DNSName::new("example.com").unwrap()),
epoch(),
);

// Test a policy that stipulates that a given extension MUST be present.
let extension_policy = ExtensionPolicy::present(
Expand Down Expand Up @@ -438,7 +443,11 @@ mod tests {
let cert_pem = v1_cert_pem();
let cert = cert(&cert_pem);
let ops = NullOps {};
let policy = Policy::new(ops, None, epoch());
let policy = Policy::new(
ops,
Subject::DNS(DNSName::new("example.com").unwrap()),
epoch(),
);

// Test a policy that stipulates that a given extension CAN be present.
let extension_policy = ExtensionPolicy::maybe_present(
Expand Down Expand Up @@ -474,7 +483,11 @@ mod tests {
let cert_pem = v1_cert_pem();
let cert = cert(&cert_pem);
let ops = NullOps {};
let policy = Policy::new(ops, None, epoch());
let policy = Policy::new(
ops,
Subject::DNS(DNSName::new("example.com").unwrap()),
epoch(),
);

// Test a policy that stipulates that a given extension MUST NOT be present.
let extension_policy = ExtensionPolicy::not_present(BASIC_CONSTRAINTS_OID);
Expand Down Expand Up @@ -506,7 +519,11 @@ mod tests {
let cert_pem = v1_cert_pem();
let cert = cert(&cert_pem);
let ops = NullOps {};
let policy = Policy::new(ops, None, epoch());
let policy = Policy::new(
ops,
Subject::DNS(DNSName::new("example.com").unwrap()),
epoch(),
);

// Test a present policy that stipulates that a given extension MUST be critical.
let extension_policy = ExtensionPolicy::present(
Expand Down Expand Up @@ -534,7 +551,11 @@ mod tests {
let cert_pem = v1_cert_pem();
let cert = cert(&cert_pem);
let ops = NullOps {};
let policy = Policy::new(ops, None, epoch());
let policy = Policy::new(
ops,
Subject::DNS(DNSName::new("example.com").unwrap()),
epoch(),
);

// Test a maybe present policy that stipulates that a given extension MUST be critical.
let extension_policy = ExtensionPolicy::maybe_present(
Expand Down
17 changes: 4 additions & 13 deletions src/rust/cryptography-x509-validation/src/policy/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,7 @@ pub struct Policy<'a, B: CryptoOps> {

/// A subject (i.e. DNS name or other name format) that any EE certificates
/// validated by this policy must match.
/// If `None`, the EE certificate must not contain a SAN.
pub subject: Option<Subject<'a>>,
pub subject: Subject<'a>,

/// The validation time. All certificates validated by this policy must
/// be valid at this time.
Expand Down Expand Up @@ -265,7 +264,7 @@ pub struct Policy<'a, B: CryptoOps> {
impl<'a, B: CryptoOps> Policy<'a, B> {
/// Create a new policy with defaults for the certificate profile defined in
/// the CA/B Forum's Basic Requirements.
pub fn new(ops: B, subject: Option<Subject<'a>>, time: asn1::DateTime) -> Self {
pub fn new(ops: B, subject: Subject<'a>, time: asn1::DateTime) -> Self {
Self {
ops,
max_chain_depth: 8,
Expand Down Expand Up @@ -449,26 +448,18 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
match (&self.subject, san_ext) {
// If we're given both an expected name and the cert has a SAN,
// then we attempt to match them.
(Some(sub), Some(san)) => {
(sub, Some(san)) => {
woodruffw marked this conversation as resolved.
Show resolved Hide resolved
let san: SubjectAlternativeName<'_> = san.value()?;
match sub.matches(&san) {
true => Ok(()),
false => Err(PolicyError::Other("EE cert has no matching SAN")),
}
}
// If we aren't given a name but the cert contains a SAN,
// we complain loudly (under the theory that the user has misused
// our API and actually intended to match against the SAN).
(None, Some(_)) => Err(PolicyError::Other(
"EE cert has subjectAltName but no expected name given to match against",
)),
// If we're given an expected name but the cert doesn't contain a
// SAN, we error.
(Some(_), None) => Err(PolicyError::Other(
(_, None) => Err(PolicyError::Other(
"EE cert has no subjectAltName but expected name given",
)),
// No expected name and no SAN, no problem.
(None, None) => Ok(()),
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/rust/src/x509/verify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,19 +174,19 @@ fn build_subject_owner(
fn build_subject<'a>(
py: pyo3::Python<'_>,
subject: &'a SubjectOwner,
) -> pyo3::PyResult<Option<Subject<'a>>> {
) -> pyo3::PyResult<Subject<'a>> {
match subject {
SubjectOwner::DNSName(dns_name) => {
let dns_name = DNSName::new(dns_name)
.ok_or_else(|| pyo3::exceptions::PyValueError::new_err("invalid domain name"))?;

Ok(Some(Subject::DNS(dns_name)))
Ok(Subject::DNS(dns_name))
}
SubjectOwner::IPAddress(ip_addr) => {
let ip_addr = IPAddress::from_bytes(ip_addr.as_bytes(py))
.ok_or_else(|| pyo3::exceptions::PyValueError::new_err("invalid IP address"))?;

Ok(Some(Subject::IP(ip_addr)))
Ok(Subject::IP(ip_addr))
}
}
}
Expand Down
Loading