(initial) ML-DSA support #84
Conversation
| @@ -2072,4 +2072,492 @@ int wh_Client_CmacCancelableResponse(whClientContext* c, Cmac* cmac, | |||
| return ret; | |||
| } | |||
| #endif /* WOLFSSL_CMAC */ | |||
|
|
|||
| #ifdef HAVE_DILITHIUM | |||
There was a problem hiding this comment.
Yuck. This should be spelled HAVE_MLDSA or HAVE_FIPS204. No change. Just complaining
| return WH_ERROR_BADARGS; | ||
| } | ||
|
|
||
| *outId = (whKeyId)((intptr_t)key->devCtx); |
There was a problem hiding this comment.
| *outId = (whKeyId)((intptr_t)key->devCtx); | |
| *outId = WH_DEVCTX_TO_KEYID(key->devCtx); |
| return WH_ERROR_BADARGS; | ||
| } | ||
|
|
||
| key->devCtx = (void*)((intptr_t)keyId); |
There was a problem hiding this comment.
| key->devCtx = (void*)((intptr_t)keyId); | |
| key->devCtx = WH_KEYID_TO_DEVCTX(keyId); |
| return ret; | ||
| } | ||
|
|
||
| static int _MlDsaMakeKey(whClientContext* ctx, int size, int level, |
There was a problem hiding this comment.
Can you add a declaration of this at the top? I swear we were getting complaints about this in the HighTec compiler
| } | ||
|
|
||
| int wh_Client_MlDsaMakeExportKey(whClientContext* ctx, int level, MlDsaKey* key, | ||
| int size, WC_RNG* rng) |
There was a problem hiding this comment.
RNG should be removed. The server will use its own rng.
There was a problem hiding this comment.
fixed in #86. Not sure what I was thinking here...
| int ret = CRYPTOCB_UNAVAILABLE; | ||
|
|
||
| /* Extract info parameters */ | ||
| WC_RNG* rng = info->pk.pqc_sig_kg.rng; |
There was a problem hiding this comment.
No need for client-side rng
| word32 in_len = info->pk.pqc_sign.inlen; | ||
| byte* out = info->pk.pqc_sign.out; | ||
| word32* out_len = info->pk.pqc_sign.outlen; | ||
| WC_RNG* rng = info->pk.pqc_sign.rng; |
There was a problem hiding this comment.
No client-side rng needed
| @@ -108,6 +108,11 @@ int wh_CommClient_SendRequest(whCommClient* context, uint16_t magic, | |||
| return WH_ERROR_BADARGS; | |||
| } | |||
|
|
|||
| /* Check if the data size is within allowed limits */ | |||
| if (data_size > WOLFHSM_CFG_COMM_DATA_LEN) { | |||
There was a problem hiding this comment.
How did we miss this?? I assume the lower-level does a sanity check but we should have caught it here. Nice!
| whNvmMetadata* cacheMeta; | ||
| uint16_t der_size; | ||
|
|
||
| const uint16_t MAX_MLDSA_DER_SIZE = 5000; |
There was a problem hiding this comment.
Isn't this defined somewhere?
|
|
||
| #define WOLFHSM_CFG_NVM_OBJECT_COUNT 30 | ||
| #define WOLFHSM_CFG_SERVER_KEYCACHE_COUNT 9 | ||
| #define WOLFHSM_CFG_SERVER_KEYCACHE_BUFSIZE 300 | ||
| #define WOLFHSM_CFG_SERVER_KEYCACHE_BIG_COUNT 2 | ||
| #define WOLFHSM_CFG_SERVER_KEYCACHE_BIG_BUFSIZE 1280 | ||
| #define WOLFHSM_CFG_SERVER_KEYCACHE_BIG_BUFSIZE WOLFHSM_CFG_COMM_DATA_LEN |
There was a problem hiding this comment.
This is a clever change. :)
Adds initial support for ML-DSA to wolfHSM. Note that this requires wolfSSL/wolfssl#8177 for wolfCrypt "blind deserialization" of ML-DSA DER keys. CI tests will fail until this merges. You can manually test against this branch if desired.
This PR does not include DMA support, which will be required (due to large key/signature sizes) for this to be reasonable to use on real hardware. I'm working on that part now, but wanted initial support to be merged in first.
This PR also fixes a few bugs in the transport mem and comm context layers, where improperly sized buffers could result in buffer overflows.