Skip to content

Commit c8f6421

Browse files
lealem47lealem47
committed
Address peer review comments
1 parent 8e26329 commit c8f6421

4 files changed

Lines changed: 141 additions & 114 deletions

File tree

‎src/pk.c‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5501,6 +5501,50 @@ int wolfSSL_ED25519_verify(const unsigned char *msg, unsigned int msgSz,
55015501

55025502
#endif /* OPENSSL_EXTRA && HAVE_ED25519 */
55035503

5504+
#if (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) && \
5505+
defined(HAVE_ED25519)
5506+
/* Allocate and initialize a new ed25519_key.
5507+
*
5508+
* @param [in] heap Heap hint for memory allocation.
5509+
* @param [in] devId Device identifier for crypto callbacks.
5510+
* @return Allocated and initialized ed25519_key on success.
5511+
* @return NULL on failure.
5512+
*/
5513+
ed25519_key* wolfSSL_ED25519_new(void* heap, int devId)
5514+
{
5515+
ed25519_key* key;
5516+
5517+
WOLFSSL_ENTER("wolfSSL_ED25519_new");
5518+
5519+
key = (ed25519_key*)XMALLOC(sizeof(ed25519_key), heap,
5520+
DYNAMIC_TYPE_ED25519);
5521+
if (key == NULL) {
5522+
WOLFSSL_ERROR_MSG("wolfSSL_ED25519_new malloc failure");
5523+
}
5524+
else if (wc_ed25519_init_ex(key, heap, devId) != 0) {
5525+
WOLFSSL_ERROR_MSG("wolfSSL_ED25519_new init failure");
5526+
XFREE(key, heap, DYNAMIC_TYPE_ED25519);
5527+
key = NULL;
5528+
}
5529+
5530+
return key;
5531+
}
5532+
5533+
/* Free an ed25519_key allocated with wolfSSL_ED25519_new.
5534+
*
5535+
* @param [in] key ed25519_key to free. May be NULL.
5536+
*/
5537+
void wolfSSL_ED25519_free(ed25519_key* key)
5538+
{
5539+
if (key != NULL) {
5540+
void* heap = key->heap;
5541+
WOLFSSL_ENTER("wolfSSL_ED25519_free");
5542+
wc_ed25519_free(key);
5543+
XFREE(key, heap, DYNAMIC_TYPE_ED25519);
5544+
}
5545+
}
5546+
#endif /* (OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL) && HAVE_ED25519 */
5547+
55045548
/*******************************************************************************
55055549
* END OF ED25519 API
55065550
******************************************************************************/
@@ -5954,6 +5998,49 @@ int wolfSSL_ED448_verify(const unsigned char *msg, unsigned int msgSz,
59545998
}
59555999
#endif /* OPENSSL_EXTRA && HAVE_ED448 */
59566000

6001+
#if (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) && \
6002+
defined(HAVE_ED448)
6003+
/* Allocate and initialize a new ed448_key.
6004+
*
6005+
* @param [in] heap Heap hint for memory allocation.
6006+
* @param [in] devId Device identifier for crypto callbacks.
6007+
* @return Allocated and initialized ed448_key on success.
6008+
* @return NULL on failure.
6009+
*/
6010+
ed448_key* wolfSSL_ED448_new(void* heap, int devId)
6011+
{
6012+
ed448_key* key;
6013+
6014+
WOLFSSL_ENTER("wolfSSL_ED448_new");
6015+
6016+
key = (ed448_key*)XMALLOC(sizeof(ed448_key), heap, DYNAMIC_TYPE_ED448);
6017+
if (key == NULL) {
6018+
WOLFSSL_ERROR_MSG("wolfSSL_ED448_new malloc failure");
6019+
}
6020+
else if (wc_ed448_init_ex(key, heap, devId) != 0) {
6021+
WOLFSSL_ERROR_MSG("wolfSSL_ED448_new init failure");
6022+
XFREE(key, heap, DYNAMIC_TYPE_ED448);
6023+
key = NULL;
6024+
}
6025+
6026+
return key;
6027+
}
6028+
6029+
/* Free an ed448_key allocated with wolfSSL_ED448_new.
6030+
*
6031+
* @param [in] key ed448_key to free. May be NULL.
6032+
*/
6033+
void wolfSSL_ED448_free(ed448_key* key)
6034+
{
6035+
if (key != NULL) {
6036+
void* heap = key->heap;
6037+
WOLFSSL_ENTER("wolfSSL_ED448_free");
6038+
wc_ed448_free(key);
6039+
XFREE(key, heap, DYNAMIC_TYPE_ED448);
6040+
}
6041+
}
6042+
#endif /* (OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL) && HAVE_ED448 */
6043+
59576044
/*******************************************************************************
59586045
* END OF ED448 API
59596046
******************************************************************************/

‎wolfcrypt/src/evp_pk.c‎

Lines changed: 40 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -257,15 +257,10 @@ static int d2iTryEd25519Key(WOLFSSL_EVP_PKEY** out, const unsigned char* mem,
257257
heap = (*out)->heap;
258258
}
259259

260-
edKey = (ed25519_key*)XMALLOC(sizeof(ed25519_key), heap,
261-
DYNAMIC_TYPE_ED25519);
260+
edKey = wolfSSL_ED25519_new(heap, INVALID_DEVID);
262261
if (edKey == NULL) {
263262
return 0;
264263
}
265-
if (wc_ed25519_init_ex(edKey, heap, INVALID_DEVID) != 0) {
266-
XFREE(edKey, heap, DYNAMIC_TYPE_ED25519);
267-
return 0;
268-
}
269264

270265
/* Try decoding data as an Ed25519 private/public key. The input may be
271266
* either a DER-encoded SubjectPublicKeyInfo / PKCS#8 PrivateKeyInfo
@@ -293,22 +288,23 @@ static int d2iTryEd25519Key(WOLFSSL_EVP_PKEY** out, const unsigned char* mem,
293288
}
294289

295290
if (!isEdKey) {
296-
wc_ed25519_free(edKey);
297-
XFREE(edKey, heap, DYNAMIC_TYPE_ED25519);
291+
wolfSSL_ED25519_free(edKey);
298292
return WOLFSSL_FATAL_ERROR;
299293
}
300294

301295
/* Create an EVP PKEY object holding the input bytes. These are the
302296
* SPKI / PKCS#8 DER on the structured path, or the raw 32-byte key
303-
* on the raw fallback path. */
304-
ret = d2i_make_pkey(out, mem, keyIdx, priv, WC_EVP_PKEY_ED25519);
297+
* on the raw fallback path. If the caller already populated the EVP
298+
* PKEY with the input bytes (pkey.ptr set), skip the allocate/copy. */
299+
if (*out == NULL || (*out)->pkey.ptr == NULL) {
300+
ret = d2i_make_pkey(out, mem, keyIdx, priv, WC_EVP_PKEY_ED25519);
301+
}
305302
if (ret == 1) {
306303
(*out)->ownEd25519 = 1;
307304
(*out)->ed25519 = edKey;
308305
}
309306
else {
310-
wc_ed25519_free(edKey);
311-
XFREE(edKey, heap, DYNAMIC_TYPE_ED25519);
307+
wolfSSL_ED25519_free(edKey);
312308
}
313309

314310
return ret;
@@ -341,14 +337,10 @@ static int d2iTryEd448Key(WOLFSSL_EVP_PKEY** out, const unsigned char* mem,
341337
heap = (*out)->heap;
342338
}
343339

344-
edKey = (ed448_key*)XMALLOC(sizeof(ed448_key), heap, DYNAMIC_TYPE_ED448);
340+
edKey = wolfSSL_ED448_new(heap, INVALID_DEVID);
345341
if (edKey == NULL) {
346342
return 0;
347343
}
348-
if (wc_ed448_init_ex(edKey, heap, INVALID_DEVID) != 0) {
349-
XFREE(edKey, heap, DYNAMIC_TYPE_ED448);
350-
return 0;
351-
}
352344

353345
/* Try decoding data as an Ed448 private/public key. The input may be
354346
* either a DER-encoded SubjectPublicKeyInfo / PKCS#8 PrivateKeyInfo
@@ -376,22 +368,23 @@ static int d2iTryEd448Key(WOLFSSL_EVP_PKEY** out, const unsigned char* mem,
376368
}
377369

378370
if (!isEdKey) {
379-
wc_ed448_free(edKey);
380-
XFREE(edKey, heap, DYNAMIC_TYPE_ED448);
371+
wolfSSL_ED448_free(edKey);
381372
return WOLFSSL_FATAL_ERROR;
382373
}
383374

384375
/* Create an EVP PKEY object holding the input bytes. These are the
385376
* SPKI / PKCS#8 DER on the structured path, or the raw 57-byte key
386-
* on the raw fallback path. */
387-
ret = d2i_make_pkey(out, mem, keyIdx, priv, WC_EVP_PKEY_ED448);
377+
* on the raw fallback path. If the caller already populated the EVP
378+
* PKEY with the input bytes (pkey.ptr set), skip the allocate/copy. */
379+
if (*out == NULL || (*out)->pkey.ptr == NULL) {
380+
ret = d2i_make_pkey(out, mem, keyIdx, priv, WC_EVP_PKEY_ED448);
381+
}
388382
if (ret == 1) {
389383
(*out)->ownEd448 = 1;
390384
(*out)->ed448 = edKey;
391385
}
392386
else {
393-
wc_ed448_free(edKey);
394-
XFREE(edKey, heap, DYNAMIC_TYPE_ED448);
387+
wolfSSL_ED448_free(edKey);
395388
}
396389

397390
return ret;
@@ -556,7 +549,7 @@ static int d2iTryDhKey(WOLFSSL_EVP_PKEY** out, const unsigned char* mem,
556549
static int d2iTryAltDhKey(WOLFSSL_EVP_PKEY** out, const unsigned char* mem,
557550
long memSz, int priv)
558551
{
559-
WOLFSSL_DH* dhObj;
552+
WOLFSSL_DH* dhObj = NULL;
560553
word32 keyIdx = 0;
561554
DhKey* key = NULL;
562555
int elements;
@@ -565,22 +558,25 @@ static int d2iTryAltDhKey(WOLFSSL_EVP_PKEY** out, const unsigned char* mem,
565558
/* Create DH key object from data. */
566559
dhObj = wolfSSL_DH_new();
567560
if (dhObj == NULL) {
568-
return 0;
561+
ret = 0;
569562
}
570563

571-
key = (DhKey*)dhObj->internal;
572-
/* Try decoding data as a DH public key. */
573-
if (wc_DhKeyDecode(mem, &keyIdx, key, (word32)memSz) != 0) {
574-
wolfSSL_DH_free(dhObj);
575-
return WOLFSSL_FATAL_ERROR;
576-
}
577-
/* DH key has data and is external to DH object. */
578-
elements = ELEMENT_P | ELEMENT_G | ELEMENT_Q | ELEMENT_PUB;
579-
if (priv) {
580-
elements |= ELEMENT_PRV;
564+
if (ret == 1) {
565+
key = (DhKey*)dhObj->internal;
566+
/* Try decoding data as a DH public key. */
567+
if (wc_DhKeyDecode(mem, &keyIdx, key, (word32)memSz) != 0) {
568+
ret = WOLFSSL_FATAL_ERROR;
569+
}
581570
}
582-
if (SetDhExternal_ex(dhObj, elements) != WOLFSSL_SUCCESS ) {
583-
ret = 0;
571+
if (ret == 1) {
572+
/* DH key has data and is external to DH object. */
573+
elements = ELEMENT_P | ELEMENT_G | ELEMENT_Q | ELEMENT_PUB;
574+
if (priv) {
575+
elements |= ELEMENT_PRV;
576+
}
577+
if (SetDhExternal_ex(dhObj, elements) != WOLFSSL_SUCCESS) {
578+
ret = 0;
579+
}
584580
}
585581
if (ret == 1) {
586582
/* Create an EVP PKEY object. */
@@ -591,7 +587,7 @@ static int d2iTryAltDhKey(WOLFSSL_EVP_PKEY** out, const unsigned char* mem,
591587
(*out)->ownDh = 1;
592588
(*out)->dh = dhObj;
593589
}
594-
if (ret == 0) {
590+
else if (dhObj != NULL) {
595591
wolfSSL_DH_free(dhObj);
596592
}
597593

@@ -1210,93 +1206,23 @@ static WOLFSSL_EVP_PKEY* d2i_evp_pkey(int type, WOLFSSL_EVP_PKEY** out,
12101206
#endif /* WOLFSSL_QT || OPENSSL_ALL || WOLFSSL_OPENSSH */
12111207
#ifdef HAVE_ED25519
12121208
case WC_EVP_PKEY_ED25519:
1213-
{
1214-
ed25519_key* edKey;
1215-
word32 keyIdx = 0;
1216-
int isEdKey;
1217-
1218-
edKey = (ed25519_key*)XMALLOC(sizeof(ed25519_key), local->heap,
1219-
DYNAMIC_TYPE_ED25519);
1220-
if (edKey == NULL) {
1209+
/* local->pkey.ptr already holds the input bytes, so
1210+
* d2iTryEd25519Key will skip the d2i_make_pkey allocate/copy
1211+
* and just decode into local->ed25519. */
1212+
if (d2iTryEd25519Key(&local, p, local->pkey_sz, priv) != 1) {
12211213
wolfSSL_EVP_PKEY_free(local);
12221214
return NULL;
12231215
}
1224-
if (wc_ed25519_init_ex(edKey, local->heap, INVALID_DEVID) != 0) {
1225-
XFREE(edKey, local->heap, DYNAMIC_TYPE_ED25519);
1226-
wolfSSL_EVP_PKEY_free(local);
1227-
return NULL;
1228-
}
1229-
if (priv) {
1230-
isEdKey = (wc_Ed25519PrivateKeyDecode(p, &keyIdx, edKey,
1231-
(word32)local->pkey_sz) == 0);
1232-
if (!isEdKey && local->pkey_sz == ED25519_KEY_SIZE) {
1233-
isEdKey = (wc_ed25519_import_private_only(p,
1234-
(word32)local->pkey_sz, edKey) == 0);
1235-
}
1236-
}
1237-
else {
1238-
isEdKey = (wc_Ed25519PublicKeyDecode(p, &keyIdx, edKey,
1239-
(word32)local->pkey_sz) == 0);
1240-
if (!isEdKey && local->pkey_sz == ED25519_PUB_KEY_SIZE) {
1241-
isEdKey = (wc_ed25519_import_public(p,
1242-
(word32)local->pkey_sz, edKey) == 0);
1243-
}
1244-
}
1245-
if (!isEdKey) {
1246-
wc_ed25519_free(edKey);
1247-
XFREE(edKey, local->heap, DYNAMIC_TYPE_ED25519);
1248-
wolfSSL_EVP_PKEY_free(local);
1249-
return NULL;
1250-
}
1251-
local->ownEd25519 = 1;
1252-
local->ed25519 = edKey;
12531216
break;
1254-
}
12551217
#endif /* HAVE_ED25519 */
12561218
#ifdef HAVE_ED448
12571219
case WC_EVP_PKEY_ED448:
1258-
{
1259-
ed448_key* edKey;
1260-
word32 keyIdx = 0;
1261-
int isEdKey;
1262-
1263-
edKey = (ed448_key*)XMALLOC(sizeof(ed448_key), local->heap,
1264-
DYNAMIC_TYPE_ED448);
1265-
if (edKey == NULL) {
1220+
/* See WC_EVP_PKEY_ED25519 case above. */
1221+
if (d2iTryEd448Key(&local, p, local->pkey_sz, priv) != 1) {
12661222
wolfSSL_EVP_PKEY_free(local);
12671223
return NULL;
12681224
}
1269-
if (wc_ed448_init_ex(edKey, local->heap, INVALID_DEVID) != 0) {
1270-
XFREE(edKey, local->heap, DYNAMIC_TYPE_ED448);
1271-
wolfSSL_EVP_PKEY_free(local);
1272-
return NULL;
1273-
}
1274-
if (priv) {
1275-
isEdKey = (wc_Ed448PrivateKeyDecode(p, &keyIdx, edKey,
1276-
(word32)local->pkey_sz) == 0);
1277-
if (!isEdKey && local->pkey_sz == ED448_KEY_SIZE) {
1278-
isEdKey = (wc_ed448_import_private_only(p,
1279-
(word32)local->pkey_sz, edKey) == 0);
1280-
}
1281-
}
1282-
else {
1283-
isEdKey = (wc_Ed448PublicKeyDecode(p, &keyIdx, edKey,
1284-
(word32)local->pkey_sz) == 0);
1285-
if (!isEdKey && local->pkey_sz == ED448_PUB_KEY_SIZE) {
1286-
isEdKey = (wc_ed448_import_public(p,
1287-
(word32)local->pkey_sz, edKey) == 0);
1288-
}
1289-
}
1290-
if (!isEdKey) {
1291-
wc_ed448_free(edKey);
1292-
XFREE(edKey, local->heap, DYNAMIC_TYPE_ED448);
1293-
wolfSSL_EVP_PKEY_free(local);
1294-
return NULL;
1295-
}
1296-
local->ownEd448 = 1;
1297-
local->ed448 = edKey;
12981225
break;
1299-
}
13001226
#endif /* HAVE_ED448 */
13011227
default:
13021228
WOLFSSL_MSG("Unsupported key type");

‎wolfssl/wolfcrypt/ed25519.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,13 @@ WOLFSSL_API
188188
int wc_ed25519_delete(ed25519_key* key, ed25519_key** key_p);
189189
#endif
190190

191+
#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)
192+
WOLFSSL_API
193+
ed25519_key* wolfSSL_ED25519_new(void* heap, int devId);
194+
WOLFSSL_API
195+
void wolfSSL_ED25519_free(ed25519_key* key);
196+
#endif
197+
191198
#ifdef HAVE_ED25519_KEY_IMPORT
192199
WOLFSSL_API
193200
int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key);

‎wolfssl/wolfcrypt/ed448.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,13 @@ int wc_ed448_init(ed448_key* key);
164164
WOLFSSL_API
165165
void wc_ed448_free(ed448_key* key);
166166

167+
#if defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)
168+
WOLFSSL_API
169+
ed448_key* wolfSSL_ED448_new(void* heap, int devId);
170+
WOLFSSL_API
171+
void wolfSSL_ED448_free(ed448_key* key);
172+
#endif
173+
167174
#ifdef HAVE_ED448_KEY_IMPORT
168175
WOLFSSL_API
169176
int wc_ed448_import_public(const byte* in, word32 inLen, ed448_key* key);

0 commit comments

Comments
 (0)