Skip to content

Commit f6dda71

Browse files
SparkiDevSparkiDev
authored
Merge pull request #447 from yosuke-wolfssl/fix/f_2236
Fix ECX/ECC get_params to size export buffer from data_size
2 parents 49c52b9 + 37120c6 commit f6dda71

6 files changed

Lines changed: 187 additions & 3 deletions

File tree

‎src/wp_ecc_kmgmt.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ static int wp_ecc_get_params_enc_pub_key(wp_Ecc* ecc, OSSL_PARAM params[],
747747
p = OSSL_PARAM_locate(params, key);
748748
if (p != NULL) {
749749
int rc;
750-
word32 outLen = (word32)p->return_size;
750+
word32 outLen = (word32)p->data_size;
751751

752752
if (ecc->hasPub == 0) {
753753
ok = 0;

‎src/wp_ecx_kmgmt.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ static int wp_ecx_get_params_enc_pub_key(wp_Ecx* ecx, OSSL_PARAM params[],
547547

548548
p = OSSL_PARAM_locate(params, key);
549549
if (p != NULL) {
550-
word32 outLen = (word32)p->return_size;
550+
word32 outLen = (word32)p->data_size;
551551

552552
if (p->data == NULL) {
553553
outLen = ecx->data->len;
@@ -584,7 +584,7 @@ static int wp_ecx_get_params_priv_key(wp_Ecx* ecx, OSSL_PARAM params[])
584584

585585
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_PRIV_KEY);
586586
if (p != NULL) {
587-
word32 outLen = (word32)p->return_size;
587+
word32 outLen = (word32)p->data_size;
588588

589589
if (p->data == NULL) {
590590
outLen = ecx->data->len;

‎test/test_ecc.c‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,83 @@ int test_eckeygen_p256(void *data)
519519

520520
return err;
521521
}
522+
523+
/* get_params for the ECC encoded public key must size the export buffer from
524+
* data_size, not a stale/zero return_size. Compare a stale-return_size export
525+
* against a known-good fresh export of the same key. */
526+
int test_ecc_p256_get_params_stale_ret(void *data)
527+
{
528+
int err = 0;
529+
EVP_PKEY_CTX *ctx = NULL;
530+
EVP_PKEY *key = NULL;
531+
unsigned char ref[128];
532+
unsigned char readback[128];
533+
size_t refLen = 0;
534+
OSSL_PARAM params[2];
535+
536+
(void)data;
537+
538+
PRINT_MSG("P-256 encoded pub key get_params with stale return_size");
539+
540+
err = (ctx = EVP_PKEY_CTX_new_from_name(wpLibCtx, "EC", NULL)) == NULL;
541+
if (err == 0) {
542+
err = EVP_PKEY_keygen_init(ctx) != 1;
543+
}
544+
if (err == 0) {
545+
err = EVP_PKEY_CTX_set_ec_paramgen_curve_nid(ctx,
546+
NID_X9_62_prime256v1) != 1;
547+
}
548+
if (err == 0) {
549+
err = EVP_PKEY_keygen(ctx, &key) != 1;
550+
}
551+
552+
/* Reference export via a fresh param. Its return_size defaults to
553+
* OSSL_PARAM_UNMODIFIED, so this block succeeds even with the bug; the
554+
* stale-return_size query below is the actual regression check. */
555+
if (err == 0) {
556+
params[0] = OSSL_PARAM_construct_octet_string(
557+
OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY, ref, sizeof(ref));
558+
params[1] = OSSL_PARAM_construct_end();
559+
if (EVP_PKEY_get_params(key, params) != 1) {
560+
PRINT_ERR_MSG("reference encoded pub key get_params failed");
561+
err = 1;
562+
}
563+
else {
564+
refLen = params[0].return_size;
565+
}
566+
}
567+
if (err == 0 && (refLen == 0 || refLen > sizeof(ref))) {
568+
PRINT_ERR_MSG("unexpected encoded pub key length: %zu", refLen);
569+
err = 1;
570+
}
571+
572+
/* Same query with return_size forced to 0 to mimic a reused param. */
573+
if (err == 0) {
574+
memset(readback, 0, sizeof(readback));
575+
params[0] = OSSL_PARAM_construct_octet_string(
576+
OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY, readback, sizeof(readback));
577+
params[0].return_size = 0;
578+
params[1] = OSSL_PARAM_construct_end();
579+
if (EVP_PKEY_get_params(key, params) != 1) {
580+
PRINT_ERR_MSG("encoded pub key get_params failed with stale "
581+
"return_size");
582+
err = 1;
583+
}
584+
}
585+
if (err == 0 && params[0].return_size != refLen) {
586+
PRINT_ERR_MSG("encoded pub key return_size wrong: %zu vs %zu",
587+
params[0].return_size, refLen);
588+
err = 1;
589+
}
590+
if (err == 0 && memcmp(readback, ref, refLen) != 0) {
591+
PRINT_ERR_MSG("encoded pub key bytes not returned correctly");
592+
err = 1;
593+
}
594+
595+
EVP_PKEY_free(key);
596+
EVP_PKEY_CTX_free(ctx);
597+
return err;
598+
}
522599
#endif /* WP_HAVE_EC_P256 */
523600

524601
#ifdef WP_HAVE_EC_P384

‎test/test_ecx.c‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,109 @@ int test_ecx_x25519_raw_priv_roundtrip(void *data)
792792
return err;
793793
}
794794

795+
/* get_params must size the export buffer from the caller's data_size, not
796+
* return_size. A reused param can carry a stale/zero return_size from a prior
797+
* size query; using it would truncate or fail the export. */
798+
int test_ecx_x25519_get_params_stale_ret(void *data)
799+
{
800+
int err = 0;
801+
EVP_PKEY *pkey = NULL;
802+
static const unsigned char privKey[] = {
803+
0x77, 0x07, 0x6d, 0x0a, 0x73, 0x18, 0xa5, 0x7d,
804+
0x3c, 0x16, 0xc1, 0x72, 0x51, 0xb2, 0x66, 0x45,
805+
0xdf, 0x4c, 0x2f, 0x87, 0xeb, 0xc0, 0x99, 0x2a,
806+
0xb1, 0x77, 0xfb, 0xa5, 0x1d, 0xb9, 0x2c, 0x2a
807+
};
808+
/* RFC 7748 Section 6.1 Alice public key, derived from privKey above. */
809+
static const unsigned char pubKey[] = {
810+
0x85, 0x20, 0xf0, 0x09, 0x89, 0x30, 0xa7, 0x54,
811+
0x74, 0x8b, 0x7d, 0xdc, 0xb4, 0x3e, 0xf7, 0x5a,
812+
0x0d, 0xbf, 0x3a, 0x0d, 0x26, 0x38, 0x1a, 0xf4,
813+
0xeb, 0xa4, 0xa9, 0x8e, 0xaa, 0x9b, 0x4e, 0x6a
814+
};
815+
unsigned char readback[32];
816+
OSSL_PARAM params[2];
817+
818+
(void)data;
819+
820+
PRINT_MSG("X25519 get_params with stale (zero) return_size");
821+
822+
pkey = EVP_PKEY_new_raw_private_key_ex(wpLibCtx, "X25519", NULL,
823+
privKey, sizeof(privKey));
824+
if (pkey == NULL) {
825+
PRINT_ERR_MSG("Failed to import X25519 private key");
826+
err = 1;
827+
}
828+
829+
/* Private key: full 32-byte buffer but return_size forced to 0. */
830+
if (err == 0) {
831+
memset(readback, 0, sizeof(readback));
832+
params[0] = OSSL_PARAM_construct_octet_string(
833+
OSSL_PKEY_PARAM_PRIV_KEY, readback, sizeof(readback));
834+
params[0].return_size = 0;
835+
params[1] = OSSL_PARAM_construct_end();
836+
if (EVP_PKEY_get_params(pkey, params) != 1) {
837+
PRINT_ERR_MSG("priv key get_params failed with stale return_size");
838+
err = 1;
839+
}
840+
}
841+
if (err == 0 && params[0].return_size != sizeof(privKey)) {
842+
PRINT_ERR_MSG("priv key return_size wrong: %zu", params[0].return_size);
843+
err = 1;
844+
}
845+
if (err == 0 && memcmp(readback, privKey, sizeof(privKey)) != 0) {
846+
PRINT_ERR_MSG("priv key bytes not returned correctly");
847+
err = 1;
848+
}
849+
850+
/* Public key: same stale-return_size condition on the export path. */
851+
if (err == 0) {
852+
memset(readback, 0, sizeof(readback));
853+
params[0] = OSSL_PARAM_construct_octet_string(
854+
OSSL_PKEY_PARAM_PUB_KEY, readback, sizeof(readback));
855+
params[0].return_size = 0;
856+
params[1] = OSSL_PARAM_construct_end();
857+
if (EVP_PKEY_get_params(pkey, params) != 1) {
858+
PRINT_ERR_MSG("pub key get_params failed with stale return_size");
859+
err = 1;
860+
}
861+
}
862+
if (err == 0 && params[0].return_size != sizeof(pubKey)) {
863+
PRINT_ERR_MSG("pub key return_size wrong: %zu", params[0].return_size);
864+
err = 1;
865+
}
866+
if (err == 0 && memcmp(readback, pubKey, sizeof(pubKey)) != 0) {
867+
PRINT_ERR_MSG("pub key bytes not returned correctly");
868+
err = 1;
869+
}
870+
871+
/* Encoded public key: same helper, exercised via its own key string. */
872+
if (err == 0) {
873+
memset(readback, 0, sizeof(readback));
874+
params[0] = OSSL_PARAM_construct_octet_string(
875+
OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY, readback, sizeof(readback));
876+
params[0].return_size = 0;
877+
params[1] = OSSL_PARAM_construct_end();
878+
if (EVP_PKEY_get_params(pkey, params) != 1) {
879+
PRINT_ERR_MSG("enc pub key get_params failed with stale "
880+
"return_size");
881+
err = 1;
882+
}
883+
}
884+
if (err == 0 && params[0].return_size != sizeof(pubKey)) {
885+
PRINT_ERR_MSG("enc pub key return_size wrong: %zu",
886+
params[0].return_size);
887+
err = 1;
888+
}
889+
if (err == 0 && memcmp(readback, pubKey, sizeof(pubKey)) != 0) {
890+
PRINT_ERR_MSG("enc pub key bytes not returned correctly");
891+
err = 1;
892+
}
893+
894+
EVP_PKEY_free(pkey);
895+
return err;
896+
}
897+
795898
/* A zero-length private key octet string must be rejected without an
796899
* out-of-bounds read (privData[len-1] with len==0). */
797900
int test_ecx_import_zero_priv(void *data)

‎test/unit.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@ TEST_CASE test_case[] = {
416416
#endif
417417
#ifdef WP_HAVE_ECKEYGEN
418418
TEST_DECL(test_eckeygen_p256, NULL),
419+
TEST_DECL(test_ecc_p256_get_params_stale_ret, NULL),
419420
#endif
420421
#ifdef WP_HAVE_ECDH
421422
#ifdef WP_HAVE_ECKEYGEN
@@ -527,6 +528,7 @@ TEST_CASE test_case[] = {
527528
TEST_DECL(test_ecx_null_init, NULL),
528529
#ifdef WP_HAVE_X25519
529530
TEST_DECL(test_ecx_x25519_raw_priv_roundtrip, NULL),
531+
TEST_DECL(test_ecx_x25519_get_params_stale_ret, NULL),
530532
TEST_DECL(test_ecx_import_zero_priv, NULL),
531533
#endif
532534
TEST_DECL(test_ecx_dup, NULL),

‎test/unit.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ int test_eckeygen_p224(void *data);
395395

396396
#ifdef WP_HAVE_EC_P256
397397
int test_eckeygen_p256(void *data);
398+
int test_ecc_p256_get_params_stale_ret(void *data);
398399
#endif /* WP_HAVE_EC_P256 */
399400

400401
#ifdef WP_HAVE_EC_P384
@@ -543,6 +544,7 @@ int test_ecx_misc(void *data);
543544
int test_ecx_null_init(void *data);
544545
#ifdef WP_HAVE_X25519
545546
int test_ecx_x25519_raw_priv_roundtrip(void *data);
547+
int test_ecx_x25519_get_params_stale_ret(void *data);
546548
int test_ecx_import_zero_priv(void *data);
547549
#endif /* WP_HAVE_X25519 */
548550
int test_ecx_dup(void *data);

0 commit comments

Comments
 (0)