From 59665a44b509b9970efbdc012ea0fb53e0bcad39 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 18 Mar 2022 11:27:45 -0700 Subject: [PATCH 1/2] Fixes for allowing server to have a public key set when using external key with PK callbacks. --- src/internal.c | 95 +++++++++++++++++++++++++++++++-------------- src/ssl.c | 56 +++++++++++++++++++------- src/tls13.c | 26 ++++++++----- wolfcrypt/src/asn.c | 4 +- wolfssl/internal.h | 2 +- wolfssl/test.h | 7 +--- 6 files changed, 128 insertions(+), 62 deletions(-) diff --git a/src/internal.c b/src/internal.c index 0516b8efc..a5f3317fb 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5775,16 +5775,22 @@ int InitSSL_Suites(WOLFSSL* ssl) return NO_PRIVATE_KEY; } - /* allow no private key if using PK callbacks and CB is set */ - #ifdef HAVE_PK_CALLBACKS - if (wolfSSL_CTX_IsPrivatePkSet(ssl->ctx)) { - WOLFSSL_MSG("Using PK for server private key"); - } - else - #endif if (!ssl->buffers.key || !ssl->buffers.key->buffer) { - WOLFSSL_MSG("Server missing private key"); - return NO_PRIVATE_KEY; + /* allow no private key if using existing key */ + #ifdef WOLF_PRIVATE_KEY_ID + if (ssl->devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) + #endif + ) { + WOLFSSL_MSG("Allowing no server private key (external)"); + } + else + #endif + { + WOLFSSL_MSG("Server missing private key"); + return NO_PRIVATE_KEY; + } } } #endif @@ -22380,19 +22386,24 @@ int DecodePrivateKey(WOLFSSL *ssl, word16* length) int keySz; word32 idx; -#ifdef HAVE_PK_CALLBACKS - /* allow no private key if using PK callbacks and CB is set */ - if (wolfSSL_IsPrivatePkSet(ssl)) { - *length = GetPrivateKeySigSize(ssl); - return 0; - } - else -#endif - /* make sure private key exists */ if (ssl->buffers.key == NULL || ssl->buffers.key->buffer == NULL) { - WOLFSSL_MSG("Private key missing!"); - ERROR_OUT(NO_PRIVATE_KEY, exit_dpk); + /* allow no private key if using external */ + #ifdef WOLF_PRIVATE_KEY_ID + if (ssl->devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) + #endif + ) { + *length = GetPrivateKeySigSize(ssl); + return 0; + } + else + #endif + { + WOLFSSL_MSG("Private key missing!"); + ERROR_OUT(NO_PRIVATE_KEY, exit_dpk); + } } #ifdef WOLF_PRIVATE_KEY_ID @@ -22479,8 +22490,12 @@ int DecodePrivateKey(WOLFSSL *ssl, word16* length) ret = wc_RsaPrivateKeyDecode(ssl->buffers.key->buffer, &idx, (RsaKey*)ssl->hsKey, ssl->buffers.key->length); #ifdef WOLF_PRIVATE_KEY_ID - /* if using crypto or PK callbacks allow using a public key */ - if (ret != 0 && ssl->devId != INVALID_DEVID) { + /* if using external key then allow using a public key */ + if (ret != 0 && (ssl->devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) + #endif + )) { WOLFSSL_MSG("Trying RSA public key with crypto callbacks"); idx = 0; ret = wc_RsaPublicKeyDecode(ssl->buffers.key->buffer, &idx, @@ -22534,8 +22549,12 @@ int DecodePrivateKey(WOLFSSL *ssl, word16* length) (ecc_key*)ssl->hsKey, ssl->buffers.key->length); #ifdef WOLF_PRIVATE_KEY_ID - /* if using crypto or PK callbacks allow using a public key */ - if (ret != 0 && ssl->devId != INVALID_DEVID) { + /* if using external key then allow using a public key */ + if (ret != 0 && (ssl->devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) + #endif + )) { WOLFSSL_MSG("Trying ECC public key with crypto callbacks"); idx = 0; ret = wc_EccPublicKeyDecode(ssl->buffers.key->buffer, &idx, @@ -22587,13 +22606,17 @@ int DecodePrivateKey(WOLFSSL *ssl, word16* length) (ed25519_key*)ssl->hsKey, ssl->buffers.key->length); #ifdef WOLF_PRIVATE_KEY_ID - /* if using crypto or PK callbacks allow using a public key */ - if (ret != 0 && ssl->devId != INVALID_DEVID) { + /* if using external key then allow using a public key */ + if (ret != 0 && (ssl->devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) + #endif + )) { WOLFSSL_MSG("Trying ED25519 public key with crypto callbacks"); idx = 0; ret = wc_Ed25519PublicKeyDecode(ssl->buffers.key->buffer, &idx, - (ed25519_key*)ssl->hsKey, - ssl->buffers.key->length); + (ed25519_key*)ssl->hsKey, + ssl->buffers.key->length); } #endif if (ret == 0) { @@ -22640,6 +22663,20 @@ int DecodePrivateKey(WOLFSSL *ssl, word16* length) ret = wc_Ed448PrivateKeyDecode(ssl->buffers.key->buffer, &idx, (ed448_key*)ssl->hsKey, ssl->buffers.key->length); + #ifdef WOLF_PRIVATE_KEY_ID + /* if using external key then allow using a public key */ + if (ret != 0 && (ssl->devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) + #endif + )) { + WOLFSSL_MSG("Trying ED25519 public key with crypto callbacks"); + idx = 0; + ret = wc_Ed448PublicKeyDecode(ssl->buffers.key->buffer, &idx, + (ed448_key*)ssl->hsKey, + ssl->buffers.key->length); + } + #endif if (ret == 0) { WOLFSSL_MSG("Using ED448 private key"); @@ -26876,7 +26913,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, #ifndef NO_CERTS -#ifdef HAVE_PK_CALLBACKS +#ifdef WOLF_PRIVATE_KEY_ID int GetPrivateKeySigSize(WOLFSSL* ssl) { int sigSz = 0; diff --git a/src/ssl.c b/src/ssl.c index 5cbd9332a..9f3fe33e1 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -5697,7 +5697,11 @@ static int ProcessBufferTryDecode(WOLFSSL_CTX* ctx, WOLFSSL* ssl, DerBuffer* der *idx = 0; ret = wc_RsaPrivateKeyDecode(der->buffer, idx, key, der->length); #ifdef WOLF_PRIVATE_KEY_ID - if (ret != 0 && devId != INVALID_DEVID) { + if (ret != 0 && (devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ctx) + #endif + )) { /* if using crypto or PK callbacks, try public key decode */ *idx = 0; ret = wc_RsaPublicKeyDecode(der->buffer, idx, key, der->length); @@ -5769,7 +5773,11 @@ static int ProcessBufferTryDecode(WOLFSSL_CTX* ctx, WOLFSSL* ssl, DerBuffer* der *idx = 0; ret = wc_EccPrivateKeyDecode(der->buffer, idx, key, der->length); #ifdef WOLF_PRIVATE_KEY_ID - if (ret != 0 && devId != INVALID_DEVID) { + if (ret != 0 && (devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ctx) + #endif + )) { /* if using crypto or PK callbacks, try public key decode */ *idx = 0; ret = wc_EccPublicKeyDecode(der->buffer, idx, key, der->length); @@ -5836,7 +5844,11 @@ static int ProcessBufferTryDecode(WOLFSSL_CTX* ctx, WOLFSSL* ssl, DerBuffer* der *idx = 0; ret = wc_Ed25519PrivateKeyDecode(der->buffer, idx, key, der->length); #ifdef WOLF_PRIVATE_KEY_ID - if (ret != 0 && devId != INVALID_DEVID) { + if (ret != 0 && (devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ctx) + #endif + )) { /* if using crypto or PK callbacks, try public key decode */ *idx = 0; ret = wc_Ed25519PublicKeyDecode(der->buffer, idx, key, der->length); @@ -5905,6 +5917,17 @@ static int ProcessBufferTryDecode(WOLFSSL_CTX* ctx, WOLFSSL* ssl, DerBuffer* der if (ret == 0) { *idx = 0; ret = wc_Ed448PrivateKeyDecode(der->buffer, idx, key, der->length); + #ifdef WOLF_PRIVATE_KEY_ID + if (ret != 0 && (devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ctx) + #endif + )) { + /* if using crypto or PK callbacks, try public key decode */ + *idx = 0; + ret = wc_Ed448PublicKeyDecode(der->buffer, idx, key, der->length); + } + #endif if (ret == 0) { /* check for minimum key size and then free */ int minKeySz = ssl ? ssl->options.minEccKeySz : @@ -14823,7 +14846,6 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, #ifndef NO_CERTS /* in case used set_accept_state after init */ - /* allow no private key if using PK callbacks and CB is set */ if (!havePSK && !haveAnon && !haveMcast) { #ifdef OPENSSL_EXTRA if (ssl->ctx->certSetupCb != NULL) { @@ -14842,17 +14864,23 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, return WOLFSSL_FATAL_ERROR; } - #ifdef HAVE_PK_CALLBACKS - if (wolfSSL_CTX_IsPrivatePkSet(ssl->ctx)) { - WOLFSSL_MSG("Using PK for server private key"); - } - else - #endif if (!ssl->buffers.key || !ssl->buffers.key->buffer) { - WOLFSSL_MSG("accept error: server key required"); - ssl->error = NO_PRIVATE_KEY; - WOLFSSL_ERROR(ssl->error); - return WOLFSSL_FATAL_ERROR; + /* allow no private key if using existing key */ + #ifdef WOLF_PRIVATE_KEY_ID + if (ssl->devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) + #endif + ) { + WOLFSSL_MSG("Allowing no server private key (external)"); + } + else + #endif + { + WOLFSSL_MSG("accept error: server key required"); + WOLFSSL_ERROR(ssl->error = NO_PRIVATE_KEY); + return WOLFSSL_FATAL_ERROR; + } } } } diff --git a/src/tls13.c b/src/tls13.c index b92a0c882..7c467c6fe 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -9520,7 +9520,6 @@ int wolfSSL_accept_TLSv13(WOLFSSL* ssl) #endif /* WOLFSSL_WOLFSENTRY_HOOKS */ #ifndef NO_CERTS - /* allow no private key if using PK callbacks and CB is set */ #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) if (!havePSK) #endif @@ -9542,16 +9541,23 @@ int wolfSSL_accept_TLSv13(WOLFSSL* ssl) return WOLFSSL_FATAL_ERROR; } - #ifdef HAVE_PK_CALLBACKS - if (wolfSSL_CTX_IsPrivatePkSet(ssl->ctx)) { - WOLFSSL_MSG("Using PK for server private key"); - } - else - #endif if (!ssl->buffers.key || !ssl->buffers.key->buffer) { - WOLFSSL_MSG("accept error: server key required"); - WOLFSSL_ERROR(ssl->error = NO_PRIVATE_KEY); - return WOLFSSL_FATAL_ERROR; + /* allow no private key if using existing key */ + #ifdef WOLF_PRIVATE_KEY_ID + if (ssl->devId != INVALID_DEVID + #ifdef HAVE_PK_CALLBACKS + || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) + #endif + ) { + WOLFSSL_MSG("Allowing no server private key (external)"); + } + else + #endif + { + WOLFSSL_MSG("accept error: server key required"); + WOLFSSL_ERROR(ssl->error = NO_PRIVATE_KEY); + return WOLFSSL_FATAL_ERROR; + } } } } diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 6a38d0c48..6964957bc 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -19802,7 +19802,7 @@ int PemToDer(const unsigned char* buff, long longSz, int type, } #endif else { - #if defined(WOLF_CRYPTO_CB) || defined(HAVE_PK_CALLBACKS) + #ifdef WOLF_PRIVATE_KEY_ID /* allow loading a public key for use with crypto or PK callbacks */ type = PUBLICKEY_TYPE; header = BEGIN_PUB_KEY; @@ -19926,7 +19926,7 @@ int PemToDer(const unsigned char* buff, long longSz, int type, *keyFormat = DSAk; #endif } - #if defined(WOLF_CRYPTO_CB) || defined(HAVE_PK_CALLBACKS) + #ifdef WOLF_PRIVATE_KEY_ID else if (type == PUBLICKEY_TYPE) { #ifndef NO_RSA if (header == BEGIN_RSA_PUB) diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 652d5dfea..bea8af6e0 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1735,7 +1735,7 @@ WOLFSSL_LOCAL int CreateDevPrivateKey(void** pkey, byte* data, word32 length, void* heap, int devId); #endif WOLFSSL_LOCAL int DecodePrivateKey(WOLFSSL *ssl, word16* length); -#ifdef HAVE_PK_CALLBACKS +#ifdef WOLF_PRIVATE_KEY_ID WOLFSSL_LOCAL int GetPrivateKeySigSize(WOLFSSL* ssl); #ifndef NO_ASN WOLFSSL_LOCAL int InitSigPkCb(WOLFSSL* ssl, SignatureCtx* sigCtx); diff --git a/wolfssl/test.h b/wolfssl/test.h index d0abe2dfd..4edc4c312 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -3729,14 +3729,9 @@ typedef struct PkCbInfo { #ifdef TEST_PK_PRIVKEY union { #ifdef HAVE_ECC + /* only ECC PK callback with TLS v1.2 needs this */ ecc_key ecc; #endif - #ifdef HAVE_CURVE25519 - curve25519_key curve; - #endif - #ifdef HAVE_CURVE448 - curve448_key curve; - #endif } keyGen; int hasKeyGen; #endif From aa38d995389f2bb78613d29b89d020586a98d38b Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 21 Mar 2022 13:45:40 -0700 Subject: [PATCH 2/2] Fix for TLS PK callback issue with Ed25519/Ed448 and public key not being set. --- src/internal.c | 6 +++--- src/ssl.c | 12 ++++++++---- src/tls13.c | 2 +- wolfssl/test.h | 14 ++++++++++++-- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/internal.c b/src/internal.c index a5f3317fb..88403d1cf 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4877,7 +4877,7 @@ int Ed25519CheckPubKey(WOLFSSL* ssl) int ret = 0; /* Public key required for signing. */ - if (!key->pubKeySet) { + if (key != NULL && !key->pubKeySet) { DerBuffer* leaf = ssl->buffers.certificate; DecodedCert* cert = (DecodedCert*)XMALLOC(sizeof(*cert), ssl->heap, DYNAMIC_TYPE_DCERT); @@ -5211,7 +5211,7 @@ int Ed448CheckPubKey(WOLFSSL* ssl) int ret = 0; /* Public key required for signing. */ - if (!key->pubKeySet) { + if (key != NULL && !key->pubKeySet) { DerBuffer* leaf = ssl->buffers.certificate; DecodedCert* cert = (DecodedCert*)XMALLOC(sizeof(*cert), ssl->heap, DYNAMIC_TYPE_DCERT); @@ -5786,7 +5786,7 @@ int InitSSL_Suites(WOLFSSL* ssl) WOLFSSL_MSG("Allowing no server private key (external)"); } else - #endif + #endif { WOLFSSL_MSG("Server missing private key"); return NO_PRIVATE_KEY; diff --git a/src/ssl.c b/src/ssl.c index 9f3fe33e1..115d368ce 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -5851,7 +5851,8 @@ static int ProcessBufferTryDecode(WOLFSSL_CTX* ctx, WOLFSSL* ssl, DerBuffer* der )) { /* if using crypto or PK callbacks, try public key decode */ *idx = 0; - ret = wc_Ed25519PublicKeyDecode(der->buffer, idx, key, der->length); + ret = wc_Ed25519PublicKeyDecode(der->buffer, idx, key, + der->length); } #endif if (ret == 0) { @@ -5925,7 +5926,8 @@ static int ProcessBufferTryDecode(WOLFSSL_CTX* ctx, WOLFSSL* ssl, DerBuffer* der )) { /* if using crypto or PK callbacks, try public key decode */ *idx = 0; - ret = wc_Ed448PublicKeyDecode(der->buffer, idx, key, der->length); + ret = wc_Ed448PublicKeyDecode(der->buffer, idx, key, + der->length); } #endif if (ret == 0) { @@ -6131,7 +6133,8 @@ int ProcessBuffer(WOLFSSL_CTX* ctx, const unsigned char* buff, #ifdef HAVE_PKCS8 /* if private key try and remove PKCS8 header */ if (type == PRIVATEKEY_TYPE) { - if ((ret = ToTraditional_ex(der->buffer, der->length, &algId)) > 0) { + if ((ret = ToTraditional_ex(der->buffer, der->length, + &algId)) > 0) { /* Found PKCS8 header */ /* ToTraditional_ex moves buff and returns adjusted length */ der->length = ret; @@ -14872,7 +14875,8 @@ int wolfSSL_DTLS_SetCookieSecret(WOLFSSL* ssl, || wolfSSL_CTX_IsPrivatePkSet(ssl->ctx) #endif ) { - WOLFSSL_MSG("Allowing no server private key (external)"); + WOLFSSL_MSG("Allowing no server private key " + "(external)"); } else #endif diff --git a/src/tls13.c b/src/tls13.c index 7c467c6fe..d5f0b0c6e 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -9552,7 +9552,7 @@ int wolfSSL_accept_TLSv13(WOLFSSL* ssl) WOLFSSL_MSG("Allowing no server private key (external)"); } else - #endif + #endif { WOLFSSL_MSG("accept error: server key required"); WOLFSSL_ERROR(ssl->error = NO_PRIVATE_KEY); diff --git a/wolfssl/test.h b/wolfssl/test.h index 4edc4c312..d3f57ad46 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -4027,8 +4027,13 @@ static WC_INLINE int myEd25519Sign(WOLFSSL* ssl, const byte* in, word32 inSz, ret = wc_ed25519_init(&myKey); if (ret == 0) { ret = wc_Ed25519PrivateKeyDecode(keyBuf, &idx, &myKey, keySz); - if (ret == 0) + if (ret == 0) { + ret = wc_ed25519_make_public(&myKey, myKey.p, ED25519_PUB_KEY_SIZE); + } + if (ret == 0) { + myKey.pubKeySet = 1; ret = wc_ed25519_sign_msg(in, inSz, out, outSz, &myKey); + } wc_ed25519_free(&myKey); } @@ -4191,8 +4196,13 @@ static WC_INLINE int myEd448Sign(WOLFSSL* ssl, const byte* in, word32 inSz, ret = wc_ed448_init(&myKey); if (ret == 0) { ret = wc_Ed448PrivateKeyDecode(keyBuf, &idx, &myKey, keySz); - if (ret == 0) + if (ret == 0) { + ret = wc_ed448_make_public(&myKey, myKey.p, ED448_PUB_KEY_SIZE); + } + if (ret == 0) { + myKey.pubKeySet = 1; ret = wc_ed448_sign_msg(in, inSz, out, outSz, &myKey, NULL, 0); + } wc_ed448_free(&myKey); }