diff --git a/src/internal.c b/src/internal.c index 64b16c485..b558ef04b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -18903,6 +18903,9 @@ static int DoServerKeyExchange(WOLFSSL* ssl, const byte* input, curveId = wc_ecc_get_oid(curveOid, NULL, NULL); if (wc_ecc_import_x963_ex(input + args->idx, length, ssl->peerEccKey, curveId) != 0) { + #ifdef WOLFSSL_EXTRA_ALERTS + SendAlert(ssl, alert_fatal, illegal_parameter); + #endif ERROR_OUT(ECC_PEERKEY_ERROR, exit_dske); } @@ -26005,6 +26008,22 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, } } + if ((ret = wc_curve25519_check_public( + input + args->idx, args->length, + EC25519_LITTLE_ENDIAN)) != 0) { + #ifdef WOLFSSL_EXTRA_ALERTS + if (ret == BUFFER_E) + SendAlert(ssl, alert_fatal, decode_error); + else if (ret == ECC_OUT_OF_RANGE_E) + SendAlert(ssl, alert_fatal, bad_record_mac); + else { + SendAlert(ssl, alert_fatal, + illegal_parameter); + } + #endif + ERROR_OUT(ECC_PEERKEY_ERROR, exit_dcke); + } + if (wc_curve25519_import_public_ex( input + args->idx, args->length, ssl->peerX25519Key, diff --git a/src/tls.c b/src/tls.c index 955fe98bc..ce91a672a 100644 --- a/src/tls.c +++ b/src/tls.c @@ -6925,13 +6925,19 @@ static int TLSX_KeyShare_ProcessX25519(WOLFSSL* ssl, WOLFSSL_BUFFER(keyShareEntry->ke, keyShareEntry->keLen); #endif - /* Point is validated by import function. */ - if (wc_curve25519_import_public_ex(keyShareEntry->ke, keyShareEntry->keLen, - peerX25519Key, + if (wc_curve25519_check_public(keyShareEntry->ke, keyShareEntry->keLen, EC25519_LITTLE_ENDIAN) != 0) { ret = ECC_PEERKEY_ERROR; } + if (ret == 0) { + if (wc_curve25519_import_public_ex(keyShareEntry->ke, + keyShareEntry->keLen, peerX25519Key, + EC25519_LITTLE_ENDIAN) != 0) { + ret = ECC_PEERKEY_ERROR; + } + } + if (ret == 0) { ssl->ecdhCurveOID = ECC_X25519_OID; diff --git a/wolfcrypt/src/curve25519.c b/wolfcrypt/src/curve25519.c index 741c55e8b..942ffef80 100644 --- a/wolfcrypt/src/curve25519.c +++ b/wolfcrypt/src/curve25519.c @@ -257,6 +257,64 @@ int wc_curve25519_import_public_ex(const byte* in, word32 inLen, return 0; } +/* Check the public key value (big or little endian) + * + * pub Public key bytes. + * pubSz Size of public key in bytes. + * endian Big-endian or little-endian. + * returns BAD_FUNC_ARGS when pub is NULL, + * BUFFER_E when size of public key is zero; + * ECC_OUT_OF_RANGE_E if the high bit is set; + * ECC_BAD_ARG_E if key length is not 32 bytes, public key value is + * zero or one; and + * 0 otherwise. + */ +int wc_curve25519_check_public(const byte* pub, word32 pubSz, int endian) +{ + word32 i; + + if (pub == NULL) + return BAD_FUNC_ARG; + + /* Check for empty key data */ + if (pubSz == 0) + return BUFFER_E; + + /* Check key length */ + if (pubSz != CURVE25519_KEYSIZE) + return ECC_BAD_ARG_E; + + + if (endian == EC25519_LITTLE_ENDIAN) { + /* Check for value of zero or one */ + for (i = pubSz - 1; i > 0; i--) { + if (pub[i] != 0) + break; + } + if (i == 0 && (pub[0] == 0 || pub[0] == 1)) + return ECC_BAD_ARG_E; + + /* Check high bit set */ + if (pub[CURVE25519_KEYSIZE-1] & 0x80) + return ECC_OUT_OF_RANGE_E; + } + else { + /* Check for value of zero or one */ + for (i = 0; i < pubSz-1; i++) { + if (pub[i] != 0) + break; + } + if (i == pubSz - 1 && (pub[i] == 0 || pub[i] == 1)) + return ECC_BAD_ARG_E; + + /* Check high bit set */ + if (pub[0] & 0x80) + return ECC_OUT_OF_RANGE_E; + } + + return 0; +} + #endif /* HAVE_CURVE25519_KEY_IMPORT */ diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 9223ae7a7..79f5fef6b 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -18718,6 +18718,122 @@ static int curve25519_overflow_test(void) return 0; } + +/* Test the wc_curve25519_check_public API. + * + * returns 0 on success and -ve on failure. + */ +static int curve25519_check_public_test(void) +{ + /* Little-endian values that will fail */ + byte fail_le[][CURVE25519_KEYSIZE] = { + { + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 + }, + { + 0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 + }, + { + 0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x81 + }, + }; + /* Big-endian values that will fail */ + byte fail_be[][CURVE25519_KEYSIZE] = { + { + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 + }, + { + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01 + }, + { + 0x81,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01 + }, + }; + /* Good or valid public value */ + byte good[CURVE25519_KEYSIZE] = { + 0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01 + }; + int i; + + /* Parameter checks */ + /* NULL pointer */ + if (wc_curve25519_check_public(NULL, 0, EC25519_LITTLE_ENDIAN) != + BAD_FUNC_ARG) { + return -10300; + } + if (wc_curve25519_check_public(NULL, 0, EC25519_BIG_ENDIAN) != + BAD_FUNC_ARG) { + return -10301; + } + /* Length of 0 treated differntly to other invalid lengths for TLS */ + if (wc_curve25519_check_public(good, 0, EC25519_LITTLE_ENDIAN) != BUFFER_E) + return -10302; + if (wc_curve25519_check_public(good, 0, EC25519_BIG_ENDIAN) != BUFFER_E) + return -10303; + + /* Length not CURVE25519_KEYSIZE */ + for (i = 1; i < CURVE25519_KEYSIZE + 2; i++) { + if (i == CURVE25519_KEYSIZE) + continue; + if (wc_curve25519_check_public(good, i, EC25519_LITTLE_ENDIAN) != + ECC_BAD_ARG_E) { + return -10310 - i; + } + if (wc_curve25519_check_public(good, i, EC25519_BIG_ENDIAN) != + ECC_BAD_ARG_E) { + return -10350 - i; + } + } + + /* Little-endian fail cases */ + for (i = 0; i < (int)(sizeof(fail_le) / sizeof(fail_le)); i++) { + if (wc_curve25519_check_public(fail_le[i], CURVE25519_KEYSIZE, + EC25519_LITTLE_ENDIAN) == 0) { + return -10390 - i; + } + } + /* Big-endian fail cases */ + for (i = 0; i < (int)(sizeof(fail_be) / sizeof(fail_be)); i++) { + if (wc_curve25519_check_public(fail_be[i], CURVE25519_KEYSIZE, + EC25519_BIG_ENDIAN) == 0) { + return -10394 - i; + } + } + + /* Check a valid public value works! */ + if (wc_curve25519_check_public(good, CURVE25519_KEYSIZE, + EC25519_LITTLE_ENDIAN) != 0) { + return -10398; + } + if (wc_curve25519_check_public(good, CURVE25519_KEYSIZE, + EC25519_BIG_ENDIAN) != 0) { + return -10399; + } + + return 0; +} + #endif /* HAVE_CURVE25519_SHARED_SECRET && HAVE_CURVE25519_KEY_IMPORT */ int curve25519_test(void) @@ -18895,6 +19011,9 @@ int curve25519_test(void) ret = curve25519_overflow_test(); if (ret != 0) return ret; + ret = curve25519_check_public_test(); + if (ret != 0) + return ret; #endif /* HAVE_CURVE25519_SHARED_SECRET && HAVE_CURVE25519_KEY_IMPORT */ /* clean up keys when done */ diff --git a/wolfssl/wolfcrypt/curve25519.h b/wolfssl/wolfcrypt/curve25519.h index 0fb1ea1aa..5f85fd9f4 100644 --- a/wolfssl/wolfcrypt/curve25519.h +++ b/wolfssl/wolfcrypt/curve25519.h @@ -128,6 +128,8 @@ int wc_curve25519_import_public(const byte* in, word32 inLen, WOLFSSL_API int wc_curve25519_import_public_ex(const byte* in, word32 inLen, curve25519_key* key, int endian); +WOLFSSL_API +int wc_curve25519_check_public(const byte* pub, word32 pubSz, int endian); WOLFSSL_API int wc_curve25519_export_public(curve25519_key* key, byte* out, word32* outLen);