From 22e3dcb7808bb06cd18c3231e34a5930e796cc48 Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Mon, 25 Jan 2016 13:30:37 -0500 Subject: 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 --- 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 +- 9 files changed, 54 insertions(+), 229 deletions(-) (limited to 'ssl') diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index b1f6ed2..8b3e941 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 c53ef9a..49e8f46 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 d0f17bb..6a4f92f 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 8e2d7c4..8b25b0e7 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 4570fbb..e6b9bbd 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 f10570b..d7a7d01 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 5feef6e..25ec77e 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 446a678..e0e0cb9 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 7d451da..de1cac7 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; -- cgit v1.1