Repository navigation
Conversation
|
Member
Author
|
@wolfSSL-Fenrir-bot review balanced |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Named device keys cannot reach the make-public callbacks because the software private-key flag is checked first.
2 open findings
What changed in this PR
Adds Ed25519/Ed448 device-private-key references and Ed448 CryptoCb support across wolfCrypt and TLS.
Changes:
- Adds ID/label initialization APIs and key metadata.
- Adds Ed448 make-public/check-key callbacks and TLS integration.
- Extends software-device and API/TLS test coverage.
| File | Description |
|---|---|
wolfssl/wolfcrypt/types.h |
Adds Ed448 callback operation types. |
wolfssl/wolfcrypt/ed448.h |
Exposes Ed448 ID/label support. |
wolfssl/wolfcrypt/ed25519.h |
Exposes Ed25519 ID/label support. |
wolfssl/wolfcrypt/cryptocb.h |
Defines Ed448 callback payloads. |
wolfcrypt/test/test.c |
Exercises new Ed448 callbacks. |
wolfcrypt/src/ed448.c |
Implements Ed448 initialization and callback dispatch. |
wolfcrypt/src/ed25519.c |
Implements Ed25519 ID/label initialization. |
wolfcrypt/src/cryptocb.c |
Implements callback routing and private-reference detection. |
tests/swdev/swdev.c |
Adds software Ed448 callback handlers. |
tests/api/test_tls13.h |
Registers device-key TLS tests. |
tests/api/test_tls13.c |
Tests TLS handshakes using device keys. |
tests/api/test_ed448.h |
Registers Ed448 initialization tests. |
tests/api/test_ed448.c |
Tests Ed448 ID/label APIs. |
tests/api/test_ed25519.h |
Registers Ed25519 initialization tests. |
tests/api/test_ed25519.c |
Tests Ed25519 ID/label APIs. |
src/ssl_api_pk.c |
Checks certificate/device-key pairs. |
src/internal.c |
Creates TLS handshake device-key objects. |
doc/dox_comments/header_files/ed448.h |
Documents Ed448 initialization APIs. |
doc/dox_comments/header_files/ed25519.h |
Documents Ed25519 initialization APIs. |
doc/dox_comments/header_files/cryptocb.h |
Documents callback behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11680
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Coverage: 10 of 14 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_ed25519.c, tests/api/test_ed448.c, tests/api/test_tls13.c, wolfssl/wolfcrypt/ed25519.h
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Balanced
padelsbach
reviewed
Oct 8, 2026
| ret = wc_ed25519_init_id(edKey, data, (int)length, heap, devId); | ||
| } | ||
| if (ret == 0) { | ||
| *pkey = (void*)edKey; |
Contributor
There was a problem hiding this comment.
AI says this results in the device being asked to check the keypair on every TLS handshake because Ed25519CheckPubKey calls wc_ed25519_import_public which imports the key as untrusted. Possibly not what we want.
This is a rather indirect code path, so I'm not totally sure. Can you check?
Member
Author
There was a problem hiding this comment.
Good catch, you are right! I fixed that now: for id/label keys the cert public key is now imported as trusted, so the pair is only checked by wolfSSL_CTX_check_private_key, same as RSA/ECC device keys already. This also lets WOLF_CRYPTO_CB_ONLY_ED25519/ED448 builds perform a handshake with a device that doesn't implement CHECK_KEY.
| return EXPECT_RESULT(); | ||
| } | ||
|
|
||
| #if defined(WOLFSSL_TLS13) && defined(WOLF_PRIVATE_KEY_ID) && \ |
Contributor
There was a problem hiding this comment.
Should we test TLS1.2 as well?
Member
Author
There was a problem hiding this comment.
Added TLS1.2 tests now as well.
Ed25519 can hand public key derivation and key validation to a crypto callback device through WC_PK_TYPE_ED25519_MAKE_PUB and WC_PK_TYPE_ED25519_CHECK_KEY. Ed448 only routed sign and verify, so a device holding an Ed448 key could not derive its public key or check it against a public key. Add WC_PK_TYPE_ED448_MAKE_PUB and WC_PK_TYPE_ED448_CHECK_KEY with the same payloads as the Ed25519 ones. wc_ed448_make_public and wc_ed448_check_key now try the device first and fall back to software when the device returns CRYPTOCB_UNAVAILABLE. The software test device in tests/swdev gains Ed448 sign, verify, make_pub and check_key handlers. With --enable-swdev every unbound Ed448 operation is now served by it instead of falling back to software in the callback library. The wolfCrypt crypto callback test counts the new callbacks and fails if key generation or wc_ed448_check_key does not reach the device.
RSA, ECC and the PQC signature algorithms can reference a private key held by a crypto callback device through an id or label (WOLF_PRIVATE_KEY_ID), both in wolfCrypt and through wolfSSL_CTX_use_PrivateKey_Id / _Label. Ed25519 and Ed448 already dispatch sign and verify through crypto callbacks but had no way to name such a key, so a TLS server with an Ed25519 or Ed448 certificate could not use a device key: no key object could be created for it and wolfSSL_CTX_check_private_key failed. Add wc_ed25519_init_id, wc_ed25519_init_label, wc_ed448_init_id and wc_ed448_init_label, with the id and label stored at the end of the key structures and limited to 32 bytes, like ECC. Code outside ed25519.c and ed448.c tests ED25519_MAX_ID_LEN and ED448_MAX_ID_LEN rather than WOLF_PRIVATE_KEY_ID, because FIPS v6 builds use pinned ed25519.h and ed448.h that have neither the fields nor the functions. wc_ed25519_make_public and wc_ed448_make_public no longer reject a key named by id or label before asking the device, so a device can derive the public key of a key it holds. When the device declines, or the key has no device, they still return ECC_PRIV_KEY_E rather than deriving from an empty software key. In the TLS layer, CreateDevPrivateKey and DecodePrivateKey_ex build Ed25519 and Ed448 device keys for ed25519_sa_algo and ed448_sa_algo. check_cert_key_dev imports the certificate's public key into the device key and validates the pair through the Ed25519 or Ed448 check_key callback. It calls the callback directly rather than wc_ed*_check_key, whose software fallback would only validate the public key of a device key, so a device that declines the check fails it. Builds without Ed25519 or Ed448 key import cannot do that, so they keep failing the check as before. A device key named by id or label has no private key in software, so the check_key callbacks previously reported checkPriv as 0 for it and a device would only have validated the public key. checkPriv is now also set when the key carries an id or label. Tests cover the init argument checks, make_public on a device key, and a TLS 1.3 test per curve where a callback device holds the server key: the pair check succeeds for both id and label, fails against a certificate for another key, with an id longer than the key can hold or when the device declines the check, and the handshake is signed by the device after one more pair check when the certificate public key is loaded into the device key.
Ed25519CheckPubKey and Ed448CheckPubKey import the certificate public key into the handshake key when it has none. The import was untrusted, so for a key referenced by id or label every handshake sent CHECK_KEY to the device. Import it as trusted for those keys instead: the pair is checked once by wolfSSL_CTX_check_private_key, as for RSA and ECC device keys. This also lets a WOLF_CRYPTO_CB_ONLY_ED25519/ED448 build complete a handshake with a device that does not implement CHECK_KEY. Add TLS 1.2 handshake tests for Ed25519 and Ed448 device keys to test_tls.c. Both the TLS 1.2 and TLS 1.3 tests now require that the handshake does not reach the device's CHECK_KEY.
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.
Description
RSA, ECC and the PQC signature algorithms can reference a private key held by a crypto callback device through an id or label (
WOLF_PRIVATE_KEY_ID), both in wolfCrypt and throughwolfSSL_CTX_use_PrivateKey_Id/_Label. Ed25519 and Ed448 already route sign and verify through crypto callbacks, but had no way to name such a key. A TLS server with an Ed25519 or Ed448 certificate could not use a device key: no key object could be created for it andwolfSSL_CTX_check_private_keyfailed.Add make_pub and check_key crypto callbacks for Ed448
WC_PK_TYPE_ED448_MAKE_PUBandWC_PK_TYPE_ED448_CHECK_KEY, with the same payloads as the existing Ed25519 ones.wc_ed448_make_publicandwc_ed448_check_keytry the device first and fall back to software onCRYPTOCB_UNAVAILABLE.tests/swdevgains Ed448 sign, verify, make_pub and check_key handlers, so with--enable-swdevevery unbound Ed448 operation is served by it.Support device private keys by id or label for Ed25519 and Ed448
wc_ed25519_init_id,wc_ed25519_init_label,wc_ed448_init_idandwc_ed448_init_label. The id and label are stored at the end of the key structs and limited to 32 bytes, like ECC.CreateDevPrivateKeyandDecodePrivateKey_exbuild Ed25519 and Ed448 device keys.check_cert_key_devimports the certificate public key into the device key and checks the pair through the check_key callback. It calls the callback directly rather thanwc_ed*_check_key, whose software fallback cannot see a device key, so a device that declines the check fails it.checkPrivfor a key named by id or label, not only when a software private key is set.ed25519.canded448.ctestsED25519_MAX_ID_LEN/ED448_MAX_ID_LENinstead ofWOLF_PRIVATE_KEY_ID, because FIPS v6 builds use pinneded25519.handed448.hthat have neither the fields nor the functions.Testing
wolfcrypt/test/test.ccounts the new Ed448 callbacks.testwolfcryptandunit.testwith--enable-all, plus--enable-ed25519 --enable-ed448 --enable-cryptocbwith and withoutWOLF_CRYPTO_CB_FIND; build-only with PK callbacks and no cryptocb, with noWOLF_PRIVATE_KEY_ID, and withNO_ED25519_KEY_IMPORT/NO_ED448_KEY_IMPORT.make checkwith the cryptocb-only CI base config and--enable-swdev(Linux).WCv6.0.0-RC5and cryptocb enabled. This is a simulation without the FIPS bundle; a realfips-check.sh v6.0.0run with cryptocb would confirm it.