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 a9c5dfdbfa4c61d9ea5b1c12e451178b5a99c2ce..97254ec9678d2158dcac853d6c9ec1fdedeb33ec 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -3,6 +3,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]
 
   *) Add load_crls() function to apps tidying load_certs() too. Add option
      to verify utility to allow additional CRLs to be included.

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 f7f8be7e29037dff4d3b990a267bb7979fe42449..0b96093c887061b948ab11dfb4a3f6bab0887ac4 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2099,8 +2099,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
@@ -2193,6 +2195,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"},