Preserve X.509 extension insertion order. (5e083028) · Commits · Public Repositories / Rust Openssl

openssl/src/x509/mod.rs

+77 −6

Original line number	Diff line number	Diff line
		@@ -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<ExtensionType, Extension>,
		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<I>(mut self, exts: I) -> X509Generator
		where I: IntoIterator<Item = extension::Extension>
		{
		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<Extension>,
		/// A map of used to keep track of added extensions and their indexes in `self.extensions`.
		indexes: HashMap<ExtensionType, usize>,
		}

		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<Extension>
		}

		impl<'a> Iterator for ExtensionsIter<'a> {
		type Item = (ExtensionType, &'a Extension);

		fn next(&mut self) -> Option<Self::Item> {
		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)]

openssl/src/x509/tests.rs

+24 −0

Original line number	Diff line number	Diff line
		@@ -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();