Add CRL extraction logic - #275
Conversation
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #275
Scan targets checked: wolfclu-bugs, wolfclu-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
ad17d95 to
23283ad
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #275
Scan targets checked: wolfclu-bugs, wolfclu-src
No new issues found in the changed files. ✅
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #275
Scan targets checked: wolfclu-bugs, wolfclu-src
No new issues found in the changed files. ✅
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #275
Scan targets checked: wolfclu-bugs, wolfclu-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #275
Scan targets checked: wolfclu-bugs, wolfclu-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #275
Scan targets checked: wolfclu-bugs, wolfclu-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #275
Scan targets checked: wolfclu-bugs, wolfclu-src
Findings: 2
Low (2)
No-op selfSigned guard in wolfCLU_BuildAndSignNative
File: src/x509/clu_cert_setup.c:1969
Function: wolfCLU_BuildAndSignNative
Category: Dead/unreachable code
wolfCLU_X509FillCert is called with caCert=NULL, which already unconditionally sets cert->selfSigned = 1 internally; the !isCSR guarded reassignment has no effect for either CSR or cert generation.
Recommendation: Remove the redundant reassignment, or pass appropriate caCert semantics so the !isCSR condition actually differentiates behavior.
Referenced code: src/x509/clu_cert_setup.c:1969-1971 (3 lines)
No-op selfSigned guard in wolfCLU_CertSignNative
File: src/x509/clu_x509_sign.c:1881
Function: wolfCLU_CertSignNative
Category: Dead/unreachable code
wolfCLU_X509FillCert already sets cert->selfSigned = 1 whenever caCert == x509 (or NULL); this follow-up assignment duplicates that with no observable effect.
Recommendation: Remove the redundant block since wolfCLU_X509FillCert already computes selfSigned from caCert/x509.
Referenced code: src/x509/clu_x509_sign.c:1881-1883 (3 lines)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #275
Scan targets checked: wolfclu-bugs, wolfclu-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| inForm = wolfCLU_checkInform(optarg); | ||
| /* Format is auto-detected; -inform is a compat no-op. */ | ||
| if (optarg != NULL) { | ||
| wolfCLU_convertToLower(optarg, (int)XSTRLEN(optarg)); |
There was a problem hiding this comment.
🔵 [Low] -inform value can alias the cert-to-verify argument and is lowercased in place · Logic errors
-inform is newly registered as required_argument, so optarg can be argv[argc-1], the same slot already captured as verifyCert. Unlike -CAfile and -untrusted, this case has no aliasing check, and wolfCLU_convertToLower rewrites that argv slot, so the certificate path is silently lowercased before load_cert_from_file.
Related known finding #5020 (similar but distinct): Both involve wolfCLU_x509Verify preselecting the final argv element and an option argument aliasing verifyCert, but the faulting operation here lowercases the aliased -inform value in place; issue 5020 aliases -CAfile and verifyCert to enable self-verification. Their root effects and required patches differ.
Fix: Reject optarg == verifyCert for -inform as the -CAfile/-untrusted cases do, and compare case-insensitively instead of mutating argv.
| if (ret == WOLFCLU_SUCCESS && !partialChain) { | ||
| hardErr = WOLFCLU_SUCCESS; | ||
| if (should_skip_non_ca_cert(caX509, legacyCa, &hardErr)) { | ||
| wolfCLU_LogError("CA file does not assert " |
There was a problem hiding this comment.
⚪ [Info] DER fallback reports bundle-oriented and inaccurate basicConstraints diagnostics · Incorrect error handling
In the single-cert DER path, should_skip_non_ca_cert first logs "Skipping CA bundle cert..." although nothing is skipped and the input is not a bundle, and the caller then reports "CA file does not assert basicConstraints CA:TRUE" for a certificate that carries no basicConstraints extension at all.
Fix: Distinguish the absent-extension case from CA:FALSE in the DER path and drop the bundle wording for single-certificate input.
| } | ||
| else { /* PEM_BLOCK_CRL */ | ||
| #ifdef HAVE_CRL | ||
| if (crlCheck) { |
There was a problem hiding this comment.
🔵 [Low] CRL blocks in -CAfile are silently ignored unless -crl_check is passed · Certificate validation bypass
The bundle scanner parses PEM_BLOCK_CRL blocks only when crlCheck is set, so wolfssl verify -CAfile ca-and-crl.pem revoked-cert.pem reports OK despite the bundle carrying that certificate's CRL. The previous wolfSSL_X509_LOOKUP_load_file path enabled WOLFSSL_CRL_CHECK whenever the CA file contained a CRL block, so this silently drops revocation enforcement for callers that omit the flag.
Related known finding #5020 (similar but distinct): Both affect wolfCLU_x509Verify and can produce incorrect certificate-validation success, but the candidate conditionally skips CRL processing due to missing crlCheck, whereas issue 5020 aliases the CA file as the verification target through pre-option argv handling. Their root causes and faulting operations differ, and each requires a separate patch.
Fix: Either load CRL blocks unconditionally and enable CRL checking when any are present, or emit a warning when the bundle contains CRL data but -crl_check was not given.
Introduces fully integrated CRL (Certificate Revocation List) loading and verification support.
Depends on #276