Here's an updated version; this mainly simplifies code, per comments from Andres (things were a bit too baroque in places due to the way the code had evolved, and I hadn't gone over it to simplify it).
The only behavior change is that the renegotiation is requested 1kB before the limit is hit: the raise to 1% of the configured limit was removed. The "fixup" is an incremental on top of the previous version; the other one is the full v2 patch. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
*** a/src/backend/libpq/be-secure.c --- b/src/backend/libpq/be-secure.c *************** *** 324,344 **** secure_write(Port *port, void *ptr, size_t len) if (port->ssl) { int err; - static long renegotiation_count = 0; /* * If SSL renegotiations are enabled and we're getting close to the * limit, start one now; but avoid it if there's one already in ! * progress. Request the renegotiation before the limit has actually ! * expired; we give it 1% of the specified limit, or 1kB, whichever is ! * greater. */ if (ssl_renegotiation_limit && !in_ssl_renegotiation && ! port->count > Min((ssl_renegotiation_limit - 1) * 1024L, ! (long) rint(ssl_renegotiation_limit * 1024L * 0.99))) { in_ssl_renegotiation = true; SSL_set_session_id_context(port->ssl, (void *) &SSL_context, sizeof(SSL_context)); if (SSL_renegotiate(port->ssl) <= 0) --- 324,352 ---- if (port->ssl) { int err; /* * If SSL renegotiations are enabled and we're getting close to the * limit, start one now; but avoid it if there's one already in ! * progress. Request the renegotiation 1kB before the limit has ! * actually expired. */ if (ssl_renegotiation_limit && !in_ssl_renegotiation && ! port->count > (ssl_renegotiation_limit - 1) * 1024L) { in_ssl_renegotiation = true; + /* + * The way we determine that a renegotiation has completed is by + * observing OpenSSL's internal renegotiation counter. Make sure + * we start out at zero, and assume that the renegotiation is + * complete when the counter advances. + * + * OpenSSL provides SSL_renegotiation_pending(), but this doesn't + * seem to work in testing. + */ + SSL_clear_num_renegotiations(port->ssl); + SSL_set_session_id_context(port->ssl, (void *) &SSL_context, sizeof(SSL_context)); if (SSL_renegotiate(port->ssl) <= 0) *************** *** 347,379 **** secure_write(Port *port, void *ptr, size_t len) errmsg("SSL failure during renegotiation start"))); else { ! int handshake; ! int retries = 20; /* ! * The way we determine that a renegotiation has completed is ! * by observing OpenSSL's internal renegotiation counter. ! * Memoize it when starting the renegotiation, and assume that ! * the renegotiation is complete when the counter advances. ! * ! * OpenSSL provides SSL_renegotiation_pending(), but this ! * doesn't seem to work in testing. */ ! renegotiation_count = SSL_num_renegotiations(port->ssl); ! ! /* ! * A handshake can fail, so be prepared to retry it, but only a ! * few times ! */ ! for (;;) { ! handshake = SSL_do_handshake(port->ssl); ! if (handshake > 0) break; /* done */ ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("SSL handshake failure on renegotiation, retrying"))); ! if (retries-- <= 0) ereport(FATAL, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("unable to complete SSL handshake"))); --- 355,374 ---- errmsg("SSL failure during renegotiation start"))); else { ! int retries; /* ! * A handshake can fail, so be prepared to retry it, but only ! * a few times. */ ! for (retries = 0; retries++;) { ! if (SSL_do_handshake(port->ssl) > 0) break; /* done */ ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("SSL handshake failure on renegotiation, retrying"))); ! if (retries >= 20) ereport(FATAL, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("unable to complete SSL handshake"))); *************** *** 427,439 **** wloop: /* is renegotiation complete? */ if (in_ssl_renegotiation && ! SSL_num_renegotiations(port->ssl) > renegotiation_count) { in_ssl_renegotiation = false; port->count = 0; - /* avoid overflow issues by resetting counter */ - SSL_clear_num_renegotiations(port->ssl); - renegotiation_count = SSL_num_renegotiations(port->ssl); } /* --- 422,431 ---- /* is renegotiation complete? */ if (in_ssl_renegotiation && ! SSL_num_renegotiations(port->ssl) >= 1) { in_ssl_renegotiation = false; port->count = 0; } /*
*** a/src/backend/libpq/be-secure.c --- b/src/backend/libpq/be-secure.c *************** *** 101,106 **** char *ssl_crl_file; --- 101,109 ---- */ int ssl_renegotiation_limit; + /* are we in the middle of a renegotiation? */ + static bool in_ssl_renegotiation = false; + #ifdef USE_SSL static SSL_CTX *SSL_context = NULL; static bool ssl_loaded_verify_locations = false; *************** *** 322,350 **** secure_write(Port *port, void *ptr, size_t len) { int err; ! if (ssl_renegotiation_limit && port->count > ssl_renegotiation_limit * 1024L) { SSL_set_session_id_context(port->ssl, (void *) &SSL_context, sizeof(SSL_context)); if (SSL_renegotiate(port->ssl) <= 0) ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), ! errmsg("SSL renegotiation failure"))); ! if (SSL_do_handshake(port->ssl) <= 0) ! ereport(COMMERROR, ! (errcode(ERRCODE_PROTOCOL_VIOLATION), ! errmsg("SSL renegotiation failure"))); ! if (port->ssl->state != SSL_ST_OK) ! ereport(COMMERROR, ! (errcode(ERRCODE_PROTOCOL_VIOLATION), ! errmsg("SSL failed to send renegotiation request"))); ! port->ssl->state |= SSL_ST_ACCEPT; ! SSL_do_handshake(port->ssl); ! if (port->ssl->state != SSL_ST_OK) ! ereport(COMMERROR, ! (errcode(ERRCODE_PROTOCOL_VIOLATION), ! errmsg("SSL renegotiation failure"))); ! port->count = 0; } wloop: --- 325,379 ---- { int err; ! /* ! * If SSL renegotiations are enabled and we're getting close to the ! * limit, start one now; but avoid it if there's one already in ! * progress. Request the renegotiation 1kB before the limit has ! * actually expired. ! */ ! if (ssl_renegotiation_limit && !in_ssl_renegotiation && ! port->count > (ssl_renegotiation_limit - 1) * 1024L) { + in_ssl_renegotiation = true; + + /* + * The way we determine that a renegotiation has completed is by + * observing OpenSSL's internal renegotiation counter. Make sure + * we start out at zero, and assume that the renegotiation is + * complete when the counter advances. + * + * OpenSSL provides SSL_renegotiation_pending(), but this doesn't + * seem to work in testing. + */ + SSL_clear_num_renegotiations(port->ssl); + SSL_set_session_id_context(port->ssl, (void *) &SSL_context, sizeof(SSL_context)); if (SSL_renegotiate(port->ssl) <= 0) ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), ! errmsg("SSL failure during renegotiation start"))); ! else ! { ! int retries; ! ! /* ! * A handshake can fail, so be prepared to retry it, but only ! * a few times. ! */ ! for (retries = 0; retries++;) ! { ! if (SSL_do_handshake(port->ssl) > 0) ! break; /* done */ ! ereport(COMMERROR, ! (errcode(ERRCODE_PROTOCOL_VIOLATION), ! errmsg("SSL handshake failure on renegotiation, retrying"))); ! if (retries >= 20) ! ereport(FATAL, ! (errcode(ERRCODE_PROTOCOL_VIOLATION), ! errmsg("unable to complete SSL handshake"))); ! } ! } } wloop: *************** *** 390,395 **** wloop: --- 419,443 ---- n = -1; break; } + + /* is renegotiation complete? */ + if (in_ssl_renegotiation && + SSL_num_renegotiations(port->ssl) >= 1) + { + in_ssl_renegotiation = false; + port->count = 0; + } + + /* + * if renegotiation is still ongoing, and we've gone beyond the limit, + * kill the connection now -- continuing to use it can be considered a + * security problem. + */ + if (in_ssl_renegotiation && + port->count > ssl_renegotiation_limit * 1024L) + ereport(FATAL, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("SSL failed to renegotiate connection before limit expired"))); } else #endif *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *************** *** 125,131 **** extern char *default_tablespace; extern char *temp_tablespaces; extern bool ignore_checksum_failure; extern bool synchronize_seqscans; - extern int ssl_renegotiation_limit; extern char *SSLCipherSuites; #ifdef TRACE_SORT --- 125,130 ---- *** a/src/include/libpq/libpq-be.h --- b/src/include/libpq/libpq-be.h *************** *** 93,98 **** typedef struct --- 93,103 ---- #endif /* + * SSL renegotiations + */ + extern int ssl_renegotiation_limit; + + /* * This is used by the postmaster in its communication with frontends. It * contains all state information needed during this communication before the * backend is run. The Port structure is kept in malloc'd memory and is
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers