Unverified Commit 5efceaab authored by Alex Gaynor's avatar Alex Gaynor Committed by GitHub
Browse files

Merge pull request #1854 from alex/davids-openssl-of-horrors

Fix a series of security issues
parents 690eeb2a 6ced4f30
Loading
Loading
Loading
Loading
+7 −0
Original line number Diff line number Diff line
@@ -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
+1 −0
Original line number Diff line number Diff line
@@ -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);
}

+5 −0
Original line number Diff line number Diff line
@@ -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)]
+69 −90
Original line number Diff line number Diff line
@@ -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<String>,
    items: Vec<String>,
}

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<X509Extension, ErrorStack> {
        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<String>,
    items: Vec<RustGeneralName>,
}

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

+137 −8
Original line number Diff line number Diff line
@@ -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<X509Extension, ErrorStack> {
        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<X509Extension, ErrorStack> {
        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<X509Extension, ErrorStack> {
        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<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());

        #[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<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;

            #[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 {
Loading