From a0473d82c6ad0d4f5f1e2f778a20ae374317720a Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 1 Dec 2014 11:10:38 +0000 Subject: [PATCH] MTU fixes patch --- apps/s_client.c | 16 +++++++- apps/s_server.c | 18 ++++++++- crypto/bio/bio.h | 4 ++ crypto/bio/bss_dgram.c | 46 ++++++++++++++++++++-- ssl/d1_both.c | 103 +++++++++++++++++++++++++++++++------------------ ssl/d1_lib.c | 32 +++++++++++++-- ssl/dtls1.h | 4 ++ ssl/ssl.h | 8 ++++ ssl/ssl_lib.c | 13 ------- ssl/ssl_locl.h | 3 +- 10 files changed, 184 insertions(+), 63 deletions(-) diff --git a/apps/s_client.c b/apps/s_client.c index 3ba6605..9f4be61 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -1307,10 +1307,22 @@ re_start: BIO_ctrl(sbio, BIO_CTRL_DGRAM_SET_SEND_TIMEOUT, 0, &timeout); } - if (socket_mtu > 28) + if (socket_mtu) { + if(socket_mtu < DTLS_get_link_min_mtu(con)) + { + BIO_printf(bio_err,"MTU too small. Must be at least %d\n", + DTLS_get_link_min_mtu(con)); + BIO_free(sbio); + goto shut; + } SSL_set_options(con, SSL_OP_NO_QUERY_MTU); - SSL_set_mtu(con, socket_mtu - 28); + if(!DTLS_set_link_mtu(con, socket_mtu)) + { + BIO_printf(bio_err, "Failed to set MTU\n"); + BIO_free(sbio); + goto shut; + } } else /* want to do MTU discovery */ diff --git a/apps/s_server.c b/apps/s_server.c index 8198d7f..a6f090b 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -2035,10 +2035,24 @@ static int sv_body(char *hostname, int s, unsigned char *context) BIO_ctrl(sbio, BIO_CTRL_DGRAM_SET_SEND_TIMEOUT, 0, &timeout); } - if (socket_mtu > 28) + if (socket_mtu) { + if(socket_mtu < DTLS_get_link_min_mtu(con)) + { + BIO_printf(bio_err,"MTU too small. Must be at least %d\n", + DTLS_get_link_min_mtu(con)); + ret = -1; + BIO_free(sbio); + goto err; + } SSL_set_options(con, SSL_OP_NO_QUERY_MTU); - SSL_set_mtu(con, socket_mtu - 28); + if(!DTLS_set_link_mtu(con, socket_mtu)) + { + BIO_printf(bio_err, "Failed to set MTU\n"); + ret = -1; + BIO_free(sbio); + goto err; + } } else /* want to do MTU discovery */ diff --git a/crypto/bio/bio.h b/crypto/bio/bio.h index 05699ab..32eba71 100644 --- a/crypto/bio/bio.h +++ b/crypto/bio/bio.h @@ -175,6 +175,8 @@ extern "C" { #define BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT 45 /* Next DTLS handshake timeout to * adjust socket timeouts */ +#define BIO_CTRL_DGRAM_GET_MTU_OVERHEAD 49 + #ifndef OPENSSL_NO_SCTP /* SCTP stuff */ #define BIO_CTRL_DGRAM_SCTP_SET_IN_HANDSHAKE 50 @@ -607,6 +609,8 @@ int BIO_ctrl_reset_read_request(BIO *b); (int)BIO_ctrl(b, BIO_CTRL_DGRAM_GET_PEER, 0, (char *)peer) #define BIO_dgram_set_peer(b,peer) \ (int)BIO_ctrl(b, BIO_CTRL_DGRAM_SET_PEER, 0, (char *)peer) +#define BIO_dgram_get_mtu_overhead(b) \ + (unsigned int)BIO_ctrl((b), BIO_CTRL_DGRAM_GET_MTU_OVERHEAD, 0, NULL) /* These two aren't currently implemented */ /* int BIO_get_ex_num(BIO *bio); */ diff --git a/crypto/bio/bss_dgram.c b/crypto/bio/bss_dgram.c index 8990909..0e5a52f 100644 --- a/crypto/bio/bss_dgram.c +++ b/crypto/bio/bss_dgram.c @@ -454,6 +454,36 @@ static int dgram_write(BIO *b, const char *in, int inl) return(ret); } +static long dgram_get_mtu_overhead(bio_dgram_data *data) + { + long ret; + + switch (data->peer.sa.sa_family) + { + case AF_INET: + /* Assume this is UDP - 20 bytes for IP, 8 bytes for UDP */ + ret = 28; + break; +#if OPENSSL_USE_IPV6 + case AF_INET6: +#ifdef IN6_IS_ADDR_V4MAPPED + if (IN6_IS_ADDR_V4MAPPED(&data->peer.sa_in6.sin6_addr)) + /* Assume this is UDP - 20 bytes for IP, 8 bytes for UDP */ + ret = 28; + else +#endif + /* Assume this is UDP - 40 bytes for IP, 8 bytes for UDP */ + ret = 48; + break; +#endif + default: + /* We don't know. Go with the historical default */ + ret = 28; + break; + } + return ret; + } + static long dgram_ctrl(BIO *b, int cmd, long num, void *ptr) { long ret=1; @@ -630,23 +660,24 @@ static long dgram_ctrl(BIO *b, int cmd, long num, void *ptr) #endif break; case BIO_CTRL_DGRAM_GET_FALLBACK_MTU: + ret = -dgram_get_mtu_overhead(data); switch (data->peer.sa.sa_family) { case AF_INET: - ret = 576 - 20 - 8; + ret += 576; break; #if OPENSSL_USE_IPV6 case AF_INET6: #ifdef IN6_IS_ADDR_V4MAPPED if (IN6_IS_ADDR_V4MAPPED(&data->peer.sa_in6.sin6_addr)) - ret = 576 - 20 - 8; + ret += 576; else #endif - ret = 1280 - 40 - 8; + ret += 1280; break; #endif default: - ret = 576 - 20 - 8; + ret += 576; break; } break; @@ -847,6 +878,9 @@ static long dgram_ctrl(BIO *b, int cmd, long num, void *ptr) ret = 0; break; #endif + case BIO_CTRL_DGRAM_GET_MTU_OVERHEAD: + ret = dgram_get_mtu_overhead(data); + break; default: ret=0; break; @@ -1367,6 +1401,10 @@ static long dgram_sctp_ctrl(BIO *b, int cmd, long num, void *ptr) * Returns always 1. */ break; + case BIO_CTRL_DGRAM_GET_MTU_OVERHEAD: + /* We allow transport protocol fragmentation so this is irrelevant */ + ret = 0; + break; case BIO_CTRL_DGRAM_SCTP_SET_IN_HANDSHAKE: if (num > 0) data->in_handshake = 1; diff --git a/ssl/d1_both.c b/ssl/d1_both.c index de8bab8..0781ca5 100644 --- a/ssl/d1_both.c +++ b/ssl/d1_both.c @@ -156,9 +156,8 @@ static unsigned char bitmask_start_values[] = {0xff, 0xfe, 0xfc, 0xf8, 0xf0, 0xe static unsigned char bitmask_end_values[] = {0xff, 0x01, 0x03, 0x07, 0x0f, 0x1f, 0x3f, 0x7f}; /* XDTLS: figure out the right values */ -static unsigned int g_probable_mtu[] = {1500 - 28, 512 - 28, 256 - 28}; +static const unsigned int g_probable_mtu[] = {1500, 512, 256}; -static unsigned int dtls1_guess_mtu(unsigned int curr_mtu); static void dtls1_fix_message_header(SSL *s, unsigned long frag_off, unsigned long frag_len); static unsigned char *dtls1_write_message_header(SSL *s, @@ -219,29 +218,48 @@ dtls1_hm_fragment_free(hm_fragment *frag) OPENSSL_free(frag); } +static int dtls1_query_mtu(SSL *s) +{ + if(s->d1->link_mtu) + { + s->d1->mtu = s->d1->link_mtu-BIO_dgram_get_mtu_overhead(SSL_get_wbio(s)); + s->d1->link_mtu = 0; + } + + /* AHA! Figure out the MTU, and stick to the right size */ + if (s->d1->mtu < dtls1_min_mtu(s)) + { + if(!(SSL_get_options(s) & SSL_OP_NO_QUERY_MTU)) + { + s->d1->mtu = + BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_QUERY_MTU, 0, NULL); + + /* I've seen the kernel return bogus numbers when it doesn't know + * (initial write), so just make sure we have a reasonable number */ + if (s->d1->mtu < dtls1_min_mtu(s)) + { + /* Set to min mtu */ + s->d1->mtu = dtls1_min_mtu(s); + BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_SET_MTU, + s->d1->mtu, NULL); + } + } + else + return 0; + } + return 1; +} + /* send s->init_buf in records of type 'type' (SSL3_RT_HANDSHAKE or SSL3_RT_CHANGE_CIPHER_SPEC) */ int dtls1_do_write(SSL *s, int type) { int ret; int curr_mtu; + int retry = 1; unsigned int len, frag_off, mac_size, blocksize; - /* AHA! Figure out the MTU, and stick to the right size */ - if (s->d1->mtu < dtls1_min_mtu() && !(SSL_get_options(s) & SSL_OP_NO_QUERY_MTU)) - { - s->d1->mtu = - BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_QUERY_MTU, 0, NULL); - - /* I've seen the kernel return bogus numbers when it doesn't know - * (initial write), so just make sure we have a reasonable number */ - if (s->d1->mtu < dtls1_min_mtu()) - { - s->d1->mtu = 0; - s->d1->mtu = dtls1_guess_mtu(s->d1->mtu); - BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_SET_MTU, - s->d1->mtu, NULL); - } - } + if(!dtls1_query_mtu(s)) + return -1; #if 0 mtu = s->d1->mtu; @@ -265,7 +283,7 @@ int dtls1_do_write(SSL *s, int type) } #endif - OPENSSL_assert(s->d1->mtu >= dtls1_min_mtu()); /* should have something reasonable now */ + OPENSSL_assert(s->d1->mtu >= dtls1_min_mtu(s)); /* should have something reasonable now */ if ( s->init_off == 0 && type == SSL3_RT_HANDSHAKE) OPENSSL_assert(s->init_num == @@ -318,12 +336,18 @@ int dtls1_do_write(SSL *s, int type) len += DTLS1_HM_HEADER_LENGTH; } + if ( len < DTLS1_HM_HEADER_LENGTH ) + { + /* + * len is so small that we really can't do anything sensible + * so fail + */ + return -1; + } dtls1_fix_message_header(s, frag_off, len - DTLS1_HM_HEADER_LENGTH); dtls1_write_message_header(s, (unsigned char *)&s->init_buf->data[s->init_off]); - - OPENSSL_assert(len >= DTLS1_HM_HEADER_LENGTH); } ret=dtls1_write_bytes(s,type,&s->init_buf->data[s->init_off], @@ -336,12 +360,23 @@ int dtls1_do_write(SSL *s, int type) * is fine and wait for an alert to handle the * retransmit */ - if ( BIO_ctrl(SSL_get_wbio(s), + if ( retry && BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_MTU_EXCEEDED, 0, NULL) > 0 ) - s->d1->mtu = BIO_ctrl(SSL_get_wbio(s), - BIO_CTRL_DGRAM_QUERY_MTU, 0, NULL); + { + if(!(SSL_get_options(s) & SSL_OP_NO_QUERY_MTU)) + { + if(!dtls1_query_mtu(s)) + return -1; + /* Have one more go */ + retry = 0; + } + else + return -1; + } else + { return(-1); + } } else { @@ -1380,28 +1415,20 @@ dtls1_write_message_header(SSL *s, unsigned char *p) return p; } -unsigned int -dtls1_min_mtu(void) +unsigned int +dtls1_link_min_mtu(void) { return (g_probable_mtu[(sizeof(g_probable_mtu) / sizeof(g_probable_mtu[0])) - 1]); } -static unsigned int -dtls1_guess_mtu(unsigned int curr_mtu) +unsigned int +dtls1_min_mtu(SSL *s) { - unsigned int i; - - if ( curr_mtu == 0 ) - return g_probable_mtu[0] ; - - for ( i = 0; i < sizeof(g_probable_mtu)/sizeof(g_probable_mtu[0]); i++) - if ( curr_mtu > g_probable_mtu[i]) - return g_probable_mtu[i]; - - return curr_mtu; + return dtls1_link_min_mtu()-BIO_dgram_get_mtu_overhead(SSL_get_wbio(s)); } + void dtls1_get_message_header(unsigned char *data, struct hm_header_st *msg_hdr) { diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c index f61f718..87c36ad 100644 --- a/ssl/d1_lib.c +++ b/ssl/d1_lib.c @@ -113,6 +113,9 @@ int dtls1_new(SSL *s) d1->cookie_len = sizeof(s->d1->cookie); } + d1->link_mtu = 0; + d1->mtu = 0; + if( ! d1->unprocessed_rcds.q || ! d1->processed_rcds.q || ! d1->buffered_messages || ! d1->sent_messages || ! d1->buffered_app_data.q) { @@ -206,6 +209,7 @@ void dtls1_clear(SSL *s) pqueue sent_messages; pqueue buffered_app_data; unsigned int mtu; + unsigned int link_mtu; if (s->d1) { @@ -215,6 +219,7 @@ void dtls1_clear(SSL *s) sent_messages = s->d1->sent_messages; buffered_app_data = s->d1->buffered_app_data.q; mtu = s->d1->mtu; + link_mtu = s->d1->link_mtu; dtls1_clear_queues(s); @@ -228,6 +233,7 @@ void dtls1_clear(SSL *s) if (SSL_get_options(s) & SSL_OP_NO_QUERY_MTU) { s->d1->mtu = mtu; + s->d1->link_mtu = link_mtu; } s->d1->unprocessed_rcds.q = unprocessed_rcds; @@ -262,7 +268,22 @@ long dtls1_ctrl(SSL *s, int cmd, long larg, void *parg) case DTLS_CTRL_LISTEN: ret = dtls1_listen(s, parg); break; - + case DTLS_CTRL_SET_LINK_MTU: + if (larg < (long)dtls1_link_min_mtu()) + return 0; + s->d1->link_mtu = larg; + return 1; + case DTLS_CTRL_GET_LINK_MIN_MTU: + return (long)dtls1_link_min_mtu(); + case SSL_CTRL_SET_MTU: + /* + * We may not have a BIO set yet so can't call dtls1_min_mtu() + * We'll have to make do with dtls1_link_min_mtu() and max overhead + */ + if (larg < (long)dtls1_link_min_mtu() - DTLS1_MAX_MTU_OVERHEAD) + return 0; + s->d1->mtu = larg; + return larg; default: ret = ssl3_ctrl(s, cmd, larg, parg); break; @@ -401,12 +422,17 @@ void dtls1_stop_timer(SSL *s) int dtls1_check_timeout_num(SSL *s) { + unsigned int mtu; + s->d1->timeout.num_alerts++; /* Reduce MTU after 2 unsuccessful retransmissions */ - if (s->d1->timeout.num_alerts > 2) + if (s->d1->timeout.num_alerts > 2 + && !(SSL_get_options(s) & SSL_OP_NO_QUERY_MTU)) { - s->d1->mtu = BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_GET_FALLBACK_MTU, 0, NULL); + mtu = BIO_ctrl(SSL_get_wbio(s), BIO_CTRL_DGRAM_GET_FALLBACK_MTU, 0, NULL); + if(mtu < s->d1->mtu) + s->d1->mtu = mtu; } if (s->d1->timeout.num_alerts > DTLS1_TMO_ALERT_COUNT) diff --git a/ssl/dtls1.h b/ssl/dtls1.h index e65d501..fd168e7 100644 --- a/ssl/dtls1.h +++ b/ssl/dtls1.h @@ -115,6 +115,9 @@ extern "C" { #define DTLS1_SCTP_AUTH_LABEL "EXPORTER_DTLS_OVER_SCTP" #endif +/* Max MTU overhead we know about so far is 40 for IPv6 + 8 for UDP */ +#define DTLS1_MAX_MTU_OVERHEAD 48 + typedef struct dtls1_bitmap_st { unsigned long map; /* track 32 packets on 32-bit systems @@ -229,6 +232,7 @@ typedef struct dtls1_state_st /* Is set when listening for new connections with dtls1_listen() */ unsigned int listen; + unsigned int link_mtu; /* max on-the-wire DTLS packet size */ unsigned int mtu; /* max DTLS packet size */ struct hm_header_st w_msg_hdr; diff --git a/ssl/ssl.h b/ssl/ssl.h index 593579e..377ceff 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -672,6 +672,10 @@ struct ssl_session_st SSL_ctrl((ssl),SSL_CTRL_MODE,0,NULL) #define SSL_set_mtu(ssl, mtu) \ SSL_ctrl((ssl),SSL_CTRL_SET_MTU,(mtu),NULL) +#define DTLS_set_link_mtu(ssl, mtu) \ + SSL_ctrl((ssl),DTLS_CTRL_SET_LINK_MTU,(mtu),NULL) +#define DTLS_get_link_min_mtu(ssl) \ + SSL_ctrl((ssl),DTLS_CTRL_GET_LINK_MIN_MTU,0,NULL) #define SSL_get_secure_renegotiation_support(ssl) \ SSL_ctrl((ssl), SSL_CTRL_GET_RI_SUPPORT, 0, NULL) @@ -1609,6 +1613,10 @@ DECLARE_PEM_rw(SSL_SESSION, SSL_SESSION) #define SSL_CTRL_GET_EXTRA_CHAIN_CERTS 82 #define SSL_CTRL_CLEAR_EXTRA_CHAIN_CERTS 83 + +#define DTLS_CTRL_SET_LINK_MTU 120 +#define DTLS_CTRL_GET_LINK_MIN_MTU 121 + #define DTLSv1_get_timeout(ssl, arg) \ SSL_ctrl(ssl,DTLS_CTRL_GET_TIMEOUT,0, (void *)arg) #define DTLSv1_handle_timeout(ssl) \ diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 14d143d..cd94f30 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -1080,19 +1080,6 @@ long SSL_ctrl(SSL *s,int cmd,long larg,void *parg) l=s->max_cert_list; s->max_cert_list=larg; return(l); - case SSL_CTRL_SET_MTU: -#ifndef OPENSSL_NO_DTLS1 - if (larg < (long)dtls1_min_mtu()) - return 0; -#endif - - if (SSL_version(s) == DTLS1_VERSION || - SSL_version(s) == DTLS1_BAD_VER) - { - s->d1->mtu = larg; - return larg; - } - return 0; case SSL_CTRL_SET_MAX_SEND_FRAGMENT: if (larg < 512 || larg > SSL3_RT_MAX_PLAIN_LENGTH) return 0; diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 1b98947..7f4c375 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -983,7 +983,8 @@ void dtls1_stop_timer(SSL *s); int dtls1_is_timer_expired(SSL *s); void dtls1_double_timeout(SSL *s); int dtls1_send_newsession_ticket(SSL *s); -unsigned int dtls1_min_mtu(void); +unsigned int dtls1_min_mtu(SSL *s); +unsigned int dtls1_link_min_mtu(void); /* some client-only functions */ int ssl3_client_hello(SSL *s); -- 2.1.0