Skip to content

scep: advertise cipher capabilities the server can actually honour - #33

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:mainfrom
yosuke-wolfssl:fix/f_8023
Open

yosuke-wolfssl wants to merge 1 commit into
wolfSSL:mainfrom
yosuke-wolfssl:fix/f_8023

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Problem

The SCEP test server selects its content-encryption OID at compile time —
AES128CBCb under WOLFSSL_AES_128 && HAVE_AES_CBC, DES3b otherwise — but
handle_get_ca_caps() hardcoded AES into both GetCACaps bodies. A wolfSSL
lacking AES-128 therefore advertised a cipher it cannot decrypt. A compliant
external client follows RFC 8894 §3.5.2, encrypts its pkcsPKIEnvelope with
AES-128-CBC, and the server cannot open it.

Self-tests could not see this: the in-tree client's AUTO path gates on the
same local macros, so it falls back to 3DES regardless of what the server
advertised. Only a third-party client breaks.

Withholding the AES token alone is not sufficient — RFC 8894 §3.5.2 Table 7
states SCEPStandard implies AES, so a 3DES-only server still emitting
SCEPStandard re-advertises AES by implication.

Closes f-8023.

Fix (src/scep/scep_server.c)

SCEP_SRV_CIPHER_CAP and SCEP_SRV_STD_CAP now sit in the same #if cascade
as SCEP_SRV_ENC_OID, so the advertisement cannot drift from the cipher:

Build Cipher OID GetCACaps tokens
AES-128-CBC available AES128CBCb AES, SCEPStandard
3DES fallback DES3b DES3, no SCEPStandard

DES3 is Table 7's keyword for the fallback. caps.scep_standard is parsed
into WolfCertScepCaps but 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.aes and caps.scep_standard gated on the same two
macros, caps.renewal unconditionally. Four token mutations, before → after:

Mutation Before After
AES dropped passes fails
AES loses \r\n (fuses to AESRenewal) passes fails
SCEPStandard dropped passes fails
SCEPStandard loses \r\n fails fails

Verification

  • Default build: 29/29, clean under -Wall -Wextra -Wshadow -Wpedantic.
  • Against a wolfSSL built -DNO_AES_128: server advertises
    POSTPKIOperation / SHA-256 / DES3 / Renewal, and SCEP enrollment completes
    over 3DES. Reverting the fix, that same build advertises AES.
  • Build-system parity and the freestanding ARM compile are clean.

Not in this PR

  • No CI arm builds the 3DES branch. --disable-aescbc cannot produce one:
    wolfSSL's own wolfcrypt/src/pkcs7.c calls wc_AesCbcEncrypt unguarded, so
    NO_AES_CBC with --enable-pkcs7 does not compile. -DNO_AES_128 does work
    and is the recipe for a future config.
  • parse_negative and scep_msg fail on a NO_AES_128 build — both call
    wolfcert_scep_envelop(..., AES128CBCb, ...) unguarded. Pre-existing, and
    would need guarding before such an arm could land.
  • The client does not parse the DES3 keyword. Behaviour is already correct
    (it defaults to 3DES when AES is absent), but it cannot distinguish "CA said
    3DES" from "CA said nothing". Adding it means a new WolfCertScepCaps field.

- 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
@yosuke-wolfssl yosuke-wolfssl self-assigned this Sep 17, 2026
Copilot AI lite review requested due to automatic review settings September 17, 2026 05:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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 DES3 token, but this test only asserts that AES and SCEPStandard are absent. A regression that omits or misspells DES3 would 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 wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread tests/integration/test_scep_roundtrip.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants