From 2c943282f061a02363c32e0fe804192ee19d91e2 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Fri, 1 Jul 2022 12:21:16 +1000 Subject: [PATCH 1/4] Ed25519/Ed448: assume public key is not trusted In defense against attack, assume the imported public key is not trusted and check it matches the private key if set. Added APIs that allow application to explicitly trust public key. Original APIs default to not trusting public key. --- doc/dox_comments/header_files/ed25519.h | 106 +++++++++++++++++++++++- doc/dox_comments/header_files/ed448.h | 100 +++++++++++++++++++++- tests/api.c | 18 ++-- wolfcrypt/src/ecc.c | 2 +- wolfcrypt/src/ed25519.c | 74 +++++++++++++---- wolfcrypt/src/ed448.c | 77 +++++++++++++---- wolfcrypt/test/test.c | 4 +- wolfssl/wolfcrypt/ed25519.h | 7 ++ wolfssl/wolfcrypt/ed448.h | 7 ++ 9 files changed, 350 insertions(+), 45 deletions(-) diff --git a/doc/dox_comments/header_files/ed25519.h b/doc/dox_comments/header_files/ed25519.h index 167765c2d..83f302c3d 100644 --- a/doc/dox_comments/header_files/ed25519.h +++ b/doc/dox_comments/header_files/ed25519.h @@ -564,7 +564,8 @@ void wc_ed25519_free(ed25519_key* key); \brief This function imports a public ed25519_key pair from a buffer containing the public key. This function will handle both compressed and - uncompressed keys. + uncompressed keys. The public key is checked that it matches the private + key when one is present. \return 0 Returned on successfully importing the ed25519_key. \return BAD_FUNC_ARG Returned if in or key evaluate to NULL, or inLen is @@ -588,12 +589,54 @@ void wc_ed25519_free(ed25519_key* key); } \endcode + \sa wc_ed25519_import_public_ex \sa wc_ed25519_import_private_key + \sa wc_ed25519_import_private_key_ex \sa wc_ed25519_export_public */ int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key); +/*! + \ingroup ED25519 + + \brief This function imports a public ed25519_key pair from a buffer + containing the public key. This function will handle both compressed and + uncompressed keys. Check public key matches private key, when present, + when not trusted. + + \return 0 Returned on successfully importing the ed25519_key. + \return BAD_FUNC_ARG Returned if in or key evaluate to NULL, or inLen is + less than the size of an Ed25519 key. + + \param [in] in Pointer to the buffer containing the public key. + \param [in] inLen Length of the buffer containing the public key. + \param [in,out] key Pointer to the ed25519_key object in which to store the + public key. + \param [in] trusted Public key data is trusted or not. + + _Example_ + \code + int ret; + byte pub[] = { initialize Ed25519 public key }; + + ed_25519 key; + wc_ed25519_init_key(&key); + ret = wc_ed25519_import_public_ex(pub, sizeof(pub), &key, 1); + if (ret != 0) { + // error importing key + } + \endcode + + \sa wc_ed25519_import_public + \sa wc_ed25519_import_private_key + \sa wc_ed25519_import_private_key_ex + \sa wc_ed25519_export_public +*/ + +int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, + int trusted); + /*! \ingroup ED25519 @@ -618,14 +661,16 @@ int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key); ed25519_key key; wc_ed25519_init_key(&key); - ret = wc_ed25519_import_private_key(priv, sizeof(priv), &key); + ret = wc_ed25519_import_private_only(priv, sizeof(priv), &key); if (ret != 0) { // error importing private key } \endcode \sa wc_ed25519_import_public + \sa wc_ed25519_import_public_ex \sa wc_ed25519_import_private_key + \sa wc_ed25519_import_private_key_ex \sa wc_ed25519_export_private_only */ @@ -637,7 +682,8 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, \brief This function imports a public/private Ed25519 key pair from a pair of buffers. This function will handle both compressed and - uncompressed keys. + uncompressed keys. The public key is assumed to be untrusted and is + checked against the private key. \return 0 Returned on successfully importing the ed25519_key. \return BAD_FUNC_ARG Returned if in or key evaluate to NULL, or if @@ -667,13 +713,60 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, \endcode \sa wc_ed25519_import_public + \sa wc_ed25519_import_public_ex \sa wc_ed25519_import_private_only + \sa wc_ed25519_import_private_key_ex \sa wc_ed25519_export_private */ int wc_ed25519_import_private_key(const byte* priv, word32 privSz, const byte* pub, word32 pubSz, ed25519_key* key); +/*! + \ingroup ED25519 + + \brief This function imports a public/private Ed25519 key pair from a + pair of buffers. This function will handle both compressed and + uncompressed keys. The public is checked against private key if not trusted. + + \return 0 Returned on successfully importing the ed25519_key. + \return BAD_FUNC_ARG Returned if in or key evaluate to NULL, or if + either privSz is less than ED25519_KEY_SIZE or pubSz is less than + ED25519_PUB_KEY_SIZE. + + \param [in] priv Pointer to the buffer containing the private key. + \param [in] privSz Length of the private key. + \param [in] pub Pointer to the buffer containing the public key. + \param [in] pubSz Length of the public key. + \param [in,out] key Pointer to the ed25519_key object in which to store the + imported private/public key pair. + \param [in] trusted Public key data is trusted or not. + + _Example_ + \code + int ret; + byte priv[] = { initialize with 32 byte private key }; + byte pub[] = { initialize with the corresponding public key }; + + ed25519_key key; + wc_ed25519_init_key(&key); + ret = wc_ed25519_import_private_key(priv, sizeof(priv), pub, sizeof(pub), + &key, 1); + if (ret != 0) { + // error importing key + } + \endcode + + \sa wc_ed25519_import_public + \sa wc_ed25519_import_public_ex + \sa wc_ed25519_import_private_only + \sa wc_ed25519_import_private_key + \sa wc_ed25519_export_private +*/ + +int wc_ed25519_import_private_key_ex(const byte* priv, word32 privSz, + const byte* pub, word32 pubSz, ed25519_key* key, int trusted); + /*! \ingroup ED25519 @@ -710,6 +803,7 @@ int wc_ed25519_import_private_key(const byte* priv, word32 privSz, \endcode \sa wc_ed25519_import_public + \sa wc_ed25519_import_public_ex \sa wc_ed25519_export_private_only */ @@ -750,6 +844,7 @@ int wc_ed25519_export_public(ed25519_key* key, byte* out, word32* outLen); \sa wc_ed25519_export_public \sa wc_ed25519_import_private_key + \sa wc_ed25519_import_private_key_ex */ int wc_ed25519_export_private_only(ed25519_key* key, byte* out, word32* outLen); @@ -792,6 +887,7 @@ int wc_ed25519_export_private_only(ed25519_key* key, byte* out, word32* outLen); \endcode \sa wc_ed25519_import_private_key + \sa wc_ed25519_import_private_key_ex \sa wc_ed25519_export_private_only */ @@ -866,7 +962,8 @@ int wc_ed25519_export_key(ed25519_key* key, ed25519_key key; wc_ed25519_init_key(&key); - wc_ed25519_import_private_key(priv, sizeof(priv), pub, sizeof(pub), &key); + wc_ed25519_import_private_key_ex(priv, sizeof(priv), pub, sizeof(pub), &key, + 1); ret = wc_ed25519_check_key(&key); if (ret != 0) { // error checking key @@ -874,6 +971,7 @@ int wc_ed25519_export_key(ed25519_key* key, \endcode \sa wc_ed25519_import_private_key + \sa wc_ed25519_import_private_key_ex */ int wc_ed25519_check_key(ed25519_key* key); diff --git a/doc/dox_comments/header_files/ed448.h b/doc/dox_comments/header_files/ed448.h index 75550aa11..a3ea82088 100644 --- a/doc/dox_comments/header_files/ed448.h +++ b/doc/dox_comments/header_files/ed448.h @@ -447,7 +447,8 @@ void wc_ed448_free(ed448_key* key); \brief This function imports a public ed448_key pair from a buffer containing the public key. This function will handle both compressed and - uncompressed keys. + uncompressed keys. The public key is checked that it matches the private + key when one is present. \return 0 Returned on successfully importing the ed448_key. \return BAD_FUNC_ARG Returned if in or key evaluate to NULL, or inLen is @@ -471,12 +472,54 @@ void wc_ed448_free(ed448_key* key); } \endcode + \sa wc_ed448_import_public_ex \sa wc_ed448_import_private_key + \sa wc_ed448_import_private_key_ex \sa wc_ed448_export_public */ int wc_ed448_import_public(const byte* in, word32 inLen, ed448_key* key); +/*! + \ingroup ED448 + + \brief This function imports a public ed448_key pair from a buffer + containing the public key. This function will handle both compressed and + uncompressed keys. Check public key matches private key, when present, + when not trusted. + + \return 0 Returned on successfully importing the ed448_key. + \return BAD_FUNC_ARG Returned if in or key evaluate to NULL, or inLen is + less than the size of an Ed448 key. + + \param [in] in Pointer to the buffer containing the public key. + \param [in] inLen Length of the buffer containing the public key. + \param [in,out] key Pointer to the ed448_key object in which to store the + public key. + \param [in] trusted Public key data is trusted or not. + + _Example_ + \code + int ret; + byte pub[] = { initialize Ed448 public key }; + + ed_448 key; + wc_ed448_init_key(&key); + ret = wc_ed448_import_public_ex(pub, sizeof(pub), &key, 1); + if (ret != 0) { + // error importing key + } + \endcode + + \sa wc_ed448_import_public + \sa wc_ed448_import_private_key + \sa wc_ed448_import_private_key_ex + \sa wc_ed448_export_public +*/ + +int wc_ed448_import_public_ex(const byte* in, word32 inLen, ed448_key* key, + int trusted); + /*! \ingroup ED448 @@ -506,7 +549,9 @@ int wc_ed448_import_public(const byte* in, word32 inLen, ed448_key* key); \endcode \sa wc_ed448_import_public + \sa wc_ed448_import_public_ex \sa wc_ed448_import_private_key + \sa wc_ed448_import_private_key_ex \sa wc_ed448_export_private_only */ @@ -548,13 +593,60 @@ int wc_ed448_import_private_only(const byte* priv, word32 privSz, \endcode \sa wc_ed448_import_public + \sa wc_ed448_import_public_ex \sa wc_ed448_import_private_only + \sa wc_ed448_import_private_key_ex \sa wc_ed448_export_private */ int wc_ed448_import_private_key(const byte* priv, word32 privSz, const byte* pub, word32 pubSz, ed448_key* key); +/*! + \ingroup ED448 + + \brief This function imports a public/private Ed448 key pair from a + pair of buffers. This function will handle both compressed and + uncompressed keys. The public is checked against private key if not trusted. + + \return 0 Returned on successfully importing the Ed448 key. + \return BAD_FUNC_ARG Returned if in or key evaluate to NULL, or if + either privSz is less than ED448_KEY_SIZE or pubSz is less than + ED448_PUB_KEY_SIZE. + + \param [in] priv Pointer to the buffer containing the private key. + \param [in] privSz Length of the private key. + \param [in] pub Pointer to the buffer containing the public key. + \param [in] pubSz Length of the public key. + \param [in,out] key Pointer to the ed448_key object in which to store the + imported private/public key pair. + \param [in] trusted Public key data is trusted or not. + + _Example_ + \code + int ret; + byte priv[] = { initialize with 57 byte private key }; + byte pub[] = { initialize with the corresponding public key }; + + ed448_key key; + wc_ed448_init_key(&key); + ret = wc_ed448_import_private_key_ex(priv, sizeof(priv), pub, sizeof(pub), + &key, 1); + if (ret != 0) { + // error importing key + } + \endcode + + \sa wc_ed448_import_public + \sa wc_ed448_import_public_ex + \sa wc_ed448_import_private_only + \sa wc_ed448_import_private_key + \sa wc_ed448_export_private +*/ + +int wc_ed448_import_private_key_ex(const byte* priv, word32 privSz, + const byte* pub, word32 pubSz, ed448_key* key, int trusted); + /*! \ingroup ED448 @@ -591,6 +683,7 @@ int wc_ed448_import_private_key(const byte* priv, word32 privSz, \endcode \sa wc_ed448_import_public + \sa wc_ed448_import_public_ex \sa wc_ed448_export_private_only */ @@ -631,6 +724,7 @@ int wc_ed448_export_public(ed448_key* key, byte* out, word32* outLen); \sa wc_ed448_export_public \sa wc_ed448_import_private_key + \sa wc_ed448_import_private_key_ex */ int wc_ed448_export_private_only(ed448_key* key, byte* out, word32* outLen); @@ -747,7 +841,8 @@ int wc_ed448_export_key(ed448_key* key, ed448_key key; wc_ed448_init_key(&key); - wc_ed448_import_private_key(priv, sizeof(priv), pub, sizeof(pub), &key); + wc_ed448_import_private_key_ex(priv, sizeof(priv), pub, sizeof(pub), &key, + 1); ret = wc_ed448_check_key(&key); if (ret != 0) { // error checking key @@ -755,6 +850,7 @@ int wc_ed448_export_key(ed448_key* key, \endcode \sa wc_ed448_import_private_key + \sa wc_ed448_import_private_key_ex */ int wc_ed448_check_key(ed448_key* key); diff --git a/tests/api.c b/tests/api.c index a19bcf2b1..7e2e14ddc 100644 --- a/tests/api.c +++ b/tests/api.c @@ -21230,7 +21230,7 @@ static int test_wc_ed25519_import_public (void) printf(testingFmt, "wc_ed25519_import_public()"); if (ret == 0) { - ret = wc_ed25519_import_public(in, inlen, &pubKey); + ret = wc_ed25519_import_public_ex(in, inlen, &pubKey, 1); if (ret == 0 && XMEMCMP(in, pubKey.p, inlen) == 0) { ret = 0; @@ -21300,8 +21300,8 @@ static int test_wc_ed25519_import_private_key (void) printf(testingFmt, "wc_ed25519_import_private_key()"); if (ret == 0) { - ret = wc_ed25519_import_private_key(privKey, privKeySz, pubKey, - pubKeySz, &key); + ret = wc_ed25519_import_private_key_ex(privKey, privKeySz, pubKey, + pubKeySz, &key, 1); if (ret == 0 && (XMEMCMP(pubKey, key.p, privKeySz) != 0 || XMEMCMP(privKey, key.k, pubKeySz) != 0)) { ret = WOLFSSL_FATAL_ERROR; @@ -21313,7 +21313,8 @@ static int test_wc_ed25519_import_private_key (void) ret = wc_ed25519_export_private(&key, bothKeys, &bothKeysSz); if (ret == 0) { - ret = wc_ed25519_import_private_key(bothKeys, bothKeysSz, NULL, 0, &key); + ret = wc_ed25519_import_private_key_ex(bothKeys, bothKeysSz, NULL, 0, + &key, 1); if (ret == 0 && (XMEMCMP(pubKey, key.p, privKeySz) != 0 || XMEMCMP(privKey, key.k, pubKeySz) != 0)) { ret = WOLFSSL_FATAL_ERROR; @@ -23051,7 +23052,7 @@ static int test_wc_ed448_import_public (void) printf(testingFmt, "wc_ed448_import_public()"); if (ret == 0) { - ret = wc_ed448_import_public(in, inlen, &pubKey); + ret = wc_ed448_import_public_ex(in, inlen, &pubKey, 1); if (ret == 0 && XMEMCMP(in, pubKey.p, inlen) == 0) { ret = 0; @@ -23123,8 +23124,8 @@ static int test_wc_ed448_import_private_key (void) printf(testingFmt, "wc_ed448_import_private_key()"); if (ret == 0) { - ret = wc_ed448_import_private_key(privKey, privKeySz, pubKey, pubKeySz, - &key); + ret = wc_ed448_import_private_key_ex(privKey, privKeySz, pubKey, + pubKeySz, &key, 1); if (ret == 0 && (XMEMCMP(pubKey, key.p, privKeySz) != 0 || XMEMCMP(privKey, key.k, pubKeySz) != 0)) { ret = WOLFSSL_FATAL_ERROR; @@ -23136,7 +23137,8 @@ static int test_wc_ed448_import_private_key (void) ret = wc_ed448_export_private(&key, bothKeys, &bothKeysSz); if (ret == 0) { - ret = wc_ed448_import_private_key(bothKeys, bothKeysSz, NULL, 0, &key); + ret = wc_ed448_import_private_key_ex(bothKeys, bothKeysSz, NULL, 0, + &key, 1); if (ret == 0 && (XMEMCMP(pubKey, key.p, privKeySz) != 0 || XMEMCMP(privKey, key.k, pubKeySz) != 0)) { ret = WOLFSSL_FATAL_ERROR; diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 9397c43bd..ca70c217b 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -60,7 +60,7 @@ Possible ECC enable options: * USE_ECC_B_PARAM: Enable ECC curve B param default: off * (on for HAVE_COMP_KEY) * WOLFSSL_ECC_CURVE_STATIC: default off (on for windows) - * For the ECC curve paramaters `ecc_set_type` use fixed + * For the ECC curve parameters `ecc_set_type` use fixed * array for hex string * WC_ECC_NONBLOCK: Enable non-blocking support for sign/verify. * Requires SP with WOLFSSL_SP_NONBLOCK diff --git a/wolfcrypt/src/ed25519.c b/wolfcrypt/src/ed25519.c index 8304a46ea..ff5bd7ebd 100644 --- a/wolfcrypt/src/ed25519.c +++ b/wolfcrypt/src/ed25519.c @@ -242,6 +242,7 @@ int wc_ed25519_make_key(WC_RNG* rng, int keySz, ed25519_key* key) /* put public key after private key, on the same buffer */ XMEMMOVE(key->k + ED25519_KEY_SIZE, key->p, ED25519_PUB_KEY_SIZE); + key->privKeySet = 1; key->pubKeySet = 1; return ret; @@ -941,11 +942,13 @@ int wc_ed25519_export_public(ed25519_key* key, byte* out, word32* outLen) #ifdef HAVE_ED25519_KEY_IMPORT /* Imports a compressed/uncompressed public key. - in the byte array containing the public key - inLen the length of the byte array being passed in - key ed25519 key struct to put the public key in + in the byte array containing the public key + inLen the length of the byte array being passed in + key ed25519 key struct to put the public key in + trusted whether the public key is trusted to match private key if set */ -int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key) +int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, + int trusted) { int ret; @@ -969,6 +972,10 @@ int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key) LTC_PKHA_Ed25519_PointDecompress(key->p, ED25519_PUB_KEY_SIZE, &pubKey); #endif key->pubKeySet = 1; + + if (key->privKeySet && (!trusted)) { + return wc_ed25519_check_key(key); + } return 0; } @@ -991,6 +998,9 @@ int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key) if (ret == 0) key->pubKeySet = 1; #endif /* FREESCALE_LTC_ECC */ + if ((ret == 0) && key->privKeySet && (!trusted)) { + return wc_ed25519_check_key(key); + } return ret; } @@ -1006,6 +1016,9 @@ int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key) LTC_PKHA_Ed25519_PointDecompress(key->p, ED25519_PUB_KEY_SIZE, &pubKey); #endif key->pubKeySet = 1; + if (key->privKeySet && (!trusted)) { + return wc_ed25519_check_key(key); + } return 0; } @@ -1013,6 +1026,16 @@ int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key) return BAD_FUNC_ARG; } +/* + Imports a compressed/uncompressed public key. + in the byte array containing the public key + inLen the length of the byte array being passed in + key ed25519 key struct to put the public key in + */ +int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key) +{ + return wc_ed25519_import_public_ex(in, inLen, key, 0); +} /* For importing a private key. @@ -1029,6 +1052,7 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, return BAD_FUNC_ARG; XMEMCPY(key->k, priv, ED25519_KEY_SIZE); + key->privKeySet = 1; return 0; } @@ -1036,17 +1060,21 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, /* Import an ed25519 private and public keys from byte array(s). * - * priv [in] Array holding private key from wc_ed25519_export_private_only(), - * or private+public keys from wc_ed25519_export_private(). - * privSz [in] Number of bytes of data in private key array. - * pub [in] Array holding public key (or NULL). - * pubSz [in] Number of bytes of data in public key array (or 0). - * key [in] Ed25519 private/public key. + * priv [in] Array holding private key from + * wc_ed25519_export_private_only(), or private+public keys from + * wc_ed25519_export_private(). + * privSz [in] Number of bytes of data in private key array. + * pub [in] Array holding public key (or NULL). + * pubSz [in] Number of bytes of data in public key array (or 0). + * key [in] Ed25519 private/public key. + * trusted [in] Indicates whether the public key data is trusted. + * When 0, checks public key matches private key. + * When 1, doesn't check public key matches private key. * returns BAD_FUNC_ARG when a required parameter is NULL or an invalid * combination of keys/lengths is supplied, 0 otherwise. */ -int wc_ed25519_import_private_key(const byte* priv, word32 privSz, - const byte* pub, word32 pubSz, ed25519_key* key) +int wc_ed25519_import_private_key_ex(const byte* priv, word32 privSz, + const byte* pub, word32 pubSz, ed25519_key* key, int trusted) { int ret; @@ -1069,18 +1097,36 @@ int wc_ed25519_import_private_key(const byte* priv, word32 privSz, return BAD_FUNC_ARG; } + XMEMCPY(key->k, priv, ED25519_KEY_SIZE); + key->privKeySet = 1; + /* import public key */ - ret = wc_ed25519_import_public(pub, pubSz, key); + ret = wc_ed25519_import_public_ex(pub, pubSz, key, trusted); if (ret != 0) return ret; /* make the private key (priv + pub) */ - XMEMCPY(key->k, priv, ED25519_KEY_SIZE); XMEMCPY(key->k + ED25519_KEY_SIZE, key->p, ED25519_PUB_KEY_SIZE); return ret; } +/* Import an ed25519 private and public keys from byte array(s). + * + * priv [in] Array holding private key from wc_ed25519_export_private_only(), + * or private+public keys from wc_ed25519_export_private(). + * privSz [in] Number of bytes of data in private key array. + * pub [in] Array holding public key (or NULL). + * pubSz [in] Number of bytes of data in public key array (or 0). + * key [in] Ed25519 private/public key. + * returns BAD_FUNC_ARG when a required parameter is NULL or an invalid + * combination of keys/lengths is supplied, 0 otherwise. + */ +int wc_ed25519_import_private_key(const byte* priv, word32 privSz, + const byte* pub, word32 pubSz, ed25519_key* key) +{ + return wc_ed25519_import_private_key_ex(priv, privSz, pub, pubSz, key, 0); +} #endif /* HAVE_ED25519_KEY_IMPORT */ diff --git a/wolfcrypt/src/ed448.c b/wolfcrypt/src/ed448.c index ea092d18b..0225c2f04 100644 --- a/wolfcrypt/src/ed448.c +++ b/wolfcrypt/src/ed448.c @@ -228,6 +228,7 @@ int wc_ed448_make_key(WC_RNG* rng, int keySz, ed448_key* key) ret = wc_RNG_GenerateBlock(rng, key->k, ED448_KEY_SIZE); } if (ret == 0) { + key->privKeySet = 1; ret = wc_ed448_make_public(key, key->p, ED448_PUB_KEY_SIZE); if (ret != 0) { ForceZero(key->k, ED448_KEY_SIZE); @@ -893,13 +894,17 @@ int wc_ed448_export_public(ed448_key* key, byte* out, word32* outLen) /* Import a compressed or uncompressed ed448 public key from a byte array. * Public key encoded in big-endian. * - * in [in] Array holding public key. - * inLen [in] Number of bytes of data in array. - * key [in] Ed448 public key. + * in [in] Array holding public key. + * inLen [in] Number of bytes of data in array. + * key [in] Ed448 public key. + * trusted [in] Indicates whether the public key data is trusted. + * When 0, checks public key matches private key. + * When 1, doesn't check public key matches private key. * returns BAD_FUNC_ARG when a parameter is NULL or key format is not supported, * 0 otherwise. */ -int wc_ed448_import_public(const byte* in, word32 inLen, ed448_key* key) +int wc_ed448_import_public_ex(const byte* in, word32 inLen, ed448_key* key, + int trusted) { int ret = 0; @@ -939,9 +944,29 @@ int wc_ed448_import_public(const byte* in, word32 inLen, ed448_key* key) } } + if ((ret == 0) && key->privKeySet && (!trusted)) { + /* Check untrusted public key data matches private key. */ + ret = wc_ed448_check_key(key); + } + return ret; } +/* Import a compressed or uncompressed ed448 public key from a byte array. + * + * Public key encoded in big-endian. + * Public key is not trusted and is checked against private key if set. + * + * in [in] Array holding public key. + * inLen [in] Number of bytes of data in array. + * key [in] Ed448 public key. + * returns BAD_FUNC_ARG when a parameter is NULL or key format is not supported, + * 0 otherwise. + */ +int wc_ed448_import_public(const byte* in, word32 inLen, ed448_key* key) +{ + return wc_ed448_import_public_ex(in, inLen, key, 0); +} /* Import an ed448 private key from a byte array. * @@ -969,6 +994,7 @@ int wc_ed448_import_private_only(const byte* priv, word32 privSz, if (ret == 0) { XMEMCPY(key->k, priv, ED448_KEY_SIZE); + key->privKeySet = 1; } return ret; @@ -977,17 +1003,20 @@ int wc_ed448_import_private_only(const byte* priv, word32 privSz, /* Import an ed448 private and public keys from byte array(s). * - * priv [in] Array holding private key from wc_ed448_export_private_only(), - * or private+public keys from wc_ed448_export_private(). - * privSz [in] Number of bytes of data in private key array. - * pub [in] Array holding public key (or NULL). - * pubSz [in] Number of bytes of data in public key array (or 0). - * key [in] Ed448 private/public key. + * priv [in] Array holding private key from wc_ed448_export_private_only(), + * or private+public keys from wc_ed448_export_private(). + * privSz [in] Number of bytes of data in private key array. + * pub [in] Array holding public key (or NULL). + * pubSz [in] Number of bytes of data in public key array (or 0). + * key [in] Ed448 private/public key. + * trusted [in] Indicates whether the public key data is trusted. + * When 0, checks public key matches private key. + * When 1, doesn't check public key matches private key. * returns BAD_FUNC_ARG when a required parameter is NULL or an invalid * combination of keys/lengths is supplied, 0 otherwise. */ -int wc_ed448_import_private_key(const byte* priv, word32 privSz, - const byte* pub, word32 pubSz, ed448_key* key) +int wc_ed448_import_private_key_ex(const byte* priv, word32 privSz, + const byte* pub, word32 pubSz, ed448_key* key, int trusted) { int ret; @@ -1010,18 +1039,38 @@ int wc_ed448_import_private_key(const byte* priv, word32 privSz, return BAD_FUNC_ARG; } + XMEMCPY(key->k, priv, ED448_KEY_SIZE); + key->privKeySet = 1; + /* import public key */ - ret = wc_ed448_import_public(pub, pubSz, key); + ret = wc_ed448_import_public_ex(pub, pubSz, key, trusted); if (ret != 0) return ret; /* make the private key (priv + pub) */ - XMEMCPY(key->k, priv, ED448_KEY_SIZE); XMEMCPY(key->k + ED448_KEY_SIZE, key->p, ED448_PUB_KEY_SIZE); return ret; } +/* Import an ed448 private and public keys from byte array(s). + * + * Public key is not trusted and is checked against private key. + * + * priv [in] Array holding private key from wc_ed448_export_private_only(), + * or private+public keys from wc_ed448_export_private(). + * privSz [in] Number of bytes of data in private key array. + * pub [in] Array holding public key (or NULL). + * pubSz [in] Number of bytes of data in public key array (or 0). + * key [in] Ed448 private/public key. + * returns BAD_FUNC_ARG when a required parameter is NULL or an invalid + * combination of keys/lengths is supplied, 0 otherwise. + */ +int wc_ed448_import_private_key(const byte* priv, word32 privSz, + const byte* pub, word32 pubSz, ed448_key* key) +{ + return wc_ed448_import_private_key_ex(priv, privSz, pub, pubSz, key, 0); +} #endif /* HAVE_ED448_KEY_IMPORT */ diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index ec7d34603..7f7c7ce6f 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -27350,7 +27350,7 @@ WOLFSSL_TEST_SUBROUTINE int ed25519_test(void) if (wc_ed25519_export_public(&key, exportPKey, &exportPSz) != 0) return -11051 - i; - if (wc_ed25519_import_public(exportPKey, exportPSz, &key2) != 0) + if (wc_ed25519_import_public_ex(exportPKey, exportPSz, &key2, 1) != 0) return -11061 - i; if (wc_ed25519_export_private_only(&key, exportSKey, &exportSSz) != 0) @@ -28813,7 +28813,7 @@ WOLFSSL_TEST_SUBROUTINE int ed448_test(void) break; } - if (wc_ed448_import_public(exportPKey, exportPSz, key2) != 0) { + if (wc_ed448_import_public_ex(exportPKey, exportPSz, key2, 1) != 0) { ret = -11761 - i; break; } diff --git a/wolfssl/wolfcrypt/ed25519.h b/wolfssl/wolfcrypt/ed25519.h index 4f2171ead..b7d2ef949 100644 --- a/wolfssl/wolfcrypt/ed25519.h +++ b/wolfssl/wolfcrypt/ed25519.h @@ -96,6 +96,7 @@ struct ed25519_key { int keyId; word32 flags; #endif + word16 privKeySet:1; word16 pubKeySet:1; #ifdef WOLFSSL_ASYNC_CRYPT WC_ASYNC_DEV asyncDev; @@ -181,11 +182,17 @@ void wc_ed25519_free(ed25519_key* key); WOLFSSL_API int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key); WOLFSSL_API +int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, + int trusted); +WOLFSSL_API int wc_ed25519_import_private_only(const byte* priv, word32 privSz, ed25519_key* key); WOLFSSL_API int wc_ed25519_import_private_key(const byte* priv, word32 privSz, const byte* pub, word32 pubSz, ed25519_key* key); +WOLFSSL_API +int wc_ed25519_import_private_key_ex(const byte* priv, word32 privSz, + const byte* pub, word32 pubSz, ed25519_key* key, int trusted); #endif /* HAVE_ED25519_KEY_IMPORT */ #ifdef HAVE_ED25519_KEY_EXPORT diff --git a/wolfssl/wolfcrypt/ed448.h b/wolfssl/wolfcrypt/ed448.h index c1ce329fc..4ee888bfd 100644 --- a/wolfssl/wolfcrypt/ed448.h +++ b/wolfssl/wolfcrypt/ed448.h @@ -86,6 +86,7 @@ struct ed448_key { byte pointX[ED448_KEY_SIZE]; /* recovered X coordinate */ byte pointY[ED448_KEY_SIZE]; /* Y coordinate is the public key with The most significant bit of the final octet always zero. */ #endif + word16 privKeySet:1; word16 pubKeySet:1; #ifdef WOLFSSL_ASYNC_CRYPT WC_ASYNC_DEV asyncDev; @@ -163,11 +164,17 @@ void wc_ed448_free(ed448_key* key); WOLFSSL_API int wc_ed448_import_public(const byte* in, word32 inLen, ed448_key* key); WOLFSSL_API +int wc_ed448_import_public_ex(const byte* in, word32 inLen, ed448_key* key, + int trusted); +WOLFSSL_API int wc_ed448_import_private_only(const byte* priv, word32 privSz, ed448_key* key); WOLFSSL_API int wc_ed448_import_private_key(const byte* priv, word32 privSz, const byte* pub, word32 pubSz, ed448_key* key); +WOLFSSL_API +int wc_ed448_import_private_key_ex(const byte* priv, word32 privSz, + const byte* pub, word32 pubSz, ed448_key* key, int trusted); #endif /* HAVE_ED448_KEY_IMPORT */ #ifdef HAVE_ED448_KEY_EXPORT From bb68766bdab9565aabd67dd67eeaa1d49ab2bdff Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 1 Jul 2022 12:53:39 -0700 Subject: [PATCH 2/4] For ED255219 and ED448 if importing private only and public key is already set then check it. --- wolfcrypt/src/ed25519.c | 5 +++++ wolfcrypt/src/ed448.c | 5 +++++ wolfssl/wolfcrypt/types.h | 12 ++++++------ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/wolfcrypt/src/ed25519.c b/wolfcrypt/src/ed25519.c index ff5bd7ebd..2e6020e62 100644 --- a/wolfcrypt/src/ed25519.c +++ b/wolfcrypt/src/ed25519.c @@ -1054,6 +1054,11 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, XMEMCPY(key->k, priv, ED25519_KEY_SIZE); key->privKeySet = 1; + if (key->pubKeySet) { + /* Validate loaded public key */ + return wc_ed25519_check_key(key); + } + return 0; } diff --git a/wolfcrypt/src/ed448.c b/wolfcrypt/src/ed448.c index 0225c2f04..ffef6b050 100644 --- a/wolfcrypt/src/ed448.c +++ b/wolfcrypt/src/ed448.c @@ -997,6 +997,11 @@ int wc_ed448_import_private_only(const byte* priv, word32 privSz, key->privKeySet = 1; } + if ((ret == 0) && key->pubKeySet) { + /* Validate loaded public key */ + ret = wc_ed448_check_key(key); + } + return ret; } diff --git a/wolfssl/wolfcrypt/types.h b/wolfssl/wolfcrypt/types.h index 73ce7a9ae..6fc577087 100644 --- a/wolfssl/wolfcrypt/types.h +++ b/wolfssl/wolfcrypt/types.h @@ -44,8 +44,8 @@ decouple library dependencies with standard string, memory and so on. /* * This struct is used multiple time by other structs and - * needs to be defined somwhere that all structs can import - * (with minimal depencencies). + * needs to be defined somewhere that all structs can import + * (with minimal dependencies). */ #ifdef HAVE_EX_DATA #ifdef HAVE_EX_DATA_CLEANUP_HOOKS @@ -208,14 +208,14 @@ decouple library dependencies with standard string, memory and so on. #elif defined(WC_16BIT_CPU) #undef WORD64_AVAILABLE typedef word16 wolfssl_word; - #define MP_16BIT /* for mp_int, mp_word needs to be twice as big as - mp_digit, no 64 bit type so make mp_digit 16 bit */ + #define MP_16BIT /* for mp_int, mp_word needs to be twice as big as \ + * mp_digit, no 64 bit type so make mp_digit 16 bit */ #else #undef WORD64_AVAILABLE typedef word32 wolfssl_word; - #define MP_16BIT /* for mp_int, mp_word needs to be twice as big as - mp_digit, no 64 bit type so make mp_digit 16 bit */ + #define MP_16BIT /* for mp_int, mp_word needs to be twice as big as \ + * mp_digit, no 64 bit type so make mp_digit 16 bit */ #endif typedef struct w64wrapper { From 4a962b7fb2eed6e1102ef96a3e46c2218a24ad0b Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Tue, 5 Jul 2022 09:02:05 +1000 Subject: [PATCH 3/4] Ed25519/448: improvements Check lengths of buffers in import functions. priv/pub key set flag set on success only. --- wolfcrypt/src/ed25519.c | 58 ++++++++++++++++++++--------------------- wolfcrypt/src/ed448.c | 34 +++++++++++++++--------- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/wolfcrypt/src/ed25519.c b/wolfcrypt/src/ed25519.c index 2e6020e62..add7fb19f 100644 --- a/wolfcrypt/src/ed25519.c +++ b/wolfcrypt/src/ed25519.c @@ -950,7 +950,7 @@ int wc_ed25519_export_public(ed25519_key* key, byte* out, word32* outLen) int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, int trusted) { - int ret; + int ret = 0; /* sanity check on arguments */ if (in == NULL || key == NULL) @@ -961,7 +961,7 @@ int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, /* compressed prefix according to draft http://www.ietf.org/id/draft-koch-eddsa-for-openpgp-02.txt */ - if (in[0] == 0x40 && inLen > ED25519_PUB_KEY_SIZE) { + if (in[0] == 0x40 && inLen == ED25519_PUB_KEY_SIZE + 1) { /* key is stored in compressed format so just copy in */ XMEMCPY(key->p, (in + 1), ED25519_PUB_KEY_SIZE); #ifdef FREESCALE_LTC_ECC @@ -971,16 +971,9 @@ int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, pubKey.Y = key->pointY; LTC_PKHA_Ed25519_PointDecompress(key->p, ED25519_PUB_KEY_SIZE, &pubKey); #endif - key->pubKeySet = 1; - - if (key->privKeySet && (!trusted)) { - return wc_ed25519_check_key(key); - } - return 0; } - /* importing uncompressed public key */ - if (in[0] == 0x04 && inLen > 2*ED25519_PUB_KEY_SIZE) { + else if (in[0] == 0x04 && inLen > 2*ED25519_PUB_KEY_SIZE) { #ifdef FREESCALE_LTC_ECC /* reverse bytes for little endian byte order */ for (int i = 0; i < ED25519_KEY_SIZE; i++) @@ -989,24 +982,15 @@ int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, key->pointY[i] = *(in + 2*ED25519_KEY_SIZE - i); } XMEMCPY(key->p, key->pointY, ED25519_KEY_SIZE); - key->pubKeySet = 1; - ret = 0; #else /* pass in (x,y) and store compressed key */ ret = ge_compress_key(key->p, in+1, in+1+ED25519_PUB_KEY_SIZE, ED25519_PUB_KEY_SIZE); - if (ret == 0) - key->pubKeySet = 1; #endif /* FREESCALE_LTC_ECC */ - if ((ret == 0) && key->privKeySet && (!trusted)) { - return wc_ed25519_check_key(key); - } - return ret; } - /* if not specified compressed or uncompressed check key size if key size is equal to compressed key size copy in key */ - if (inLen == ED25519_PUB_KEY_SIZE) { + else if (inLen == ED25519_PUB_KEY_SIZE) { XMEMCPY(key->p, in, ED25519_PUB_KEY_SIZE); #ifdef FREESCALE_LTC_ECC /* recover X coordinate */ @@ -1015,15 +999,23 @@ int wc_ed25519_import_public_ex(const byte* in, word32 inLen, ed25519_key* key, pubKey.Y = key->pointY; LTC_PKHA_Ed25519_PointDecompress(key->p, ED25519_PUB_KEY_SIZE, &pubKey); #endif + } + else { + ret = BAD_FUNC_ARG; + } + + if (ret == 0) { key->pubKeySet = 1; if (key->privKeySet && (!trusted)) { - return wc_ed25519_check_key(key); + ret = wc_ed25519_check_key(key); } - return 0; + } + if (ret != 0) { + key->pubKeySet = 0; } /* bad public key format */ - return BAD_FUNC_ARG; + return ret; } /* @@ -1043,12 +1035,14 @@ int wc_ed25519_import_public(const byte* in, word32 inLen, ed25519_key* key) int wc_ed25519_import_private_only(const byte* priv, word32 privSz, ed25519_key* key) { + int ret = 0; + /* sanity check on arguments */ if (priv == NULL || key == NULL) return BAD_FUNC_ARG; /* key size check */ - if (privSz < ED25519_KEY_SIZE) + if (privSz != ED25519_KEY_SIZE) return BAD_FUNC_ARG; XMEMCPY(key->k, priv, ED25519_KEY_SIZE); @@ -1056,10 +1050,13 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, if (key->pubKeySet) { /* Validate loaded public key */ - return wc_ed25519_check_key(key); + ret = wc_ed25519_check_key(key); + } + if (ret != 0) { + key->privKeySet = 0; } - return 0; + return ret; } @@ -1081,24 +1078,25 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, int wc_ed25519_import_private_key_ex(const byte* priv, word32 privSz, const byte* pub, word32 pubSz, ed25519_key* key, int trusted) { - int ret; + int ret; /* sanity check on arguments */ if (priv == NULL || key == NULL) return BAD_FUNC_ARG; /* key size check */ - if (privSz < ED25519_KEY_SIZE) + if (privSz != ED25519_KEY_SIZE && privSz != ED25519_PRV_KEY_SIZE) return BAD_FUNC_ARG; if (pub == NULL) { if (pubSz != 0) return BAD_FUNC_ARG; - if (privSz < ED25519_PRV_KEY_SIZE) + if (privSz != ED25519_PRV_KEY_SIZE) return BAD_FUNC_ARG; pub = priv + ED25519_KEY_SIZE; pubSz = ED25519_PUB_KEY_SIZE; - } else if (pubSz < ED25519_PUB_KEY_SIZE) { + } + else if (pubSz < ED25519_PUB_KEY_SIZE) { return BAD_FUNC_ARG; } diff --git a/wolfcrypt/src/ed448.c b/wolfcrypt/src/ed448.c index ffef6b050..d306d068f 100644 --- a/wolfcrypt/src/ed448.c +++ b/wolfcrypt/src/ed448.c @@ -913,7 +913,7 @@ int wc_ed448_import_public_ex(const byte* in, word32 inLen, ed448_key* key, ret = BAD_FUNC_ARG; } - if (inLen < ED448_PUB_KEY_SIZE) { + if (inLen != ED448_PUB_KEY_SIZE) { ret = BAD_FUNC_ARG; } @@ -923,20 +923,16 @@ int wc_ed448_import_public_ex(const byte* in, word32 inLen, ed448_key* key, if (in[0] == 0x40 && inLen > ED448_PUB_KEY_SIZE) { /* key is stored in compressed format so just copy in */ XMEMCPY(key->p, (in + 1), ED448_PUB_KEY_SIZE); - key->pubKeySet = 1; } /* importing uncompressed public key */ else if (in[0] == 0x04 && inLen > 2*ED448_PUB_KEY_SIZE) { /* pass in (x,y) and store compressed key */ ret = ge448_compress_key(key->p, in+1, in+1+ED448_PUB_KEY_SIZE); - if (ret == 0) - key->pubKeySet = 1; } else if (inLen == ED448_PUB_KEY_SIZE) { /* if not specified compressed or uncompressed check key size * if key size is equal to compressed key size copy in key */ XMEMCPY(key->p, in, ED448_PUB_KEY_SIZE); - key->pubKeySet = 1; } else { /* bad public key format */ @@ -944,9 +940,17 @@ int wc_ed448_import_public_ex(const byte* in, word32 inLen, ed448_key* key, } } - if ((ret == 0) && key->privKeySet && (!trusted)) { - /* Check untrusted public key data matches private key. */ - ret = wc_ed448_check_key(key); + if (ret == 0) { + key->pubKeySet = 1; + if (key->privKeySet && (!trusted)) { + /* Check untrusted public key data matches private key. */ + ret = wc_ed448_check_key(key); + } + } + + if ((ret != 0) && (key != NULL)) { + /* No public key set on failure. */ + key->pubKeySet = 0; } return ret; @@ -988,7 +992,7 @@ int wc_ed448_import_private_only(const byte* priv, word32 privSz, } /* key size check */ - if ((ret == 0) && (privSz < ED448_KEY_SIZE)) { + if ((ret == 0) && (privSz != ED448_KEY_SIZE)) { ret = BAD_FUNC_ARG; } @@ -1002,6 +1006,11 @@ int wc_ed448_import_private_only(const byte* priv, word32 privSz, ret = wc_ed448_check_key(key); } + if ((ret != 0) && (key != NULL)) { + /* No private key set on error. */ + key->privKeySet = 0; + } + return ret; } @@ -1030,17 +1039,18 @@ int wc_ed448_import_private_key_ex(const byte* priv, word32 privSz, return BAD_FUNC_ARG; /* key size check */ - if (privSz < ED448_KEY_SIZE) + if (privSz != ED448_KEY_SIZE && privSz != ED448_PRV_KEY_SIZE) return BAD_FUNC_ARG; if (pub == NULL) { if (pubSz != 0) return BAD_FUNC_ARG; - if (privSz < ED448_PRV_KEY_SIZE) + if (privSz != ED448_PRV_KEY_SIZE) return BAD_FUNC_ARG; pub = priv + ED448_KEY_SIZE; pubSz = ED448_PUB_KEY_SIZE; - } else if (pubSz < ED448_PUB_KEY_SIZE) { + } + else if (pubSz < ED448_PUB_KEY_SIZE) { return BAD_FUNC_ARG; } From 4caffee590df58337646648a14949349c364dc4c Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Tue, 5 Jul 2022 13:44:31 +0200 Subject: [PATCH 4/4] ForceZero the private key on import error --- wolfcrypt/src/ed25519.c | 6 +++++- wolfcrypt/src/ed448.c | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/wolfcrypt/src/ed25519.c b/wolfcrypt/src/ed25519.c index add7fb19f..436997668 100644 --- a/wolfcrypt/src/ed25519.c +++ b/wolfcrypt/src/ed25519.c @@ -1054,6 +1054,7 @@ int wc_ed25519_import_private_only(const byte* priv, word32 privSz, } if (ret != 0) { key->privKeySet = 0; + ForceZero(key->k, ED25519_KEY_SIZE); } return ret; @@ -1105,8 +1106,11 @@ int wc_ed25519_import_private_key_ex(const byte* priv, word32 privSz, /* import public key */ ret = wc_ed25519_import_public_ex(pub, pubSz, key, trusted); - if (ret != 0) + if (ret != 0) { + key->privKeySet = 0; + ForceZero(key->k, ED25519_KEY_SIZE); return ret; + } /* make the private key (priv + pub) */ XMEMCPY(key->k + ED25519_KEY_SIZE, key->p, ED25519_PUB_KEY_SIZE); diff --git a/wolfcrypt/src/ed448.c b/wolfcrypt/src/ed448.c index d306d068f..0423b2b2b 100644 --- a/wolfcrypt/src/ed448.c +++ b/wolfcrypt/src/ed448.c @@ -1009,6 +1009,7 @@ int wc_ed448_import_private_only(const byte* priv, word32 privSz, if ((ret != 0) && (key != NULL)) { /* No private key set on error. */ key->privKeySet = 0; + ForceZero(key->k, ED448_KEY_SIZE); } return ret; @@ -1059,8 +1060,11 @@ int wc_ed448_import_private_key_ex(const byte* priv, word32 privSz, /* import public key */ ret = wc_ed448_import_public_ex(pub, pubSz, key, trusted); - if (ret != 0) + if (ret != 0) { + key->privKeySet = 0; + ForceZero(key->k, ED448_KEY_SIZE); return ret; + } /* make the private key (priv + pub) */ XMEMCPY(key->k + ED448_KEY_SIZE, key->p, ED448_PUB_KEY_SIZE);