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 all 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
3 changes: 3 additions & 0 deletions openssl-sys/src/handwritten/x509.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,9 @@ extern "C" {
#[cfg(ossl110)]
pub fn X509_get0_extensions(req: *const X509) -> *const stack_st_X509_EXTENSION;

#[cfg(any(ossl110, libressl281))]
pub fn X509_CRL_get_version(crl: *const X509_CRL) -> c_long;

pub fn X509_CRL_set_version(crl: *mut X509_CRL, version: c_long) -> c_int;
}
const_ptr_api! {
Expand Down
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
247 changes: 244 additions & 3 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 @@ -620,7 +621,7 @@ impl X509Ref {
///
/// Note that `0` return value stands for version 1, `1` for version 2 and so on.
#[corresponds(X509_get_version)]
#[cfg(ossl110)]
#[cfg(any(ossl110, libressl281))]
#[allow(clippy::unnecessary_cast)]
pub fn version(&self) -> i32 {
unsafe { ffi::X509_get_version(self.as_ptr()) as i32 }
Expand Down Expand Up @@ -1670,6 +1671,27 @@ impl X509Revoked {
X509Revoked,
ffi::d2i_X509_REVOKED
}

pub fn new(to_revoke: &X509Ref) -> Result<Self, ErrorStack> {
unsafe { Ok(Self(Self::new_raw(to_revoke)?)) }
}

/// the caller has to ensure the pointer is freed
unsafe fn new_raw(to_revoke: &X509Ref) -> 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,225 @@ impl X509Crl {
X509Crl,
ffi::d2i_X509_CRL
}

#[cfg(any(ossl110, libressl281))]
const X509_VERSION_3: i32 = 2;
#[cfg(any(ossl110, libressl281))]
const X509_CRL_VERSION_2: i32 = 1;

// if not cfg(ossl110) issuer_cert is unused
#[allow(unused_variables)]
pub fn new(issuer_cert: &X509Ref, conf: Option<&ConfRef>) -> Result<Self, ErrorStack> {
unsafe {
let crl = Self(cvt_p(ffi::X509_CRL_new())?);

#[cfg(any(ossl110, libressl281))]
if issuer_cert.version() >= Self::X509_VERSION_3 {
use crate::x509::extension::AuthorityKeyIdentifier;

#[cfg(any(ossl110, libressl251, boringssl))]
{
// "if present, MUST be v2" (source: RFC 5280, page 55)
cvt(ffi::X509_CRL_set_version(
crl.as_ptr(),
Self::X509_CRL_VERSION_2 as c_long,
))?;
}

cvt(ffi::X509_CRL_set_issuer_name(
crl.as_ptr(),
issuer_cert.issuer_name().as_ptr(),
))?;

let context = {
let mut ctx = std::mem::MaybeUninit::<ffi::X509V3_CTX>::zeroed();
ffi::X509V3_set_ctx(
ctx.as_mut_ptr(),
issuer_cert.as_ptr(),
std::ptr::null_mut(),
std::ptr::null_mut(),
crl.as_ptr(),
0,
);
let mut ctx = ctx.assume_init();

if let Some(conf) = conf {
ffi::X509V3_set_nconf(&mut ctx, conf.as_ptr());
}

X509v3Context(ctx, PhantomData)
};

let ext = AuthorityKeyIdentifier::new().keyid(true).build(&context)?;
cvt(ffi::X509_CRL_add_ext(crl.as_ptr(), ext.as_ptr(), -1))?;
}

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)
}
}

/// Note that `0` return value stands for version 1, `1` for version 2.
#[cfg(any(ossl110, libressl281))]
#[corresponds(X509_CRL_get_version)]
pub fn version(&self) -> i32 {
unsafe { ffi::X509_CRL_get_version(self.as_ptr()) as i32 }
}

/// 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()?)),
}
}
}

/// This is an internal function, therefore the caller is expected to ensure not to call this with a CRLv1
/// Set the crl_number extension's value.
/// If the extension is not present, it will be added.
#[cfg(any(ossl110, libressl281))]
fn set_crl_number(&mut self, value: &BigNum) -> Result<(), ErrorStack> {
debug_assert_eq!(self.version(), Self::X509_CRL_VERSION_2);
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(|_| ())
}
}

/// Increment the crl number (or try to add the extension if not present)
///
/// Returns the new crl number, unless self is a crlv1, which does not support extensions
#[cfg(any(ossl110, libressl281))]
pub fn increment_crl_number(&mut self) -> Result<Option<BigNum>, ErrorStack> {
if self.version() == Self::X509_CRL_VERSION_2 {
let new_crl_number = if let Some(mut n) = self.read_crl_number()? {
n.add_word(1)?;
n
} else {
BigNum::from_u32(1)?
};

self.set_crl_number(&new_crl_number)?;

Ok(Some(new_crl_number))
} else {
Ok(None)
}
}

/// 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, None).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