diff --git a/README.OpenSource b/README.OpenSource index 7b6db84de5049fb613ab042be2d4a9801798af3b..518d65aa12076b5ed0cfc0002da8f6138c84b751 100644 --- a/README.OpenSource +++ b/README.OpenSource @@ -3,7 +3,7 @@ "Name": "openssl", "License": "Apache License V2.0", "License File": "THIRD_PARTY", - "Version Number": "0.10.47", + "Version Number": "0.10.48", "Owner": "xuelei3@huawei.com", "Upstream URL": "https://github.com/sfackler/rust-openssl", "Description": "OpenSSL bindings." diff --git a/openssl-sys/CHANGELOG.md b/openssl-sys/CHANGELOG.md index 3cb0711817a18eccac5358fc5944cb363e325aa5..8587ad22622b0cf0afeed1ba24a51d693533a406 100644 --- a/openssl-sys/CHANGELOG.md +++ b/openssl-sys/CHANGELOG.md @@ -2,6 +2,17 @@ ## [Unreleased] +## [v0.9.83] - 2023-03-23 + +### Fixed + +* Fixed version checks for LibreSSL. + +### Added + +* Added `i2d_X509_EXTENSION`. +* Added `GENERAL_NAME_new`. + ## [v0.9.82] - 2023-03-19 ### Added @@ -399,7 +410,8 @@ Fixed builds against OpenSSL built with `no-cast`. * Added `X509_verify` and `X509_REQ_verify`. * Added `EVP_MD_type` and `EVP_GROUP_get_curve_name`. -[Unreleased]: https://github.com/sfackler/rust-openssl/compare/openssl-sys-v0.9.82..master +[Unreleased]: https://github.com/sfackler/rust-openssl/compare/openssl-sys-v0.9.83..master +[v0.9.83]: https://github.com/sfackler/rust-openssl/compare/openssl-sys-v0.9.82...openssl-sys-v0.9.83 [v0.9.82]: https://github.com/sfackler/rust-openssl/compare/openssl-sys-v0.9.81...openssl-sys-v0.9.82 [v0.9.81]: https://github.com/sfackler/rust-openssl/compare/openssl-sys-v0.9.80...openssl-sys-v0.9.81 [v0.9.80]: https://github.com/sfackler/rust-openssl/compare/openssl-sys-v0.9.79...openssl-sys-v0.9.80 diff --git a/openssl-sys/Cargo.toml b/openssl-sys/Cargo.toml index ed3161c784b85f8116de4a58993f32371cd72081..ad7582ad056f69d33233677b0dff4bdb38e37ab1 100644 --- a/openssl-sys/Cargo.toml +++ b/openssl-sys/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openssl-sys" -version = "0.9.82" +version = "0.9.83" authors = [ "Alex Crichton ", "Steven Fackler ", diff --git a/openssl-sys/build/cfgs.rs b/openssl-sys/build/cfgs.rs index d925d90ad7a9e3bc4f372d7c8b143c1c8f5e6387..960515f00f0b36a2b37b5d5e36352e99fccbb0c0 100644 --- a/openssl-sys/build/cfgs.rs +++ b/openssl-sys/build/cfgs.rs @@ -31,6 +31,9 @@ pub fn get(openssl_version: Option, libressl_version: Option) -> Vec<& if libressl_version >= 0x2_09_01_00_0 { cfgs.push("libressl291"); } + if libressl_version >= 0x3_01_00_00_0 { + cfgs.push("libressl310"); + } if libressl_version >= 0x3_02_01_00_0 { cfgs.push("libressl321"); } diff --git a/openssl-sys/src/handwritten/x509.rs b/openssl-sys/src/handwritten/x509.rs index 8762e5f98d4ff6a1c7555c20eb6ffda9138b6de0..abda4110cf3e35422899349dab1fb58731c2ead7 100644 --- a/openssl-sys/src/handwritten/x509.rs +++ b/openssl-sys/src/handwritten/x509.rs @@ -550,6 +550,13 @@ extern "C" { pub fn X509_EXTENSION_get_object(ext: *mut X509_EXTENSION) -> *mut ASN1_OBJECT; pub fn X509_EXTENSION_get_data(ext: *mut X509_EXTENSION) -> *mut ASN1_OCTET_STRING; } + +const_ptr_api! { + extern "C" { + pub fn i2d_X509_EXTENSION(ext: #[const_ptr_if(ossl300)] X509_EXTENSION, pp: *mut *mut c_uchar) -> c_int; + } +} + const_ptr_api! { extern "C" { // in X509 diff --git a/openssl-sys/src/handwritten/x509v3.rs b/openssl-sys/src/handwritten/x509v3.rs index d0923e32b223c69aed550f839838eebbbd037741..4f661ca5ec053e10f428ddad881da9cfe325d1ab 100644 --- a/openssl-sys/src/handwritten/x509v3.rs +++ b/openssl-sys/src/handwritten/x509v3.rs @@ -4,6 +4,7 @@ use libc::*; pub enum CONF_METHOD {} extern "C" { + pub fn GENERAL_NAME_new() -> *mut GENERAL_NAME; pub fn GENERAL_NAME_free(name: *mut GENERAL_NAME); } diff --git a/openssl/CHANGELOG.md b/openssl/CHANGELOG.md index 7de74b8045b6514a313a1a7b294cf5fbe940452f..c6d9b303cd3b134f2aca008d4c0eb8b13a486208 100644 --- a/openssl/CHANGELOG.md +++ b/openssl/CHANGELOG.md @@ -2,6 +2,16 @@ ## [Unreleased] +## [v0.10.48] - 2023-03-23 + +### Fixed + +* Fixed injection vulnerabilities where OpenSSL's configuration mini-language could be used via `x509::extension::SubjectAlternativeName` and `x509::extension::ExtendedKeyUsage`. The mini-language can read arbitrary files amongst other things. + * As part of fixing this `SubjectAlternativeName::dir_name` and `SubjectAlternativeName::other_name` are deprecated and their implementations always `panic!`. If you have a use case for these, please file an issue. +* Fixed several NULL pointer dereferences in OpenSSL that could be triggered via `x509::X509Extension::new` and `x509::X509Extension::new_nid`. Note that these methods still accept OpenSSL's configuration mini-language, and therefore should not be used with untrusted data. +* Fixed a data-race with `x509::X509Name` that are created with `x509::X509NameBuilder` and then used concurrently. +* Fixed LibreSSL version checking. More functions should now be correctly available on LibreSSL. + ## [v0.10.47] - 2023-03-19 ### Added @@ -697,7 +707,8 @@ Look at the [release tags] for information about older releases. -[Unreleased]: https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.47...master +[Unreleased]: https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.48...master +[v0.10.48]: https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.47...openssl-v0.10.48 [v0.10.47]: https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.46...openssl-v0.10.47 [v0.10.46]: https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.45...openssl-v0.10.46 [v0.10.45]: https://github.com/sfackler/rust-openssl/compare/openssl-v0.10.44...openssl-v0.10.45 diff --git a/openssl/Cargo.toml b/openssl/Cargo.toml index 158acff5a3e675c06055a39e4b82c1b1e0bdbd6d..e49bd9163e17c2fdf2cbedd697cabce148b21a90 100644 --- a/openssl/Cargo.toml +++ b/openssl/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openssl" -version = "0.10.47" +version = "0.10.48" authors = ["Steven Fackler "] license = "Apache-2.0" description = "OpenSSL bindings" @@ -30,7 +30,7 @@ libc = "0.2" once_cell = "1.5.2" openssl-macros = { version = "0.1.0", path = "../openssl-macros" } -ffi = { package = "openssl-sys", version = "0.9.82", path = "../openssl-sys" } +ffi = { package = "openssl-sys", version = "0.9.83", path = "../openssl-sys" } [dev-dependencies] hex = "0.3" diff --git a/openssl/build.rs b/openssl/build.rs index 5cddce90c267d4e1a9fe65ebb8682777703240df..0a974b33e69769bfa558e91f060e0b1545c32c1e 100644 --- a/openssl/build.rs +++ b/openssl/build.rs @@ -16,8 +16,57 @@ fn main() { return; } - if let Ok(v) = env::var("DEP_OPENSSL_LIBRESSL_VERSION") { - println!("cargo:rustc-cfg=libressl{}", v); + if let Ok(v) = env::var("DEP_OPENSSL_LIBRESSL_VERSION_NUMBER") { + let version = u64::from_str_radix(&v, 16).unwrap(); + + if version >= 0x2_05_00_00_0 { + println!("cargo:rustc-cfg=libressl250"); + } + if version >= 0x2_05_01_00_0 { + println!("cargo:rustc-cfg=libressl251"); + } + if version >= 0x2_06_01_00_0 { + println!("cargo:rustc-cfg=libressl261"); + } + if version >= 0x2_07_00_00_0 { + println!("cargo:rustc-cfg=libressl270"); + } + if version >= 0x2_07_01_00_0 { + println!("cargo:rustc-cfg=libressl271"); + } + if version >= 0x2_07_03_00_0 { + println!("cargo:rustc-cfg=libressl273"); + } + if version >= 0x2_08_00_00_0 { + println!("cargo:rustc-cfg=libressl280"); + } + if version >= 0x2_09_01_00_0 { + println!("cargo:rustc-cfg=libressl291"); + } + if version >= 0x3_01_00_00_0 { + println!("cargo:rustc-cfg=libressl310"); + } + if version >= 0x3_02_01_00_0 { + println!("cargo:rustc-cfg=libressl321"); + } + if version >= 0x3_03_02_00_0 { + println!("cargo:rustc-cfg=libressl332"); + } + if version >= 0x3_04_00_00_0 { + println!("cargo:rustc-cfg=libressl340"); + } + if version >= 0x3_05_00_00_0 { + println!("cargo:rustc-cfg=libressl350"); + } + if version >= 0x3_06_00_00_0 { + println!("cargo:rustc-cfg=libressl360"); + } + if version >= 0x3_06_01_00_0 { + println!("cargo:rustc-cfg=libressl361"); + } + if version >= 0x3_07_00_00_0 { + println!("cargo:rustc-cfg=libressl370"); + } } if let Ok(vars) = env::var("DEP_OPENSSL_CONF") { @@ -50,6 +99,9 @@ fn main() { if version >= 0x3_00_00_00_0 { println!("cargo:rustc-cfg=ossl300"); } + if version >= 0x3_01_00_00_0 { + println!("cargo:rustc-cfg=ossl310"); + } } if let Ok(version) = env::var("DEP_OPENSSL_LIBRESSL_VERSION_NUMBER") { diff --git a/openssl/src/asn1.rs b/openssl/src/asn1.rs index 55de049c0872f06ea8d80af9418024c3fb06eca1..c0178c7e65c39daf90be277b6dc9a6c700df81b6 100644 --- a/openssl/src/asn1.rs +++ b/openssl/src/asn1.rs @@ -39,6 +39,7 @@ use crate::bio::MemBio; use crate::bn::{BigNum, BigNumRef}; use crate::error::ErrorStack; use crate::nid::Nid; +use crate::stack::Stackable; use crate::string::OpensslString; use crate::{cvt, cvt_p}; use openssl_macros::corresponds; @@ -592,6 +593,10 @@ foreign_type_and_impl_send_sync! { pub struct Asn1ObjectRef; } +impl Stackable for Asn1Object { + type StackType = ffi::stack_st_ASN1_OBJECT; +} + impl Asn1Object { /// Constructs an ASN.1 Object Identifier from a string representation of the OID. #[corresponds(OBJ_txt2obj)] diff --git a/openssl/src/error.rs b/openssl/src/error.rs index 064d635234fa9eb1545af14e593110e23490c5c9..e097ce6881e5324b3666c4592669d1810b8303a5 100644 --- a/openssl/src/error.rs +++ b/openssl/src/error.rs @@ -401,9 +401,12 @@ cfg_if! { #[cfg(test)] mod tests { + #[cfg(not(ossl310))] use crate::nid::Nid; #[test] + // Due to a bug in OpenSSL 3.1.0, this test can hang there. Skip for now. + #[cfg(not(ossl310))] fn test_error_library_code() { let stack = Nid::create("not-an-oid", "invalid", "invalid").unwrap_err(); let errors = stack.errors(); diff --git a/openssl/src/ssl/test/mod.rs b/openssl/src/ssl/test/mod.rs index 1eb9fe4bad61b346b355f75ce34703c3e0d169d0..03dc89e5c36db17ae66be6a1792b69b395957849 100644 --- a/openssl/src/ssl/test/mod.rs +++ b/openssl/src/ssl/test/mod.rs @@ -84,17 +84,21 @@ fn verify_trusted_with_set_cert() { #[test] fn verify_untrusted_callback_override_ok() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + let server = Server::builder().build(); let mut client = server.client(); client .ctx() .set_verify_callback(SslVerifyMode::PEER, |_, x509| { + CALLED_BACK.store(true, Ordering::SeqCst); assert!(x509.current_cert().is_some()); true }); client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); } #[test] @@ -113,6 +117,8 @@ fn verify_untrusted_callback_override_bad() { #[test] fn verify_trusted_callback_override_ok() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + let server = Server::builder().build(); let mut client = server.client(); @@ -120,11 +126,13 @@ fn verify_trusted_callback_override_ok() { client .ctx() .set_verify_callback(SslVerifyMode::PEER, |_, x509| { + CALLED_BACK.store(true, Ordering::SeqCst); assert!(x509.current_cert().is_some()); true }); client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); } #[test] @@ -144,21 +152,27 @@ fn verify_trusted_callback_override_bad() { #[test] fn verify_callback_load_certs() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + let server = Server::builder().build(); let mut client = server.client(); client .ctx() .set_verify_callback(SslVerifyMode::PEER, |_, x509| { + CALLED_BACK.store(true, Ordering::SeqCst); assert!(x509.current_cert().is_some()); true }); client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); } #[test] fn verify_trusted_get_error_ok() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + let server = Server::builder().build(); let mut client = server.client(); @@ -166,11 +180,13 @@ fn verify_trusted_get_error_ok() { client .ctx() .set_verify_callback(SslVerifyMode::PEER, |_, x509| { + CALLED_BACK.store(true, Ordering::SeqCst); assert_eq!(x509.error(), X509VerifyResult::OK); true }); client.connect(); + assert!(CALLED_BACK.load(Ordering::SeqCst)); } #[test] @@ -469,8 +485,11 @@ fn test_alpn_server_select_none_fatal() { #[test] #[cfg(any(ossl102, libressl261))] fn test_alpn_server_select_none() { + static CALLED_BACK: AtomicBool = AtomicBool::new(false); + let mut server = Server::builder(); server.ctx().set_alpn_select_callback(|_, client| { + CALLED_BACK.store(true, Ordering::SeqCst); ssl::select_next_proto(b"\x08http/1.1\x08spdy/3.1", client).ok_or(ssl::AlpnError::NOACK) }); let server = server.build(); @@ -479,6 +498,7 @@ fn test_alpn_server_select_none() { client.ctx().set_alpn_protos(b"\x06http/2").unwrap(); let s = client.connect(); assert_eq!(None, s.ssl().selected_alpn_protocol()); + assert!(CALLED_BACK.load(Ordering::SeqCst)); } #[test] @@ -595,7 +615,7 @@ fn refcount_ssl_context() { { let new_ctx_a = SslContext::builder(SslMethod::tls()).unwrap().build(); - let _new_ctx_b = ssl.set_ssl_context(&new_ctx_a); + ssl.set_ssl_context(&new_ctx_a).unwrap(); } } @@ -731,7 +751,7 @@ fn connector_no_hostname_still_verifies() { } #[test] -fn connector_no_hostname_can_disable_verify() { +fn connector_can_disable_verify() { let server = Server::builder().build(); let mut connector = SslConnector::builder(SslMethod::tls()).unwrap(); @@ -742,8 +762,7 @@ fn connector_no_hostname_can_disable_verify() { let mut s = connector .configure() .unwrap() - .verify_hostname(false) - .connect("foobar.com", s) + .connect("fizzbuzz.com", s) .unwrap(); s.read_exact(&mut [0]).unwrap(); } diff --git a/openssl/src/x509/extension.rs b/openssl/src/x509/extension.rs index ebbea1c8854a06d42eea30c342a55042c07ffca8..f04d227960adfcba0fe1cc34819f33fc5d256c0d 100644 --- a/openssl/src/x509/extension.rs +++ b/openssl/src/x509/extension.rs @@ -18,9 +18,11 @@ //! ``` use std::fmt::Write; +use crate::asn1::Asn1Object; use crate::error::ErrorStack; use crate::nid::Nid; -use crate::x509::{X509Extension, X509v3Context}; +use crate::x509::{GeneralName, Stack, X509Extension, X509v3Context}; +use foreign_types::ForeignType; /// An extension which indicates whether a certificate is a CA certificate. pub struct BasicConstraints { @@ -222,18 +224,7 @@ impl KeyUsage { /// for which the certificate public key can be used for. pub struct ExtendedKeyUsage { critical: bool, - server_auth: bool, - client_auth: bool, - code_signing: bool, - email_protection: bool, - time_stamping: bool, - ms_code_ind: bool, - ms_code_com: bool, - ms_ctl_sign: bool, - ms_sgc: bool, - ms_efs: bool, - ns_sgc: bool, - other: Vec, + items: Vec, } impl Default for ExtendedKeyUsage { @@ -247,18 +238,7 @@ impl ExtendedKeyUsage { pub fn new() -> ExtendedKeyUsage { ExtendedKeyUsage { critical: false, - server_auth: false, - client_auth: false, - code_signing: false, - email_protection: false, - time_stamping: false, - ms_code_ind: false, - ms_code_com: false, - ms_ctl_sign: false, - ms_sgc: false, - ms_efs: false, - ns_sgc: false, - other: vec![], + items: vec![], } } @@ -270,101 +250,74 @@ impl ExtendedKeyUsage { /// Sets the `serverAuth` flag to `true`. pub fn server_auth(&mut self) -> &mut ExtendedKeyUsage { - self.server_auth = true; - self + self.other("serverAuth") } /// Sets the `clientAuth` flag to `true`. pub fn client_auth(&mut self) -> &mut ExtendedKeyUsage { - self.client_auth = true; - self + self.other("clientAuth") } /// Sets the `codeSigning` flag to `true`. pub fn code_signing(&mut self) -> &mut ExtendedKeyUsage { - self.code_signing = true; - self + self.other("codeSigning") } /// Sets the `emailProtection` flag to `true`. pub fn email_protection(&mut self) -> &mut ExtendedKeyUsage { - self.email_protection = true; - self + self.other("emailProtection") } /// Sets the `timeStamping` flag to `true`. pub fn time_stamping(&mut self) -> &mut ExtendedKeyUsage { - self.time_stamping = true; - self + self.other("timeStamping") } /// Sets the `msCodeInd` flag to `true`. pub fn ms_code_ind(&mut self) -> &mut ExtendedKeyUsage { - self.ms_code_ind = true; - self + self.other("msCodeInd") } /// Sets the `msCodeCom` flag to `true`. pub fn ms_code_com(&mut self) -> &mut ExtendedKeyUsage { - self.ms_code_com = true; - self + self.other("msCodeCom") } /// Sets the `msCTLSign` flag to `true`. pub fn ms_ctl_sign(&mut self) -> &mut ExtendedKeyUsage { - self.ms_ctl_sign = true; - self + self.other("msCTLSign") } /// Sets the `msSGC` flag to `true`. pub fn ms_sgc(&mut self) -> &mut ExtendedKeyUsage { - self.ms_sgc = true; - self + self.other("msSGC") } /// Sets the `msEFS` flag to `true`. pub fn ms_efs(&mut self) -> &mut ExtendedKeyUsage { - self.ms_efs = true; - self + self.other("msEFS") } /// Sets the `nsSGC` flag to `true`. pub fn ns_sgc(&mut self) -> &mut ExtendedKeyUsage { - self.ns_sgc = true; - self + self.other("nsSGC") } /// Sets a flag not already defined. pub fn other(&mut self, other: &str) -> &mut ExtendedKeyUsage { - self.other.push(other.to_owned()); + self.items.push(other.to_string()); self } /// Return the `ExtendedKeyUsage` extension as an `X509Extension`. pub fn build(&self) -> Result { - let mut value = String::new(); - let mut first = true; - append(&mut value, &mut first, self.critical, "critical"); - append(&mut value, &mut first, self.server_auth, "serverAuth"); - append(&mut value, &mut first, self.client_auth, "clientAuth"); - append(&mut value, &mut first, self.code_signing, "codeSigning"); - append( - &mut value, - &mut first, - self.email_protection, - "emailProtection", - ); - append(&mut value, &mut first, self.time_stamping, "timeStamping"); - append(&mut value, &mut first, self.ms_code_ind, "msCodeInd"); - append(&mut value, &mut first, self.ms_code_com, "msCodeCom"); - append(&mut value, &mut first, self.ms_ctl_sign, "msCTLSign"); - append(&mut value, &mut first, self.ms_sgc, "msSGC"); - append(&mut value, &mut first, self.ms_efs, "msEFS"); - append(&mut value, &mut first, self.ns_sgc, "nsSGC"); - for other in &self.other { - append(&mut value, &mut first, true, other); + let mut stack = Stack::new()?; + for item in &self.items { + stack.push(Asn1Object::from_str(item)?)?; + } + unsafe { + X509Extension::new_internal(Nid::EXT_KEY_USAGE, self.critical, stack.as_ptr().cast()) } - X509Extension::new_nid(None, None, Nid::EXT_KEY_USAGE, &value) } } @@ -463,11 +416,19 @@ impl AuthorityKeyIdentifier { } } +enum RustGeneralName { + Dns(String), + Email(String), + Uri(String), + Ip(String), + Rid(String), +} + /// An extension that allows additional identities to be bound to the subject /// of the certificate. pub struct SubjectAlternativeName { critical: bool, - names: Vec, + items: Vec, } impl Default for SubjectAlternativeName { @@ -481,7 +442,7 @@ impl SubjectAlternativeName { pub fn new() -> SubjectAlternativeName { SubjectAlternativeName { critical: false, - names: vec![], + items: vec![], } } @@ -493,55 +454,73 @@ impl SubjectAlternativeName { /// Sets the `email` flag. pub fn email(&mut self, email: &str) -> &mut SubjectAlternativeName { - self.names.push(format!("email:{}", email)); + self.items.push(RustGeneralName::Email(email.to_string())); self } /// Sets the `uri` flag. pub fn uri(&mut self, uri: &str) -> &mut SubjectAlternativeName { - self.names.push(format!("URI:{}", uri)); + self.items.push(RustGeneralName::Uri(uri.to_string())); self } /// Sets the `dns` flag. pub fn dns(&mut self, dns: &str) -> &mut SubjectAlternativeName { - self.names.push(format!("DNS:{}", dns)); + self.items.push(RustGeneralName::Dns(dns.to_string())); self } /// Sets the `rid` flag. pub fn rid(&mut self, rid: &str) -> &mut SubjectAlternativeName { - self.names.push(format!("RID:{}", rid)); + self.items.push(RustGeneralName::Rid(rid.to_string())); self } /// Sets the `ip` flag. pub fn ip(&mut self, ip: &str) -> &mut SubjectAlternativeName { - self.names.push(format!("IP:{}", ip)); + self.items.push(RustGeneralName::Ip(ip.to_string())); self } /// Sets the `dirName` flag. - pub fn dir_name(&mut self, dir_name: &str) -> &mut SubjectAlternativeName { - self.names.push(format!("dirName:{}", dir_name)); - self + /// + /// Not currently actually supported, always panics. + #[deprecated = "dir_name is deprecated and always panics. Please file a bug if you have a use case for this."] + pub fn dir_name(&mut self, _dir_name: &str) -> &mut SubjectAlternativeName { + unimplemented!( + "This has not yet been adapted for the new internals. File a bug if you need this." + ); } /// Sets the `otherName` flag. - pub fn other_name(&mut self, other_name: &str) -> &mut SubjectAlternativeName { - self.names.push(format!("otherName:{}", other_name)); - self + /// + /// Not currently actually supported, always panics. + #[deprecated = "other_name is deprecated and always panics. Please file a bug if you have a use case for this."] + pub fn other_name(&mut self, _other_name: &str) -> &mut SubjectAlternativeName { + unimplemented!( + "This has not yet been adapted for the new internals. File a bug if you need this." + ); } /// Return a `SubjectAlternativeName` extension as an `X509Extension`. - pub fn build(&self, ctx: &X509v3Context<'_>) -> Result { - let mut value = String::new(); - let mut first = true; - append(&mut value, &mut first, self.critical, "critical"); - for name in &self.names { - append(&mut value, &mut first, true, name); + pub fn build(&self, _ctx: &X509v3Context<'_>) -> Result { + let mut stack = Stack::new()?; + for item in &self.items { + let gn = match item { + RustGeneralName::Dns(s) => GeneralName::new_dns(s.as_bytes())?, + RustGeneralName::Email(s) => GeneralName::new_email(s.as_bytes())?, + RustGeneralName::Uri(s) => GeneralName::new_uri(s.as_bytes())?, + RustGeneralName::Ip(s) => { + GeneralName::new_ip(s.parse().map_err(|_| ErrorStack::get())?)? + } + RustGeneralName::Rid(s) => GeneralName::new_rid(Asn1Object::from_str(s)?)?, + }; + stack.push(gn)?; + } + + unsafe { + X509Extension::new_internal(Nid::SUBJECT_ALT_NAME, self.critical, stack.as_ptr().cast()) } - X509Extension::new_nid(None, Some(ctx), Nid::SUBJECT_ALT_NAME, &value) } } diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 4f08bbc66752c707f49ac9a09fc8f6654d76cb96..5b5591875018f56a8672e22acc5f34737b58ed31 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -9,9 +9,9 @@ use cfg_if::cfg_if; use foreign_types::{ForeignType, ForeignTypeRef, Opaque}; -use libc::{c_int, c_long, c_uint}; +use libc::{c_int, c_long, c_uint, c_void}; use std::cmp::{self, Ordering}; -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; use std::error::Error; use std::ffi::{CStr, CString}; use std::fmt; @@ -24,7 +24,8 @@ use std::slice; use std::str; use crate::asn1::{ - Asn1BitStringRef, Asn1IntegerRef, Asn1ObjectRef, Asn1StringRef, Asn1TimeRef, Asn1Type, + Asn1BitStringRef, Asn1IntegerRef, Asn1Object, Asn1ObjectRef, Asn1StringRef, Asn1TimeRef, + Asn1Type, }; use crate::bio::MemBioSlice; use crate::conf::ConfRef; @@ -806,6 +807,9 @@ impl X509Extension { /// Some extension types, such as `subjectAlternativeName`, require an `X509v3Context` to be /// provided. /// + /// DO NOT CALL THIS WITH UNTRUSTED `value`: `value` is an OpenSSL + /// mini-language that can read arbitrary files. + /// /// See the extension module for builder types which will construct certain common extensions. pub fn new( conf: Option<&ConfRef>, @@ -815,14 +819,30 @@ impl X509Extension { ) -> Result { let name = CString::new(name).unwrap(); let value = CString::new(value).unwrap(); + let mut ctx; unsafe { ffi::init(); let conf = conf.map_or(ptr::null_mut(), ConfRef::as_ptr); - let context = context.map_or(ptr::null_mut(), X509v3Context::as_ptr); + let context_ptr = match context { + Some(c) => c.as_ptr(), + None => { + ctx = mem::zeroed(); + + ffi::X509V3_set_ctx( + &mut ctx, + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + 0, + ); + &mut ctx + } + }; let name = name.as_ptr() as *mut _; let value = value.as_ptr() as *mut _; - cvt_p(ffi::X509V3_EXT_nconf(conf, context, name, value)).map(X509Extension) + cvt_p(ffi::X509V3_EXT_nconf(conf, context_ptr, name, value)).map(X509Extension) } } @@ -832,6 +852,9 @@ impl X509Extension { /// Some extension types, such as `nid::SUBJECT_ALTERNATIVE_NAME`, require an `X509v3Context` to /// be provided. /// + /// DO NOT CALL THIS WITH UNTRUSTED `value`: `value` is an OpenSSL + /// mini-language that can read arbitrary files. + /// /// See the extension module for builder types which will construct certain common extensions. pub fn new_nid( conf: Option<&ConfRef>, @@ -840,17 +863,42 @@ impl X509Extension { value: &str, ) -> Result { let value = CString::new(value).unwrap(); + let mut ctx; unsafe { ffi::init(); let conf = conf.map_or(ptr::null_mut(), ConfRef::as_ptr); - let context = context.map_or(ptr::null_mut(), X509v3Context::as_ptr); + let context_ptr = match context { + Some(c) => c.as_ptr(), + None => { + ctx = mem::zeroed(); + + ffi::X509V3_set_ctx( + &mut ctx, + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + ptr::null_mut(), + 0, + ); + &mut ctx + } + }; let name = name.as_raw(); let value = value.as_ptr() as *mut _; - cvt_p(ffi::X509V3_EXT_nconf_nid(conf, context, name, value)).map(X509Extension) + cvt_p(ffi::X509V3_EXT_nconf_nid(conf, context_ptr, name, value)).map(X509Extension) } } + pub(crate) unsafe fn new_internal( + nid: Nid, + critical: bool, + value: *mut c_void, + ) -> Result { + ffi::init(); + cvt_p(ffi::X509V3_EXT_i2d(nid.as_raw(), critical as _, value)).map(X509Extension) + } + /// Adds an alias for an extension /// /// # Safety @@ -863,6 +911,15 @@ impl X509Extension { } } +impl X509ExtensionRef { + to_der! { + /// Serializes the Extension to its standard DER encoding. + #[corresponds(i2d_X509_EXTENSION)] + to_der, + ffi::i2d_X509_EXTENSION + } +} + /// A builder used to construct an `X509Name`. pub struct X509NameBuilder(X509Name); @@ -988,7 +1045,10 @@ impl X509NameBuilder { /// Return an `X509Name`. pub fn build(self) -> X509Name { - self.0 + // Round-trip through bytes because OpenSSL is not const correct and + // names in a "modified" state compute various things lazily. This can + // lead to data-races because OpenSSL doesn't have locks or anything. + X509Name::from_der(&self.0.to_der().unwrap()).unwrap() } } @@ -1715,6 +1775,75 @@ foreign_type_and_impl_send_sync! { pub struct GeneralNameRef; } +impl GeneralName { + unsafe fn new( + type_: c_int, + asn1_type: Asn1Type, + value: &[u8], + ) -> Result { + ffi::init(); + let gn = GeneralName::from_ptr(cvt_p(ffi::GENERAL_NAME_new())?); + (*gn.as_ptr()).type_ = type_; + let s = cvt_p(ffi::ASN1_STRING_type_new(asn1_type.as_raw()))?; + ffi::ASN1_STRING_set(s, value.as_ptr().cast(), value.len().try_into().unwrap()); + + #[cfg(boringssl)] + { + (*gn.as_ptr()).d.ptr = s.cast(); + } + #[cfg(not(boringssl))] + { + (*gn.as_ptr()).d = s.cast(); + } + + Ok(gn) + } + + pub(crate) fn new_email(email: &[u8]) -> Result { + unsafe { GeneralName::new(ffi::GEN_EMAIL, Asn1Type::IA5STRING, email) } + } + + pub(crate) fn new_dns(dns: &[u8]) -> Result { + unsafe { GeneralName::new(ffi::GEN_DNS, Asn1Type::IA5STRING, dns) } + } + + pub(crate) fn new_uri(uri: &[u8]) -> Result { + unsafe { GeneralName::new(ffi::GEN_URI, Asn1Type::IA5STRING, uri) } + } + + pub(crate) fn new_ip(ip: IpAddr) -> Result { + match ip { + IpAddr::V4(addr) => unsafe { + GeneralName::new(ffi::GEN_IPADD, Asn1Type::OCTET_STRING, &addr.octets()) + }, + IpAddr::V6(addr) => unsafe { + GeneralName::new(ffi::GEN_IPADD, Asn1Type::OCTET_STRING, &addr.octets()) + }, + } + } + + pub(crate) fn new_rid(oid: Asn1Object) -> Result { + unsafe { + ffi::init(); + let gn = cvt_p(ffi::GENERAL_NAME_new())?; + (*gn).type_ = ffi::GEN_RID; + + #[cfg(boringssl)] + { + (*gn).d.registeredID = oid.as_ptr(); + } + #[cfg(not(boringssl))] + { + (*gn).d = oid.as_ptr().cast(); + } + + mem::forget(oid); + + Ok(GeneralName::from_ptr(gn)) + } + } +} + impl GeneralNameRef { fn ia5_string(&self, ffi_type: c_int) -> Option<&str> { unsafe { diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 5c563a21921f8595871406d3dd68726b5054610d..57734f2665a17ea964ccfc7a57c51ede876ed9ae 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -25,7 +25,7 @@ use crate::x509::X509PurposeId; #[cfg(any(ossl102, libressl261))] use crate::x509::X509PurposeRef; use crate::x509::{ - CrlStatus, X509Crl, X509Name, X509Req, X509StoreContext, X509VerifyResult, X509, + CrlStatus, X509Crl, X509Extension, X509Name, X509Req, X509StoreContext, X509VerifyResult, X509, }; use hex::{self, FromHex}; #[cfg(any(ossl102, libressl261))] @@ -287,6 +287,60 @@ fn x509_builder() { assert_eq!(serial, x509.serial_number().to_bn().unwrap()); } +#[test] +fn x509_extension_new() { + assert!(X509Extension::new(None, None, "crlDistributionPoints", "section").is_err()); + assert!(X509Extension::new(None, None, "proxyCertInfo", "").is_err()); + assert!(X509Extension::new(None, None, "certificatePolicies", "").is_err()); + assert!(X509Extension::new(None, None, "subjectAltName", "dirName:section").is_err()); +} + +#[test] +fn x509_extension_to_der() { + let builder = X509::builder().unwrap(); + + for (ext, expected) in [ + ( + BasicConstraints::new().critical().ca().build().unwrap(), + b"0\x0f\x06\x03U\x1d\x13\x01\x01\xff\x04\x050\x03\x01\x01\xff" as &[u8], + ), + ( + SubjectAlternativeName::new() + .dns("example.com,DNS:example2.com") + .build(&builder.x509v3_context(None, None)) + .unwrap(), + b"0'\x06\x03U\x1d\x11\x04 0\x1e\x82\x1cexample.com,DNS:example2.com", + ), + ( + SubjectAlternativeName::new() + .rid("1.2.3.4") + .uri("https://example.com") + .build(&builder.x509v3_context(None, None)) + .unwrap(), + b"0#\x06\x03U\x1d\x11\x04\x1c0\x1a\x88\x03*\x03\x04\x86\x13https://example.com", + ), + ( + ExtendedKeyUsage::new() + .server_auth() + .other("2.999.1") + .other("clientAuth") + .build() + .unwrap(), + b"0\x22\x06\x03U\x1d%\x04\x1b0\x19\x06\x08+\x06\x01\x05\x05\x07\x03\x01\x06\x03\x887\x01\x06\x08+\x06\x01\x05\x05\x07\x03\x02", + ), + ] { + assert_eq!(&ext.to_der().unwrap(), expected); + } +} + +#[test] +fn eku_invalid_other() { + assert!(ExtendedKeyUsage::new() + .other("1.1.1.1.1,2.2.2.2.2") + .build() + .is_err()); +} + #[test] fn x509_req_builder() { let pkey = pkey();