[openssl.org #2089] [PATCH] DTLS Fragment size bug

[Misha Aizatulin via RT]

DTLS fragmentation doesn't consider the additional data required with  
using encryption, so the packet size then exceeds the MTU when  
fragmentation is performed. This patch subtracts the size of the mac  
and the additional encryption bytes of the maximum possible length for  
a fragment, if necessary.

Thanks to Daniel Mentz for finding this bug.

Regards,
Robin


--- ssl/d1_both.c 1 Oct 2009 00:03:50 -0000 1.14.2.15
+++ ssl/d1_both.c 30 Oct 2009 15:16:40 -0000
@@ -177,7 +177,7 @@
  {
  int ret;
  int curr_mtu;
- unsigned int len, frag_off;
+ unsigned int len, frag_off, mac_size, blocksize;

  /* AHA!  Figure out the MTU, and stick to the right size */
  if ( ! (SSL_get_options(s) & SSL_OP_NO_QUERY_MTU))
@@ -225,11 +225,22 @@
  OPENSSL_assert(s->init_num ==
  (int)s->d1->w_msg_hdr.msg_len + DTLS1_HM_HEADER_LENGTH);

+ if (s->write_hash)
+ mac_size = EVP_MD_CTX_size(s->write_hash);
+ else
+ mac_size = 0;
+
+ if (s->enc_write_ctx &&
+ (EVP_CIPHER_mode( s->enc_write_ctx->cipher) & EVP_CIPH_CBC_MODE))
+ blocksize = 2 * EVP_CIPHER_block_size(s->enc_write_ctx->cipher);
+ else
+ blocksize = 0;
+
  frag_off = 0;
  while( s->init_num)
  {
  curr_mtu = s->d1->mtu - BIO_wpending(SSL_get_wbio(s)) -
- DTLS1_RT_HEADER_LENGTH;
+ DTLS1_RT_HEADER_LENGTH - mac_size - blocksize;

  if ( curr_mtu <= DTLS1_HM_HEADER_LENGTH)
  {
@@ -237,7 +248,8 @@
  ret = BIO_flush(SSL_get_wbio(s));
  if ( ret <= 0)
  return ret;
- curr_mtu = s->d1->mtu - DTLS1_RT_HEADER_LENGTH;
+ curr_mtu = s->d1->mtu - DTLS1_RT_HEADER_LENGTH -
+ mac_size - blocksize;
  }

  if ( s->init_num > curr_mtu)

[Misha Aizatulin via RT]

On Fri, 2009-10-30 at 18:11 +0100, Robin Seggelmann via RT wrote:
> DTLS fragmentation doesn't consider the additional data required with  
> using encryption, so the packet size then exceeds the MTU when  
> fragmentation is performed. This patch subtracts the size of the mac  
> and the additional encryption bytes of the maximum possible length for  
> a fragment, if necessary.
>
> Thanks to Daniel Mentz for finding this bug.
>

...

> + if (s->write_hash)
> + mac_size = EVP_MD_CTX_size(s->write_hash);
> + else
> + mac_size = 0;
> +

This was applied to the 0.9.8 branch too, where it causes a SEGV. It
should be EVP_MD_size() there.

Index: ssl/d1_both.c
===================================================================
RCS file: /home/dwmw2/openssl-cvs/openssl/ssl/d1_both.c,v
retrieving revision 1.4.2.21
diff -u -p -r1.4.2.21 d1_both.c
--- ssl/d1_both.c 2 Nov 2009 13:36:56 -0000 1.4.2.21
+++ ssl/d1_both.c 3 Nov 2009 20:41:54 -0000
@@ -226,7 +226,7 @@ int dtls1_do_write(SSL *s, int type)
  (int)s->d1->w_msg_hdr.msg_len + DTLS1_HM_HEADER_LENGTH);
 
  if (s->write_hash)
- mac_size = EVP_MD_CTX_size(s->write_hash);
+ mac_size = EVP_MD_size(s->write_hash);
  else
  mac_size = 0;
 


--
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@...                              Intel Corporation


______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@...
Automated List Manager                           majordomo@...