Merge pull request #114 from nox/nox/openssl-fixes

Port security fixes from the openssl crate (fixes #111)

boring/src/asn1.rs

@@ -38,6 +38,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};
 
@@ -78,6 +79,79 @@ impl fmt::Display for Asn1GeneralizedTimeRef {
     }
 }
 
+/// The type of an ASN.1 value.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub struct Asn1Type(c_int);
+
+#[allow(missing_docs)] // no need to document the constants
+impl Asn1Type {
+    pub const EOC: Asn1Type = Asn1Type(ffi::V_ASN1_EOC);
+
+    pub const BOOLEAN: Asn1Type = Asn1Type(ffi::V_ASN1_BOOLEAN);
+
+    pub const INTEGER: Asn1Type = Asn1Type(ffi::V_ASN1_INTEGER);
+
+    pub const BIT_STRING: Asn1Type = Asn1Type(ffi::V_ASN1_BIT_STRING);
+
+    pub const OCTET_STRING: Asn1Type = Asn1Type(ffi::V_ASN1_OCTET_STRING);
+
+    pub const NULL: Asn1Type = Asn1Type(ffi::V_ASN1_NULL);
+
+    pub const OBJECT: Asn1Type = Asn1Type(ffi::V_ASN1_OBJECT);
+
+    pub const OBJECT_DESCRIPTOR: Asn1Type = Asn1Type(ffi::V_ASN1_OBJECT_DESCRIPTOR);
+
+    pub const EXTERNAL: Asn1Type = Asn1Type(ffi::V_ASN1_EXTERNAL);
+
+    pub const REAL: Asn1Type = Asn1Type(ffi::V_ASN1_REAL);
+
+    pub const ENUMERATED: Asn1Type = Asn1Type(ffi::V_ASN1_ENUMERATED);
+
+    pub const UTF8STRING: Asn1Type = Asn1Type(ffi::V_ASN1_UTF8STRING);
+
+    pub const SEQUENCE: Asn1Type = Asn1Type(ffi::V_ASN1_SEQUENCE);
+
+    pub const SET: Asn1Type = Asn1Type(ffi::V_ASN1_SET);
+
+    pub const NUMERICSTRING: Asn1Type = Asn1Type(ffi::V_ASN1_NUMERICSTRING);
+
+    pub const PRINTABLESTRING: Asn1Type = Asn1Type(ffi::V_ASN1_PRINTABLESTRING);
+
+    pub const T61STRING: Asn1Type = Asn1Type(ffi::V_ASN1_T61STRING);
+
+    pub const TELETEXSTRING: Asn1Type = Asn1Type(ffi::V_ASN1_TELETEXSTRING);
+
+    pub const VIDEOTEXSTRING: Asn1Type = Asn1Type(ffi::V_ASN1_VIDEOTEXSTRING);
+
+    pub const IA5STRING: Asn1Type = Asn1Type(ffi::V_ASN1_IA5STRING);
+
+    pub const UTCTIME: Asn1Type = Asn1Type(ffi::V_ASN1_UTCTIME);
+
+    pub const GENERALIZEDTIME: Asn1Type = Asn1Type(ffi::V_ASN1_GENERALIZEDTIME);
+
+    pub const GRAPHICSTRING: Asn1Type = Asn1Type(ffi::V_ASN1_GRAPHICSTRING);
+
+    pub const ISO64STRING: Asn1Type = Asn1Type(ffi::V_ASN1_ISO64STRING);
+
+    pub const VISIBLESTRING: Asn1Type = Asn1Type(ffi::V_ASN1_VISIBLESTRING);
+
+    pub const GENERALSTRING: Asn1Type = Asn1Type(ffi::V_ASN1_GENERALSTRING);
+
+    pub const UNIVERSALSTRING: Asn1Type = Asn1Type(ffi::V_ASN1_UNIVERSALSTRING);
+
+    pub const BMPSTRING: Asn1Type = Asn1Type(ffi::V_ASN1_BMPSTRING);
+
+    /// Constructs an `Asn1Type` from a raw OpenSSL value.
+    pub fn from_raw(value: c_int) -> Self {
+        Asn1Type(value)
+    }
+
+    /// Returns the raw OpenSSL value represented by this type.
+    pub fn as_raw(&self) -> c_int {
+        self.0
+    }
+}
+
 /// Difference between two ASN1 times.
 ///
 /// This `struct` is created by the [`diff`] method on [`Asn1TimeRef`]. See its
@@ -482,6 +556,10 @@ foreign_type_and_impl_send_sync! {
     pub struct Asn1Object;
 }
 
+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.

boring/src/x509/extension.rs

@@ -17,9 +17,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 {
@@ -221,18 +223,7 @@ impl KeyUsage {
 /// for which the certificate public key can be used for.
 pub struct ExtendedKeyUsage {
     critical: bool,
-    server_auth: bool,
+    items: Vec<String>,
-    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>,
 }
 
 impl Default for ExtendedKeyUsage {
@@ -246,18 +237,7 @@ impl ExtendedKeyUsage {
     pub fn new() -> ExtendedKeyUsage {
         ExtendedKeyUsage {
             critical: false,
-            server_auth: false,
+            items: vec![],
-            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![],
         }
     }
 
@@ -269,95 +249,69 @@ impl ExtendedKeyUsage {
 
     /// Sets the `serverAuth` flag to `true`.
     pub fn server_auth(&mut self) -> &mut ExtendedKeyUsage {
-        self.server_auth = true;
+        self.other("serverAuth")
-        self
     }
 
     /// Sets the `clientAuth` flag to `true`.
     pub fn client_auth(&mut self) -> &mut ExtendedKeyUsage {
-        self.client_auth = true;
+        self.other("clientAuth")
-        self
     }
 
     /// Sets the `codeSigning` flag to `true`.
     pub fn code_signing(&mut self) -> &mut ExtendedKeyUsage {
-        self.code_signing = true;
+        self.other("codeSigning")
-        self
     }
 
     /// Sets the `timeStamping` flag to `true`.
     pub fn time_stamping(&mut self) -> &mut ExtendedKeyUsage {
-        self.time_stamping = true;
+        self.other("timeStamping")
-        self
     }
 
     /// Sets the `msCodeInd` flag to `true`.
     pub fn ms_code_ind(&mut self) -> &mut ExtendedKeyUsage {
-        self.ms_code_ind = true;
+        self.other("msCodeInd")
-        self
     }
 
     /// Sets the `msCodeCom` flag to `true`.
     pub fn ms_code_com(&mut self) -> &mut ExtendedKeyUsage {
-        self.ms_code_com = true;
+        self.other("msCodeCom")
-        self
     }
 
     /// Sets the `msCTLSign` flag to `true`.
     pub fn ms_ctl_sign(&mut self) -> &mut ExtendedKeyUsage {
-        self.ms_ctl_sign = true;
+        self.other("msCTLSign")
-        self
     }
 
     /// Sets the `msSGC` flag to `true`.
     pub fn ms_sgc(&mut self) -> &mut ExtendedKeyUsage {
-        self.ms_sgc = true;
+        self.other("msSGC")
-        self
     }
 
     /// Sets the `msEFS` flag to `true`.
     pub fn ms_efs(&mut self) -> &mut ExtendedKeyUsage {
-        self.ms_efs = true;
+        self.other("msEFS")
-        self
     }
 
     /// Sets the `nsSGC` flag to `true`.
     pub fn ns_sgc(&mut self) -> &mut ExtendedKeyUsage {
-        self.ns_sgc = true;
+        self.other("nsSGC")
-        self
     }
 
     /// 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 stack = Stack::new()?;
-        let mut first = true;
+        for item in &self.items {
-        append(&mut value, &mut first, self.critical, "critical");
+            stack.push(Asn1Object::from_str(item)?)?;
-        append(&mut value, &mut first, self.server_auth, "serverAuth");
+        }
-        append(&mut value, &mut first, self.client_auth, "clientAuth");
+        unsafe {
-        append(&mut value, &mut first, self.code_signing, "codeSigning");
+            X509Extension::new_internal(Nid::EXT_KEY_USAGE, self.critical, stack.as_ptr().cast())
-        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);
         }
-        X509Extension::new_nid(None, None, Nid::EXT_KEY_USAGE, &value)
     }
 }
 
@@ -456,11 +410,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 +436,7 @@ impl SubjectAlternativeName {
     pub fn new() -> SubjectAlternativeName {
         SubjectAlternativeName {
             critical: false,
-            names: vec![],
+            items: vec![],
         }
     }
 
@@ -486,55 +448,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));
+    /// Not currently actually supported, always panics.
-        self
+    #[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));
+    /// Not currently actually supported, always panics.
-        self
+    #[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> {
+    pub fn build(&self, _ctx: &X509v3Context<'_>) -> Result<X509Extension, ErrorStack> {
-        let mut value = String::new();
+        let mut stack = Stack::new()?;
-        let mut first = true;
+        for item in &self.items {
-        append(&mut value, &mut first, self.critical, "critical");
+            let gn = match item {
-        for name in &self.names {
+                RustGeneralName::Dns(s) => GeneralName::new_dns(s.as_bytes())?,
-            append(&mut value, &mut first, true, name);
+                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)
     }
 }
 

boring/src/x509/mod.rs

@@ -9,19 +9,23 @@
 
 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};
+use crate::asn1::{
+    Asn1BitStringRef, Asn1IntegerRef, Asn1Object, Asn1ObjectRef, Asn1StringRef, Asn1TimeRef,
+    Asn1Type,
+};
 use crate::bio::MemBioSlice;
 use crate::conf::ConfRef;
 use crate::error::ErrorStack;
@@ -741,6 +745,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>,
@@ -750,14 +757,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))
+            cvt_p(ffi::X509V3_EXT_nconf(conf, context_ptr, name, value))
                 .map(|p| X509Extension::from_ptr(p))
         }
     }
@@ -768,6 +791,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>,
@@ -776,17 +802,51 @@ 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))
+            cvt_p(ffi::X509V3_EXT_nconf_nid(conf, context_ptr, name, value))
                 .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`.
@@ -823,6 +883,33 @@ impl X509NameBuilder {
         }
     }
 
+    /// Add a field entry by str with a specific type.
+    ///
+    /// This corresponds to [`X509_NAME_add_entry_by_txt`].
+    ///
+    /// [`X509_NAME_add_entry_by_txt`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_NAME_add_entry_by_txt.html
+    pub fn append_entry_by_text_with_type(
+        &mut self,
+        field: &str,
+        value: &str,
+        ty: Asn1Type,
+    ) -> Result<(), ErrorStack> {
+        unsafe {
+            let field = CString::new(field).unwrap();
+            assert!(value.len() <= c_int::max_value() as usize);
+            cvt(ffi::X509_NAME_add_entry_by_txt(
+                self.0.as_ptr(),
+                field.as_ptr() as *mut _,
+                ty.as_raw(),
+                value.as_ptr(),
+                value.len() as c_int,
+                -1,
+                0,
+            ))
+            .map(|_| ())
+        }
+    }
+
     /// Add a field entry by NID.
     ///
     /// This corresponds to [`X509_NAME_add_entry_by_NID`].
@@ -844,9 +931,38 @@ impl X509NameBuilder {
         }
     }
 
+    /// Add a field entry by NID with a specific type.
+    ///
+    /// This corresponds to [`X509_NAME_add_entry_by_NID`].
+    ///
+    /// [`X509_NAME_add_entry_by_NID`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_NAME_add_entry_by_NID.html
+    pub fn append_entry_by_nid_with_type(
+        &mut self,
+        field: Nid,
+        value: &str,
+        ty: Asn1Type,
+    ) -> Result<(), ErrorStack> {
+        unsafe {
+            assert!(value.len() <= c_int::max_value() as usize);
+            cvt(ffi::X509_NAME_add_entry_by_NID(
+                self.0.as_ptr(),
+                field.as_raw(),
+                ty.as_raw(),
+                value.as_ptr() as *mut _,
+                value.len() as c_int,
+                -1,
+                0,
+            ))
+            .map(|_| ())
+        }
+    }
+
     /// 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()
     }
 }
 
@@ -876,6 +992,18 @@ impl X509Name {
         let file = CString::new(file.as_ref().as_os_str().to_str().unwrap()).unwrap();
         unsafe { cvt_p(ffi::SSL_load_client_CA_file(file.as_ptr())).map(|p| Stack::from_ptr(p)) }
     }
+
+    from_der! {
+        /// Deserializes a DER-encoded X509 name structure.
+        ///
+        /// This corresponds to [`d2i_X509_NAME`].
+        ///
+        /// [`d2i_X509_NAME`]: https://www.openssl.org/docs/manmaster/man3/d2i_X509_NAME.html
+        from_der,
+        X509Name,
+        ffi::d2i_X509_NAME,
+        ::libc::c_long
+    }
 }
 
 impl Stackable for X509Name {
@@ -900,6 +1028,16 @@ impl X509NameRef {
             loc: -1,
         }
     }
+
+    to_der! {
+        /// Serializes the certificate into a DER-encoded X509 name structure.
+        ///
+        /// This corresponds to [`i2d_X509_NAME`].
+        ///
+        /// [`i2d_X509_NAME`]: https://www.openssl.org/docs/man1.1.0/crypto/i2d_X509_NAME.html
+        to_der,
+        ffi::i2d_X509_NAME
+    }
 }
 
 impl fmt::Debug for X509NameRef {
@@ -1292,6 +1430,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 {

boring/src/x509/tests.rs

@@ -12,7 +12,7 @@ use crate::x509::extension::{
     SubjectKeyIdentifier,
 };
 use crate::x509::store::X509StoreBuilder;
-use crate::x509::{X509Name, X509Req, X509StoreContext, X509VerifyResult, X509};
+use crate::x509::{X509Extension, X509Name, X509Req, X509StoreContext, X509VerifyResult, X509};
 
 fn pkey() -> PKey<Private> {
     let rsa = Rsa::generate(2048).unwrap();
@@ -250,6 +250,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();
@@ -379,3 +433,26 @@ fn test_verify_fails() {
         .init(&store, &cert, &chain, |c| c.verify_cert())
         .unwrap());
 }
+
+#[test]
+fn test_save_subject_der() {
+    let cert = include_bytes!("../../test/cert.pem");
+    let cert = X509::from_pem(cert).unwrap();
+
+    let der = cert.subject_name().to_der().unwrap();
+    println!("der: {:?}", der);
+    assert!(!der.is_empty());
+}
+
+#[test]
+fn test_load_subject_der() {
+    // The subject from ../../test/cert.pem
+    const SUBJECT_DER: &[u8] = &[
+        48, 90, 49, 11, 48, 9, 6, 3, 85, 4, 6, 19, 2, 65, 85, 49, 19, 48, 17, 6, 3, 85, 4, 8, 12,
+        10, 83, 111, 109, 101, 45, 83, 116, 97, 116, 101, 49, 33, 48, 31, 6, 3, 85, 4, 10, 12, 24,
+        73, 110, 116, 101, 114, 110, 101, 116, 32, 87, 105, 100, 103, 105, 116, 115, 32, 80, 116,
+        121, 32, 76, 116, 100, 49, 19, 48, 17, 6, 3, 85, 4, 3, 12, 10, 102, 111, 111, 98, 97, 114,
+        46, 99, 111, 109,
+    ];
+    X509Name::from_der(SUBJECT_DER).unwrap();
+}