From 22e3dcb7808bb06cd18c3231e34a5930e796cc48 Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Mon, 25 Jan 2016 13:30:37 -0500 Subject: [PATCH] Remove TLS heartbeat, disable DTLS heartbeat To enable heartbeats for DTLS, configure with enable-heartbeats. Heartbeats for TLS have been completely removed. This addresses RT 3647 Reviewed-by: Richard Levitte --- CHANGES | 5 + Configure | 1 + apps/openssl.c | 3 + include/openssl/ssl.h | 10 +- include/openssl/ssl3.h | 2 +- include/openssl/tls1.h | 34 +++-- ssl/d1_lib.c | 14 +- ssl/record/rec_layer_d1.c | 2 +- ssl/record/rec_layer_s3.c | 16 --- ssl/s3_lib.c | 23 ++-- ssl/ssl_err.c | 2 - ssl/ssl_locl.h | 5 - ssl/ssl_utst.c | 1 - ssl/t1_lib.c | 216 +++++-------------------------- ssl/t1_trce.c | 4 +- test/heartbeat_test.c | 95 -------------- test/recipes/90-test_heartbeat.t | 2 +- 17 files changed, 95 insertions(+), 340 deletions(-) diff --git a/CHANGES b/CHANGES index cb71a13c05..29a00bce89 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,11 @@ Changes between 1.0.2f and 1.1.0 [xx XXX xxxx] + *) Heartbeat for TLS has been removed and is disabled by default + for DTLS; configure with enable-heartbeats. Code that uses the + old #define's might need to be updated. + [Emilia Käsper, Rich Salz] + *) Rename REF_CHECK to REF_DEBUG. [Rich Salz] diff --git a/Configure b/Configure index 4198ff74ce..3dc6a42999 100755 --- a/Configure +++ b/Configure @@ -318,6 +318,7 @@ my %disabled = ( # "what" => "comment" [or special keyword "experimental "unit-test" => "default", "zlib" => "default", "crypto-mdebug" => "default", + "heartbeats" => "default", ); my @experimental = (); diff --git a/apps/openssl.c b/apps/openssl.c index e9c24d0572..732cdf105a 100644 --- a/apps/openssl.c +++ b/apps/openssl.c @@ -748,6 +748,9 @@ static void list_disabled(void) #ifdef OPENSSL_NO_GOST BIO_puts(bio_out, "GOST\n"); #endif +#ifdef OPENSSL_NO_HEARTBEATS + BIO_puts(bio_out, "HEARTBEATS\n"); +#endif #ifdef OPENSSL_NO_HMAC BIO_puts(bio_out, "HMAC\n"); #endif diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index c4b9826ccb..d51c2d4840 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -601,7 +601,7 @@ unsigned long SSL_set_options(SSL *s, unsigned long op); # ifndef OPENSSL_NO_HEARTBEATS # define SSL_heartbeat(ssl) \ - SSL_ctrl((ssl),SSL_CTRL_TLS_EXT_SEND_HEARTBEAT,0,NULL) + SSL_ctrl((ssl),SSL_CTRL_DTLS_EXT_SEND_HEARTBEAT,0,NULL) # endif # define SSL_CTX_set_cert_flags(ctx,op) \ @@ -1177,9 +1177,9 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION) # define SSL_CTRL_SET_TLS_EXT_SRP_STRENGTH 80 # define SSL_CTRL_SET_TLS_EXT_SRP_PASSWORD 81 # ifndef OPENSSL_NO_HEARTBEATS -# define SSL_CTRL_TLS_EXT_SEND_HEARTBEAT 85 -# define SSL_CTRL_GET_TLS_EXT_HEARTBEAT_PENDING 86 -# define SSL_CTRL_SET_TLS_EXT_HEARTBEAT_NO_REQUESTS 87 +# define SSL_CTRL_DTLS_EXT_SEND_HEARTBEAT 85 +# define SSL_CTRL_GET_DTLS_EXT_HEARTBEAT_PENDING 86 +# define SSL_CTRL_SET_DTLS_EXT_HEARTBEAT_NO_REQUESTS 87 # endif # define DTLS_CTRL_GET_TIMEOUT 73 # define DTLS_CTRL_HANDLE_TIMEOUT 74 @@ -2125,11 +2125,9 @@ void ERR_load_SSL_strings(void); # define SSL_F_TLS1_CHECK_SERVERHELLO_TLSEXT 274 # define SSL_F_TLS1_EXPORT_KEYING_MATERIAL 314 # define SSL_F_TLS1_GET_CURVELIST 338 -# define SSL_F_TLS1_HEARTBEAT 315 # define SSL_F_TLS1_PREPARE_CLIENTHELLO_TLSEXT 275 # define SSL_F_TLS1_PREPARE_SERVERHELLO_TLSEXT 276 # define SSL_F_TLS1_PRF 284 -# define SSL_F_TLS1_PROCESS_HEARTBEAT 341 # define SSL_F_TLS1_SETUP_KEY_BLOCK 211 # define SSL_F_TLS1_SET_SERVER_SIGALGS 335 # define SSL_F_TLS_CLIENT_KEY_EXCHANGE_POST_WORK 354 diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index 325fa94191..ecbe24715d 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -306,7 +306,7 @@ extern "C" { # define SSL3_RT_ALERT 21 # define SSL3_RT_HANDSHAKE 22 # define SSL3_RT_APPLICATION_DATA 23 -# define TLS1_RT_HEARTBEAT 24 +# define DTLS1_RT_HEARTBEAT 24 /* Pseudo content types to indicate additional parameters */ # define TLS1_RT_CRYPTO 0x1000 diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index bf21b8e966..0f0d4a3713 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h @@ -391,14 +391,32 @@ SSL_CTX_ctrl(ssl,SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB_ARG,0, (void *)arg) SSL_CTX_callback_ctrl(ssl,SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB,(void (*)(void))cb) # ifndef OPENSSL_NO_HEARTBEATS -# define SSL_TLSEXT_HB_ENABLED 0x01 -# define SSL_TLSEXT_HB_DONT_SEND_REQUESTS 0x02 -# define SSL_TLSEXT_HB_DONT_RECV_REQUESTS 0x04 - -# define SSL_get_tlsext_heartbeat_pending(ssl) \ - SSL_ctrl((ssl),SSL_CTRL_GET_TLS_EXT_HEARTBEAT_PENDING,0,NULL) -# define SSL_set_tlsext_heartbeat_no_requests(ssl, arg) \ - SSL_ctrl((ssl),SSL_CTRL_SET_TLS_EXT_HEARTBEAT_NO_REQUESTS,arg,NULL) +# define SSL_DTLSEXT_HB_ENABLED 0x01 +# define SSL_DTLSEXT_HB_DONT_SEND_REQUESTS 0x02 +# define SSL_DTLSEXT_HB_DONT_RECV_REQUESTS 0x04 +# define SSL_get_dtlsext_heartbeat_pending(ssl) \ + SSL_ctrl((ssl),SSL_CTRL_GET_DTLS_EXT_HEARTBEAT_PENDING,0,NULL) +# define SSL_set_dtlsext_heartbeat_no_requests(ssl, arg) \ + SSL_ctrl((ssl),SSL_CTRL_SET_DTLS_EXT_HEARTBEAT_NO_REQUESTS,arg,NULL) + +# if OPENSSL_API_COMPAT < 0x10100000L +# define SSL_CTRL_TLS_EXT_SEND_HEARTBEAT \ + SSL_CTRL_DTLS_EXT_SEND_HEARTBEAT +# define SSL_CTRL_GET_TLS_EXT_HEARTBEAT_PENDING \ + SSL_CTRL_GET_DTLS_EXT_HEARTBEAT_PENDING +# define SSL_CTRL_SET_TLS_EXT_HEARTBEAT_NO_REQUESTS \ + SSL_CTRL_SET_DTLS_EXT_HEARTBEAT_NO_REQUESTS +# define SSL_TLSEXT_HB_ENABLED \ + SSL_DTLSEXT_HB_ENABLED +# define SSL_TLSEXT_HB_DONT_SEND_REQUESTS \ + SSL_DTLSEXT_HB_DONT_SEND_REQUESTS +# define SSL_TLSEXT_HB_DONT_RECV_REQUESTS \ + SSL_DTLSEXT_HB_DONT_RECV_REQUESTS +# define SSL_get_tlsext_heartbeat_pending(ssl) \ + SSL_get_dtlsext_heartbeat_pending(ssl) +# define SSL_set_tlsext_heartbeat_no_requests(ssl, arg) \ + SSL_set_dtlsext_heartbeat_no_requests(ssl, arg) +# endif # endif /* PSK ciphersuites from 4279 */ diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index b1f6ed207d..8b3e941570 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -916,7 +916,7 @@ int dtls1_process_heartbeat(SSL *s, unsigned char *p, unsigned int length) unsigned int padding = 16; /* Use minimum padding */ if (s->msg_callback) - s->msg_callback(0, s->version, TLS1_RT_HEARTBEAT, + s->msg_callback(0, s->version, DTLS1_RT_HEARTBEAT, p, length, s, s->msg_callback_arg); /* Read type and payload length first */ @@ -961,10 +961,10 @@ int dtls1_process_heartbeat(SSL *s, unsigned char *p, unsigned int length) return -1; } - r = dtls1_write_bytes(s, TLS1_RT_HEARTBEAT, buffer, write_length); + r = dtls1_write_bytes(s, DTLS1_RT_HEARTBEAT, buffer, write_length); if (r >= 0 && s->msg_callback) - s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT, + s->msg_callback(1, s->version, DTLS1_RT_HEARTBEAT, buffer, write_length, s, s->msg_callback_arg); OPENSSL_free(buffer); @@ -998,8 +998,8 @@ int dtls1_heartbeat(SSL *s) unsigned int padding = 16; /* Use minimum padding */ /* Only send if peer supports and accepts HB requests... */ - if (!(s->tlsext_heartbeat & SSL_TLSEXT_HB_ENABLED) || - s->tlsext_heartbeat & SSL_TLSEXT_HB_DONT_SEND_REQUESTS) { + if (!(s->tlsext_heartbeat & SSL_DTLSEXT_HB_ENABLED) || + s->tlsext_heartbeat & SSL_DTLSEXT_HB_DONT_SEND_REQUESTS) { SSLerr(SSL_F_DTLS1_HEARTBEAT, SSL_R_TLS_HEARTBEAT_PEER_DOESNT_ACCEPT); return -1; } @@ -1050,10 +1050,10 @@ int dtls1_heartbeat(SSL *s) goto err; } - ret = dtls1_write_bytes(s, TLS1_RT_HEARTBEAT, buf, 3 + payload + padding); + ret = dtls1_write_bytes(s, DTLS1_RT_HEARTBEAT, buf, 3 + payload + padding); if (ret >= 0) { if (s->msg_callback) - s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT, + s->msg_callback(1, s->version, DTLS1_RT_HEARTBEAT, buf, 3 + payload + padding, s, s->msg_callback_arg); diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c index c53ef9a1ac..49e8f462f4 100644 --- a/ssl/record/rec_layer_d1.c +++ b/ssl/record/rec_layer_d1.c @@ -628,7 +628,7 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, dest_len = &s->rlayer.d->alert_fragment_len; } #ifndef OPENSSL_NO_HEARTBEATS - else if (SSL3_RECORD_get_type(rr) == TLS1_RT_HEARTBEAT) { + else if (SSL3_RECORD_get_type(rr) == DTLS1_RT_HEARTBEAT) { /* We allow a 0 return */ if (dtls1_process_heartbeat(s, SSL3_RECORD_get_data(rr), SSL3_RECORD_get_length(rr)) < 0) { diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index d0f17bb0c7..6a4f92f9ba 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1176,22 +1176,6 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, dest = s->rlayer.alert_fragment; dest_len = &s->rlayer.alert_fragment_len; } -#ifndef OPENSSL_NO_HEARTBEATS - else if (SSL3_RECORD_get_type(rr)== TLS1_RT_HEARTBEAT) { - /* We can ignore 0 return values */ - if (tls1_process_heartbeat(s, SSL3_RECORD_get_data(rr), - SSL3_RECORD_get_length(rr)) < 0) { - return -1; - } - - /* Exit and notify application to read again */ - SSL3_RECORD_set_length(rr, 0); - s->rwstate = SSL_READING; - BIO_clear_retry_flags(SSL_get_rbio(s)); - BIO_set_retry_read(SSL_get_rbio(s)); - return (-1); - } -#endif if (dest_maxlen > 0) { n = dest_maxlen - *dest_len; /* available space in 'dest' */ diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 8e2d7c4ff7..8b25b0e76b 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3594,23 +3594,24 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) break; #ifndef OPENSSL_NO_HEARTBEATS - case SSL_CTRL_TLS_EXT_SEND_HEARTBEAT: + case SSL_CTRL_DTLS_EXT_SEND_HEARTBEAT: if (SSL_IS_DTLS(s)) ret = dtls1_heartbeat(s); - else - ret = tls1_heartbeat(s); break; - case SSL_CTRL_GET_TLS_EXT_HEARTBEAT_PENDING: - ret = s->tlsext_hb_pending; + case SSL_CTRL_GET_DTLS_EXT_HEARTBEAT_PENDING: + if (SSL_IS_DTLS(s)) + ret = s->tlsext_hb_pending; break; - case SSL_CTRL_SET_TLS_EXT_HEARTBEAT_NO_REQUESTS: - if (larg) - s->tlsext_heartbeat |= SSL_TLSEXT_HB_DONT_RECV_REQUESTS; - else - s->tlsext_heartbeat &= ~SSL_TLSEXT_HB_DONT_RECV_REQUESTS; - ret = 1; + case SSL_CTRL_SET_DTLS_EXT_HEARTBEAT_NO_REQUESTS: + if (SSL_IS_DTLS(s)) { + if (larg) + s->tlsext_heartbeat |= SSL_DTLSEXT_HB_DONT_RECV_REQUESTS; + else + s->tlsext_heartbeat &= ~SSL_DTLSEXT_HB_DONT_RECV_REQUESTS; + ret = 1; + } break; #endif diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 4570fbb83f..e6b9bbdb9c 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -278,13 +278,11 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_TLS1_EXPORT_KEYING_MATERIAL), "tls1_export_keying_material"}, {ERR_FUNC(SSL_F_TLS1_GET_CURVELIST), "tls1_get_curvelist"}, - {ERR_FUNC(SSL_F_TLS1_HEARTBEAT), "tls1_heartbeat"}, {ERR_FUNC(SSL_F_TLS1_PREPARE_CLIENTHELLO_TLSEXT), "TLS1_PREPARE_CLIENTHELLO_TLSEXT"}, {ERR_FUNC(SSL_F_TLS1_PREPARE_SERVERHELLO_TLSEXT), "TLS1_PREPARE_SERVERHELLO_TLSEXT"}, {ERR_FUNC(SSL_F_TLS1_PRF), "tls1_PRF"}, - {ERR_FUNC(SSL_F_TLS1_PROCESS_HEARTBEAT), "tls1_process_heartbeat"}, {ERR_FUNC(SSL_F_TLS1_SETUP_KEY_BLOCK), "tls1_setup_key_block"}, {ERR_FUNC(SSL_F_TLS1_SET_SERVER_SIGALGS), "tls1_set_server_sigalgs"}, {ERR_FUNC(SSL_F_TLS_CLIENT_KEY_EXCHANGE_POST_WORK), diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index f10570b50d..d7a7d01763 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1832,8 +1832,6 @@ const SSL_METHOD *func_name(void) \ struct openssl_ssl_test_functions { int (*p_ssl_init_wbio_buffer) (SSL *s, int push); int (*p_ssl3_setup_buffers) (SSL *s); - int (*p_tls1_process_heartbeat) (SSL *s, - unsigned char *p, unsigned int length); int (*p_dtls1_process_heartbeat) (SSL *s, unsigned char *p, unsigned int length); }; @@ -2053,9 +2051,7 @@ __owur int ssl_prepare_clienthello_tlsext(SSL *s); __owur int ssl_prepare_serverhello_tlsext(SSL *s); # ifndef OPENSSL_NO_HEARTBEATS -__owur int tls1_heartbeat(SSL *s); __owur int dtls1_heartbeat(SSL *s); -__owur int tls1_process_heartbeat(SSL *s, unsigned char *p, unsigned int length); __owur int dtls1_process_heartbeat(SSL *s, unsigned char *p, unsigned int length); # endif @@ -2151,7 +2147,6 @@ void custom_exts_free(custom_ext_methods *exts); # define ssl_init_wbio_buffer SSL_test_functions()->p_ssl_init_wbio_buffer # define ssl3_setup_buffers SSL_test_functions()->p_ssl3_setup_buffers -# define tls1_process_heartbeat SSL_test_functions()->p_tls1_process_heartbeat # define dtls1_process_heartbeat SSL_test_functions()->p_dtls1_process_heartbeat # endif diff --git a/ssl/ssl_utst.c b/ssl/ssl_utst.c index 5feef6e2f3..25ec77e20b 100644 --- a/ssl/ssl_utst.c +++ b/ssl/ssl_utst.c @@ -59,7 +59,6 @@ static const struct openssl_ssl_test_functions ssl_test_functions = { ssl_init_wbio_buffer, ssl3_setup_buffers, - tls1_process_heartbeat, dtls1_process_heartbeat }; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 446a678dee..e0e0cb95ac 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1389,20 +1389,22 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf, i2d_X509_EXTENSIONS(s->tlsext_ocsp_exts, &ret); } #ifndef OPENSSL_NO_HEARTBEATS - /* Add Heartbeat extension */ - if ((limit - ret - 4 - 1) < 0) - return NULL; - s2n(TLSEXT_TYPE_heartbeat, ret); - s2n(1, ret); - /*- - * Set mode: - * 1: peer may send requests - * 2: peer not allowed to send requests - */ - if (s->tlsext_heartbeat & SSL_TLSEXT_HB_DONT_RECV_REQUESTS) - *(ret++) = SSL_TLSEXT_HB_DONT_SEND_REQUESTS; - else - *(ret++) = SSL_TLSEXT_HB_ENABLED; + if (SSL_IS_DTLS(s)) { + /* Add Heartbeat extension */ + if ((limit - ret - 4 - 1) < 0) + return NULL; + s2n(TLSEXT_TYPE_heartbeat, ret); + s2n(1, ret); + /*- + * Set mode: + * 1: peer may send requests + * 2: peer not allowed to send requests + */ + if (s->tlsext_heartbeat & SSL_DTLSEXT_HB_DONT_RECV_REQUESTS) + *(ret++) = SSL_DTLSEXT_HB_DONT_SEND_REQUESTS; + else + *(ret++) = SSL_DTLSEXT_HB_ENABLED; + } #endif #ifndef OPENSSL_NO_NEXTPROTONEG @@ -1637,7 +1639,7 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, } #ifndef OPENSSL_NO_HEARTBEATS /* Add Heartbeat extension if we've received one */ - if (s->tlsext_heartbeat & SSL_TLSEXT_HB_ENABLED) { + if (SSL_IS_DTLS(s) && (s->tlsext_heartbeat & SSL_DTLSEXT_HB_ENABLED)) { if ((limit - ret - 4 - 1) < 0) return NULL; s2n(TLSEXT_TYPE_heartbeat, ret); @@ -1647,10 +1649,10 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, * 1: peer may send requests * 2: peer not allowed to send requests */ - if (s->tlsext_heartbeat & SSL_TLSEXT_HB_DONT_RECV_REQUESTS) - *(ret++) = SSL_TLSEXT_HB_DONT_SEND_REQUESTS; + if (s->tlsext_heartbeat & SSL_DTLSEXT_HB_DONT_RECV_REQUESTS) + *(ret++) = SSL_DTLSEXT_HB_DONT_SEND_REQUESTS; else - *(ret++) = SSL_TLSEXT_HB_ENABLED; + *(ret++) = SSL_DTLSEXT_HB_ENABLED; } #endif @@ -1878,8 +1880,8 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al) OPENSSL_free(s->s3->alpn_selected); s->s3->alpn_selected = NULL; #ifndef OPENSSL_NO_HEARTBEATS - s->tlsext_heartbeat &= ~(SSL_TLSEXT_HB_ENABLED | - SSL_TLSEXT_HB_DONT_SEND_REQUESTS); + s->tlsext_heartbeat &= ~(SSL_DTLSEXT_HB_ENABLED | + SSL_DTLSEXT_HB_DONT_SEND_REQUESTS); #endif #ifndef OPENSSL_NO_EC @@ -2197,7 +2199,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al) s->tlsext_status_type = -1; } #ifndef OPENSSL_NO_HEARTBEATS - else if (type == TLSEXT_TYPE_heartbeat) { + else if (SSL_IS_DTLS(s) && type == TLSEXT_TYPE_heartbeat) { unsigned int hbtype; if (!PACKET_get_1(&subpkt, &hbtype) @@ -2207,11 +2209,11 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al) } switch (hbtype) { case 0x01: /* Client allows us to send HB requests */ - s->tlsext_heartbeat |= SSL_TLSEXT_HB_ENABLED; + s->tlsext_heartbeat |= SSL_DTLSEXT_HB_ENABLED; break; case 0x02: /* Client doesn't accept HB requests */ - s->tlsext_heartbeat |= SSL_TLSEXT_HB_ENABLED; - s->tlsext_heartbeat |= SSL_TLSEXT_HB_DONT_SEND_REQUESTS; + s->tlsext_heartbeat |= SSL_DTLSEXT_HB_ENABLED; + s->tlsext_heartbeat |= SSL_DTLSEXT_HB_DONT_SEND_REQUESTS; break; default: *al = SSL_AD_ILLEGAL_PARAMETER; @@ -2356,8 +2358,8 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al) OPENSSL_free(s->s3->alpn_selected); s->s3->alpn_selected = NULL; #ifndef OPENSSL_NO_HEARTBEATS - s->tlsext_heartbeat &= ~(SSL_TLSEXT_HB_ENABLED | - SSL_TLSEXT_HB_DONT_SEND_REQUESTS); + s->tlsext_heartbeat &= ~(SSL_DTLSEXT_HB_ENABLED | + SSL_DTLSEXT_HB_DONT_SEND_REQUESTS); #endif #ifdef TLSEXT_TYPE_encrypt_then_mac @@ -2519,7 +2521,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al) s->s3->alpn_selected_len = len; } #ifndef OPENSSL_NO_HEARTBEATS - else if (type == TLSEXT_TYPE_heartbeat) { + else if (SSL_IS_DTLS(s) && type == TLSEXT_TYPE_heartbeat) { unsigned int hbtype; if (!PACKET_get_1(&spkt, &hbtype)) { *al = SSL_AD_DECODE_ERROR; @@ -2527,11 +2529,11 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al) } switch (hbtype) { case 0x01: /* Server allows us to send HB requests */ - s->tlsext_heartbeat |= SSL_TLSEXT_HB_ENABLED; + s->tlsext_heartbeat |= SSL_DTLSEXT_HB_ENABLED; break; case 0x02: /* Server doesn't accept HB requests */ - s->tlsext_heartbeat |= SSL_TLSEXT_HB_ENABLED; - s->tlsext_heartbeat |= SSL_TLSEXT_HB_DONT_SEND_REQUESTS; + s->tlsext_heartbeat |= SSL_DTLSEXT_HB_ENABLED; + s->tlsext_heartbeat |= SSL_DTLSEXT_HB_DONT_SEND_REQUESTS; break; default: *al = SSL_AD_ILLEGAL_PARAMETER; @@ -3627,160 +3629,6 @@ int SSL_get_shared_sigalgs(SSL *s, int idx, return s->cert->shared_sigalgslen; } -#ifndef OPENSSL_NO_HEARTBEATS -int tls1_process_heartbeat(SSL *s, unsigned char *p, unsigned int length) -{ - unsigned char *pl; - unsigned short hbtype; - unsigned int payload; - unsigned int padding = 16; /* Use minimum padding */ - - if (s->msg_callback) - s->msg_callback(0, s->version, TLS1_RT_HEARTBEAT, - p, length, - s, s->msg_callback_arg); - - /* Read type and payload length first */ - if (1 + 2 + 16 > length) - return 0; /* silently discard */ - hbtype = *p++; - n2s(p, payload); - if (1 + 2 + payload + 16 > length) - return 0; /* silently discard per RFC 6520 sec. 4 */ - pl = p; - - if (hbtype == TLS1_HB_REQUEST) { - unsigned char *buffer, *bp; - int r; - - /* - * Allocate memory for the response, size is 1 bytes message type, - * plus 2 bytes payload length, plus payload, plus padding - */ - buffer = OPENSSL_malloc(1 + 2 + payload + padding); - if (buffer == NULL) { - SSLerr(SSL_F_TLS1_PROCESS_HEARTBEAT, ERR_R_MALLOC_FAILURE); - return -1; - } - bp = buffer; - - /* Enter response type, length and copy payload */ - *bp++ = TLS1_HB_RESPONSE; - s2n(payload, bp); - memcpy(bp, pl, payload); - bp += payload; - /* Random padding */ - if (RAND_bytes(bp, padding) <= 0) { - OPENSSL_free(buffer); - return -1; - } - - r = ssl3_write_bytes(s, TLS1_RT_HEARTBEAT, buffer, - 3 + payload + padding); - - if (r >= 0 && s->msg_callback) - s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT, - buffer, 3 + payload + padding, - s, s->msg_callback_arg); - - OPENSSL_free(buffer); - - if (r < 0) - return r; - } else if (hbtype == TLS1_HB_RESPONSE) { - unsigned int seq; - - /* - * We only send sequence numbers (2 bytes unsigned int), and 16 - * random bytes, so we just try to read the sequence number - */ - n2s(pl, seq); - - if (payload == 18 && seq == s->tlsext_hb_seq) { - s->tlsext_hb_seq++; - s->tlsext_hb_pending = 0; - } - } - - return 0; -} - -int tls1_heartbeat(SSL *s) -{ - unsigned char *buf, *p; - int ret = -1; - unsigned int payload = 18; /* Sequence number + random bytes */ - unsigned int padding = 16; /* Use minimum padding */ - - /* Only send if peer supports and accepts HB requests... */ - if (!(s->tlsext_heartbeat & SSL_TLSEXT_HB_ENABLED) || - s->tlsext_heartbeat & SSL_TLSEXT_HB_DONT_SEND_REQUESTS) { - SSLerr(SSL_F_TLS1_HEARTBEAT, SSL_R_TLS_HEARTBEAT_PEER_DOESNT_ACCEPT); - return -1; - } - - /* ...and there is none in flight yet... */ - if (s->tlsext_hb_pending) { - SSLerr(SSL_F_TLS1_HEARTBEAT, SSL_R_TLS_HEARTBEAT_PENDING); - return -1; - } - - /* ...and no handshake in progress. */ - if (SSL_in_init(s) || ossl_statem_get_in_handshake(s)) { - SSLerr(SSL_F_TLS1_HEARTBEAT, SSL_R_UNEXPECTED_MESSAGE); - return -1; - } - - /*- - * Create HeartBeat message, we just use a sequence number - * as payload to distuingish different messages and add - * some random stuff. - * - Message Type, 1 byte - * - Payload Length, 2 bytes (unsigned int) - * - Payload, the sequence number (2 bytes uint) - * - Payload, random bytes (16 bytes uint) - * - Padding - */ - buf = OPENSSL_malloc(1 + 2 + payload + padding); - if (buf == NULL) { - SSLerr(SSL_F_TLS1_HEARTBEAT, ERR_R_MALLOC_FAILURE); - return -1; - } - p = buf; - /* Message Type */ - *p++ = TLS1_HB_REQUEST; - /* Payload length (18 bytes here) */ - s2n(payload, p); - /* Sequence number */ - s2n(s->tlsext_hb_seq, p); - /* 16 random bytes */ - if (RAND_bytes(p, 16) <= 0) { - SSLerr(SSL_F_TLS1_HEARTBEAT, ERR_R_INTERNAL_ERROR); - goto err; - } - p += 16; - /* Random padding */ - if (RAND_bytes(p, padding) <= 0) { - SSLerr(SSL_F_TLS1_HEARTBEAT, ERR_R_INTERNAL_ERROR); - goto err; - } - - ret = ssl3_write_bytes(s, TLS1_RT_HEARTBEAT, buf, 3 + payload + padding); - if (ret >= 0) { - if (s->msg_callback) - s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT, - buf, 3 + payload + padding, - s, s->msg_callback_arg); - - s->tlsext_hb_pending = 1; - } - - err: - OPENSSL_free(buf); - return ret; -} -#endif - #define MAX_SIGALGLEN (TLSEXT_hash_num * TLSEXT_signature_num * 2) typedef struct { diff --git a/ssl/t1_trce.c b/ssl/t1_trce.c index 7d451da741..de1cac7c70 100644 --- a/ssl/t1_trce.c +++ b/ssl/t1_trce.c @@ -116,7 +116,7 @@ static ssl_trace_tbl ssl_content_tbl[] = { {SSL3_RT_ALERT, "Alert"}, {SSL3_RT_HANDSHAKE, "Handshake"}, {SSL3_RT_APPLICATION_DATA, "ApplicationData"}, - {TLS1_RT_HEARTBEAT, "HeartBeat"} + {DTLS1_RT_HEARTBEAT, "HeartBeat"} }; /* Handshake types */ @@ -1400,7 +1400,7 @@ void SSL_trace(int write_p, int version, int content_type, SSL_alert_type_string_long(msg[0] << 8), msg[0], SSL_alert_desc_string_long(msg[1]), msg[1]); } - case TLS1_RT_HEARTBEAT: + case DTLS1_RT_HEARTBEAT: ssl_print_heartbeat(bio, 4, msg, msglen); break; diff --git a/test/heartbeat_test.c b/test/heartbeat_test.c index dd7bf11d00..453615a8f4 100644 --- a/test/heartbeat_test.c +++ b/test/heartbeat_test.c @@ -144,24 +144,6 @@ static int dummy_handshake(SSL *s) return 1; } -static HEARTBEAT_TEST_FIXTURE set_up_tls(const char *const test_case_name) -{ - HEARTBEAT_TEST_FIXTURE fixture = set_up(test_case_name, - TLSv1_server_method()); - fixture.process_heartbeat = tls1_process_heartbeat; - fixture.s->handshake_func = dummy_handshake; - - /* - * As per do_ssl3_write(), skipping the following from the beginning of - * the returned heartbeat message: type-1 byte; version-2 bytes; length-2 - * bytes And then skipping the 1-byte type encoded by process_heartbeat - * for a total of 6 bytes, at which point we can grab the length and the - * payload we seek. - */ - fixture.return_payload_offset = 6; - return fixture; -} - static void tear_down(HEARTBEAT_TEST_FIXTURE fixture) { ERR_print_errors_fp(stderr); @@ -360,79 +342,6 @@ static int test_dtls1_heartbleed_excessive_plaintext_length() EXECUTE_HEARTBEAT_TEST(); } -static int test_tls1_not_bleeding() -{ - SETUP_HEARTBEAT_TEST_FIXTURE(tls); - /* Three-byte pad at the beginning for type and payload length */ - unsigned char payload_buf[MAX_PRINTABLE_CHARACTERS + 4] = - " Not bleeding, sixteen spaces of padding" " "; - const int payload_buf_len = honest_payload_size(payload_buf); - - fixture.payload = &payload_buf[0]; - fixture.sent_payload_len = payload_buf_len; - fixture.expected_return_value = 0; - fixture.expected_payload_len = payload_buf_len; - fixture.expected_return_payload = - "Not bleeding, sixteen spaces of padding"; - EXECUTE_HEARTBEAT_TEST(); -} - -static int test_tls1_not_bleeding_empty_payload() -{ - int payload_buf_len; - - SETUP_HEARTBEAT_TEST_FIXTURE(tls); - /* - * Three-byte pad at the beginning for type and payload length, plus a - * NUL at the end - */ - unsigned char payload_buf[4 + MAX_PRINTABLE_CHARACTERS]; - memset(payload_buf, ' ', MIN_PADDING_SIZE + 3); - payload_buf[MIN_PADDING_SIZE + 3] = '\0'; - payload_buf_len = honest_payload_size(payload_buf); - - fixture.payload = &payload_buf[0]; - fixture.sent_payload_len = payload_buf_len; - fixture.expected_return_value = 0; - fixture.expected_payload_len = payload_buf_len; - fixture.expected_return_payload = ""; - EXECUTE_HEARTBEAT_TEST(); -} - -static int test_tls1_heartbleed() -{ - SETUP_HEARTBEAT_TEST_FIXTURE(tls); - /* Three-byte pad at the beginning for type and payload length */ - unsigned char payload_buf[MAX_PRINTABLE_CHARACTERS + 4] = - " HEARTBLEED "; - - fixture.payload = &payload_buf[0]; - fixture.sent_payload_len = MAX_PRINTABLE_CHARACTERS; - fixture.expected_return_value = 0; - fixture.expected_payload_len = 0; - fixture.expected_return_payload = ""; - EXECUTE_HEARTBEAT_TEST(); -} - -static int test_tls1_heartbleed_empty_payload() -{ - SETUP_HEARTBEAT_TEST_FIXTURE(tls); - /* - * Excluding the NUL at the end, one byte short of type + payload length - * + minimum padding - */ - unsigned char payload_buf[MAX_PRINTABLE_CHARACTERS + 4]; - memset(payload_buf, ' ', MIN_PADDING_SIZE + 2); - payload_buf[MIN_PADDING_SIZE + 2] = '\0'; - - fixture.payload = &payload_buf[0]; - fixture.sent_payload_len = MAX_PRINTABLE_CHARACTERS; - fixture.expected_return_value = 0; - fixture.expected_payload_len = 0; - fixture.expected_return_payload = ""; - EXECUTE_HEARTBEAT_TEST(); -} - # undef EXECUTE_HEARTBEAT_TEST # undef SETUP_HEARTBEAT_TEST_FIXTURE @@ -445,10 +354,6 @@ int main(int argc, char *argv[]) ADD_TEST(test_dtls1_heartbleed); ADD_TEST(test_dtls1_heartbleed_empty_payload); ADD_TEST(test_dtls1_heartbleed_excessive_plaintext_length); - ADD_TEST(test_tls1_not_bleeding); - ADD_TEST(test_tls1_not_bleeding_empty_payload); - ADD_TEST(test_tls1_heartbleed); - ADD_TEST(test_tls1_heartbleed_empty_payload); result = run_tests(argv[0]); ERR_print_errors_fp(stderr); diff --git a/test/recipes/90-test_heartbeat.t b/test/recipes/90-test_heartbeat.t index 660f630f9e..a139c9a882 100644 --- a/test/recipes/90-test_heartbeat.t +++ b/test/recipes/90-test_heartbeat.t @@ -2,4 +2,4 @@ use OpenSSL::Test::Simple; -simple_test("test_heartbeat", "heartbeat_test"); +simple_test("test_heartbeat", "heartbeat_test", "heartbeats"); -- 2.25.1