scep: advertise cipher capabilities the server can actually honour - #33
Open
yosuke-wolfssl wants to merge 1 commit into
Open
yosuke-wolfssl wants to merge 1 commit into
yosuke-wolfssl wants to merge 1 commit into
Conversation
- scep_server.c defines SCEP_SRV_CIPHER_CAP and SCEP_SRV_STD_CAP alongside SCEP_SRV_ENC_OID: AES plus SCEPStandard under WOLFSSL_AES_128 && HAVE_AES_CBC, and DES3 with no SCEPStandard on the 3DES branch. - handle_get_ca_caps() builds both GetCACaps bodies from those two macros in place of the literal AES and SCEPStandard lines. - test_scep_roundtrip reads the server's rendered capability list back, asserting caps.aes and caps.scep_standard against the same two macros and caps.renewal unconditionally. Issue: F-8023
There was a problem hiding this comment.
🔵 Needs a closer look
The fallback test does not assert that the DES3 capability token is advertised.
Pull request overview
Updates SCEP capability advertisements to match the server’s available encryption cipher.
Changes:
- Aligns AES/3DES capability tokens with compiled cipher support.
- Adds integration assertions for capability negotiation.
File summaries
| File | Summary |
|---|---|
tests/integration/test_scep_roundtrip.c |
Tests advertised cipher capabilities. |
src/scep/scep_server.c |
Synchronizes GetCACaps tokens with the selected cipher. |
Review details
Suppressed comments (1)
tests/integration/test_scep_roundtrip.c:1143
- The fallback branch now advertises a new
DES3token, but this test only asserts thatAESandSCEPStandardare absent. A regression that omits or misspellsDES3would still pass while the server's actual 3DES selection remains undiscoverable to external clients. Please assert the exact fallback token (via a raw GetCACaps response check or a dedicated parsed capability field) when exercising the non-AES build.
#else
REQUIRE(caps.aes == 0);
REQUIRE(caps.scep_standard == 0);
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
requested changes
Sep 17, 2026
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #33
Scan targets checked: wolfcert-src, wolfcert-bugs
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The SCEP test server selects its content-encryption OID at compile time —
AES128CBCbunderWOLFSSL_AES_128 && HAVE_AES_CBC,DES3botherwise — buthandle_get_ca_caps()hardcodedAESinto both GetCACaps bodies. A wolfSSLlacking AES-128 therefore advertised a cipher it cannot decrypt. A compliant
external client follows RFC 8894 §3.5.2, encrypts its
pkcsPKIEnvelopewithAES-128-CBC, and the server cannot open it.
Self-tests could not see this: the in-tree client's
AUTOpath gates on thesame local macros, so it falls back to 3DES regardless of what the server
advertised. Only a third-party client breaks.
Withholding the
AEStoken alone is not sufficient — RFC 8894 §3.5.2 Table 7states
SCEPStandardimpliesAES, so a 3DES-only server still emittingSCEPStandardre-advertises AES by implication.Closes f-8023.
Fix (
src/scep/scep_server.c)SCEP_SRV_CIPHER_CAPandSCEP_SRV_STD_CAPnow sit in the same#ifcascadeas
SCEP_SRV_ENC_OID, so the advertisement cannot drift from the cipher:AES128CBCbAES,SCEPStandardDES3bDES3, noSCEPStandardDES3is Table 7's keyword for the fallback.caps.scep_standardis parsedinto
WolfCertScepCapsbut never consulted, so dropping it breaks no caller.Tests (
tests/integration/test_scep_roundtrip.c)The body is now concatenated from macros, so the test reads the server's
rendered list back:
caps.aesandcaps.scep_standardgated on the same twomacros,
caps.renewalunconditionally. Four token mutations, before → after:AESdroppedAESloses\r\n(fuses toAESRenewal)SCEPStandarddroppedSCEPStandardloses\r\nVerification
-Wall -Wextra -Wshadow -Wpedantic.-DNO_AES_128: server advertisesPOSTPKIOperation / SHA-256 / DES3 / Renewal, and SCEP enrollment completesover 3DES. Reverting the fix, that same build advertises
AES.Not in this PR
--disable-aescbccannot produce one:wolfSSL's own
wolfcrypt/src/pkcs7.ccallswc_AesCbcEncryptunguarded, soNO_AES_CBCwith--enable-pkcs7does not compile.-DNO_AES_128does workand is the recipe for a future config.
parse_negativeandscep_msgfail on aNO_AES_128build — both callwolfcert_scep_envelop(..., AES128CBCb, ...)unguarded. Pre-existing, andwould need guarding before such an arm could land.
DES3keyword. Behaviour is already correct(it defaults to 3DES when
AESis absent), but it cannot distinguish "CA said3DES" from "CA said nothing". Adding it means a new
WolfCertScepCapsfield.