Ed448: check for public key presence on export#10656
Conversation
Return PUBLIC_KEY_E for wc_ed25519_export_key if public key is not present. Return PUBLIC_KEY_E for wc_ed448_export_key if public key is not present. Rename several inLen parameters to outLen for consistency. Fix F-4427
|
| int wc_Ed25519PublicKeyToDer(const ed25519_key* key, byte* output, | ||
| int inLen); | ||
| word32 outLen); |
| ExpectIntEQ(wc_ed448_init(&key), 0); | ||
| ExpectIntEQ(wc_Ed448PublicKeyToDer(&key, derBuf, 0, 0), | ||
| WC_NO_ERR_TRACE(BUFFER_E)); | ||
| WC_NO_ERR_TRACE(PUBLIC_KEY_E)); |
| if ((ret == 0) && (!key->pubKeySet)) { | ||
| ret = PUBLIC_KEY_E; | ||
| } |
dgarske
left a comment
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: COMMENT
Findings: 2 total — 2 posted, 0 skipped
1 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] New PUBLIC_KEY_E return value not documented for wc_ed448_export_public —
wolfcrypt/src/ed448.c:1109-1143, doc/dox_comments/header_files/ed448.h:663-667 - [Low] Public-API semantics change to wc_ed25519_export_key (private-only keys now error) —
wolfcrypt/src/ed25519.c:1523-1537
Review generated by Skoll
| } | ||
|
|
||
| return ret; | ||
| } |
There was a problem hiding this comment.
🔵 [Low] Public-API semantics change to wc_ed25519_export_key (private-only keys now error)
wc_ed25519_export_key() previously deliberately swallowed PUBLIC_KEY_E from the public-part export (if (ret == WC_NO_ERR_TRACE(PUBLIC_KEY_E)) ret = 0; /* ignore no public key */). This PR removes that and now propagates the error, so a caller exporting a key that has only the private part set (e.g. imported via wc_ed25519_import_private_only) will now receive PUBLIC_KEY_E instead of success. This is the intended behavior of the fix (it makes Ed25519 consistent with Ed448) and all in-tree callers are unaffected: the only library callers are in src/pk.c (wc_ed25519_export_key at line 5320, wc_ed448_export_key at 5828), both invoked right after wc_ed*_make_key, which always sets the public key. Flagging only so the public-API behavior change and its blast radius are recorded; the doxygen for wc_ed25519_export_key/wc_ed448_export_key does not mention the PUBLIC_KEY_E return, which downstream/external callers may want documented.
Fix: Intentional and well-tested (new ed25519/ed448 no-pub export tests added); no code change required. Consider noting the new PUBLIC_KEY_E return in the export_key doxygen for completeness.
Description
Ed448: check for public key presence on export
Return PUBLIC_KEY_E for wc_ed25519_export_key if public key is not present.
Return PUBLIC_KEY_E for wc_ed448_export_key if public key is not present.
Rename several inLen parameters to outLen for consistency.
Fix F-4427
Testing
How did you test?
Checklist