From c32f064fee73783142835ac11c0595d7bcd24d35 Mon Sep 17 00:00:00 2001 From: Joe Gooch Date: Mon, 6 Feb 2012 15:21:10 -0500 Subject: [PATCH] SSL Renegotiation Patch for v2.6 final. This patch adds two new config directives: SSLHonorCipherOrder 0|1 When set to 1, server prefers Ciphers in the order specified. When 0, Server advertises no preference. SSLAllowClientRenegotiation 0|1|2 When set to 0, no client renegotiation will be honored. When 1, secure renegotiation will be honored. When 2, insecure renegotiation will be honored. It will also disable insecure renegotiation on backend HTTPS connections. Given these options, the most secure configuration would be: SSLAllowClientRenegotiation 0 SSLHonorCipherOrder 1 Ciphers "ECDHE-RSA-AES256-SHA384:AES256-SHA256:RC4:HIGH:!MD5:!aNULL:!EDH:!AESGCM" Which mitigates BEAST attacks as outlined here: http://blog.ivanristic.com/2011/10/mitigating-the-beast-attack-on-tls.html As well as renegotiation attacks. Test your server at https://www.ssllabs.com/ssldb/ --- config.c | 39 +++++++++++++++++++++++++++++++++++++-- http.c | 45 ++++++++++++++++++++++++++++++++++++++------- pound.8 | 13 +++++++++++++ pound.h | 9 +++++++++ svc.c | 31 +++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 9 deletions(-) diff --git a/config.c b/config.c index 1294d84..b066dd8 100755 --- a/config.c +++ b/config.c @@ -76,7 +76,7 @@ static regex_t ListenHTTP, ListenHTTPS, End, Address, Port, Cert, xHTTP, Client static regex_t Err414, Err500, Err501, Err503, MaxRequest, HeadRemove, RewriteLocation, RewriteDestination; static regex_t Service, ServiceName, URL, HeadRequire, HeadDeny, BackEnd, Emergency, Priority, HAport, HAportAddr; static regex_t Redirect, RedirectN, TimeOut, Session, Type, TTL, ID, DynScale; -static regex_t ClientCert, AddHeader, Ciphers, CAlist, VerifyList, CRLlist, NoHTTPS11; +static regex_t ClientCert, AddHeader, SSLAllowClientRenegotiation, SSLHonorCipherOrder, Ciphers, CAlist, VerifyList, CRLlist, NoHTTPS11; static regex_t Grace, Include, ConnTO, IgnoreCase, HTTPS, HTTPSCert, Disabled, Threads, CNName; static regmatch_t matches[5]; @@ -289,9 +289,12 @@ parse_be(const int is_emergency) } else if(!regexec(&HTTPS, lin, 4, matches, 0)) { if((res->ctx = SSL_CTX_new(SSLv23_client_method())) == NULL) conf_err("SSL_CTX_new failed - aborted"); + SSL_CTX_set_app_data(res->ctx, res); SSL_CTX_set_verify(res->ctx, SSL_VERIFY_NONE, NULL); SSL_CTX_set_mode(res->ctx, SSL_MODE_AUTO_RETRY); SSL_CTX_set_options(res->ctx, SSL_OP_ALL); + SSL_CTX_clear_options(res->ctx, SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION); + SSL_CTX_clear_options(res->ctx, SSL_OP_LEGACY_SERVER_CONNECT); sprintf(lin, "%d-Pound-%ld", getpid(), random()); SSL_CTX_set_session_id_context(res->ctx, (unsigned char *)lin, strlen(lin)); SSL_CTX_set_tmp_rsa_callback(res->ctx, RSA_tmp_callback); @@ -299,6 +302,7 @@ parse_be(const int is_emergency) } else if(!regexec(&HTTPSCert, lin, 4, matches, 0)) { if((res->ctx = SSL_CTX_new(SSLv23_client_method())) == NULL) conf_err("SSL_CTX_new failed - aborted"); + SSL_CTX_set_app_data(res->ctx, res); lin[matches[1].rm_eo] = '\0'; if(SSL_CTX_use_certificate_chain_file(res->ctx, lin + matches[1].rm_so) != 1) conf_err("SSL_CTX_use_certificate_chain_file failed - aborted"); @@ -309,6 +313,8 @@ parse_be(const int is_emergency) SSL_CTX_set_verify(res->ctx, SSL_VERIFY_NONE, NULL); SSL_CTX_set_mode(res->ctx, SSL_MODE_AUTO_RETRY); SSL_CTX_set_options(res->ctx, SSL_OP_ALL); + SSL_CTX_clear_options(res->ctx, SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION); + SSL_CTX_clear_options(res->ctx, SSL_OP_LEGACY_SERVER_CONNECT); sprintf(lin, "%d-Pound-%ld", getpid(), random()); SSL_CTX_set_session_id_context(res->ctx, (unsigned char *)lin, strlen(lin)); SSL_CTX_set_tmp_rsa_callback(res->ctx, RSA_tmp_callback); @@ -829,11 +835,15 @@ parse_HTTPS(void) SERVICE *svc; MATCHER *m; int has_addr, has_port, has_other; + long ssl_op_enable, ssl_op_disable; struct hostent *host; struct sockaddr_in in; struct sockaddr_in6 in6; POUND_CTX *pc; + ssl_op_enable = SSL_OP_ALL; + ssl_op_disable = SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION | SSL_OP_LEGACY_SERVER_CONNECT; + if((res = (LISTENER *)malloc(sizeof(LISTENER))) == NULL) conf_err("ListenHTTPS config: out of memory - aborted"); memset(res, 0, sizeof(LISTENER)); @@ -844,6 +854,7 @@ parse_HTTPS(void) res->err500 = "An internal server error occurred. Please try again later."; res->err501 = "This method may not be used."; res->err503 = "The service is not available. Please try again later."; + res->allow_cl_reneg = 0; res->log_level = log_level; if(regcomp(&res->verb, xhttp[0], REG_ICASE | REG_NEWLINE | REG_EXTENDED)) conf_err("xHTTP bad default pattern - aborted"); @@ -1029,6 +1040,23 @@ parse_HTTPS(void) strcat(res->add_head, "\r\n"); strcat(res->add_head, lin + matches[1].rm_so); } + } else if(!regexec(&SSLAllowClientRenegotiation, lin, 4, matches, 0)) { + res->allow_cl_reneg = atoi(lin + matches[1].rm_so); + if (res->allow_cl_reneg == 2) { + ssl_op_enable |= SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION; + ssl_op_disable &= ~SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION; + } else { + ssl_op_disable |= SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION; + ssl_op_enable &= ~SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION; + } + } else if(!regexec(&SSLHonorCipherOrder, lin, 4, matches, 0)) { + if (atoi(lin + matches[1].rm_so)) { + ssl_op_enable |= SSL_OP_CIPHER_SERVER_PREFERENCE; + ssl_op_disable &= ~SSL_OP_CIPHER_SERVER_PREFERENCE; + } else { + ssl_op_disable |= SSL_OP_CIPHER_SERVER_PREFERENCE; + ssl_op_enable &= ~SSL_OP_CIPHER_SERVER_PREFERENCE; + } } else if(!regexec(&Ciphers, lin, 4, matches, 0)) { has_other = 1; if(res->ctx == NULL) @@ -1105,12 +1133,15 @@ parse_HTTPS(void) conf_err("ListenHTTPS: can't set SNI callback"); #endif for(pc = res->ctx; pc; pc = pc->next) { + SSL_CTX_set_app_data(pc->ctx, res); SSL_CTX_set_mode(pc->ctx, SSL_MODE_AUTO_RETRY); - SSL_CTX_set_options(pc->ctx, SSL_OP_ALL); + SSL_CTX_set_options(pc->ctx, ssl_op_enable); + SSL_CTX_clear_options(pc->ctx, ssl_op_disable); sprintf(lin, "%d-Pound-%ld", getpid(), random()); SSL_CTX_set_session_id_context(pc->ctx, (unsigned char *)lin, strlen(lin)); SSL_CTX_set_tmp_rsa_callback(pc->ctx, RSA_tmp_callback); SSL_CTX_set_tmp_dh_callback(pc->ctx, DH_tmp_callback); + SSL_CTX_set_info_callback(pc->ctx, SSLINFO_callback); } return res; } else { @@ -1305,6 +1336,8 @@ config_parse(const int argc, char **const argv) || regcomp(&DynScale, "^[ \t]*DynScale[ \t]+([01])[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED) || regcomp(&ClientCert, "^[ \t]*ClientCert[ \t]+([0-3])[ \t]+([1-9])[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED) || regcomp(&AddHeader, "^[ \t]*AddHeader[ \t]+\"(.+)\"[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED) + || regcomp(&SSLAllowClientRenegotiation, "^[ \t]*SSLAllowClientRenegotiation[ \t]+([012])[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED) + || regcomp(&SSLHonorCipherOrder, "^[ \t]*SSLHonorCipherOrder[ \t]+([01])[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED) || regcomp(&Ciphers, "^[ \t]*Ciphers[ \t]+\"(.+)\"[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED) || regcomp(&CAlist, "^[ \t]*CAlist[ \t]+\"(.+)\"[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED) || regcomp(&VerifyList, "^[ \t]*VerifyList[ \t]+\"(.+)\"[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED) @@ -1463,6 +1496,8 @@ config_parse(const int argc, char **const argv) regfree(&DynScale); regfree(&ClientCert); regfree(&AddHeader); + regfree(&SSLAllowClientRenegotiation); + regfree(&SSLHonorCipherOrder); regfree(&Ciphers); regfree(&CAlist); regfree(&VerifyList); diff --git a/http.c b/http.c index bb2ce8b..d420b69 100755 --- a/http.c +++ b/http.c @@ -246,6 +246,11 @@ copy_chunks(BIO *const cl, BIO *const be, LONG *res_bytes, const int no_write, c static int err_to = -1; +typedef struct { + int timeout; + RENEG_STATE *reneg_state; +} BIO_ARG; + /* * Time-out for client read/gets * the SSL manual says not to do it, but it works well enough anyway... @@ -253,18 +258,32 @@ static int err_to = -1; static long bio_callback(BIO *const bio, const int cmd, const char *argp, int argi, long argl, long ret) { + BIO_ARG *bio_arg; struct pollfd p; int to, p_res, p_err; if(cmd != BIO_CB_READ && cmd != BIO_CB_WRITE) return ret; + //logmsg(LOG_NOTICE, "bio callback"); /* a time-out already occured */ - if((to = *((int *)BIO_get_callback_arg(bio)) * 1000) < 0) { + if((bio_arg = (BIO_ARG*)BIO_get_callback_arg(bio))==NULL) return ret; + if((to = bio_arg->timeout * 1000) < 0) { errno = ETIMEDOUT; return -1; } + /* Renegotiations */ + //logmsg(LOG_NOTICE, "RENEG STATE %d", bio_arg->reneg_state==NULL?-1:*bio_arg->reneg_state); + if (bio_arg->reneg_state != NULL && *bio_arg->reneg_state == RENEG_ABORT) { + logmsg(LOG_NOTICE, "REJECTING renegotiated session"); + errno = ECONNABORTED; + return -1; + } + + //logmsg(LOG_NOTICE, "TO %d", to); + if (to == 0) return ret; + for(;;) { memset(&p, 0, sizeof(p)); BIO_get_fd(bio, &p.fd); @@ -299,7 +318,7 @@ bio_callback(BIO *const bio, const int cmd, const char *argp, int argi, long arg return -1; case 0: /* timeout - mark the BIO as unusable for the future */ - BIO_set_callback_arg(bio, (char *)&err_to); + bio_arg->timeout = err_to; #ifdef EBUG logmsg(LOG_WARNING, "(%lx) CALLBACK timeout poll after %d secs: %s", pthread_self(), to / 1000, strerror(p_err)); @@ -503,7 +522,14 @@ do_http(thr_arg *arg) regmatch_t matches[4]; struct linger l; double start_req, end_req; - + RENEG_STATE reneg_state; + BIO_ARG ba1, ba2; + + reneg_state = RENEG_INIT; + ba1.reneg_state = &reneg_state; + ba2.reneg_state = &reneg_state; + ba1.timeout = 0; + ba2.timeout = 0; from_host = ((thr_arg *)arg)->from_host; memcpy(&from_host_addr, from_host.ai_addr, from_host.ai_addrlen); from_host.ai_addr = (struct sockaddr *)&from_host_addr; @@ -512,6 +538,8 @@ do_http(thr_arg *arg) free(((thr_arg *)arg)->from_host.ai_addr); free(arg); + if(lstn->allow_cl_reneg) reneg_state = RENEG_ALLOW; + n = 1; setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, (void *)&n, sizeof(n)); l.l_onoff = 1; @@ -535,10 +563,11 @@ do_http(thr_arg *arg) close(sock); return; } - if(lstn->to > 0) { - BIO_set_callback_arg(cl, (char *)&lstn->to); + //if(lstn->to > 0) { + ba1.timeout = lstn->to; + BIO_set_callback_arg(cl, (char *)&ba1); BIO_set_callback(cl, bio_callback); - } + //} if(lstn->ctx != NULL) { if((ssl = SSL_new(lstn->ctx->ctx)) == NULL) { @@ -547,6 +576,7 @@ do_http(thr_arg *arg) BIO_free_all(cl); return; } + SSL_set_app_data(ssl, &reneg_state); SSL_set_bio(ssl, cl, cl); if((bb = BIO_new(BIO_f_ssl())) == NULL) { logmsg(LOG_WARNING, "(%lx) BIO_new(Bio_f_ssl()) failed", pthread_self()); @@ -848,7 +878,8 @@ do_http(thr_arg *arg) } BIO_set_close(be, BIO_CLOSE); if(backend->to > 0) { - BIO_set_callback_arg(be, (char *)&backend->to); + ba2.timeout = backend->to; + BIO_set_callback_arg(be, (char *)&ba2); BIO_set_callback(be, bio_callback); } if(backend->ctx != NULL) { diff --git a/pound.8 b/pound.8 index f878a4d..b95e794 100755 --- a/pound.8 +++ b/pound.8 @@ -501,6 +501,19 @@ string in the same format as in OpenSSL and .I SSL_CTX_set_cipher_list(3). .TP +\fBSSLHonorCipherOrder\fR 0|1 +If this value is 1, the server will broadcast a preference to use \fBCiphers\fR in the +order supplied in the \fBCiphers\fR directive. If the value is 0, the server will treat +the Ciphers list as the list of Ciphers it will accept, but no preference will be +indicated. Default value is 0. +.TP +\fBSSLAllowClientRenegotiation\fR 0|1|2 +If this value is 0, client initiated renegotiation will be disabled. This will mitigate +DoS exploits based on client renegotiation, regardless of the patch status of clients and +servers related to "Secure renegotiation". If the value is 1, secure renegotiation is +supported. If the value is 2, insecure renegotiation is supported, with unpatched +clients. /fBThis can lead to a DoS and a Man in the Middle attack!/fR Default value is 0. +.TP \fBCAlist\fR "CAcert_file" Set the list of "trusted" CA's for this server. The CAcert_file is a file containing a sequence of CA certificates (PEM format). The names of the defined CA certificates diff --git a/pound.h b/pound.h index 114db58..3feb0fd 100755 --- a/pound.h +++ b/pound.h @@ -404,6 +404,7 @@ typedef struct _listener { int rewr_dest; /* rewrite destination header */ int disabled; /* true if the listener is disabled */ int log_level; /* log level for this listener */ + int allow_cl_reneg; /* Allow Client SSL Renegotiation */ SERVICE *services; struct _listener *next; } LISTENER; @@ -419,6 +420,9 @@ typedef struct _thr_arg { struct _thr_arg *next; } thr_arg; /* argument to processing threads: socket, origin */ +/* Track SSL handshare/renegotiation so we can reject client-renegotiations. */ +typedef enum { RENEG_INIT=0, RENEG_REJECT, RENEG_ALLOW, RENEG_ABORT } RENEG_STATE; + /* Header types */ #define HEADER_ILLEGAL -1 #define HEADER_OTHER 0 @@ -590,6 +594,11 @@ extern RSA *RSA_tmp_callback(SSL *, int, int); */ extern DH *DH_tmp_callback(SSL *, int, int); +/* + * Renegotiation callback + */ +extern void SSLINFO_callback(const SSL *s, int where, int rc); + /* * expiration stuff */ diff --git a/svc.c b/svc.c index fca3e3b..8c33a10 100755 --- a/svc.c +++ b/svc.c @@ -1797,3 +1797,34 @@ thr_control(void *arg) close(ctl); } } + +void +SSLINFO_callback(const SSL *ssl, int where, int rc) +{ + RENEG_STATE *reneg_state; + + /* Get our thr_arg where we're tracking this connection info */ + if ((reneg_state = (RENEG_STATE *)SSL_get_app_data(ssl)) == NULL) return; + + /* If we're rejecting renegotiations, move to ABORT if Client Hello is being read. */ + if ((where & SSL_CB_ACCEPT_LOOP) && *reneg_state == RENEG_REJECT) { + int state = SSL_get_state(ssl); + + if (state == SSL3_ST_SR_CLNT_HELLO_A || state == SSL23_ST_SR_CLNT_HELLO_A) { + *reneg_state = RENEG_ABORT; + logmsg(LOG_WARNING,"rejecting client initiated renegotiation"); + } + } + else if (where & SSL_CB_HANDSHAKE_DONE && *reneg_state == RENEG_INIT) { + // Reject any followup renegotiations + *reneg_state = RENEG_REJECT; + } + + //if (where & SSL_CB_HANDSHAKE_START) logmsg(LOG_DEBUG, "handshake start"); + //else if (where & SSL_CB_HANDSHAKE_DONE) logmsg(LOG_DEBUG, "handshake done"); + //else if (where & SSL_CB_LOOP) logmsg(LOG_DEBUG, "loop"); + //else if (where & SSL_CB_READ) logmsg(LOG_DEBUG, "read"); + //else if (where & SSL_CB_WRITE) logmsg(LOG_DEBUG, "write"); + //else if (where & SSL_CB_ALERT) logmsg(LOG_DEBUG, "alert"); +} +