Compression handling on session resume was badly broken: it always

used compression algorithms in client hello (a legacy from when
the compression algorithm wasn't serialized with SSL_SESSION).

diff --git a/CHANGES b/CHANGES
index bba32ef79b0313338d682572c3ce7dc626ff3aec..9f15d3d6a1a74023b792a7a34320c6763a33dbeb 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -44,6 +44,11 @@
 
  Changes between 0.9.8m (?) and 1.0.0  [xx XXX xxxx]
 
+  *) Fix compression algorithm handling: if resuming a session use the
+     compression algorithm of the resumed session instead of determining
+     it from client hello again. Don't allow server to change algorithm.
+     [Steve Henson]
+
   *) Constify crypto/cast (i.e., <openssl/cast.h>): a CAST_KEY doesn't
      change when encrypting or decrypting.
      [Bodo Moeller]

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 5c74f1750bdb6cb2ee9aa77a854b12b582497531..fadf638cc979c5c384dd90f2ccefdcb01bb21e95 100644 (file)
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1088,7 +1088,50 @@ int ssl3_get_client_hello(SSL *s)
         * algorithms from the client, starting at q. */
        s->s3->tmp.new_compression=NULL;
 #ifndef OPENSSL_NO_COMP
-       if (!(s->options & SSL_OP_NO_COMPRESSION) && s->ctx->comp_methods)
+       /* This only happens if we have a cache hit */
+       if (s->session->compress_meth != 0)
+               {
+               int m, comp_id = s->session->compress_meth;
+               /* Perform sanity checks on resumed compression algorithm */
+               /* Can't disable compression */
+               if (s->options & SSL_OP_NO_COMPRESSION)
+                       {
+                       al=SSL_AD_INTERNAL_ERROR;
+                       SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_INCONSISTENT_COMPRESSION);
+                       goto f_err;
+                       }
+               /* Look for resumed compression method */
+               for (m = 0; m < sk_SSL_COMP_num(s->ctx->comp_methods); m++)
+                       {
+                       comp=sk_SSL_COMP_value(s->ctx->comp_methods,m);
+                       if (comp_id == comp->id)
+                               {
+                               s->s3->tmp.new_compression=comp;
+                               break;
+                               }
+                       }
+               if (s->s3->tmp.new_compression == NULL)
+                       {
+                       al=SSL_AD_INTERNAL_ERROR;
+                       SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_INVALID_COMPRESSION_ALGORITHM);
+                       goto f_err;
+                       }
+               /* Look for resumed method in compression list */
+               for (m = 0; m < i; m++)
+                       {
+                       if (q[m] == comp_id)
+                               break;
+                       }
+               if (m >= i)
+                       {
+                       al=SSL_AD_ILLEGAL_PARAMETER;
+                       SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_REQUIRED_COMPRESSSION_ALGORITHM_MISSING);
+                       goto f_err;
+                       }
+               }
+       else if (s->hit)
+               comp = NULL;
+       else if (!(s->options & SSL_OP_NO_COMPRESSION) && s->ctx->comp_methods)
                { /* See if we have a match */
                int m,nn,o,v,done=0;
 
@@ -1112,6 +1155,16 @@ int ssl3_get_client_hello(SSL *s)
                else
                        comp=NULL;
                }
+#else
+       /* If compression is disabled we'd better not try to resume a session
+        * using compression.
+        */
+       if (s->session->compress_id != 0)
+               {
+               al=SSL_AD_INTERNAL_ERROR;
+               SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,SSL_R_INCONSISTENT_COMPRESSION);
+               goto f_err;
+               }
 #endif
 
        /* Given s->session->ciphers and SSL_get_ciphers, we must

diff --git a/ssl/ssl.h b/ssl/ssl.h
index 3c3ab46efd3da8271efb2dd44a71f07825a0df7a..d6c254e48e0fc4190a0ed84afdbcf33ccbdbd3fe 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2105,8 +2105,10 @@ void ERR_load_SSL_strings(void);
 #define SSL_R_HTTPS_PROXY_REQUEST                       155
 #define SSL_R_HTTP_REQUEST                              156
 #define SSL_R_ILLEGAL_PADDING                           283
+#define SSL_R_INCONSISTENT_COMPRESSION                  340
 #define SSL_R_INVALID_CHALLENGE_LENGTH                  158
 #define SSL_R_INVALID_COMMAND                           280
+#define SSL_R_INVALID_COMPRESSION_ALGORITHM             341
 #define SSL_R_INVALID_PURPOSE                           278
 #define SSL_R_INVALID_STATUS_RESPONSE                   328
 #define SSL_R_INVALID_TICKET_KEYS_LENGTH                325
@@ -2199,6 +2201,7 @@ void ERR_load_SSL_strings(void);
 #define SSL_R_RENEGOTIATION_ENCODING_ERR                336
 #define SSL_R_RENEGOTIATION_MISMATCH                    337
 #define SSL_R_REQUIRED_CIPHER_MISSING                   215
+#define SSL_R_REQUIRED_COMPRESSSION_ALGORITHM_MISSING   342
 #define SSL_R_REUSE_CERT_LENGTH_NOT_ZERO                216
 #define SSL_R_REUSE_CERT_TYPE_NOT_ZERO                  217
 #define SSL_R_REUSE_CIPHER_LIST_NOT_ZERO                218

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index f47e4a5099053836f61120b2342a1b72a1e5def1..44f2f6bbc3ad1b4d02d02663ea9960594885dc2a 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -357,8 +357,10 @@ static ERR_STRING_DATA SSL_str_reasons[]=
 {ERR_REASON(SSL_R_HTTPS_PROXY_REQUEST)   ,"https proxy request"},
 {ERR_REASON(SSL_R_HTTP_REQUEST)          ,"http request"},
 {ERR_REASON(SSL_R_ILLEGAL_PADDING)       ,"illegal padding"},
+{ERR_REASON(SSL_R_INCONSISTENT_COMPRESSION),"inconsistent compression"},
 {ERR_REASON(SSL_R_INVALID_CHALLENGE_LENGTH),"invalid challenge length"},
 {ERR_REASON(SSL_R_INVALID_COMMAND)       ,"invalid command"},
+{ERR_REASON(SSL_R_INVALID_COMPRESSION_ALGORITHM),"invalid compression algorithm"},
 {ERR_REASON(SSL_R_INVALID_PURPOSE)       ,"invalid purpose"},
 {ERR_REASON(SSL_R_INVALID_STATUS_RESPONSE),"invalid status response"},
 {ERR_REASON(SSL_R_INVALID_TICKET_KEYS_LENGTH),"invalid ticket keys length"},
@@ -451,6 +453,7 @@ static ERR_STRING_DATA SSL_str_reasons[]=
 {ERR_REASON(SSL_R_RENEGOTIATION_ENCODING_ERR),"renegotiation encoding err"},
 {ERR_REASON(SSL_R_RENEGOTIATION_MISMATCH),"renegotiation mismatch"},
 {ERR_REASON(SSL_R_REQUIRED_CIPHER_MISSING),"required cipher missing"},
+{ERR_REASON(SSL_R_REQUIRED_COMPRESSSION_ALGORITHM_MISSING),"required compresssion algorithm missing"},
 {ERR_REASON(SSL_R_REUSE_CERT_LENGTH_NOT_ZERO),"reuse cert length not zero"},
 {ERR_REASON(SSL_R_REUSE_CERT_TYPE_NOT_ZERO),"reuse cert type not zero"},
 {ERR_REASON(SSL_R_REUSE_CIPHER_LIST_NOT_ZERO),"reuse cipher list not zero"},