From 5e0830286ee0d2529ac7364781b9633128848c33 Mon Sep 17 00:00:00 2001 From: Jimmy Cuadra Date: Thu, 28 Jan 2016 04:06:52 -0800 Subject: Preserve X.509 extension insertion order. Ensures that extensions that are order-dependent are inserted in the same order when calling out to OpenSSL during certificate signing. Fixes #327. --- openssl/src/x509/mod.rs | 83 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 77 insertions(+), 6 deletions(-) (limited to 'openssl/src/x509/mod.rs') diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index f31de89b..a69f61d5 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)] -- cgit v1.2.3 From 8ab4b545411cb705872e327bf46044241c2d8e74 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Thu, 28 Jan 2016 23:37:27 -0800 Subject: Revert "impl Clone for PKey and X509 by using their 'references' member" --- openssl/src/x509/mod.rs | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'openssl/src/x509/mod.rs') diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index a69f61d5..8cc34cad 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -509,20 +509,6 @@ impl<'ctx> X509<'ctx> { } } -extern "C" { - fn rust_X509_clone(x509: *mut ffi::X509); -} - -impl<'ctx> Clone for X509<'ctx> { - fn clone(&self) -> X509<'ctx> { - unsafe { rust_X509_clone(self.handle) } - /* FIXME: given that we now have refcounting control, 'owned' should be uneeded, the 'ctx - * is probably also uneeded. We can remove both to condense the x509 api quite a bit - */ - X509::new(self.handle, true) - } -} - impl<'ctx> Drop for X509<'ctx> { fn drop(&mut self) { if self.owned { -- cgit v1.2.3 From 627f394d595562346187b8210b1aeeb225223914 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sun, 31 Jan 2016 20:38:36 +0000 Subject: Revert "Revert "impl Clone for PKey and X509 by using their 'references' member"" --- openssl/src/x509/mod.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'openssl/src/x509/mod.rs') diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 8cc34cad..a69f61d5 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -509,6 +509,20 @@ impl<'ctx> X509<'ctx> { } } +extern "C" { + fn rust_X509_clone(x509: *mut ffi::X509); +} + +impl<'ctx> Clone for X509<'ctx> { + fn clone(&self) -> X509<'ctx> { + unsafe { rust_X509_clone(self.handle) } + /* FIXME: given that we now have refcounting control, 'owned' should be uneeded, the 'ctx + * is probably also uneeded. We can remove both to condense the x509 api quite a bit + */ + X509::new(self.handle, true) + } +} + impl<'ctx> Drop for X509<'ctx> { fn drop(&mut self) { if self.owned { -- cgit v1.2.3