diff --git a/examples/client/client.c b/examples/client/client.c index 81470f3225..16c4061229 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -2470,6 +2470,12 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) ((func_args*)args)->return_code = -1; /* error state */ +#ifdef HAVE_PK_CALLBACKS + /* The ECC callbacks read keyGenCnt whether or not certificates are + * compiled in, so this cannot sit inside the NO_CERTS block below. */ + XMEMSET(&pkCbInfo, 0, sizeof(pkCbInfo)); +#endif + #ifndef NO_RSA verifyCert = caCertFile; ourCert = cliCertFile; @@ -5117,6 +5123,10 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) exit: +#ifdef HAVE_PK_CALLBACKS + CleanupPkCallbackContexts(&pkCbInfo); +#endif + #ifdef WOLFSSL_WOLFSENTRY_HOOKS wolfsentry_ret = wolfsentry_shutdown(WOLFSENTRY_CONTEXT_ARGS_OUT_EX4(&wolfsentry, NULL)); diff --git a/examples/server/server.c b/examples/server/server.c index a0de5cb90f..41ada9fc13 100644 --- a/examples/server/server.c +++ b/examples/server/server.c @@ -1878,6 +1878,12 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) ((func_args*)args)->return_code = -1; /* error state */ +#ifdef HAVE_PK_CALLBACKS + /* The ECC callbacks read keyGenCnt whether or not certificates are + * compiled in, so this cannot sit inside the NO_CERTS block below. */ + XMEMSET(&pkCbInfo, 0, sizeof(pkCbInfo)); +#endif + #ifndef NO_RSA verifyCert = cliCertFile; ourCert = svrCertFile; @@ -4196,6 +4202,10 @@ THREAD_RETURN WOLFSSL_THREAD server_test(void* args) exit: +#ifdef HAVE_PK_CALLBACKS + CleanupPkCallbackContexts(&pkCbInfo); +#endif + #ifdef WOLFSSL_WOLFSENTRY_HOOKS wolfsentry_ret = wolfsentry_shutdown(WOLFSENTRY_CONTEXT_ARGS_OUT_EX4(&wolfsentry, NULL)); diff --git a/scripts/pkcallbacks.test b/scripts/pkcallbacks.test index 753bf7d621..b350356e0c 100755 --- a/scripts/pkcallbacks.test +++ b/scripts/pkcallbacks.test @@ -105,15 +105,24 @@ restore_file_system() { } trap restore_file_system EXIT +# $1 is the version flag pair to run with, empty for the build's default. run_test() { - echo -e "\nStarting example server for pkcallbacks test...\n" + version_args="$1" + + echo -e "\nStarting example server for pkcallbacks test ${version_args:-default}...\n" + + # Back to the port zero hack for every run: the previous run left its + # allocated port here, and reusing a now-known port is exactly what the + # hack exists to avoid when another make check is running alongside. + pk_port=0 remove_ready_file # starts the server on pk_port, -R generates ready file to be used as a # mutex lock, -P does pkcallbacks. We capture the processid # into the variable server_pid - $TIMEOUT_KILL_2M ./examples/server/server -P -R "$ready_file" -p $pk_port & + $TIMEOUT_KILL_2M ./examples/server/server -P -R "$ready_file" -p $pk_port \ + $version_args & server_pid=$! counter=0 @@ -137,12 +146,13 @@ run_test() { pk_port=`cat "$ready_file"` # starts client on pk_port with pkcallbacks, captures the output from client - capture_out=$(./examples/client/client -P -p $pk_port 2>&1) + capture_out=$(./examples/client/client -P -p $pk_port $version_args 2>&1) client_result=$? if [ $client_result != 0 ] then echo -e "client failed!" + echo "$capture_out" do_cleanup exit 1 fi @@ -161,8 +171,19 @@ run_test() { ######### begin program ######### -# run the test -run_test +# The build's default version, then each version explicitly. Running only the +# default used to hide that the ECC PK callbacks could not complete a TLS v1.3 +# handshake: the default is TLS v1.2 wherever it is compiled in, so the 1.3 +# path was never taken here. +run_test "" + +if ./examples/client/client -V | grep -q 3; then + run_test "-v 3" +fi + +if ./examples/client/client -V | grep -q 4; then + run_test "-v 4" +fi # If we get to this, success echo "Success!" diff --git a/wolfssl/test.h b/wolfssl/test.h index 055b5b4391..57906a3b79 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -3432,39 +3432,116 @@ static WC_INLINE int wolfSSL_PrintStatsConn(WOLFSSL_MEM_CONN_STATS* stats) #ifdef HAVE_PK_CALLBACKS +/* How many generated ECC keys one connection can have to hold at once. A TLS + * v1.3 client offers a key share per group it is willing to start with, and + * the example client offers SM2 alongside secp256r1 where both are built, so + * one is not enough; a HelloRetryRequest then adds the group the server + * names. */ +#define PKCB_MAX_ECC_KEYGEN 4 + typedef struct PkCbInfo { const char* ourKey; -#ifdef TEST_PK_PRIVKEY - union { - #ifdef HAVE_ECC - /* only ECC PK callback with TLS v1.2 needs this */ - ecc_key ecc; - #endif - } keyGen; - int hasKeyGen; +#ifdef HAVE_ECC + /* Our own ephemeral keys, kept out of the library's key objects. TLS v1.3 + * generates each key share in the key gen callback and computes the shared + * secret in a later callback that is handed only the peer's key, so the + * private halves have to live here until the group is settled. Held by + * pointer: an ecc_key is several kilobytes, and this struct is a stack + * local in the example client and server. */ + ecc_key* keyGen[PKCB_MAX_ECC_KEYGEN]; + int keyGenCnt; #endif } PkCbInfo; #ifdef HAVE_ECC +/* The key we generated for a curve, or NULL if we have none for it. */ +static WC_INLINE ecc_key* myEccKeptKey(PkCbInfo* cbInfo, int ecc_curve) +{ + int i; + + if (cbInfo == NULL) + return NULL; + + for (i = 0; i < cbInfo->keyGenCnt; i++) { + if (cbInfo->keyGen[i]->dp != NULL && + cbInfo->keyGen[i]->dp->id == ecc_curve) { + return cbInfo->keyGen[i]; + } + } + + return NULL; +} + +static WC_INLINE void myEccFreeKeptKeys(PkCbInfo* cbInfo) +{ + int i; + + if (cbInfo == NULL) + return; + + for (i = 0; i < cbInfo->keyGenCnt; i++) { + wc_ecc_free(cbInfo->keyGen[i]); + XFREE(cbInfo->keyGen[i], NULL, DYNAMIC_TYPE_ECC); + cbInfo->keyGen[i] = NULL; + } + cbInfo->keyGenCnt = 0; +} + +/* Whether this connection has to keep its own private key. TEST_PK_PRIVKEY + * asks for it on every version to model an application that never hands the + * library a private key; TLS v1.3 needs it either way, because the shared + * secret callback only receives the peer's key. Test the 1.3 versions rather + * than ordering the enum: the DTLS values sort above the TLS ones, so DTLS + * v1.2 would otherwise be taken for a 1.3. */ +static WC_INLINE int myEccKeepPrivKey(WOLFSSL* ssl) +{ +#ifdef TEST_PK_PRIVKEY + (void)ssl; + return 1; +#else + int version = wolfSSL_GetVersion(ssl); + + return version == WOLFSSL_TLSV1_3 || version == WOLFSSL_DTLSV1_3; +#endif +} + static WC_INLINE int myEccKeyGen(WOLFSSL* ssl, ecc_key* key, word32 keySz, int ecc_curve, void* ctx) { int ret; PkCbInfo* cbInfo = (PkCbInfo*)ctx; - ecc_key* new_key; - -#ifdef TEST_PK_PRIVKEY - new_key = cbInfo ? &cbInfo->keyGen.ecc : key; -#else - new_key = key; -#endif - - (void)ssl; - (void)cbInfo; + ecc_key* new_key = key; WOLFSSL_PKMSG("PK ECC KeyGen: keySz %u, Curve ID %d\n", keySz, ecc_curve); + if (cbInfo != NULL && myEccKeepPrivKey(ssl)) { + /* A key we already kept for this curve is stale - a TLS v1.3 client + * regenerates its key share when the server names a group in a + * HelloRetryRequest - so release it and take its slot back. */ + new_key = myEccKeptKey(cbInfo, ecc_curve); + if (new_key != NULL) { + wc_ecc_free(new_key); + } + else if (cbInfo->keyGenCnt < PKCB_MAX_ECC_KEYGEN) { + new_key = (ecc_key*)XMALLOC(sizeof(ecc_key), NULL, + DYNAMIC_TYPE_ECC); + if (new_key == NULL) { + WOLFSSL_PKMSG("PK ECC KeyGen: out of memory\n"); + return MEMORY_E; + } + /* Zero it before it is counted: a wc_ecc_init() failure below + * still leaves it for myEccFreeKeptKeys() to release. */ + XMEMSET(new_key, 0, sizeof(ecc_key)); + cbInfo->keyGen[cbInfo->keyGenCnt++] = new_key; + } + else { + WOLFSSL_PKMSG("PK ECC KeyGen: no room to keep curve %d\n", + ecc_curve); + return MEMORY_E; + } + } + ret = wc_ecc_init(new_key); if (ret == 0) { WC_RNG *rng = wolfSSL_GetRNG(ssl); @@ -3472,7 +3549,6 @@ static WC_INLINE int myEccKeyGen(WOLFSSL* ssl, ecc_key* key, word32 keySz, /* create new key */ ret = wc_ecc_make_key_ex(rng, (int) keySz, new_key, ecc_curve); - #ifdef TEST_PK_PRIVKEY if (ret == 0 && new_key != key) { byte qx[MAX_ECC_BYTES], qy[MAX_ECC_BYTES]; word32 qxLen = sizeof(qx), qyLen = sizeof(qy); @@ -3486,10 +3562,6 @@ static WC_INLINE int myEccKeyGen(WOLFSSL* ssl, ecc_key* key, word32 keySz, (void)qxLen; (void)qyLen; } - if (ret == 0 && cbInfo != NULL) { - cbInfo->hasKeyGen = 1; - } - #endif } WOLFSSL_PKMSG("PK ECC KeyGen: ret %d\n", ret); @@ -3620,32 +3692,39 @@ static WC_INLINE int myEccSharedSecret(WOLFSSL* ssl, ecc_key* otherKey, /* for client: create and export public key */ if (side == WOLFSSL_CLIENT_END) { - #ifdef TEST_PK_PRIVKEY - privKey = cbInfo ? &cbInfo->keyGen.ecc : &tmpKey; - #else - privKey = &tmpKey; - #endif pubKey = otherKey; /* TLS v1.2 and older we must generate a key here for the client only. - * TLS v1.3 calls key gen early with key share. Test the 1.3 versions - * rather than ordering the enum: the DTLS values sort above the TLS - * ones, so DTLS v1.2 would otherwise be taken for a 1.3 and skipped. */ + * TLS v1.3 calls key gen early with each key share it offers, and + * myEccKeepPrivKey() made that callback leave the private halves in + * cbInfo for us; the peer's key names the group the server settled on. + * Test the 1.3 versions rather than ordering the enum: the DTLS values + * sort above the TLS ones, so DTLS v1.2 would otherwise be taken for a + * 1.3 and skipped. */ if (version != WOLFSSL_TLSV1_3 && version != WOLFSSL_DTLSV1_3) { - ret = myEccKeyGen(ssl, privKey, 0, otherKey->dp->id, ctx); + ret = myEccKeyGen(ssl, &tmpKey, 0, otherKey->dp->id, ctx); if (ret == 0) { + privKey = myEccKeptKey(cbInfo, otherKey->dp->id); + if (privKey == NULL) + privKey = &tmpKey; ret = wc_ecc_export_x963(privKey, pubKeyDer, pubKeySz); } } + else { + privKey = myEccKeptKey(cbInfo, otherKey->dp->id); + if (privKey == NULL) { + WOLFSSL_PKMSG("PK ECC PMS: no key kept for curve %d\n", + otherKey->dp->id); + ret = ECC_CURVE_OID_E; + } + } } /* for server: import public key */ else if (side == WOLFSSL_SERVER_END) { - #ifdef TEST_PK_PRIVKEY - privKey = cbInfo ? &cbInfo->keyGen.ecc : otherKey; - #else - privKey = otherKey; - #endif + privKey = myEccKeptKey(cbInfo, otherKey->dp->id); + if (privKey == NULL) + privKey = otherKey; pubKey = &tmpKey; ret = wc_ecc_import_x963_ex(pubKeyDer, *pubKeySz, pubKey, @@ -3655,7 +3734,7 @@ static WC_INLINE int myEccSharedSecret(WOLFSSL* ssl, ecc_key* otherKey, ret = BAD_FUNC_ARG; } - if (privKey == NULL || pubKey == NULL) { + if (ret == 0 && (privKey == NULL || pubKey == NULL)) { ret = BAD_FUNC_ARG; } @@ -3678,13 +3757,6 @@ static WC_INLINE int myEccSharedSecret(WOLFSSL* ssl, ecc_key* otherKey, #endif } -#ifdef TEST_PK_PRIVKEY - if (cbInfo && cbInfo->hasKeyGen) { - wc_ecc_free(&cbInfo->keyGen.ecc); - cbInfo->hasKeyGen = 0; - } -#endif - wc_ecc_free(&tmpKey); WOLFSSL_PKMSG("PK ECC PMS: ret %d, PubKeySz %u, OutLen %u\n", ret, *pubKeySz, *outlen); @@ -4708,6 +4780,11 @@ static WC_INLINE void SetupPkCallbacks(WOLFSSL_CTX* ctx) static WC_INLINE void SetupPkCallbackContexts(WOLFSSL* ssl, void* myCtx) { #ifdef HAVE_ECC + /* The kept keys belong to one connection: whatever a previous one left + * behind is stale, and a TLS v1.2 handshake would otherwise derive + * with it instead of the key the library just handed us. */ + myEccFreeKeptKeys((PkCbInfo*)myCtx); + wolfSSL_SetEccKeyGenCtx(ssl, myCtx); wolfSSL_SetEccSignCtx(ssl, myCtx); wolfSSL_SetEccVerifyCtx(ssl, myCtx); @@ -4761,6 +4838,15 @@ static WC_INLINE void SetupPkCallbackContexts(WOLFSSL* ssl, void* myCtx) #endif } +/* Release what the callbacks kept for the last connection. */ +static WC_INLINE void CleanupPkCallbackContexts(void* myCtx) +{ + (void)myCtx; + #ifdef HAVE_ECC + myEccFreeKeptKeys((PkCbInfo*)myCtx); + #endif +} + #endif /* HAVE_PK_CALLBACKS */ #ifdef USE_WOLFSSL_IO