Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CRL extensions and the Authority Information Access e… #2003

Merged
merged 2 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions openssl/src/x509/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1746,6 +1746,17 @@ unsafe impl ExtensionType for CertificateIssuer {
type Output = Stack<GeneralName>;
}

/// The CRL extension identifying how to access information and services for the issuer of the CRL
pub enum AuthorityInformationAccess {}

// SAFETY: AuthorityInformationAccess is defined to be a stack of AccessDescription in the RFC
// and in OpenSSL.
unsafe impl ExtensionType for AuthorityInformationAccess {
const NID: Nid = Nid::from_raw(ffi::NID_info_access);

type Output = Stack<AccessDescription>;
}

foreign_type_and_impl_send_sync! {
type CType = ffi::X509_CRL;
fn drop = ffi::X509_CRL_free;
Expand Down Expand Up @@ -1915,6 +1926,36 @@ impl X509CrlRef {
{
unsafe { cvt_n(ffi::X509_CRL_verify(self.as_ptr(), key.as_ptr())).map(|n| n != 0) }
}

/// Get the criticality and value of an extension.
///
/// This returns None if the extension is not present or occurs multiple times.
#[corresponds(X509_CRL_get_ext_d2i)]
pub fn extension<T: ExtensionType>(&self) -> Result<Option<(bool, T::Output)>, ErrorStack> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfackler This change looks good to me, I'd like to check that you're happy with carrying on with this approach to extensions?

let mut critical = -1;
let out = unsafe {
// SAFETY: self.as_ptr() is a valid pointer to an X509_CRL.
let ext = ffi::X509_CRL_get_ext_d2i(
self.as_ptr(),
T::NID.as_raw(),
&mut critical as *mut _,
ptr::null_mut(),
);
// SAFETY: Extensions's contract promises that the type returned by
// OpenSSL here is T::Output.
T::Output::from_ptr_opt(ext as *mut _)
};
match (critical, out) {
(0, Some(out)) => Ok(Some((false, out))),
(1, Some(out)) => Ok(Some((true, out))),
// -1 means the extension wasn't found, -2 means multiple were found.
(-1 | -2, _) => Ok(None),
// A critical value of 0 or 1 suggests success, but a null pointer
// was returned so something went wrong.
(0 | 1, None) => Err(ErrorStack::get()),
(c_int::MIN..=-2 | 2.., _) => panic!("OpenSSL should only return -2, -1, 0, or 1 for an extension's criticality but it returned {}", critical),
}
}
}

/// The result of peer certificate verification.
Expand Down
20 changes: 19 additions & 1 deletion openssl/src/x509/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use hex::{self, FromHex};
#[cfg(any(ossl102, libressl261))]
use libc::time_t;

use super::{CertificateIssuer, ReasonCode};
use super::{AuthorityInformationAccess, CertificateIssuer, ReasonCode};

fn pkey() -> PKey<Private> {
let rsa = Rsa::generate(2048).unwrap();
Expand Down Expand Up @@ -701,6 +701,24 @@ fn test_crl_entry_extensions() {
let crl = include_bytes!("../../test/entry_extensions.crl");
let crl = X509Crl::from_pem(crl).unwrap();

let (critical, access_info) = crl
.extension::<AuthorityInformationAccess>()
.unwrap()
.expect("Authority Information Access extension should be present");
assert!(
!critical,
"Authority Information Access extension is not critical"
);
assert_eq!(
access_info.len(),
1,
"Authority Information Access should have one entry"
);
assert_eq!(access_info[0].method().to_string(), "CA Issuers");
assert_eq!(
access_info[0].location().uri(),
Some("http://www.example.com/ca.crt")
);
let revoked_certs = crl.get_revoked().unwrap();
let entry = &revoked_certs[0];

Expand Down
17 changes: 9 additions & 8 deletions openssl/test/entry_extensions.crl
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
-----BEGIN X509 CRL-----
MIIBXDCCAQICAQEwCgYIKoZIzj0EAwIwETEPMA0GA1UEAwwGQ1JMIENBFw0yMzAz
MjgwOTQ5MThaFw0yMzA0MDQwOTUwMDdaMIGAMH4CFE+Y95/1pOqa6c9fUEJ8c04k
xu2PFw0yMzAzMjgwOTQ3MzNaMFcwLwYDVR0dAQH/BCUwI6QhMB8xCzAJBgNVBAYT
AkdCMRAwDgYDVQQDDAdUZXN0IENBMAoGA1UdFQQDCgEBMBgGA1UdGAQRGA8yMDIz
MDMyODA5NDQ0MFqgPTA7MB8GA1UdIwQYMBaAFNX1GZ0RWuC+4gz1wuy5H32T2W+R
MAoGA1UdFAQDAgEUMAwGA1UdHAQFMAOEAf8wCgYIKoZIzj0EAwIDSAAwRQIgbl7x
W+WVAb+zlvKcJLmHVuC+gbqR4jqwGIHHgQl2J8kCIQCo/sAF5sDqy/cL+fbzBeUe
YoY2h6lIkj9ENwU8ZCt03w==
MIIBojCCAUkCAQEwCgYIKoZIzj0EAwIwHTEbMBkGA1UEAwwSY3J5cHRvZ3JhcGh5
LmlvIENBFw0yMzA3MjUxNDA1MzlaFw0yMzA4MDExNDA1MzlaMIGAMH4CFE+Y95/1
pOqa6c9fUEJ8c04kxu2PFw0yMzA3MjUxNDA1MzlaMFcwLwYDVR0dAQH/BCUwI6Qh
MB8xCzAJBgNVBAYTAkdCMRAwDgYDVQQDDAdUZXN0IENBMAoGA1UdFQQDCgEBMBgG
A1UdGAQRGA8yMDIzMDcyNTE0MDUzOVqgeDB2MB8GA1UdIwQYMBaAFK6qKNgsGefh
XexO9WsIwiQ/73R8MAoGA1UdFAQDAgEUMAwGA1UdHAQFMAOEAf8wOQYIKwYBBQUH
AQEELTArMCkGCCsGAQUFBzAChh1odHRwOi8vd3d3LmV4YW1wbGUuY29tL2NhLmNy
dDAKBggqhkjOPQQDAgNHADBEAiB22SXxFnQUB41uxfyCvg2dAs2nFiR0r8jft/cd
G8zcKAIgeYkNOzRn4lyopK6J94rhm8jIIuJRj3Ns9XcH+91N370=
-----END X509 CRL-----