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

Extend X509Crl functionality #2174

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3b79c62
add two additional constructors for Asn1Time
gweisert Feb 15, 2024
834e375
extend X509Crl with revoke and sign capabilities
gweisert Feb 15, 2024
e2756d3
add binding for ASN1_INTEGER_new
gweisert Feb 17, 2024
235f8b1
add crl method to set last updated time
gweisert Feb 17, 2024
84f5937
extend crl's revoke method to update last_update time and add doc com…
gweisert Feb 17, 2024
bf4c7e1
add support for crl's crl_number extension
gweisert Feb 17, 2024
45534ac
use bignum for crl_number extension related functions
gweisert Feb 17, 2024
57e376a
fix crl_revoke test
gweisert Feb 17, 2024
cd0bf42
fix c related integer issues
gweisert Feb 17, 2024
b44bcc5
fix incompatibilities with older openssl versions
gweisert Feb 17, 2024
30f4f30
fix X509Crl::new leaking the Crl if an error occurs between allocatio…
gweisert Feb 17, 2024
f98474f
remove binding for ASN1_INTEGER_new, it is no longer used
gweisert Feb 17, 2024
62a449c
fix typo
gweisert Feb 17, 2024
f738016
boringssl compatibility
gweisert Feb 17, 2024
4213ae2
avoid unsigned to signed casting
gweisert Feb 17, 2024
d7a8f3f
fix potential memory leak in X509Revoked::new_raw
gweisert Feb 17, 2024
75397e8
use seconds_from_now instead of from_period in asn1 time methods
Feb 29, 2024
3ccd389
Merge branch 'sfackler:master' into implement_crl_revoke
gweisert Mar 2, 2024
41fd2a4
add binding for X509_CRL_get_version
Mar 3, 2024
a86bf40
add method to retrieve a CRL's version
Mar 3, 2024
fc468e5
rework X509Crl::new
Mar 3, 2024
a53177e
add conditional compilation directives
Mar 3, 2024
3e4fbdb
linting
Mar 3, 2024
974a8d8
extend availability of X509_CRL_get_version and related function to l…
Mar 3, 2024
04ceb33
Merge branch 'sfackler:master' into implement_crl_revoke
gweisert Mar 22, 2024
812b73e
use non-owning X509 where sensible
Mar 22, 2024
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
12 changes: 11 additions & 1 deletion openssl/src/asn1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,19 @@ impl Asn1Time {
}
}

/// Creates a new time with the current time
pub fn now() -> Result<Asn1Time, ErrorStack> {
Asn1Time::seconds_from_now(0)
}

/// Creates a new time on specified interval in days from now
pub fn days_from_now(days: u32) -> Result<Asn1Time, ErrorStack> {
Asn1Time::from_period(days as c_long * 60 * 60 * 24)
Asn1Time::seconds_from_now(days as c_long * 60 * 60 * 24)
}

/// Creates a new time on specified interval in seconds from now
pub fn seconds_from_now(seconds: c_long) -> Result<Asn1Time, ErrorStack> {
gweisert marked this conversation as resolved.
Show resolved Hide resolved
Asn1Time::from_period(seconds)
}

/// Creates a new time from the specified `time_t` value
Expand Down
177 changes: 175 additions & 2 deletions openssl/src/x509/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ use std::slice;
use std::str;

use crate::asn1::{
Asn1BitStringRef, Asn1Enumerated, Asn1IntegerRef, Asn1Object, Asn1ObjectRef,
Asn1OctetStringRef, Asn1StringRef, Asn1TimeRef, Asn1Type,
Asn1BitStringRef, Asn1Enumerated, Asn1Integer, Asn1IntegerRef, Asn1Object, Asn1ObjectRef,
Asn1OctetStringRef, Asn1StringRef, Asn1Time, Asn1TimeRef, Asn1Type,
};
use crate::bio::MemBioSlice;
use crate::bn::BigNum;
use crate::conf::ConfRef;
use crate::error::ErrorStack;
use crate::ex_data::Index;
Expand Down Expand Up @@ -1670,6 +1671,27 @@ impl X509Revoked {
X509Revoked,
ffi::d2i_X509_REVOKED
}

pub fn new(to_revoke: &X509) -> Result<Self, ErrorStack> {
gweisert marked this conversation as resolved.
Show resolved Hide resolved
unsafe { Ok(Self(Self::new_raw(to_revoke)?)) }
}

/// the caller has to ensure the pointer is freed
unsafe fn new_raw(to_revoke: &X509) -> Result<*mut ffi::X509_REVOKED, ErrorStack> {
let result = cvt_p(ffi::X509_REVOKED_new())?;

if ffi::X509_REVOKED_set_serialNumber(result, to_revoke.serial_number().as_ptr()) <= 0 {
ffi::X509_REVOKED_free(result);
return Err(ErrorStack::get());
}
if ffi::X509_REVOKED_set_revocationDate(result, crate::asn1::Asn1Time::now()?.as_ptr()) <= 0
{
ffi::X509_REVOKED_free(result);
return Err(ErrorStack::get());
}

Ok(result)
}
}

impl X509RevokedRef {
Expand Down Expand Up @@ -1845,6 +1867,157 @@ impl X509Crl {
X509Crl,
ffi::d2i_X509_CRL
}

pub fn new(issuer_cert: &X509) -> Result<Self, ErrorStack> {
unsafe {
let crl = Self(cvt_p(ffi::X509_CRL_new())?);
#[cfg(ossl110)]
{
cvt(ffi::X509_CRL_set_version(
crl.as_ptr(),
issuer_cert.version() as c_long,
gweisert marked this conversation as resolved.
Show resolved Hide resolved
))?;
}
cvt(ffi::X509_CRL_set_issuer_name(
crl.as_ptr(),
issuer_cert.issuer_name().as_ptr(),
))?;

cfg_if!(
if #[cfg(any(ossl110, libressl270, boringssl))] {
cvt(ffi::X509_CRL_set1_lastUpdate(crl.as_ptr(), Asn1Time::now()?.as_ptr())).map(|_| ())?
} else {
cvt(ffi::X509_CRL_set_lastUpdate(crl.as_ptr(), Asn1Time::now()?.as_ptr())).map(|_| ())?
}
);

Ok(crl)
}
}

/// use a negative value to set a time before 'now'
pub fn set_last_update(&mut self, seconds_from_now: Option<i32>) -> Result<(), ErrorStack> {
let time = Asn1Time::seconds_from_now(seconds_from_now.unwrap_or(0) as c_long)?;
cfg_if!(
if #[cfg(any(ossl110, libressl270, boringssl))] {
unsafe {
cvt(ffi::X509_CRL_set1_lastUpdate(self.as_ptr(), time.as_ptr())).map(|_| ())?
};
} else {
unsafe {
cvt(ffi::X509_CRL_set_lastUpdate(self.as_ptr(), time.as_ptr())).map(|_| ())?
};
}
);

Ok(())
}

pub fn set_next_update_from_now(&mut self, seconds_from_now: i32) -> Result<(), ErrorStack> {
cfg_if!(
if #[cfg(any(ossl110, libressl270, boringssl))] {
unsafe {
cvt(ffi::X509_CRL_set1_nextUpdate(
self.as_ptr(),
Asn1Time::seconds_from_now(seconds_from_now as c_long)?.as_ptr(),
))
.map(|_| ())?;
}
} else {
unsafe {
cvt(ffi::X509_CRL_set_nextUpdate(
self.as_ptr(),
Asn1Time::seconds_from_now(seconds_from_now as c_long)?.as_ptr(),
))
.map(|_| ())?;
}
}
);

Ok(())
}

pub fn entry_count(&mut self) -> usize {
self.get_revoked()
.map(|stack| stack.len())
.unwrap_or_default()
}

pub fn sign<T>(&mut self, key: &PKeyRef<T>, hash: MessageDigest) -> Result<(), ErrorStack>
where
T: HasPrivate,
{
unsafe {
cvt(ffi::X509_CRL_sign(
self.as_ptr(),
key.as_ptr(),
hash.as_ptr(),
))
.map(|_| ())
}
}

/// Read the value of the crl_number extensions.
/// Returns None if the extension is not present.
pub fn read_crl_number(&self) -> Result<Option<BigNum>, ErrorStack> {
unsafe {
let mut crit = 0;
let number = Asn1Integer::from_ptr_opt(std::mem::transmute(ffi::X509_CRL_get_ext_d2i(
self.as_ptr(),
ffi::NID_crl_number,
&mut crit,
std::ptr::null_mut(),
)));
match number {
None => {
if crit == -1 {
// extension was not found
Ok(None)
} else {
Err(ErrorStack::get())
}
}

Some(number) => Ok(Some(number.to_bn()?)),
}
}
}

/// Set the crl_number extension's value.
/// If the extension is not present, it will be added.
pub fn set_crl_number(&mut self, value: &BigNum) -> Result<(), ErrorStack> {
unsafe {
let value = Asn1Integer::from_bn(value)?;
cvt(ffi::X509_CRL_add1_ext_i2d(
self.as_ptr(),
ffi::NID_crl_number,
std::mem::transmute(value.as_ptr()),
0,
#[allow(clippy::useless_conversion)]
ffi::X509V3_ADD_REPLACE.try_into().expect("This is an openssl flag and should therefore always fit into the expected integer type"),
))
.map(|_| ())
}
}

/// Revoke the given certificate.
/// This function won't produce duplicate entries in case the certificate was already revoked.
/// Sets the CRL's last_updated time to the current time before returning irregardless of the given certificate.
pub fn revoke(&mut self, to_revoke: &X509) -> Result<(), ErrorStack> {
match self.get_by_cert(to_revoke) {
Comment on lines +2074 to +2075

Choose a reason for hiding this comment

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

When my PR #2207 is merged, you can change this &X509 to &X509Ref as well.

Suggested change
pub fn revoke(&mut self, to_revoke: &X509) -> Result<(), ErrorStack> {
match self.get_by_cert(to_revoke) {
pub fn revoke(&mut self, to_revoke: &X509Ref) -> Result<(), ErrorStack> {
match self.get_by_cert(to_revoke) {

Copy link
Author

Choose a reason for hiding this comment

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

Alright, I will update it once it is merged.

CrlStatus::NotRevoked => unsafe {
// we are not allowed to drop the revoked after adding it to the crl
let revoked = X509Revoked::new_raw(to_revoke)?;
if ffi::X509_CRL_add0_revoked(self.as_ptr(), revoked) == 0 {
return Err(ErrorStack::get());
};
},

_ => { /* do nothing, already revoked */ }
}

self.set_last_update(Some(0))
}
}

impl X509CrlRef {
Expand Down
60 changes: 60 additions & 0 deletions openssl/src/x509/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,66 @@ fn test_load_crl() {
);
}

#[test]
fn test_crl_sign() {
let ca = include_bytes!("../../test/crl-ca.crt");
let ca = X509::from_pem(ca).unwrap();
let pkey = include_bytes!("../../test/rsa.pem");
let pkey = PKey::private_key_from_pem(pkey).unwrap();

let mut crl = X509Crl::new(&ca).unwrap();
crl.sign(&pkey, MessageDigest::sha256()).unwrap();
assert!(crl.verify(&pkey).unwrap());
}

#[test]
fn test_crl_revoke() {
let ca = include_bytes!("../../test/crl-ca.crt");
let ca = X509::from_pem(ca).unwrap();

let crl = include_bytes!("../../test/test.crl");
let mut crl = X509Crl::from_der(crl).unwrap();
assert!(crl.verify(&ca.public_key().unwrap()).unwrap());

// ensure revoking an already revoked cert does not change the revoked count
{
let already_revoked_cert = include_bytes!("../../test/subca.crt");
let already_revoked_cert = X509::from_pem(already_revoked_cert).unwrap();

let count_before = crl.entry_count();
crl.revoke(&already_revoked_cert).unwrap();
assert_eq!(
count_before,
crl.entry_count(),
"clr's entry count should not change when trying to revoke an already revoked cert"
);

let revoked = match crl.get_by_cert(&already_revoked_cert) {
CrlStatus::Revoked(revoked) => revoked,
_ => panic!("cert should be revoked"),
};
assert_eq!(
revoked.serial_number().to_bn().unwrap(),
already_revoked_cert.serial_number().to_bn().unwrap(),
"revoked and cert serial numbers should match"
);
}

// ensure revoke does correctly add a new revoked cert to the crl
{
let cert = include_bytes!("../../test/cert.pem");
let cert = X509::from_pem(cert).unwrap();

let count_before = crl.entry_count();
crl.revoke(&cert).unwrap();
assert_eq!(
count_before + 1,
crl.entry_count(),
"clr's entry count should have incremented by one after revoking a cert"
);
}
}

#[test]
fn test_crl_entry_extensions() {
let crl = include_bytes!("../../test/entry_extensions.crl");
Expand Down