Parse RFC 5958 PKCS#8 (OneAsymmetricKey) publicKey trailer in ToTraditional_ex()#10483
Parse RFC 5958 PKCS#8 (OneAsymmetricKey) publicKey trailer in ToTraditional_ex()#10483cconlon wants to merge 1 commit into
Conversation
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10483
Scan targets checked: wolfcrypt-bugs, wolfcrypt-src
No new issues found in the changed files. ✅
There was a problem hiding this comment.
Pull request overview
This PR teaches ToTraditionalInline_ex2() (template ASN parser) to accept RFC 5958 OneAsymmetricKey PKCS#8 buffers carrying an optional [1] publicKey trailer, while enforcing that the trailer is only allowed when version == v1. It also extends the algorithm-parameter validation switch to assert "no AlgorithmIdentifier parameters" for several post-quantum and stateful-hash signature OIDs, and adjusts wc_CreatePKCS8Key() to skip the new trailing template slot. The change closes a gap reported by wolfJCE when calling ToTraditional_ex() on ML-DSA key bundles.
Changes:
- Add
[1] publicKeyslot topkcs8KeyASNtemplate and enforce v0-must-not-have-publicKey rule. - Add explicit "no parameters in AlgorithmIdentifier" checks for Falcon, ML-DSA / Dilithium-draft, SLH-DSA, LMS, XMSS.
- Add unit tests covering hand-crafted v0/v1 buffers, encoder→parser roundtrips, and negative cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| wolfcrypt/src/asn.c | Extends pkcs8KeyASN template; enforces RFC 5958 version/publicKey policy; adds parameter-validation arms for PQ/stateful-hash OIDs; updates wc_CreatePKCS8Key to skip the extra slot. |
| tests/api/test_asn.h | Declares the four new test functions and registers them in TEST_ASN_DECLS. |
| tests/api/test_asn.c | Adds handcrafted, roundtrip, negative, and ML-DSA bad-params tests for ToTraditional_ex(). |
Comments suppressed due to low confidence (2)
wolfcrypt/src/asn.c:8999
- The seven new switch-case blocks (Falcon, Dilithium/ML-DSA, SLH-DSA, LMS, XMSS) all perform the exact same parameter check as the existing Ed25519/Ed448/X25519/X448/DH cases. Consider grouping these case labels with fall-through under a single shared check (e.g., having all "no NULL, no OBJECT_ID parameter" OIDs cascade into one body). The current copy/paste duplication makes future modifications (e.g., adding a new "no-params" algorithm) error-prone since every block must be kept in sync.
tests/api/test_asn.c:1773 - The bad-params tests for ML-DSA only exercise the ML-DSA-65 OID. The new parameter-rejection logic was added for many other OIDs as well (Falcon, ML-DSA-44/87, SLH-DSA variants, LMS, XMSS, plus the legacy DILITHIUM_LEVEL2/3/5 draft OIDs), and none of them are exercised here. Consider adding at least one bad-params case per newly added switch arm — or parameterize this test by OID — so a regression that re-removes one of those checks is caught.
int test_ToTraditional_ex_mldsa_bad_params(void)
{
EXPECT_DECLS;
#if defined(HAVE_PKCS8) && defined(HAVE_DILITHIUM) && \
defined(WOLFSSL_ASN_TEMPLATE)
/* ML-DSA-65 OID body: 2.16.840.1.101.3.4.3.18 */
static const byte mldsaOid[] = { 0x60, 0x86, 0x48, 0x01, 0x65, 0x03,
0x04, 0x03, 0x12 };
/* Single-arc OID body, used only to occupy the OBJECT_ID slot. */
static const byte extraOid[] = { 0x01 };
byte der[64];
byte copy[64];
word32 sz;
word32 outerLenIdx;
word32 algId;
const word32 privKeySz = 4;
const byte privBody = 0xAA;
/* Bad case, algoSeq = { OID, NULL } */
sz = 0;
der[sz++] = ASN_SEQUENCE | ASN_CONSTRUCTED;
outerLenIdx = sz;
der[sz++] = 0; /* outer length, filled in below */
der[sz++] = ASN_INTEGER;
der[sz++] = 1;
der[sz++] = 0x00;
der[sz++] = ASN_SEQUENCE | ASN_CONSTRUCTED;
der[sz++] = (byte)(sizeof(mldsaOid) + 2 + 2);
der[sz++] = ASN_OBJECT_ID;
der[sz++] = sizeof(mldsaOid);
XMEMCPY(der + sz, mldsaOid, sizeof(mldsaOid)); sz += sizeof(mldsaOid);
/* Disallowed, NULL parameter after the ML-DSA OID. */
der[sz++] = ASN_TAG_NULL;
der[sz++] = 0;
der[sz++] = ASN_OCTET_STRING;
der[sz++] = (byte)(privKeySz + 2);
der[sz++] = ASN_OCTET_STRING;
der[sz++] = (byte)privKeySz;
XMEMSET(der + sz, privBody, privKeySz); sz += privKeySz;
der[outerLenIdx] = (byte)(sz - outerLenIdx - 1);
XMEMCPY(copy, der, sz);
algId = 0;
ExpectIntLT(ToTraditional_ex(copy, sz, &algId), 0);
/* Bad case, algoSeq = { OID, OBJECT_ID } */
sz = 0;
der[sz++] = ASN_SEQUENCE | ASN_CONSTRUCTED;
outerLenIdx = sz;
der[sz++] = 0;
der[sz++] = ASN_INTEGER;
der[sz++] = 1;
der[sz++] = 0x00;
der[sz++] = ASN_SEQUENCE | ASN_CONSTRUCTED;
der[sz++] = (byte)(sizeof(mldsaOid) + 2 + sizeof(extraOid) + 2);
der[sz++] = ASN_OBJECT_ID;
der[sz++] = sizeof(mldsaOid);
XMEMCPY(der + sz, mldsaOid, sizeof(mldsaOid)); sz += sizeof(mldsaOid);
/* Disallowed, OBJECT_ID parameter after the ML-DSA OID. */
der[sz++] = ASN_OBJECT_ID;
der[sz++] = sizeof(extraOid);
XMEMCPY(der + sz, extraOid, sizeof(extraOid)); sz += sizeof(extraOid);
der[sz++] = ASN_OCTET_STRING;
der[sz++] = (byte)(privKeySz + 2);
der[sz++] = ASN_OCTET_STRING;
der[sz++] = (byte)privKeySz;
XMEMSET(der + sz, privBody, privKeySz); sz += privKeySz;
der[outerLenIdx] = (byte)(sz - outerLenIdx - 1);
XMEMCPY(copy, der, sz);
algId = 0;
ExpectIntLT(ToTraditional_ex(copy, sz, &algId), 0);
#endif
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (copy[1] < 0x80) { | ||
| copy[1] = (byte)(copy[1] + 1); | ||
| copy[derSz] = 0x05; | ||
| algId = 0; | ||
| ExpectIntLT(ToTraditional_ex(copy, (word32)(derSz + 1), &algId), 0); | ||
| } | ||
| } | ||
|
|
||
| /* publicKey trailer is permitted only when version == v1 */ | ||
| if (EXPECT_SUCCESS() && (derSz > 0) && | ||
| ((size_t)(derSz + 2 + ED25519_PUB_KEY_SIZE) <= sizeof(copy))) { | ||
| word32 trailerSz = 2 + ED25519_PUB_KEY_SIZE; | ||
| XMEMCPY(copy, der, (size_t)derSz); | ||
| if (copy[1] + trailerSz < 0x80) { | ||
| copy[1] = (byte)(copy[1] + trailerSz); | ||
| copy[derSz] = ASN_CONTEXT_SPECIFIC | ASN_ASYMKEY_PUBKEY; | ||
| copy[derSz + 1] = ED25519_PUB_KEY_SIZE; | ||
| XMEMSET(copy + derSz + 2, 0xDD, ED25519_PUB_KEY_SIZE); | ||
| algId = 0; | ||
| ExpectIntLT(ToTraditional_ex(copy, | ||
| (word32)(derSz + (int)trailerSz), &algId), 0); | ||
| } | ||
| } | ||
|
|
||
| /* v1 buffer (with publicKey) plus extra trailing garbage. */ | ||
| ExpectIntGT(derSz = wc_Ed25519KeyToDer(&key, der, sizeof(der)), 0); | ||
| if (EXPECT_SUCCESS() && (derSz > 0) && | ||
| ((size_t)(derSz + 1) <= sizeof(copy))) { | ||
| XMEMCPY(copy, der, (size_t)derSz); | ||
| if (copy[1] < 0x80) { | ||
| copy[1] = (byte)(copy[1] + 1); | ||
| copy[derSz] = 0x05; | ||
| algId = 0; | ||
| ExpectIntLT(ToTraditional_ex(copy, (word32)(derSz + 1), &algId), 0); | ||
| } | ||
| } |
| /* Only v0 and v1 (RFC 5958) are supported. The [1] publicKey | ||
| * trailer is permitted only when version == v1. */ | ||
| if (version > PKCS8v1) { | ||
| ret = ASN_PARSE_E; | ||
| } | ||
| else if ((version < PKCS8v1) && | ||
| (dataASN[PKCS8KEYASN_IDX_PKEY_PUBKEY].tag != 0)) { | ||
| ret = ASN_PARSE_E; | ||
| } |
|
Description
This PR adds support for parsing RFC 5958 PKCS#8 (
OneAsymmetricKey) buffers that bundle a[1] publicKeytrailer.pkcs8KeyASNtemplate with the optional[1] publicKeyitem.publicKeypermitted only whenversion == v1.wc_CreatePKCS8Key()to skip the new trailing template slot.Gap filled for wolfJCE calling
ToTraditional_ex()for ML-DSA key bundle to extract algo ID.Testing
New tests (
tests/api/test_asn.c):-
test_ToTraditional_ex_handcrafted- hand crafted v0, v1+pubKey, v1 no-pubKey Ed25519 PKCS#8 buffers.-
test_ToTraditional_ex_roundtrip- encoder parser round-trip for Ed25519, Ed448, and ML-DSA 44/65/87 in both v0 and v1.-
test_ToTraditional_ex_negative- rejects v0+pubKey, v0+trailing-garbage, and v1+trailing-garbage.-
test_ToTraditional_ex_mldsa_bad_params- rejects ML-DSAAlgorithmIdentifiercarrying trailingNULLorOBJECT_IDparameters.--enable-asn=template(default) and--enable-asn=original.Checklist