Add CRL generation capability by padelsbach · Pull Request #315 · wolfSSL/wolfssljni

padelsbach · 2026-01-20T17:50:00Z

Add wrappers for CRL generation logic. Requires wolfSSL/wolfssl#9631 to be merged.

Android and JNI CI failures are fixed with #326. Please merge that first.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 15 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.github/workflows/linux-common.yml

.github/workflows/infer.yml

native/com_wolfssl_WolfSSLCRL.c

src/java/com/wolfssl/WolfSSLCRL.java

.github/workflows/windows-vs.yml

Copilot · 2026-01-21T21:29:26Z

.github/workflows/address-sanitizer.yml

+          # Point to PR branch until merge
+          # TODO: Switch back to wolfssl/wolfssl after merge
+          repository: padelsbach/wolfssl
+          ref: crl-generation


The workflow is pointing to a temporary repository (padelsbach/wolfssl) and branch (crl-generation) for testing. The TODO comment indicates this should be changed back to wolfSSL/wolfssl after merge. This is acceptable for development but must be reverted before merging this PR to ensure CI uses the official repository.

Suggested change

# Point to PR branch until merge

# TODO: Switch back to wolfssl/wolfssl after merge

repository: padelsbach/wolfssl

ref: crl-generation

repository: wolfSSL/wolfssl

.github/workflows/stable-wolfssl-releases.yml

Copilot · 2026-01-21T21:29:27Z

.github/workflows/scan-build.yml

+          # Point to PR branch until merge
+          # TODO: Switch back to wolfssl/wolfssl after merge
+          repository: padelsbach/wolfssl
+          ref: crl-generation


The workflow is pointing to a temporary repository (padelsbach/wolfssl) and branch (crl-generation) for testing. The TODO comment indicates this should be changed back to wolfSSL/wolfssl after merge. This is acceptable for development but must be reverted before merging this PR to ensure CI uses the official repository.

Suggested change

# Point to PR branch until merge

# TODO: Switch back to wolfssl/wolfssl after merge

repository: padelsbach/wolfssl

ref: crl-generation

# Use official wolfSSL repository for CI builds

repository: wolfSSL/wolfssl

.github/workflows/maven.yml

padelsbach · 2026-01-22T19:48:33Z

The FIPS failures are due to the use of the main wolfSSL repo and are expected to be fixed once the CRL PR is merged on the wolfSSL repo.

I will also revert the yamls at that point.

cconlon · 2026-01-22T23:54:16Z

native/com_wolfssl_WolfSSLCRL.c

+    timeBuf = (byte*)(*jenv)->GetByteArrayElements(jenv, time, NULL);
+    timeSz = (*jenv)->GetArrayLength(jenv, time);
+    if (timeBuf == NULL || 
+            timeSz < (com_wolfssl_WolfSSLCRL_CTC_DATE_SIZE + 8)) {


Could we add a small comment saying what 8 represents here?

cconlon · 2026-01-22T23:56:38Z

native/com_wolfssl_WolfSSLCRL.c

+    }
+    else {
+        /* Extract length from bytes 32-35 (assuming native byte order) */
+        timeLen = *((int*)(timeBuf + com_wolfssl_WolfSSLCRL_CTC_DATE_SIZE));


I was pondering if we should use the native CTC_DATE_SIZE here rather than the JNI-defined equivalent (com_wolfssl_WolfSSLCRL_CTC_DATE_SIZE)? If it were me I probably would use CTC_DATE_SIZE since we are calling down to native wolfSSL from this layer, and it's shorter to read.

Comment applies across all usages of that define.

cconlon · 2026-01-22T23:58:18Z

native/com_wolfssl_WolfSSLCRL.c

+    timeBuf = (byte*)(*jenv)->GetByteArrayElements(jenv, time, NULL);
+    timeSz = (*jenv)->GetArrayLength(jenv, time);
+    if (timeBuf == NULL || 
+            timeSz < (com_wolfssl_WolfSSLCRL_CTC_DATE_SIZE + 8)) {


Same comment as above, a small comment about what 8 represents would be nice.

cconlon · 2026-01-23T00:00:06Z

native/com_wolfssl_WolfSSLCRL.c

+        ret = WOLFSSL_FAILURE;
+    }
+    else {
+        ret = WOLFSSL_SUCCESS;


Would it be better to initialize ret = WOLFSSL_SUCCESS at variable declaration, then just worry about setting ret = WOLFSSL_FAILURE when needed?

cconlon · 2026-01-23T00:02:33Z

native/com_wolfssl_WolfSSLCRL.c

+        }
+    }
+
+    (*jenv)->ReleaseByteArrayElements(jenv, time, (jbyte*)timeBuf, JNI_ABORT);


Should this be wrapped in a if (timeBuf != NULL), in case GetByteArrayElements() failed up above? Same comment applies to different functions where ReleaseByteArrayElements() is called as well.

That pattern exists in places in this repo, but the large majority of calls to ReleaseByteArrayElements() don't perform the null check

cconlon · 2026-01-23T00:07:57Z

native/com_wolfssl_WolfSSLCRL.c

+
+    /* Set correct WOLFSSL_EVP_MD, does not need to be freed */
+    if (ret == WOLFSSL_FAILURE) {
+        /* skip if already failed */


This seems a little odd, at least different than how most of our native wolfSSL code looks:

if (ret == WOLFSSL_FAILURE) { /* skip if already failed */ }

In wolfSSL I think you would typically see a pattern like the following instead of the if(skip)/else:

if (ret == WOLFSSL_SUCCESS) { if (digestAlg != NULL) { ... } }

Or:

if ((ret == WOLFSSL_SUCCESS) && (digestAlg != NULL)) { ... }

cconlon · 2026-01-23T00:11:05Z

native/com_wolfssl_WolfSSLCRL.c

+        /* already in DER */
+        derBuf = keyBuf;
+        derSz = keySz;
+        ret = WOLFSSL_SUCCESS;


Hasn't ret already been initialized to WOLFSSL_SUCCESS at this point, unless it has been overridden by WOLFSSL_FAILURE (but then we shouldn't get to this point).

cconlon · 2026-01-23T00:13:42Z

native/com_wolfssl_WolfSSLCRL.c

+    defined(WOLFSSL_CERT_GEN) && !defined(NO_ASN_TIME)
+    WOLFSSL_X509_CRL* crl = (WOLFSSL_X509_CRL*)(uintptr_t)crlPtr;
+    WOLFSSL_ASN1_TIME* date = NULL;
+    char ret[32];


A comment about why 32 is used here would be good, or maybe use of MAX_TIME_STRING_SZ instead?

Ok. I'll update a couple similar methods in com_wolfssl_WolfSSLCertificate.c which follow this pattern

cconlon · 2026-01-23T00:15:16Z

native/com_wolfssl_WolfSSLCRL.c

+    defined(WOLFSSL_CERT_GEN) && !defined(NO_ASN_TIME)
+    WOLFSSL_X509_CRL* crl = (WOLFSSL_X509_CRL*)(uintptr_t)crlPtr;
+    WOLFSSL_ASN1_TIME* date = NULL;
+    char ret[32];


Same as above, for possible use of MAX_TIME_STRING_SZ

cconlon · 2026-01-23T00:22:11Z

src/java/com/wolfssl/WolfSSLCRL.java

+                    WolfSSLDebug.INFO, this.crlPtr,
+                    () -> "entered getEncoded(format: " + format + ")");
+
+                ret = write_X509_CRL(this.crlPtr, tempPath.toString(), format);


Would it be easier just to add a native wolfSSL API that returns back a byte array of the CRL? That seems easier and less error prone to me than making a temporary file, especially if this is running in some sort of restricted Java environment where file creation isn't allowed (maybe Android or such).

padelsbach force-pushed the crl-generation branch from d84fe0d to 8e8480d Compare January 20, 2026 21:28

cconlon requested a review from Copilot January 20, 2026 21:31

Copilot AI reviewed Jan 20, 2026

View reviewed changes

Copilot started reviewing on behalf of cconlon January 20, 2026 21:57 View session

padelsbach force-pushed the crl-generation branch 5 times, most recently from a016758 to 3676f25 Compare January 21, 2026 20:32

cconlon requested a review from Copilot January 21, 2026 21:24

Copilot started reviewing on behalf of cconlon January 21, 2026 21:24 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

padelsbach force-pushed the crl-generation branch 7 times, most recently from 8b9532c to 8af03a8 Compare January 22, 2026 19:07

padelsbach marked this pull request as ready for review January 22, 2026 19:48

padelsbach requested a review from cconlon January 22, 2026 20:51

padelsbach assigned cconlon Jan 22, 2026

cconlon requested changes Jan 23, 2026

View reviewed changes

cconlon assigned padelsbach and unassigned cconlon Jan 23, 2026

padelsbach force-pushed the crl-generation branch 4 times, most recently from e3c6435 to dbfe657 Compare January 28, 2026 20:44

padelsbach force-pushed the crl-generation branch from 59d9989 to 42c47d6 Compare January 30, 2026 17:59

padelsbach requested a review from cconlon February 3, 2026 18:34

padelsbach removed their assignment Feb 4, 2026

cconlon self-assigned this Feb 5, 2026

padelsbach added 2 commits February 5, 2026 12:47

Add CRL generation capability

734d745

Add WOLFSSL_PR9631_PATCH_APPLIED guard

08d007d

padelsbach force-pushed the crl-generation branch from 42c47d6 to 67d1fb0 Compare February 5, 2026 20:50

padelsbach mentioned this pull request Feb 6, 2026

Crl generation ci #327

Closed

padelsbach force-pushed the crl-generation branch from 67d1fb0 to 0a1308b Compare February 6, 2026 23:04

Add missing API, misc updates

56fa7d0

padelsbach force-pushed the crl-generation branch from 0a1308b to 56fa7d0 Compare February 7, 2026 18:32

Conversation

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

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

padelsbach commented Jan 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

padelsbach commented Jan 20, 2026 •

edited

Loading