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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
