From b465b0150ec5a34cbb9a11ca774b3939d508bbf6 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 13 Apr 2023 19:35:45 -0400 Subject: [PATCH] Introduce X509Extension::new_from_der and deprecate the bad APIs --- openssl/src/x509/extension.rs | 12 ++++++++ openssl/src/x509/mod.rs | 55 +++++++++++++++++++++++++++++++++-- openssl/src/x509/tests.rs | 18 +++++++++++- 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/openssl/src/x509/extension.rs b/openssl/src/x509/extension.rs index f04d227960..075227dec3 100644 --- a/openssl/src/x509/extension.rs +++ b/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 { 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 { 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 { 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 { let mut value = String::new(); let mut first = true; diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 00b467fb77..cba3235411 100644 --- a/openssl/src/x509/mod.rs +++ b/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, Asn1String, + 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,39 @@ 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: &[u8], + ) -> Result { + unsafe { + let s = Asn1String::from_ptr(cvt_p(ffi::ASN1_STRING_type_new( + Asn1Type::OCTET_STRING.as_raw(), + ))?); + cvt(ffi::ASN1_STRING_set( + s.as_ptr(), + der_contents.as_ptr().cast(), + der_contents.len().try_into().unwrap(), + ))?; + cvt_p(ffi::X509_EXTENSION_create_by_OBJ( + ptr::null_mut(), + oid.as_ptr(), + critical as _, + s.as_ptr().cast(), + )) + .map(X509Extension) + } + } + pub(crate) unsafe fn new_internal( nid: Nid, critical: bool, @@ -936,6 +983,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(|_| ()) diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 81801358b1..a994905a86 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -1,6 +1,6 @@ use std::cmp::Ordering; -use crate::asn1::Asn1Time; +use crate::asn1::{Asn1Object, Asn1Time}; use crate::bn::{BigNum, MsbOption}; use crate::hash::MessageDigest; use crate::nid::Nid; @@ -290,6 +290,8 @@ 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()); @@ -297,6 +299,20 @@ fn x509_extension_new() { 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, + b"\x30\x03\x01\x01\xff", + ) + .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();