x509: add minimal support for CRL building by DavidFontaineOcd · Pull Request #2538 · rust-openssl/rust-openssl

DavidFontaineOcd · 2025-12-11T15:30:23Z

Summary

This pull request introduces initial support for Certificate Revocation List (CRL) functionality. The current implementation of the library exposes many X.509 certificate features, but the APIs related to CRL creation and manipulation are still missing. This PR fills part of that gap by adding the essential types and extensions required to build and manage CRLs programmatically.

Motivation

The OpenSSL C API provides full support for generating, parsing, and extending CRLs. However, rust-openssl currently lacks high-level Rust bindings for these components. As a result, users cannot create CRLs, add revoked certificate entries, or include the standard CRL-related extensions expected in real-world PKI workflows. This PR aims to improve feature parity with OpenSSL and enable broader CRL use cases within Rust applications.

What’s Added

This contribution introduces wrappers and bindings for several key X.509 structures used in CRL creation:

X509RevokedBuilder
AccessDescriptionBuilder
DistributionPointBuilder
DistributionPointNameBuilder

Additionally, the PR adds support for CRL extensions and CRL entry extensions, which are necessary for producing standards-compliant CRLs (e.g., CRL Number, Authority Key Identifier, CRL Reason Codes, Issuing Distribution Point).

Benefits

Enables programmatic creation and customization of CRLs in Rust
Improves parity with the OpenSSL CRL API
Helps developers build complete PKI workflows without relying on external tools

ocdlroux · 2025-12-11T15:43:32Z

It seems the CI overfailed with some HTTP 503 from GitHub, some of the cases need fixing though.

botovq · 2025-12-11T15:47:19Z

necessary for producing standards-compliant CRLs

I'm sorry, but I don't follow. An RFC 5280-compliant CRL only needs the AKI and the crlNumber per 5.2. I think those two should be enough, certainly for a first PR.

ocdlroux · 2025-12-11T16:31:58Z

Hi :)

I'm sorry, but I don't follow. An RFC 5280-compliant CRL only needs the
AKI and the crlNumber per 5.2. I think those two should be enough,
certainly for a first PR.

Indeed. This pull request emerges from the fact that we implemented some CRL-related code in our company codebase out-of-tree, and since the upstream crate missed them and our hierarchy agreed to it, we figured we'd give some of the work back to the community.

This is why we implemented more extensions than strictly necessary. We're happy to split the PR in multiple chunks if you prefer it this way.

About some of the failing CIs because of ASN1_{INTEGER,ENUMERATED}_set-related c_int sizing, we're probably going to use from_bn instead, and probably remove back the ffi for ASN1_INTEGER_new, is that the idiomatic way ?

Don't hesitate if you have any preference on how you want the contributions formatted !
Thanks.

botovq · 2025-12-11T17:01:38Z

Understood. Thanks a lot for upstreaming your changes.

Indeed, splitting this up into smaller chunks will be most welcome. Then we can focus on correctness both on the standardization side and on the implementation side.

As a first step, I would recommend you omit all the things that are not required to be in the tbsCertList by RFC 5280: the builder sets the CRL version to 2 (the number 1) and ensures that nextUpdate is present (and in proper 5280 time format) as well as the CRL number extension (an ASN.1 integer greater than or equal to 0 and less than 2^159) and an Authority Key Identifier extension, and that both are non-critical. For the list of revoked certificates, let's do only userCertificate and revocationDate and make sure that these follow the usual rules. Reason codes can be added later.

And yes, for the CRL number you probably need to make a detour via a bignum and can therefore omit the addition of ASN1_INTEGER_new().

For the reason code, I think you can leave your approach but cast to c_long rather than i64, that's safe since there are only 10 of them, but let's deal with this and other enumerated types once the first PR is in good shape.

ocdlroux · 2025-12-12T09:03:05Z

Thanks for your answer !

the builder sets the CRL version to 2 (the number 1) and ensures that nextUpdate is present (and in proper 5280 time format) as well as the CRL number extension (an ASN.1 integer greater than or equal to 0 and less than 2^159) and an Authority Key Identifier extension, and that both are non-critical.

To be sure, are you recommending we nuke our X509CrlBuilder::set_version and make build fallible and the only method handling ffi calls (like the extension builders) ? This would diverge from the X509Builder design, which doesn't seem to check anything while building. If so, is there some builder pattern you'd like us to mimick from somewhere else in the codebase ?

botovq · 2025-12-12T09:51:49Z

The ecosystem suffers quite a bit from non-compliant certificates and CRLs that overly flexible tools like the openssl command line let non-experts create.

New code issuing certificates and CRLs should strive to be as standards-compliant as reasonably possible. I do not think it makes sense at all to issue a CRL that isn't version 2 in the present day and age, so I don't see the need for a set_version() in the builder. (I'm not convinced it made sense to make the certificate version customizable a decade ago - it should always have been 3, but we can't fix that now)

I wold have created a build() that fails if one of the two required extensions is missing one way or the other. Moving the ffi calls there makes sense, but these could also stay in the builder components setting the extensions on the X509 and having a check in build() to make sure the extensions were set.

ocdlroux · 2025-12-15T13:49:20Z

Hi again,

We're reworking the CrlBuilder from what you stated, and:

We were wondering, if the two required extensions aren't present, should we assert! with an explanation message (and with adequate documentation) to let the developer know that this shouldn't be used without those extensions or do we just return an Err ?
Since the builder is going to contain only rust members until the build should it take the pkey & digest to construct the Crl and sign it in one go ?

Thanks.

Edit: If we do need to construct errors, what is the idiomatic way doing that also ? e.g. for the 2^159 check for CrlNumber ?

DavidFontaineOcd · 2026-01-08T13:55:14Z

Hi,

We saw that support for version 1.0.2 was removed, so we’ve updated the branch accordingly.
Feel free to reach out about any other concerns.

Thanks (and happy new year by the way 🎉 ) !

botovq · 2026-01-08T13:59:19Z

Great, thank you. Apologies for the silence - it's been a crazy few weeks. I'll endeavour to give you feedback soon.

botovq

I think we're getting quite close. I few small nits and one suggestion regarding empty list of revoked.

botovq · 2026-01-13T08:30:00Z

+            number >= BigNum::from_u32(0)? && number < max,
+            "CrlNumber must have n ASN.1 integer greater than or equal to 0 and less than 2^159"


Suggested change

number >= BigNum::from_u32(0)? && number < max,

"CrlNumber must have n ASN.1 integer greater than or equal to 0 and less than 2^159"

!number.is_negative() && number < max,

"CrlNumber must be an ASN.1 integer greater than or equal to 0 and less than 2^159"

botovq · 2026-01-13T08:52:50Z

+        }
+    }
+
+    /// Set the lastUpdate time indicating when the CRL was issued.


lastUpdate is an OpenSSL-ism. The standard name is thisUpdate

Suggested change

/// Set the lastUpdate time indicating when the CRL was issued.

/// Set the lastUpdate (thisUpdate) time indicating when the CRL was issued.

botovq · 2026-01-13T08:54:09Z

+        unsafe { cvt(ffi::X509_CRL_set1_nextUpdate(self.0.as_ptr(), t.as_ptr())).map(|_| ()) }
+    }
+
+    /// Adds an X509 extension value to the CRL.


Suggested change

/// Adds an X509 extension value to the CRL.

/// Add an X509 extension value to the CRL.

botovq · 2026-01-13T08:54:28Z

+        self.append_extension2(&extension)
+    }
+
+    /// Adds an X509 extension value to the CRL.


Suggested change

/// Adds an X509 extension value to the CRL.

/// Add an X509 extension value to the CRL.

botovq · 2026-01-18T12:06:18Z

+    }
+
+    /// Consumes the builder, returning the `X509Revoked`.
+    pub fn build(self) -> X509Revoked {


I wonder if this should not refuse building an empty list of revoked certificates per RFC 5280, 5.1.2.6, "When there are no revoked certificates, the revoked certificates list MUST be absent."

DavidFontaineOcd · 2026-01-19T12:46:41Z

Thank you for your review.

I applied your suggestion, and added an other assert in the CrlBuilder regarding the empty revoked list.
I hope this matches what you had in mind.

botovq

One last thing for you to fix. Then I believe we're done as far as code is concerned.

Before we can land this we will have to fix the min-version test either by pinning some crate (random?) or bumping the MSRV. I'll ask you to rebase once more once that's done.

botovq · 2026-01-22T09:54:12Z

+                revoked.is_some_and(|r| !r.is_empty()),
+                "Revoked should not be empty or null"


Suggested change

revoked.is_some_and(|r| !r.is_empty()),

"Revoked should not be empty or null"

// XXX - switch to is_none_or() once MSRV is 1.82.

revoked.is_none() || revoked.is_some_and(|r| !r.is_empty()),

"Revoked must be absent or non-empty"

If we had MSRV 1.82, we could write this as revoked.is_none_or(|r| !r.is_empty()), but since we still have 1.70, this needs to be spelled out explicitly.

The reason is that the revokedCertificates element is OPTIONAL:

revokedCertificates SEQUENCE OF SEQUENCE { userCertificate CertificateSerialNumber, revocationDate Time, crlEntryExtensions Extensions OPTIONAL -- if present, version MUST be v2 } OPTIONAL,

per RFC 5280, section 5.1.2.6, which states: "When there are no revoked certificates, the revoked certificates list MUST be absent.", so the None case is valid.

DavidFontaineOcd · 2026-02-19T09:28:31Z

Hi,

Sorry it took me a very long time to rebase the last changes.

Everything seems OK here.
Let me know if there’s anything I can do.

botovq

Thanks for reviving this - I could have sworn I had merged this months ago... I reread the code and I am happy to merge once the CI is green.

If you wish to replace the asserts with the new ErrorStack::internal_error() you could do that as a follow-up.

DavidFontaineOcd force-pushed the feat/crl-full branch 5 times, most recently from 19dda81 to 82ec4d4 Compare December 16, 2025 10:48

DavidFontaineOcd force-pushed the feat/crl-full branch from 82ec4d4 to d4190a7 Compare December 24, 2025 08:02

DavidFontaineOcd force-pushed the feat/crl-full branch from d4190a7 to 867f577 Compare January 5, 2026 09:36

DavidFontaineOcd changed the title ~~Feat/crl full~~ x509: add minimal support for CRL building Jan 8, 2026

botovq reviewed Jan 18, 2026

View reviewed changes

DavidFontaineOcd force-pushed the feat/crl-full branch 4 times, most recently from a9ac7ac to 796cee4 Compare January 19, 2026 10:28

botovq reviewed Jan 22, 2026

View reviewed changes

DavidFontaineOcd force-pushed the feat/crl-full branch from 796cee4 to e6d615a Compare January 22, 2026 14:44

DavidFontaineOcd force-pushed the feat/crl-full branch from e6d615a to d12e0d7 Compare February 19, 2026 08:39

DavidFontaineOcd force-pushed the feat/crl-full branch from d12e0d7 to ca9620a Compare June 4, 2026 08:03

x509: adding minimal support for X509CrlBuilder

4dae20b

DavidFontaineOcd force-pushed the feat/crl-full branch from ca9620a to 4dae20b Compare June 4, 2026 08:16

botovq approved these changes Jun 4, 2026

View reviewed changes

botovq merged commit 9fac317 into rust-openssl:master Jun 4, 2026
173 of 174 checks passed

reaperhulk mentioned this pull request Jun 12, 2026

Release openssl 0.10.81 and openssl-sys 0.9.117 #2655

Merged

		number >= BigNum::from_u32(0)? && number < max,
		"CrlNumber must have n ASN.1 integer greater than or equal to 0 and less than 2^159"

	/// Set the lastUpdate time indicating when the CRL was issued.
	/// Set the lastUpdate (thisUpdate) time indicating when the CRL was issued.

	/// Adds an X509 extension value to the CRL.
	/// Add an X509 extension value to the CRL.

		revoked.is_some_and(\|r\| !r.is_empty()),
		"Revoked should not be empty or null"

-                revoked.is_some_and(|r| !r.is_empty()),
-                "Revoked should not be empty or null"
+                // XXX - switch to is_none_or() once MSRV is 1.82.
+                revoked.is_none() || revoked.is_some_and(|r| !r.is_empty()),
+                "Revoked must be absent or non-empty"

Conversation

DavidFontaineOcd commented Dec 11, 2025

Uh oh!

ocdlroux commented Dec 11, 2025

Uh oh!

botovq commented Dec 11, 2025

Uh oh!

ocdlroux commented Dec 11, 2025

Uh oh!

botovq commented Dec 11, 2025

Uh oh!

ocdlroux commented Dec 12, 2025

Uh oh!

botovq commented Dec 12, 2025

Uh oh!

ocdlroux commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DavidFontaineOcd commented Jan 8, 2026

Uh oh!

botovq commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

botovq left a comment

Choose a reason for hiding this comment

Uh oh!

botovq Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

botovq Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

botovq Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

botovq Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

botovq Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

DavidFontaineOcd commented Jan 19, 2026

Uh oh!

botovq left a comment

Choose a reason for hiding this comment

Uh oh!

botovq Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

DavidFontaineOcd commented Feb 19, 2026

Uh oh!

botovq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ocdlroux commented Dec 15, 2025 •

edited

Loading

botovq commented Jan 8, 2026 •

edited

Loading