Resolve an injection vulnerability in SAN creation

This commit is contained in:
Alex Gaynor 2023-03-21 20:20:26 -04:00 committed by Anthony Ramine
parent ae0cd6b98e
commit 1eea7c5271
3 changed files with 162 additions and 23 deletions

View File

@ -19,7 +19,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 {
@ -456,11 +457,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<String>,
items: Vec<RustGeneralName>,
}
impl Default for SubjectAlternativeName {
@ -474,7 +483,7 @@ impl SubjectAlternativeName {
pub fn new() -> SubjectAlternativeName {
SubjectAlternativeName {
critical: false,
names: vec![],
items: vec![],
}
}
@ -486,55 +495,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<X509Extension, ErrorStack> {
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<X509Extension, ErrorStack> {
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)
}
}

View File

@ -9,20 +9,22 @@
use crate::ffi;
use foreign_types::{ForeignType, ForeignTypeRef};
use libc::{c_int, c_long};
use libc::{c_int, c_long, c_void};
use std::convert::TryInto;
use std::error::Error;
use std::ffi::{CStr, CString};
use std::fmt;
use std::marker::PhantomData;
use std::mem;
use std::net::IpAddr;
use std::path::Path;
use std::ptr;
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;
@ -792,6 +794,24 @@ impl X509Extension {
.map(|p| X509Extension::from_ptr(p))
}
}
pub(crate) unsafe fn new_internal(
nid: Nid,
critical: bool,
value: *mut c_void,
) -> Result<X509Extension, ErrorStack> {
ffi::init();
cvt_p(ffi::X509V3_EXT_i2d(nid.as_raw(), critical as _, value))
.map(|p| X509Extension::from_ptr(p))
}
}
impl X509ExtensionRef {
to_der! {
/// Serializes the Extension to its standard DER encoding.
to_der,
ffi::i2d_X509_EXTENSION
}
}
/// A builder used to construct an `X509Name`.
@ -1369,6 +1389,60 @@ foreign_type_and_impl_send_sync! {
pub struct GeneralName;
}
impl GeneralName {
unsafe fn new(
type_: c_int,
asn1_type: Asn1Type,
value: &[u8],
) -> Result<GeneralName, ErrorStack> {
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());
(*gn.as_ptr()).d.ptr = s.cast();
Ok(gn)
}
pub(crate) fn new_email(email: &[u8]) -> Result<GeneralName, ErrorStack> {
unsafe { GeneralName::new(ffi::GEN_EMAIL, Asn1Type::IA5STRING, email) }
}
pub(crate) fn new_dns(dns: &[u8]) -> Result<GeneralName, ErrorStack> {
unsafe { GeneralName::new(ffi::GEN_DNS, Asn1Type::IA5STRING, dns) }
}
pub(crate) fn new_uri(uri: &[u8]) -> Result<GeneralName, ErrorStack> {
unsafe { GeneralName::new(ffi::GEN_URI, Asn1Type::IA5STRING, uri) }
}
pub(crate) fn new_ip(ip: IpAddr) -> Result<GeneralName, ErrorStack> {
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<GeneralName, ErrorStack> {
unsafe {
ffi::init();
let gn = cvt_p(ffi::GENERAL_NAME_new())?;
(*gn).type_ = ffi::GEN_RID;
(*gn).d.registeredID = oid.as_ptr();
mem::forget(oid);
Ok(GeneralName::from_ptr(gn))
}
}
}
impl GeneralNameRef {
fn ia5_string(&self, ffi_type: c_int) -> Option<&str> {
unsafe {

View File

@ -250,6 +250,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();