Skip to content

Add COSE_Key output options, exact size queries, and CBOR parsing helpers - #66

Open
danielinux wants to merge 14 commits into
wolfSSL:mainfrom
danielinux:webauthn-options
Open

Add COSE_Key output options, exact size queries, and CBOR parsing helpers#66
danielinux wants to merge 14 commits into
wolfSSL:mainfrom
danielinux:webauthn-options

Conversation

@danielinux

Copy link
Copy Markdown
Member

Summary

This PR fils a set of API gaps that show up when wolfCOSE is used from a CTAP2/WebAuthn
or embedded-credential context: publishing a public key without leaking the private
half, sizing a buffer before encoding into it, and parsing maps that wolfCOSE itself
does not define. No existing API changes behaviour; every addition is a new symbol or
an _ex variant whose zero-flags form is the old function.

COSE_Key encoding

  • wc_CoseKey_Encode_ex() adds a flags argument; WOLFCOSE_KEY_PUBLIC_ONLY emits
    the public half only — no -4: d for EC2/OKP, no -3: d or CRT factors for RSA, no
    -2: priv seed for an RFC 9964 AKP key. A symmetric key has no public half, so that
    combination returns WOLFCOSE_E_COSE_KEY_TYPE.

    This matters because wc_CoseKey_Encode() serialises the private key whenever the
    attached key carries one, and an ecc_key attached with wc_CoseKey_SetEcc() has
    hasPrivate set whenever it is a keypair. Encoding "the public key" of a live P-256
    keypair therefore discloses the private scalar, visible only as one extra map entry
    (112 bytes / map(6) instead of 77 / map(5)). The old function is unchanged but
    now documents that; anything that publishes a public key should use the new flag.

  • wc_CoseKey_EncodeSize() / wc_CoseKey_EncodeSize_ex() report the exact encoded
    size without writing anything or exporting key material. The result is exact rather
    than an upper bound, so it can size a buffer or reject an oversized key up front.

  • wc_CoseKey_EncodeEccRaw() encodes an EC2 COSE_Key straight from raw affine
    coordinates, with no ecc_key and none of the point-import cost, for callers that
    hold only the coordinates. It does not validate that the point is on the curve —
    encoding an unvalidated point is safe, using one is not.

COSE_Key decoding

  • wc_CoseKey_PeekInfo() reads kty/alg/crv/kid out of a buffer without
    importing anything, so a parser that accepts more than one key type no longer has to
    guess and retry against wc_CoseKey_Decode()'s required pre-attached key. It applies
    the same structural checks as the decoder, and kid is zero-copy.

CBOR helpers

  • wc_CBOR_EncoderInit() / wc_CBOR_DecoderInit() — static inline, set one
    direction of WOLFCOSE_CBOR_CTX and clear the other, so a context can't be
    half-initialised from the wrong side.
  • wc_CBOR_SkipItem() — like wc_CBOR_Skip(), but also reports the skipped item's
    start and length, zero-copy, which is what deferred or nested parsing needs (a CTAP2
    allowList entry, an embedded COSE_Key). The captured range feeds straight into
    wc_CBOR_DecoderInit() or wc_CoseKey_Decode().
  • wc_CBOR_DecodeLabel() with wc_CBOR_LabelIsInt() / wc_CBOR_LabelIsText()
    RFC 9052 allows label = int / tstr, and real COSE and CTAP2 maps use both spellings
    for the same field (3 vs "alg"). Text comparison is byte-exact, matching how CTAP2
    and COSE compare labels.

Documentation

A new "Strict Decoding" section in docs/Getting-Started.md states that every decode
entry point requires RFC 8949 §4.2.1 preferred serialization and rejects indefinite
lengths, tabulating the rejected forms, their error codes, and the neighbouring
surprises (trailing bytes, exact-size EC2 coordinates, duplicate and text labels). It
closes by explaining why this is not configurable: relaxing it would let a signature or
MAC be recomputed over a re-encoding of the same data. The header carries a matching
note and ChangeLog.md lists the new APIs.

Testing

Eleven new test cases. tests/test_cbor.c covers context init, SkipItem capture and
reparse, and int-or-text labels; tests/test_cose.c covers public-only encoding across
EC2, Ed25519, Ed448, RSA and ML-DSA (asserting the public output is strictly shorter and
re-decodes with no private half, not merely that the size query agrees), exact size
queries per key type, EncodeEccRaw on P-256/P-384/P-521 against the ecc_key path,
the RSA short-d left-padding branch via a fixed key, and PeekInfo including negative
and out-of-int32-range alg.

…g range

- WOLFCOSE_KEY_PUBLIC_ONLY on Ed25519, Ed448, RSA and ML-DSA: the public
  encoding must be shorter than the full one and decode with no private half.
  Comparing only against the size query cannot catch a no-op flag.
- wc_CoseKey_EncodeEccRaw on P-384 and P-521, including rejection of another
  curve's coordinate size.
- RSA d shorter than the modulus, from a fixed key, so the left-padding branch
  and the size/encode agreement that depends on it are exercised.
- wc_CoseKey_PeekInfo with a negative alg, and with alg and crv outside the
  int32 range.
Copilot AI lite review requested due to automatic review settings August 9, 2026 07:40
@danielinux danielinux changed the title Webauthn options Add COSE_Key output options, exact size queries, and CBOR parsing helpers Aug 9, 2026

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.

Pull request overview

This PR adds new COSE_Key and CBOR helper APIs to better support CTAP2/WebAuthn and embedded credential workflows—specifically: public-only key publication, exact encoded-size queries, non-importing COSE_Key metadata inspection, and CBOR parsing conveniences for mixed-label maps and deferred/nested parsing.

Changes:

  • Add wc_CoseKey_Encode_ex() with WOLFCOSE_KEY_PUBLIC_ONLY, plus wc_CoseKey_EncodeSize() / _ex() for exact encoded-size computation.
  • Add wc_CoseKey_EncodeEccRaw() for EC2 COSE_Key encoding directly from raw affine coordinates, and wc_CoseKey_PeekInfo() to read COSE_Key metadata without importing.
  • Add CBOR helpers: wc_CBOR_EncoderInit() / wc_CBOR_DecoderInit(), wc_CBOR_SkipItem(), and wc_CBOR_DecodeLabel() (+ label match helpers), plus documentation and tests for strict decoding expectations.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_cose.c Adds coverage for public-only encoding, exact size queries, ECC-raw encoding parity, RSA short-d padding branch, and PeekInfo behavior/range checks.
tests/test_cbor.c Adds tests for CBOR context initializers, SkipItem capture/reparse, and int-or-text label decoding/matching.
src/wolfcose.c Implements new key encoding options, exact-size query logic with overflow-checked arithmetic, ECC raw encoding helper, and COSE_Key metadata peeking.
src/wolfcose_cbor.c Implements wc_CBOR_SkipItem(), wc_CBOR_DecodeLabel(), and label comparator helpers.
include/wolfcose/wolfcose.h Exposes new APIs/flags, adds inline CBOR ctx initializers, and documents strict decoding + private-key serialization warning.
docs/Getting-Started.md Documents strict decoding requirements and common interop surprises (preferred serialization, no indefinite lengths, trailing-byte rejection, etc.).
docs/API-Reference.md Documents the new APIs and clarifies wc_CoseKey_Encode() private-material serialization behavior and strict decode expectations.
ChangeLog.md Records the newly added APIs and feature additions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@aidangarske aidangarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks amazing!

Comment thread ChangeLog.md

## New Feature Additions

* `wc_CoseKey_Encode_ex()` takes a flags argument, with

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we remove this changelog entry?

@aidangarske aidangarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Skoll Multi-Scan Review

Modes: review + review-securityOverall recommendation: COMMENT
Findings: 6 total — 6 posted, 0 skipped
6 finding(s) posted as inline comments (see file-level comments below)

Posted findings

  • [Low] [review+review-security] RSA EncodeSize can fail on very large RSA keys where Encode succeeds (scratch-buffer fallback)src/wolfcose.c:1509-1550
  • [Low] [review] Exact-size claim depends on key-alg matching the attached key for ML-DSA (and e leading zeros for RSA)src/wolfcose.c:2515-2560
  • [Low] [review] wc_CBOR_DecodeLabel integer-overflow branches are untestedsrc/wolfcose_cbor.c:711-745
  • [Info] [review] Weak/near-tautological assertion in public-only ECC testtests/test_cose.c
  • [Info] [review] Test locates RSA d via WOLFCOSE_KEY_LABEL_Y without explanationtests/test_cose.c
  • [Info] [review-security] wc_CoseKey_Encode_ex leaves ctx.cbuf uninitialized (harmless, inconsistent with sibling)src/wolfcose.c:1689-1691

Review generated by Skoll

Comment thread src/wolfcose.c
* declared MP_API, which wolfSSL exports only when built with
* WOLFSSL_PUBLIC_MP, so calling it here fails to link (undefined reference
* to sp_unsigned_bin_size) against a stock library. */
static int wolfCose_RsaExponentSize(RsaKey* rsa, size_t* eLen)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔵 [Low] RSA EncodeSize can fail on very large RSA keys where Encode succeeds (scratch-buffer fallback) · Logic

In the #if !defined(HAVE_ECC) && !defined(WOLFSSL_EXPORT_INT) fallback branch the exponent is measured with wc_RsaFlattenPublicKey(rsa, eBuf, &len, nBuf, &nLen) where nBuf[WOLFCOSE_MAX_SCRATCH_SZ] (defaults to 512, holding a modulus up to RSA-4096 exactly). For an RSA modulus larger than WOLFCOSE_MAX_SCRATCH_SZ (e.g. RSA-6144/8192 = 768/1024 bytes with the default 512 scratch), wc_RsaFlattenPublicKey returns a buffer error, so wc_CoseKey_EncodeSize()/EncodeSize_ex() returns WOLFCOSE_E_CRYPTO. The encoder path (wc_CoseKey_Encode) flattens n straight into the caller's output buffer (handling moduli up to 65535 bytes), so it succeeds on the same key. The size query and the encoder therefore disagree for very large RSA keys in this narrow no-ECC / no-WOLFSSL_EXPORT_INT build. Only e is needed here, so n should not be size-bounded by scratch. The failure is a clean error return, not memory unsafety; the common build (ECC on, or WOLFSSL_EXPORT_INT) uses only eBuf[8] and is unaffected. Severity: the review mode rated this Low/SUGGEST while review-security rated it Info; the stricter Low is kept.

Fix: Read e without also materializing n in that fallback (e.g. size n from wc_RsaEncryptSize() as the caller already does and export only e), making EncodeSize succeed wherever Encode does. If left as-is, document that the exact-size query supports RSA moduli only up to WOLFCOSE_MAX_SCRATCH_SZ in the no-ECC/no-EXPORT_INT configuration so it stays consistent with the encoder. Low priority given the exotic config and key size.

Comment thread src/wolfcose.c
else
#endif /* WOLFCOSE_HAVE_RSAPSS */
#ifdef WOLFCOSE_HAVE_MLDSA
if (key->kty == WOLFCOSE_KTY_AKP) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔵 [Low] Exact-size claim depends on key-alg matching the attached key for ML-DSA (and e leading zeros for RSA) · API contract

For AKP the size query derives the public-key length from key->alg (1312/1952/2592), whereas the encoder writes the actual wc_MlDsaKey_ExportPubRaw length (src/wolfcose.c:2010). If key->alg disagrees with the attached ML-DSA parameter set, wc_CoseKey_EncodeSize() and wc_CoseKey_Encode() return different lengths, contradicting the documented 'exact, not an upper bound' contract. The RSA e path is similar: EncodeSize strips leading zeros via wolfCose_RsaExponentSize while the encoder emits whatever wc_RsaFlattenPublicKey returns. Neither is a memory-safety issue (an undersized query just yields BUFFER_TOO_SMALL later), but the 'exact' guarantee only holds when alg and key agree.

Fix: Either document that EncodeSize assumes key->alg matches the attached key material, or size the AKP public component from the key object rather than from alg, so 'exact' holds unconditionally.

Comment thread src/wolfcose_cbor.c
ret = WOLFCOSE_E_INVALID_ARG;
}
else {
ret = wolfCose_CBOR_DecodeHead(ctx, &item);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🔵 [Low] wc_CBOR_DecodeLabel integer-overflow branches are untested · Test coverage

wc_CBOR_DecodeLabel returns WOLFCOSE_E_CBOR_OVERFLOW when a uint or negint label argument exceeds INT64_MAX. test_cbor_decode_label covers int/negint/tstr/bstr and NULL cases but never exercises the overflow branches, so a regression in the > (uint64_t)INT64_MAX guards or the -1 - (int64_t)item.val arithmetic would go unnoticed. This is the same overflow contract as wc_CBOR_DecodeInt.

Fix: Extend test_cbor_decode_label with a uint label > INT64_MAX (0x1B FF..FF) and a negint at the INT64_MIN boundary, asserting WOLFCOSE_E_CBOR_OVERFLOW and INT64_MIN respectively.

Comment thread tests/test_cose.c
/* {1,3,-1,-2,-3} = map(5) = 77 bytes */
TEST_ASSERT(pubLen == 77u && pub[0] == 0xA5u,
"pubonly ex encode omits d");
TEST_ASSERT(memcmp(pub, full, 2u) == 0 || pub[0] != full[0],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⚪ [Info] Weak/near-tautological assertion in public-only ECC test · Test quality

The assertion memcmp(pub, full, 2u) == 0 || pub[0] != full[0] is false only in the narrow case where pub[0]==full[0] but pub[1]!=full[1], so it does not meaningfully verify that the public-only map head differs from the full one. The intended property (map(5) 0xA5 vs map(6) 0xA6) is already asserted directly on the lines above, making this line redundant and confusing.

Fix: Replace with a direct check of the two map heads (TEST_ASSERT(pub[0] == 0xA5u && full[0] == 0xA6u, ...)), or drop the redundant assertion.

Comment thread tests/test_cose.c
for (i = 0; (ret == 0) && (i < mapCount); i++) {
ret = wc_CBOR_DecodeInt(&dec, &label);
if (ret == 0) {
if (label == (int64_t)WOLFCOSE_KEY_LABEL_Y) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⚪ [Info] Test locates RSA d via WOLFCOSE_KEY_LABEL_Y without explanation · Style

The RSA short-d test scans for label == WOLFCOSE_KEY_LABEL_Y (-3) to find the RSA private exponent d. This is correct because RFC 8230 places d at label -3, numerically identical to EC2's y label constant, and the encoder reuses WOLFCOSE_KEY_LABEL_Y for RSA d. But reading KEY_LABEL_Y in an RSA test is misleading. A one-line comment (or an aliased WOLFCOSE_KEY_LABEL_RSA_D constant) would make the intent obvious.

Fix: Add a clarifying comment (/* RFC 8230: RSA d shares label -3 with EC2 y */) or introduce an RSA-specific -3 alias used by both encoder and test.

Comment thread src/wolfcose.c
ret = WOLFCOSE_E_INVALID_ARG;
}
else {
ctx.buf = out;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

⚪ [Info] wc_CoseKey_Encode_ex leaves ctx.cbuf uninitialized (harmless, inconsistent with sibling) · Logic

The refactored wc_CoseKey_Encode_ex sets ctx.buf, ctx.bufSz, and ctx.idx but not ctx.cbuf, leaving the const decode pointer indeterminate. This is harmless because every encode path reads only ctx.buf, and it matches the pre-PR pattern of wc_CoseKey_Encode, so it is not a newly introduced defect. Worth noting only because the sibling raw-coordinate encoder added in the same PR (wc_CoseKey_EncodeEccRaw) explicitly sets ctx.cbuf = NULL, and the new wc_CBOR_EncoderInit() helper clears it — so the direct field assignment here is the odd one out.

Fix: Optionally set ctx.cbuf = NULL; here (or use the new wc_CBOR_EncoderInit()) for consistency and defense-in-depth. No functional change.

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.

3 participants