validation: subject is non-optional (#9846)

Signed-off-by: William Woodruff <[email protected]>
pyca · Nov 9, 2023 · 8caafd7 · 8caafd7
1 parent 7d451db
commit 8caafd7
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 13 deletions.
diff --git a/src/rust/cryptography-x509-validation/src/policy/extension.rs b/src/rust/cryptography-x509-validation/src/policy/extension.rs
@@ -176,7 +176,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};
@@ -236,7 +237,12 @@ mod tests {
         let cert_pem = v1_cert_pem();
         let cert = cert(&cert_pem);
         let ops = NullOps {};
-        let policy = Policy::new(ops, None, epoch(), None);
+        let policy = Policy::new(
+            ops,
+            Subject::DNS(DNSName::new("example.com").unwrap()),
+            epoch(),
+            None,
+        );
 
         // Test a policy that stipulates that a given extension MUST be present.
         let extension_policy = ExtensionPolicy::present(
@@ -280,7 +286,12 @@ mod tests {
         let cert_pem = v1_cert_pem();
         let cert = cert(&cert_pem);
         let ops = NullOps {};
-        let policy = Policy::new(ops, None, epoch(), None);
+        let policy = Policy::new(
+            ops,
+            Subject::DNS(DNSName::new("example.com").unwrap()),
+            epoch(),
+            None,
+        );
 
         // Test a policy that stipulates that a given extension CAN be present.
         let extension_policy = ExtensionPolicy::maybe_present(
@@ -316,7 +327,12 @@ mod tests {
         let cert_pem = v1_cert_pem();
         let cert = cert(&cert_pem);
         let ops = NullOps {};
-        let policy = Policy::new(ops, None, epoch(), None);
+        let policy = Policy::new(
+            ops,
+            Subject::DNS(DNSName::new("example.com").unwrap()),
+            epoch(),
+            None,
+        );
 
         // Test a policy that stipulates that a given extension MUST NOT be present.
         let extension_policy = ExtensionPolicy::not_present(BASIC_CONSTRAINTS_OID);
@@ -348,7 +364,12 @@ mod tests {
         let cert_pem = v1_cert_pem();
         let cert = cert(&cert_pem);
         let ops = NullOps {};
-        let policy = Policy::new(ops, None, epoch(), None);
+        let policy = Policy::new(
+            ops,
+            Subject::DNS(DNSName::new("example.com").unwrap()),
+            epoch(),
+            None,
+        );
 
         // Test a present policy that stipulates that a given extension MUST be critical.
         let extension_policy = ExtensionPolicy::present(
@@ -376,7 +397,12 @@ mod tests {
         let cert_pem = v1_cert_pem();
         let cert = cert(&cert_pem);
         let ops = NullOps {};
-        let policy = Policy::new(ops, None, epoch(), None);
+        let policy = Policy::new(
+            ops,
+            Subject::DNS(DNSName::new("example.com").unwrap()),
+            epoch(),
+            None,
+        );
 
         // Test a maybe present policy that stipulates that a given extension MUST be critical.
         let extension_policy = ExtensionPolicy::maybe_present(

diff --git a/src/rust/cryptography-x509-validation/src/policy/mod.rs b/src/rust/cryptography-x509-validation/src/policy/mod.rs
@@ -215,8 +215,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.
@@ -242,7 +241,7 @@ impl<'a, B: CryptoOps> Policy<'a, B> {
     /// the CA/B Forum's Basic Requirements.
     pub fn new(
         ops: B,
-        subject: Option<Subject<'a>>,
+        subject: Subject<'a>,
         time: asn1::DateTime,
         max_chain_depth: Option<u8>,
     ) -> Self {
@@ -398,7 +397,12 @@ mod tests {
     #[test]
     fn test_policy_critical_extensions() {
         let time = asn1::DateTime::new(2023, 9, 12, 1, 1, 1).unwrap();
-        let policy = Policy::new(NullOps {}, None, time, None);
+        let policy = Policy::new(
+            NullOps {},
+            Subject::DNS(DNSName::new("example.com").unwrap()),
+            time,
+            None,
+        );
 
         assert_eq!(
             policy.critical_ca_extensions,

diff --git a/src/rust/src/x509/verify.rs b/src/rust/src/x509/verify.rs
@@ -138,19 +138,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))
         }
     }
 }