Merge pull request #1880 from alex/extension-api-cleanup

Introduce X509Extension::new_from_der and deprecate the bad APIs

openssl/src/x509/extension.rs

@@ -67,6 +67,9 @@ impl BasicConstraints {
     }
 
     /// Return the `BasicConstraints` extension as an `X509Extension`.
+    // Temporarily silence the deprecation warning - this should be ported to
+    // `X509Extension::new_internal`.
+    #[allow(deprecated)]
     pub fn build(&self) -> Result<X509Extension, ErrorStack> {
         let mut value = String::new();
         if self.critical {
@@ -183,6 +186,9 @@ impl KeyUsage {
     }
 
     /// Return the `KeyUsage` extension as an `X509Extension`.
+    // Temporarily silence the deprecation warning - this should be ported to
+    // `X509Extension::new_internal`.
+    #[allow(deprecated)]
     pub fn build(&self) -> Result<X509Extension, ErrorStack> {
         let mut value = String::new();
         let mut first = true;
@@ -346,6 +352,9 @@ impl SubjectKeyIdentifier {
     }
 
     /// Return a `SubjectKeyIdentifier` extension as an `X509Extension`.
+    // Temporarily silence the deprecation warning - this should be ported to
+    // `X509Extension::new_internal`.
+    #[allow(deprecated)]
     pub fn build(&self, ctx: &X509v3Context<'_>) -> Result<X509Extension, ErrorStack> {
         let mut value = String::new();
         let mut first = true;
@@ -398,6 +407,9 @@ impl AuthorityKeyIdentifier {
     }
 
     /// Return a `AuthorityKeyIdentifier` extension as an `X509Extension`.
+    // Temporarily silence the deprecation warning - this should be ported to
+    // `X509Extension::new_internal`.
+    #[allow(deprecated)]
     pub fn build(&self, ctx: &X509v3Context<'_>) -> Result<X509Extension, ErrorStack> {
         let mut value = String::new();
         let mut first = true;

openssl/src/x509/mod.rs

@@ -24,8 +24,8 @@ use std::slice;
 use std::str;
 
 use crate::asn1::{
-    Asn1BitStringRef, Asn1Enumerated, Asn1IntegerRef, Asn1Object, Asn1ObjectRef, Asn1StringRef,
-    Asn1TimeRef, Asn1Type,
+    Asn1BitStringRef, Asn1Enumerated, Asn1IntegerRef, Asn1Object, Asn1ObjectRef,
+    Asn1OctetStringRef, Asn1StringRef, Asn1TimeRef, Asn1Type,
 };
 use crate::bio::MemBioSlice;
 use crate::conf::ConfRef;
@@ -842,6 +842,13 @@ impl X509Extension {
     /// mini-language that can read arbitrary files.
     ///
     /// See the extension module for builder types which will construct certain common extensions.
+    ///
+    /// This function is deprecated, `X509Extension::new_from_der` or the
+    /// types in `x509::extension` should be used in its place.
+    #[deprecated(
+        note = "Use x509::extension types or new_from_der instead",
+        since = "0.10.51"
+    )]
     pub fn new(
         conf: Option<&ConfRef>,
         context: Option<&X509v3Context<'_>>,
@@ -887,6 +894,13 @@ impl X509Extension {
     /// mini-language that can read arbitrary files.
     ///
     /// See the extension module for builder types which will construct certain common extensions.
+    ///
+    /// This function is deprecated, `X509Extension::new_from_der` or the
+    /// types in `x509::extension` should be used in its place.
+    #[deprecated(
+        note = "Use x509::extension types or new_from_der instead",
+        since = "0.10.51"
+    )]
     pub fn new_nid(
         conf: Option<&ConfRef>,
         context: Option<&X509v3Context<'_>>,
@@ -921,6 +935,31 @@ impl X509Extension {
         }
     }
 
+    /// Constructs a new X509 extension value from its OID, whether it's
+    /// critical, and its DER contents.
+    ///
+    /// The extent structure of the DER value will vary based on the
+    /// extension type, and can generally be found in the RFC defining the
+    /// extension.
+    ///
+    /// For common extension types, there are Rust APIs provided in
+    /// `openssl::x509::extensions` which are more ergonomic.
+    pub fn new_from_der(
+        oid: &Asn1ObjectRef,
+        critical: bool,
+        der_contents: &Asn1OctetStringRef,
+    ) -> Result<X509Extension, ErrorStack> {
+        unsafe {
+            cvt_p(ffi::X509_EXTENSION_create_by_OBJ(
+                ptr::null_mut(),
+                oid.as_ptr(),
+                critical as _,
+                der_contents.as_ptr(),
+            ))
+            .map(X509Extension)
+        }
+    }
+
     pub(crate) unsafe fn new_internal(
         nid: Nid,
         critical: bool,
@@ -936,6 +975,10 @@ impl X509Extension {
     ///
     /// This method modifies global state without locking and therefore is not thread safe
     #[corresponds(X509V3_EXT_add_alias)]
+    #[deprecated(
+        note = "Use x509::extension types or new_from_der and then this is not necessary",
+        since = "0.10.51"
+    )]
     pub unsafe fn add_alias(to: Nid, from: Nid) -> Result<(), ErrorStack> {
         ffi::init();
         cvt(ffi::X509V3_EXT_add_alias(to.as_raw(), from.as_raw())).map(|_| ())

openssl/src/x509/tests.rs

@@ -1,6 +1,6 @@
 use std::cmp::Ordering;
 
-use crate::asn1::Asn1Time;
+use crate::asn1::{Asn1Object, Asn1OctetString, Asn1Time};
 use crate::bn::{BigNum, MsbOption};
 use crate::hash::MessageDigest;
 use crate::nid::Nid;
@@ -290,13 +290,29 @@ fn x509_builder() {
 }
 
 #[test]
+// This tests `X509Extension::new`, even though its deprecated.
+#[allow(deprecated)]
 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_new_from_der() {
+    let ext = X509Extension::new_from_der(
+        &Asn1Object::from_str("2.5.29.19").unwrap(),
+        true,
+        &Asn1OctetString::new_from_bytes(b"\x30\x03\x01\x01\xff").unwrap(),
+    )
+    .unwrap();
+    assert_eq!(
+        ext.to_der().unwrap(),
+        b"0\x0f\x06\x03U\x1d\x13\x01\x01\xff\x04\x050\x03\x01\x01\xff"
+    );
+}
+
 #[test]
 fn x509_extension_to_der() {
     let builder = X509::builder().unwrap();