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).

CHANGES

@@ -4,6 +4,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.
      [Steve Henson]

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

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

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"},