From 1f6314746cda1f659d37b67a1dca5d0d8de7fcfd Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 19 Feb 2019 15:04:22 -0800 Subject: [PATCH 1/8] Secure Renegotiation 1. Split the wolfSSL_Rehandshake() function into wolfSSL_Rehadshake() which performs a full handshake on secure renegotiation and wolfSSL_SecureResume() which performs a session resumption on a secure renegotiation. 2. Add option to example client to perform a secure resumption instead of a full secure handshake. --- examples/client/client.c | 46 ++++++++++++++++++++++++++++++++-------- src/internal.c | 4 ++-- src/ssl.c | 22 ++++++++++++++++++- wolfssl/internal.h | 2 +- wolfssl/ssl.h | 2 ++ 5 files changed, 63 insertions(+), 13 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 339f1722c..474e89e7d 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -1208,6 +1208,7 @@ static void Usage(void) #ifdef HAVE_SECURE_RENEGOTIATION printf("%s", msg[++msgid]); /* -R */ printf("%s", msg[++msgid]); /* -i */ + printf("-4 Use resumption for renegotiation\n"); #endif printf("%s", msg[++msgid]); /* -f */ printf("%s", msg[++msgid]); /* -x */ @@ -1337,6 +1338,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) int err = 0; int scr = 0; /* allow secure renegotiation */ int forceScr = 0; /* force client initiaed scr */ + int resumeScr = 0; /* use resumption for renegotiation */ #ifndef WOLFSSL_NO_CLIENT_AUTH int useClientCert = 1; #else @@ -1452,6 +1454,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) (void)atomicUser; (void)scr; (void)forceScr; + (void)resumeScr; (void)ourKey; (void)ourCert; (void)verifyCert; @@ -1478,7 +1481,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) while ((ch = mygetopt(argc, argv, "?:" "ab:c:defgh:ijk:l:mnop:q:rstuv:wxyz" "A:B:CDE:F:GH:IJKL:M:NO:PQRS:TUVW:XYZ:" - "01:23:")) != -1) { + "01:23:4")) != -1) { switch (ch) { case '?' : if(myoptarg!=NULL) { @@ -1892,6 +1895,14 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) #endif break; + case '4' : + #ifdef HAVE_SECURE_RENEGOTIATION + scr = 1; + forceScr = 1; + resumeScr = 1; + #endif + break; + default: Usage(); XEXIT_T(MY_EX_USAGE); @@ -2826,16 +2837,33 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) printf("not doing secure renegotiation on example with" " nonblocking yet"); } else { - if (wolfSSL_Rehandshake(ssl) != WOLFSSL_SUCCESS) { - err = wolfSSL_get_error(ssl, 0); - printf("err = %d, %s\n", err, - wolfSSL_ERR_error_string(err, buffer)); - wolfSSL_free(ssl); ssl = NULL; - wolfSSL_CTX_free(ctx); ctx = NULL; - err_sys("wolfSSL_Rehandshake failed"); + if (!resumeScr) { + printf("Beginning secure rengotiation.\n"); + if (wolfSSL_Rehandshake(ssl) != WOLFSSL_SUCCESS) { + err = wolfSSL_get_error(ssl, 0); + printf("err = %d, %s\n", err, + wolfSSL_ERR_error_string(err, buffer)); + wolfSSL_free(ssl); ssl = NULL; + wolfSSL_CTX_free(ctx); ctx = NULL; + err_sys("wolfSSL_Rehandshake failed"); + } + else { + printf("RENEGOTIATION SUCCESSFUL\n"); + } } else { - printf("RENEGOTIATION SUCCESSFUL\n"); + printf("Beginning secure resumption.\n"); + if (wolfSSL_SecureResume(ssl) != WOLFSSL_SUCCESS) { + err = wolfSSL_get_error(ssl, 0); + printf("err = %d, %s\n", err, + wolfSSL_ERR_error_string(err, buffer)); + wolfSSL_free(ssl); ssl = NULL; + wolfSSL_CTX_free(ctx); ctx = NULL; + err_sys("wolfSSL_SecureResume failed"); + } + else { + printf("SECURE RESUMPTION SUCCESSFUL\n"); + } } } } diff --git a/src/internal.c b/src/internal.c index 16b35131b..56c3b8423 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9571,7 +9571,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, /* compare against previous time */ if (XMEMCMP(args->dCert->subjectHash, ssl->secure_renegotiation->subject_hash, - WC_SHA_DIGEST_SIZE) != 0) { + KEYID_SIZE) != 0) { WOLFSSL_MSG( "Peer sent different cert during scr, fatal"); args->fatal = 1; @@ -9582,7 +9582,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, /* cache peer's hash */ if (args->fatal == 0) { XMEMCPY(ssl->secure_renegotiation->subject_hash, - args->dCert->subjectHash, WC_SHA_DIGEST_SIZE); + args->dCert->subjectHash, KEYID_SIZE); } } #endif /* HAVE_SECURE_RENEGOTIATION */ diff --git a/src/ssl.c b/src/ssl.c index fc3442a61..175e3460a 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -2347,7 +2347,7 @@ int wolfSSL_UseSecureRenegotiation(WOLFSSL* ssl) /* do a secure renegotiation handshake, user forced, we discourage */ -int wolfSSL_Rehandshake(WOLFSSL* ssl) +int wolfSSL_StartSecureRenegotiation(WOLFSSL* ssl, int resume) { int ret; @@ -2364,6 +2364,9 @@ int wolfSSL_Rehandshake(WOLFSSL* ssl) return SECURE_RENEGOTIATION_E; } + if (!resume) + ssl->options.resuming = 0; + /* If the client started the renegotiation, the server will already * have processed the client's hello. */ if (ssl->options.side != WOLFSSL_SERVER_END || @@ -2383,6 +2386,11 @@ int wolfSSL_Rehandshake(WOLFSSL* ssl) } #endif + if (!resume) { + XMEMSET(ssl->session.sessionID, 0, ID_LEN); + ssl->session.sessionIDSz = 0; + } + /* reset handshake states */ ssl->options.serverState = NULL_STATE; ssl->options.clientState = NULL_STATE; @@ -2411,6 +2419,18 @@ int wolfSSL_Rehandshake(WOLFSSL* ssl) return ret; } + +int wolfSSL_Rehandshake(WOLFSSL* ssl) { + WOLFSSL_ENTER("wolfSSL_Rehandshake()"); + return wolfSSL_StartSecureRenegotiation(ssl, 0); +} + + +int wolfSSL_SecureResume(WOLFSSL* ssl) { + WOLFSSL_ENTER("wolfSSL_SecureResume()"); + return wolfSSL_StartSecureRenegotiation(ssl, 1); +} + #endif /* HAVE_SECURE_RENEGOTIATION */ /* Session Ticket */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index d4fbefaba..5d7e94df5 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2286,7 +2286,7 @@ typedef struct SecureRenegotiation { enum key_cache_state cache_status; /* track key cache state */ byte client_verify_data[TLS_FINISHED_SZ]; /* cached */ byte server_verify_data[TLS_FINISHED_SZ]; /* cached */ - byte subject_hash[WC_SHA_DIGEST_SIZE]; /* peer cert hash */ + byte subject_hash[KEYID_SIZE]; /* peer cert hash */ Keys tmp_keys; /* can't overwrite real keys yet */ } SecureRenegotiation; diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 44bc2367b..b650947f5 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -2433,7 +2433,9 @@ WOLFSSL_API int wolfSSL_NoKeyShares(WOLFSSL* ssl); #ifdef HAVE_SECURE_RENEGOTIATION WOLFSSL_API int wolfSSL_UseSecureRenegotiation(WOLFSSL* ssl); +WOLFSSL_API int wolfSSL_StartSecureRenegotiation(WOLFSSL* ssl, int resume); WOLFSSL_API int wolfSSL_Rehandshake(WOLFSSL* ssl); +WOLFSSL_API int wolfSSL_SecureResume(WOLFSSL* ssl); #endif From f78ba4649bfba74a104da55526bedd11621a161b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 20 Feb 2019 11:23:00 -0800 Subject: [PATCH 2/8] Update the help text so the Japanese translations of the new options are printed. --- examples/client/client.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index 474e89e7d..748c0a299 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -983,6 +983,11 @@ static const char* client_usage_msg[][59] = { #endif "-1 Display a result by specified language.\n" " 0: English, 1: Japanese\n", /* 58 */ +#if !defined(NO_DH) && !defined(HAVE_FIPS) && \ + !defined(HAVE_SELFTEST) && !defined(WOLFSSL_OLD_PRIME_CHECK) + "-2 Disable DH Prime check\n", /* 59 */ +#endif + "-4 Use resumption for renegotiation\n", /* 60 */ NULL, }, #ifndef NO_MULTIBYTE_PRINT @@ -1134,6 +1139,13 @@ static const char* client_usage_msg[][59] = { #endif "-1 指定された言語で結果を表示します。\n" " 0: 英語、 1: 日本語\n", /* 58 */ +#if !defined(NO_DH) && !defined(HAVE_FIPS) && \ + !defined(HAVE_SELFTEST) && !defined(WOLFSSL_OLD_PRIME_CHECK) + "-2 DHプライム番号チェックを無効にする\n", /* 59 */ +#endif +#ifdef HAVE_SECURE_RENEGOTIATION + "-4 再交渉に再開を使用\n", /* 60 */ +#endif NULL, }, #endif @@ -1208,7 +1220,6 @@ static void Usage(void) #ifdef HAVE_SECURE_RENEGOTIATION printf("%s", msg[++msgid]); /* -R */ printf("%s", msg[++msgid]); /* -i */ - printf("-4 Use resumption for renegotiation\n"); #endif printf("%s", msg[++msgid]); /* -f */ printf("%s", msg[++msgid]); /* -x */ @@ -1276,14 +1287,17 @@ static void Usage(void) #ifdef WOLFSSL_EARLY_DATA printf("%s", msg[++msgid]); /* -0 */ #endif -#if !defined(NO_DH) && !defined(HAVE_FIPS) && \ - !defined(HAVE_SELFTEST) && !defined(WOLFSSL_OLD_PRIME_CHECK) - printf("-2 Disable DH Prime check\n"); -#endif #ifdef WOLFSSL_MULTICAST printf("%s", msg[++msgid]); /* -3 */ #endif printf("%s", msg[++msgid]); /* -1 */ +#if !defined(NO_DH) && !defined(HAVE_FIPS) && \ + !defined(HAVE_SELFTEST) && !defined(WOLFSSL_OLD_PRIME_CHECK) + printf("%s", msg[++msgid]); /* -2 */ +#endif +#ifdef HAVE_SECURE_RENEGOTIATION + printf("%s", msg[++msgid]); /* -4 */ +#endif } THREAD_RETURN WOLFSSL_THREAD client_test(void* args) From a376e17aee758d3b28d6bb0d65fa2fd66b746bca Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 20 Feb 2019 11:26:33 -0800 Subject: [PATCH 3/8] Switch the bound for the XMEMSET of the sessionID when starting a renegotiation to use sizeof the sessionID rather than the constat used to set the size of the array. --- src/ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index 175e3460a..9dd044898 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -2387,7 +2387,7 @@ int wolfSSL_StartSecureRenegotiation(WOLFSSL* ssl, int resume) #endif if (!resume) { - XMEMSET(ssl->session.sessionID, 0, ID_LEN); + XMEMSET(ssl->session.sessionID, 0, sizeof(ssl->session.sessionID)); ssl->session.sessionIDSz = 0; } From 7389553bd6505cbe3c7760ed3277e32baa00ddca Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 20 Feb 2019 11:45:21 -0800 Subject: [PATCH 4/8] 1. For secure renegotiation, remove the check of the peer certificate's subject ID on renegotiation. Both endpoints are already cryptographically linked on an encrypted channel. 2. The error code list has gaps where deprecated codes were deleted, remove the redundant gaps where there aren't missing codes. --- src/internal.c | 27 --------------------------- wolfssl/error-ssl.h | 6 +----- wolfssl/internal.h | 1 - 3 files changed, 1 insertion(+), 33 deletions(-) diff --git a/src/internal.c b/src/internal.c index 56c3b8423..4b219fcc5 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9562,30 +9562,6 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, #endif } } - - #ifdef HAVE_SECURE_RENEGOTIATION - if (args->fatal == 0 && ssl->secure_renegotiation - && ssl->secure_renegotiation->enabled) { - - if (IsEncryptionOn(ssl, 0)) { - /* compare against previous time */ - if (XMEMCMP(args->dCert->subjectHash, - ssl->secure_renegotiation->subject_hash, - KEYID_SIZE) != 0) { - WOLFSSL_MSG( - "Peer sent different cert during scr, fatal"); - args->fatal = 1; - ret = SCR_DIFFERENT_CERT_E; - } - } - - /* cache peer's hash */ - if (args->fatal == 0) { - XMEMCPY(ssl->secure_renegotiation->subject_hash, - args->dCert->subjectHash, KEYID_SIZE); - } - } - #endif /* HAVE_SECURE_RENEGOTIATION */ } /* if (count > 0) */ /* Check for error */ @@ -15757,9 +15733,6 @@ const char* wolfSSL_ERR_reason_error_string(unsigned long e) case SESSION_TICKET_EXPECT_E: return "Session Ticket Error"; - case SCR_DIFFERENT_CERT_E: - return "Peer sent different cert during SCR"; - case SESSION_SECRET_CB_E: return "Session Secret Callback Error"; diff --git a/wolfssl/error-ssl.h b/wolfssl/error-ssl.h index 827ae0272..3f0ae944d 100644 --- a/wolfssl/error-ssl.h +++ b/wolfssl/error-ssl.h @@ -119,30 +119,26 @@ enum wolfSSL_ErrorCodes { SECURE_RENEGOTIATION_E = -388, /* Invalid Renegotiation Info */ SESSION_TICKET_LEN_E = -389, /* Session Ticket too large */ SESSION_TICKET_EXPECT_E = -390, /* Session Ticket missing */ - SCR_DIFFERENT_CERT_E = -391, /* SCR Different cert error */ + SESSION_SECRET_CB_E = -392, /* Session secret Cb fcn failure */ NO_CHANGE_CIPHER_E = -393, /* Finished before change cipher */ SANITY_MSG_E = -394, /* Sanity check on msg order error */ DUPLICATE_MSG_E = -395, /* Duplicate message error */ SNI_UNSUPPORTED = -396, /* SSL 3.0 does not support SNI */ SOCKET_PEER_CLOSED_E = -397, /* Underlying transport closed */ - BAD_TICKET_KEY_CB_SZ = -398, /* Bad session ticket key cb size */ BAD_TICKET_MSG_SZ = -399, /* Bad session ticket msg size */ BAD_TICKET_ENCRYPT = -400, /* Bad user ticket encrypt */ - DH_KEY_SIZE_E = -401, /* DH Key too small */ SNI_ABSENT_ERROR = -402, /* No SNI request. */ RSA_SIGN_FAULT = -403, /* RSA Sign fault */ HANDSHAKE_SIZE_ERROR = -404, /* Handshake message too large */ - UNKNOWN_ALPN_PROTOCOL_NAME_E = -405, /* Unrecognized protocol name Error*/ BAD_CERTIFICATE_STATUS_ERROR = -406, /* Bad certificate status message */ OCSP_INVALID_STATUS = -407, /* Invalid OCSP Status */ OCSP_WANT_READ = -408, /* OCSP callback response WOLFSSL_CBIO_ERR_WANT_READ */ RSA_KEY_SIZE_E = -409, /* RSA key too small */ ECC_KEY_SIZE_E = -410, /* ECC key too small */ - DTLS_EXPORT_VER_E = -411, /* export version error */ INPUT_SIZE_E = -412, /* input size too big error */ CTX_INIT_MUTEX_E = -413, /* initialize ctx mutex error */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 5d7e94df5..fd82aa107 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2286,7 +2286,6 @@ typedef struct SecureRenegotiation { enum key_cache_state cache_status; /* track key cache state */ byte client_verify_data[TLS_FINISHED_SZ]; /* cached */ byte server_verify_data[TLS_FINISHED_SZ]; /* cached */ - byte subject_hash[KEYID_SIZE]; /* peer cert hash */ Keys tmp_keys; /* can't overwrite real keys yet */ } SecureRenegotiation; From 39626bb34962a6850bf0102bc6e8cbd1a63d8910 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 21 Feb 2019 10:06:55 -0800 Subject: [PATCH 5/8] 1. Add a newline to the client's "non-blocking socket and renegotiation" notice. 2. Add suite test cases for more renegotiation setting combinations. --- examples/client/client.c | 2 +- tests/test.conf | 60 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/examples/client/client.c b/examples/client/client.c index 748c0a299..ae6f358a8 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -2849,7 +2849,7 @@ THREAD_RETURN WOLFSSL_THREAD client_test(void* args) if (scr && forceScr) { if (nonBlocking) { printf("not doing secure renegotiation on example with" - " nonblocking yet"); + " nonblocking yet\n"); } else { if (!resumeScr) { printf("Beginning secure rengotiation.\n"); diff --git a/tests/test.conf b/tests/test.conf index faad62e6e..eaece3e6e 100644 --- a/tests/test.conf +++ b/tests/test.conf @@ -2378,3 +2378,63 @@ -c ./certs/client-ecc384-cert.pem -k ./certs/client-ecc384-key.pem -A ./certs/ca-ecc384-cert.pem + +# server TLSv1.2 default with secure renegotiation (renegotiation available) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-M + +# client TLSv1.2 default with secure renegotiation (client initiated) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-i + +# server TLSv1.2 default with secure renegotiation (renegotiation available) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-M + +# client TLSv1.2 default with secure renegotiation (client initiated as resume) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-i -4 + +# server TLSv1.2 default with secure renegotiation (server initiated) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-m + +# client TLSv1.2 default with secure renegotiation (renegotiation available) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-R + +# server TLSv1.2 default with secure renegotiation (server initiated) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-m + +# client TLSv1.2 default with secure renegotiation (renegotiation available as resume) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-R -4 + +# server TLSv1.2 default with secure renegotiation (server initiated) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-m + +# client TLSv1.2 default with secure renegotiation (client initiated) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-i + +# server TLSv1.2 default with secure renegotiation (server initiated) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-m + +# client TLSv1.2 default with secure renegotiation (client initiated as resume) +-v 3 +-l ECDHE-RSA-AES128-GCM-SHA256 +-i -4 From 57d8e070f94c0eed72d1344f5537c4c222233d41 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 26 Feb 2019 14:10:44 -0800 Subject: [PATCH 6/8] 1. Remove the clearing of the sessionID from Rehandshake. 2. Put SecureResume in terms of a regular resume, using Get/SetSession and then calling Rehandshake. 3. Add the startScr after checking secure_renegotiation enabled during a resume. --- src/internal.c | 8 ++++++++ src/ssl.c | 43 +++++++++++++++++++++++-------------------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/internal.c b/src/internal.c index 4b219fcc5..c4d670343 100644 --- a/src/internal.c +++ b/src/internal.c @@ -23898,6 +23898,14 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ret = HandleTlsResumption(ssl, bogusID, &clSuites); if (ret != 0) return ret; + + #ifdef HAVE_SECURE_RENEGOTIATION + if (ssl->secure_renegotiation && + ssl->secure_renegotiation->enabled && + IsEncryptionOn(ssl, 0)) + ssl->secure_renegotiation->startScr = 1; + #endif + if (ssl->options.clientState == CLIENT_KEYEXCHANGE_COMPLETE) { WOLFSSL_LEAVE("DoClientHello", ret); WOLFSSL_END(WC_FUNC_CLIENT_HELLO_DO); diff --git a/src/ssl.c b/src/ssl.c index 9dd044898..4db81dcaf 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -2347,7 +2347,7 @@ int wolfSSL_UseSecureRenegotiation(WOLFSSL* ssl) /* do a secure renegotiation handshake, user forced, we discourage */ -int wolfSSL_StartSecureRenegotiation(WOLFSSL* ssl, int resume) +int wolfSSL_Rehandshake(WOLFSSL* ssl) { int ret; @@ -2364,9 +2364,6 @@ int wolfSSL_StartSecureRenegotiation(WOLFSSL* ssl, int resume) return SECURE_RENEGOTIATION_E; } - if (!resume) - ssl->options.resuming = 0; - /* If the client started the renegotiation, the server will already * have processed the client's hello. */ if (ssl->options.side != WOLFSSL_SERVER_END || @@ -2386,12 +2383,8 @@ int wolfSSL_StartSecureRenegotiation(WOLFSSL* ssl, int resume) } #endif - if (!resume) { - XMEMSET(ssl->session.sessionID, 0, sizeof(ssl->session.sessionID)); - ssl->session.sessionIDSz = 0; - } - /* reset handshake states */ + ssl->options.sendVerify = 0; ssl->options.serverState = NULL_STATE; ssl->options.clientState = NULL_STATE; ssl->options.connectState = CONNECT_BEGIN; @@ -2406,29 +2399,39 @@ int wolfSSL_StartSecureRenegotiation(WOLFSSL* ssl, int resume) #if !defined(NO_WOLFSSL_SERVER) && defined(HAVE_SERVER_RENEGOTIATION_INFO) if (ssl->options.side == WOLFSSL_SERVER_END) { ret = SendHelloRequest(ssl); - if (ret != 0) - return ret; + if (ret != 0) { + ssl->error = ret; + return WOLFSSL_FATAL_ERROR; + } } #endif /* NO_WOLFSSL_SERVER && HAVE_SERVER_RENEGOTIATION_INFO */ ret = InitHandshakeHashes(ssl); - if (ret !=0) - return ret; + if (ret != 0) { + ssl->error = ret; + return WOLFSSL_FATAL_ERROR; + } } ret = wolfSSL_negotiate(ssl); return ret; } -int wolfSSL_Rehandshake(WOLFSSL* ssl) { - WOLFSSL_ENTER("wolfSSL_Rehandshake()"); - return wolfSSL_StartSecureRenegotiation(ssl, 0); -} +/* do a secure resumption handshake, user forced, we discourage */ +int wolfSSL_SecureResume(WOLFSSL* ssl) +{ + WOLFSSL_SESSION* session; + int ret; - -int wolfSSL_SecureResume(WOLFSSL* ssl) { WOLFSSL_ENTER("wolfSSL_SecureResume()"); - return wolfSSL_StartSecureRenegotiation(ssl, 1); + + session = wolfSSL_get_session(ssl); + ret = wolfSSL_set_session(ssl, session); + session = NULL; + if (ret == WOLFSSL_SUCCESS) + ret = wolfSSL_Rehandshake(ssl); + + return ret; } #endif /* HAVE_SECURE_RENEGOTIATION */ From 65c72ddfe112a85cb4e3f67bd0af05f219e7930b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 26 Feb 2019 14:26:09 -0800 Subject: [PATCH 7/8] Reverted an earlier change to the renegotiation resumption. Still need to check the cert subject hash. --- src/internal.c | 24 ++++++++++++++++++++++++ wolfssl/error-ssl.h | 2 +- wolfssl/internal.h | 1 + 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index c4d670343..cf06df492 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9562,6 +9562,30 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, #endif } } + + #ifdef HAVE_SECURE_RENEGOTIATION + if (args->fatal == 0 && ssl->secure_renegotiation + && ssl->secure_renegotiation->enabled) { + + if (IsEncryptionOn(ssl, 0)) { + /* compare against previous time */ + if (XMEMCMP(args->dCert->subjectHash, + ssl->secure_renegotiation->subject_hash, + KEYID_SIZE) != 0) { + WOLFSSL_MSG( + "Peer sent different cert during scr, fatal"); + args->fatal = 1; + ret = SCR_DIFFERENT_CERT_E; + } + } + + /* cache peer's hash */ + if (args->fatal == 0) { + XMEMCPY(ssl->secure_renegotiation->subject_hash, + args->dCert->subjectHash, KEYID_SIZE); + } + } + #endif /* HAVE_SECURE_RENEGOTIATION */ } /* if (count > 0) */ /* Check for error */ diff --git a/wolfssl/error-ssl.h b/wolfssl/error-ssl.h index 3f0ae944d..c7423249c 100644 --- a/wolfssl/error-ssl.h +++ b/wolfssl/error-ssl.h @@ -119,7 +119,7 @@ enum wolfSSL_ErrorCodes { SECURE_RENEGOTIATION_E = -388, /* Invalid Renegotiation Info */ SESSION_TICKET_LEN_E = -389, /* Session Ticket too large */ SESSION_TICKET_EXPECT_E = -390, /* Session Ticket missing */ - + SCR_DIFFERENT_CERT_E = -391, /* SCR Different cert error */ SESSION_SECRET_CB_E = -392, /* Session secret Cb fcn failure */ NO_CHANGE_CIPHER_E = -393, /* Finished before change cipher */ SANITY_MSG_E = -394, /* Sanity check on msg order error */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index fd82aa107..5d7e94df5 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -2286,6 +2286,7 @@ typedef struct SecureRenegotiation { enum key_cache_state cache_status; /* track key cache state */ byte client_verify_data[TLS_FINISHED_SZ]; /* cached */ byte server_verify_data[TLS_FINISHED_SZ]; /* cached */ + byte subject_hash[KEYID_SIZE]; /* peer cert hash */ Keys tmp_keys; /* can't overwrite real keys yet */ } SecureRenegotiation; From 020b27bab263e563ecbbb5e70a34039370a06584 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 1 Mar 2019 11:00:26 -0800 Subject: [PATCH 8/8] wolfSSL_SecureResume() should be client only. Return an error if called form the server. --- src/ssl.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/ssl.c b/src/ssl.c index 4db81dcaf..cce861a31 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -2417,6 +2417,8 @@ int wolfSSL_Rehandshake(WOLFSSL* ssl) } +#ifndef NO_WOLFSSL_CLIENT + /* do a secure resumption handshake, user forced, we discourage */ int wolfSSL_SecureResume(WOLFSSL* ssl) { @@ -2425,6 +2427,14 @@ int wolfSSL_SecureResume(WOLFSSL* ssl) WOLFSSL_ENTER("wolfSSL_SecureResume()"); + if (ssl == NULL) + return BAD_FUNC_ARG; + + if (ssl->options.side == WOLFSSL_SERVER_END) { + ssl->error = SIDE_ERROR; + return SSL_FATAL_ERROR; + } + session = wolfSSL_get_session(ssl); ret = wolfSSL_set_session(ssl, session); session = NULL; @@ -2434,6 +2444,8 @@ int wolfSSL_SecureResume(WOLFSSL* ssl) return ret; } +#endif /* NO_WOLFSSL_CLIENT */ + #endif /* HAVE_SECURE_RENEGOTIATION */ /* Session Ticket */