From 482575bff434f58b80ffea34a9610d0ff265ac1f Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 21 Mar 2023 20:20:26 -0400 Subject: [PATCH 1/5] Resolve an injection vulnerability in SAN creation --- openssl-sys/src/handwritten/x509.rs | 7 ++ openssl-sys/src/handwritten/x509v3.rs | 1 + openssl/src/x509/extension.rs | 69 ++++++++++++++------ openssl/src/x509/mod.rs | 94 ++++++++++++++++++++++++++- openssl/src/x509/tests.rs | 38 +++++++++++ 5 files changed, 185 insertions(+), 24 deletions(-) diff --git a/openssl-sys/src/handwritten/x509.rs b/openssl-sys/src/handwritten/x509.rs index 8762e5f98d..abda4110cf 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 d0923e32b2..4f661ca5ec 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/src/x509/extension.rs b/openssl/src/x509/extension.rs index ebbea1c885..21d8faac35 100644 --- a/openssl/src/x509/extension.rs +++ b/openssl/src/x509/extension.rs @@ -20,7 +20,8 @@ use std::fmt::Write; use crate::error::ErrorStack; use crate::nid::Nid; -use crate::x509::{X509Extension, X509v3Context}; +use crate::x509::{Asn1Object, GeneralName, Stack, X509Extension, X509v3Context}; +use foreign_types::ForeignType; /// An extension which indicates whether a certificate is a CA certificate. pub struct BasicConstraints { @@ -463,11 +464,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 +490,7 @@ impl SubjectAlternativeName { pub fn new() -> SubjectAlternativeName { SubjectAlternativeName { critical: false, - names: vec![], + items: vec![], } } @@ -493,55 +502,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 4f08bbc667..3d8f236fd5 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; @@ -851,6 +852,15 @@ impl 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 +873,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); @@ -1715,6 +1734,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 5c563a2192..41a9bc4d61 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -287,6 +287,44 @@ fn x509_builder() { assert_eq!(serial, x509.serial_number().to_bn().unwrap()); } +#[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 x509_req_builder() { let pkey = pkey(); From 332311b597cc444a10d4acaf122ee58bd1bc8ff8 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 21 Mar 2023 20:31:02 -0400 Subject: [PATCH 2/5] Resolve an injection vulnerability in EKU creation --- openssl/src/asn1.rs | 5 ++ openssl/src/x509/extension.rs | 92 +++++++++-------------------------- openssl/src/x509/tests.rs | 8 +++ 3 files changed, 35 insertions(+), 70 deletions(-) diff --git a/openssl/src/asn1.rs b/openssl/src/asn1.rs index 55de049c08..c0178c7e65 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/x509/extension.rs b/openssl/src/x509/extension.rs index 21d8faac35..f04d227960 100644 --- a/openssl/src/x509/extension.rs +++ b/openssl/src/x509/extension.rs @@ -18,9 +18,10 @@ //! ``` use std::fmt::Write; +use crate::asn1::Asn1Object; use crate::error::ErrorStack; use crate::nid::Nid; -use crate::x509::{Asn1Object, GeneralName, Stack, X509Extension, X509v3Context}; +use crate::x509::{GeneralName, Stack, X509Extension, X509v3Context}; use foreign_types::ForeignType; /// An extension which indicates whether a certificate is a CA certificate. @@ -223,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 { @@ -248,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![], } } @@ -271,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) } } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 41a9bc4d61..91fd36790c 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -325,6 +325,14 @@ fn x509_extension_to_der() { } } +#[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(); From 78aa9aa22cfd58ac33d1e19184cec667438fd2a1 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 21 Mar 2023 20:44:15 -0400 Subject: [PATCH 3/5] Always provide an X509V3Context in X509Extension::new because OpenSSL requires it for some extensions (and segfaults without) --- openssl/src/x509/mod.rs | 40 +++++++++++++++++++++++++++++++++++---- openssl/src/x509/tests.rs | 10 +++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 3d8f236fd5..60df75ae72 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -816,14 +816,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) } } @@ -841,14 +857,30 @@ 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) } } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 91fd36790c..57734f2665 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,14 @@ 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(); From a7528056c5be6f3fbabc52c2fd02882b208d5939 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 21 Mar 2023 20:45:35 -0400 Subject: [PATCH 4/5] Document the horror show --- openssl/src/x509/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 60df75ae72..bb55cada02 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -807,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>, @@ -849,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>, From 6ced4f305e44df7ca32e478621bf4840b122f1a3 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 21 Mar 2023 20:49:48 -0400 Subject: [PATCH 5/5] Fix race condition with X509Name creation --- openssl/src/x509/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index bb55cada02..5b55918750 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -1045,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() } }