From e642fd7a1c0f506deccab37ae37f59737d4ee6f9 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Fri, 1 Jan 2010 00:44:36 +0000 Subject: [PATCH] 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 | 5 +++++ ssl/s3_srvr.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++- ssl/ssl.h | 3 +++ ssl/ssl_err.c | 3 +++ 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index a9c5dfdbfa..97254ec967 100644 --- 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 5c74f1750b..fadf638cc9 100644 --- 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 f7f8be7e29..0b96093c88 100644 --- 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 f47e4a5099..44f2f6bbc3 100644 --- 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"}, -- 2.25.1