diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index f31de89b35ed008f1d9ac94bc9d41958810ff9e3..a69f61d55ad8de1d230ef5667c139698ce06a844 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -146,8 +146,7 @@ pub struct X509Generator { bits: u32, days: u32, names: Vec<(String, String)>, - // RFC 3280 §4.2: A certificate MUST NOT include more than one instance of a particular extension. - extensions: HashMap, + extensions: Extensions, hash_type: HashType, } @@ -166,7 +165,7 @@ impl X509Generator { bits: 1024, days: 365, names: vec![], - extensions: HashMap::new(), + extensions: Extensions::new(), hash_type: HashType::SHA1, } } @@ -219,7 +218,7 @@ impl X509Generator { /// generator.add_extension(KeyUsage(vec![DigitalSignature, KeyEncipherment])); /// ``` pub fn add_extension(mut self, ext: extension::Extension) -> X509Generator { - self.extensions.insert(ext.get_type(), ext); + self.extensions.add(ext); self } @@ -237,7 +236,10 @@ impl X509Generator { pub fn add_extensions(mut self, exts: I) -> X509Generator where I: IntoIterator { - self.extensions.extend(exts.into_iter().map(|ext| (ext.get_type(), ext))); + for ext in exts { + self.extensions.add(ext); + } + self } @@ -372,7 +374,7 @@ impl X509Generator { ffi::X509_set_issuer_name(x509.handle, name); for (exttype, ext) in self.extensions.iter() { - try!(X509Generator::add_extension_internal(x509.handle, exttype, &ext.to_string())); + try!(X509Generator::add_extension_internal(x509.handle, &exttype, &ext.to_string())); } let hash_fn = self.hash_type.evp_md(); @@ -618,6 +620,75 @@ impl Drop for X509Req { } } +/// A collection of X.509 extensions. +/// +/// Upholds the invariant that a certificate MUST NOT include more than one +/// instance of a particular extension, according to RFC 3280 §4.2. Also +/// ensures that extensions are added to the certificate during signing +/// in the order they were inserted, which is required for certain +/// extensions like SubjectKeyIdentifier and AuthorityKeyIdentifier. +struct Extensions { + /// The extensions contained in the collection. + extensions: Vec, + /// A map of used to keep track of added extensions and their indexes in `self.extensions`. + indexes: HashMap, +} + +impl Extensions { + /// Creates a new `Extensions`. + pub fn new() -> Extensions { + Extensions { + extensions: vec![], + indexes: HashMap::new(), + } + } + + /// Adds a new `Extension`, replacing any existing one of the same + /// `ExtensionType`. + pub fn add(&mut self, ext: Extension) { + let ext_type = ext.get_type(); + + if let Some(index) = self.indexes.get(&ext_type) { + self.extensions[*index] = ext; + return; + } + + self.extensions.push(ext); + self.indexes.insert(ext_type, self.extensions.len() - 1); + } + + /// Returns an `ExtensionsIter` for the collection. + pub fn iter(&self) -> ExtensionsIter { + ExtensionsIter { + current: 0, + extensions: &self.extensions, + } + } +} + +/// An iterator that iterates over `(ExtensionType, Extension)` for each +/// extension in the collection. +struct ExtensionsIter<'a> { + current: usize, + extensions: &'a Vec +} + +impl<'a> Iterator for ExtensionsIter<'a> { + type Item = (ExtensionType, &'a Extension); + + fn next(&mut self) -> Option { + if self.current < self.extensions.len() { + let ext = &self.extensions[self.current]; + + self.current += 1; + + Some((ext.get_type(), ext)) + } else { + None + } + } +} + macro_rules! make_validation_error( ($ok_val:ident, $($name:ident = $val:ident,)+) => ( #[derive(Copy, Clone)] diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 43ad0dec0f1c245515560d5314453318605df6d9..35690e050959a4d42142f7a25612ebbdf3d97bed 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -39,6 +39,30 @@ fn test_cert_gen() { assert_eq!(pkey.save_pub(), cert.public_key().save_pub()); } +/// SubjectKeyIdentifier must be added before AuthorityKeyIdentifier or OpenSSL +/// is "unable to get issuer keyid." This test ensures the order of insertion +/// for extensions is preserved when the cert is signed. +#[test] +fn test_cert_gen_extension_ordering() { + get_generator() + .add_extension(OtherNid(Nid::SubjectKeyIdentifier, "hash".to_owned())) + .add_extension(OtherNid(Nid::AuthorityKeyIdentifier, "keyid:always".to_owned())) + .generate() + .expect("Failed to generate cert with order-dependent extensions"); +} + +/// Proves that a passing result from `test_cert_gen_extension_ordering` is +/// deterministic by reversing the order of extensions and asserting failure. +#[test] +fn test_cert_gen_extension_bad_ordering() { + let result = get_generator() + .add_extension(OtherNid(Nid::AuthorityKeyIdentifier, "keyid:always".to_owned())) + .add_extension(OtherNid(Nid::SubjectKeyIdentifier, "hash".to_owned())) + .generate(); + + assert!(result.is_err()); +} + #[test] fn test_req_gen() { let mut pkey = PKey::new();