Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit31cf1a1

Browse files
committed
Rework SSL renegotiation code
The existing renegotiation code was home for several bugs: it mighterroneously report that renegotiation had failed; it might try toexecute another renegotiation while the previous one was pending; itfailed to terminate the connection if the renegotiation never actuallytook place; if a renegotiation was started, the byte count was reset,even if the renegotiation wasn't completed (this isn't good from asecurity perspective because it means continuing to use a session thatshould be considered compromised due to volume of data transferred.)The new code is structured to avoid these pitfalls: renegotiation isstarted a little earlier than the limit has expired; the handshakesequence is retried until it has actually returned successfully, and nomore than that, but if it fails too many times, the connection isclosed. The byte count is reset only when the renegotiation hassucceeded, and if the renegotiation byte count limit expires, theconnection is terminated.This commit only touches the master branch, because some of the changesare controversial. If everything goes well, a back-patch might beconsidered.Per discussion started by message20130710212017.GB4941@eldon.alvh.no-ip.org
1 parent956f2db commit31cf1a1

File tree

3 files changed

+70
-18
lines changed

3 files changed

+70
-18
lines changed

‎src/backend/libpq/be-secure.c

Lines changed: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ char *ssl_crl_file;
101101
*/
102102
intssl_renegotiation_limit;
103103

104+
/* are we in the middle of a renegotiation? */
105+
staticboolin_ssl_renegotiation= false;
106+
104107
#ifdefUSE_SSL
105108
staticSSL_CTX*SSL_context=NULL;
106109
staticboolssl_loaded_verify_locations= false;
@@ -322,29 +325,55 @@ secure_write(Port *port, void *ptr, size_t len)
322325
{
323326
interr;
324327

325-
if (ssl_renegotiation_limit&&port->count>ssl_renegotiation_limit*1024L)
328+
/*
329+
* If SSL renegotiations are enabled and we're getting close to the
330+
* limit, start one now; but avoid it if there's one already in
331+
* progress. Request the renegotiation 1kB before the limit has
332+
* actually expired.
333+
*/
334+
if (ssl_renegotiation_limit&& !in_ssl_renegotiation&&
335+
port->count> (ssl_renegotiation_limit-1)*1024L)
326336
{
337+
in_ssl_renegotiation= true;
338+
339+
/*
340+
* The way we determine that a renegotiation has completed is by
341+
* observing OpenSSL's internal renegotiation counter. Make sure
342+
* we start out at zero, and assume that the renegotiation is
343+
* complete when the counter advances.
344+
*
345+
* OpenSSL provides SSL_renegotiation_pending(), but this doesn't
346+
* seem to work in testing.
347+
*/
348+
SSL_clear_num_renegotiations(port->ssl);
349+
327350
SSL_set_session_id_context(port->ssl, (void*)&SSL_context,
328351
sizeof(SSL_context));
329352
if (SSL_renegotiate(port->ssl) <=0)
330353
ereport(COMMERROR,
331354
(errcode(ERRCODE_PROTOCOL_VIOLATION),
332-
errmsg("SSL renegotiation failure")));
333-
if (SSL_do_handshake(port->ssl) <=0)
334-
ereport(COMMERROR,
335-
(errcode(ERRCODE_PROTOCOL_VIOLATION),
336-
errmsg("SSL renegotiation failure")));
337-
if (port->ssl->state!=SSL_ST_OK)
338-
ereport(COMMERROR,
339-
(errcode(ERRCODE_PROTOCOL_VIOLATION),
340-
errmsg("SSL failed to send renegotiation request")));
341-
port->ssl->state |=SSL_ST_ACCEPT;
342-
SSL_do_handshake(port->ssl);
343-
if (port->ssl->state!=SSL_ST_OK)
344-
ereport(COMMERROR,
345-
(errcode(ERRCODE_PROTOCOL_VIOLATION),
346-
errmsg("SSL renegotiation failure")));
347-
port->count=0;
355+
errmsg("SSL failure during renegotiation start")));
356+
else
357+
{
358+
intretries;
359+
360+
/*
361+
* A handshake can fail, so be prepared to retry it, but only
362+
* a few times.
363+
*/
364+
for (retries=0;retries++;)
365+
{
366+
if (SSL_do_handshake(port->ssl)>0)
367+
break;/* done */
368+
ereport(COMMERROR,
369+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
370+
errmsg("SSL handshake failure on renegotiation, retrying")));
371+
if (retries >=20)
372+
ereport(FATAL,
373+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
374+
errmsg("unable to complete SSL handshake")));
375+
}
376+
}
348377
}
349378

350379
wloop:
@@ -390,6 +419,25 @@ secure_write(Port *port, void *ptr, size_t len)
390419
n=-1;
391420
break;
392421
}
422+
423+
/* is renegotiation complete? */
424+
if (in_ssl_renegotiation&&
425+
SSL_num_renegotiations(port->ssl) >=1)
426+
{
427+
in_ssl_renegotiation= false;
428+
port->count=0;
429+
}
430+
431+
/*
432+
* if renegotiation is still ongoing, and we've gone beyond the limit,
433+
* kill the connection now -- continuing to use it can be considered a
434+
* security problem.
435+
*/
436+
if (in_ssl_renegotiation&&
437+
port->count>ssl_renegotiation_limit*1024L)
438+
ereport(FATAL,
439+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
440+
errmsg("SSL failed to renegotiate connection before limit expired")));
393441
}
394442
else
395443
#endif

‎src/backend/utils/misc/guc.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ extern char *default_tablespace;
126126
externchar*temp_tablespaces;
127127
externboolignore_checksum_failure;
128128
externboolsynchronize_seqscans;
129-
externintssl_renegotiation_limit;
130129
externchar*SSLCipherSuites;
131130

132131
#ifdefTRACE_SORT

‎src/include/libpq/libpq-be.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ typedef struct
9292
}pg_gssinfo;
9393
#endif
9494

95+
/*
96+
* SSL renegotiations
97+
*/
98+
externintssl_renegotiation_limit;
99+
95100
/*
96101
* This is used by the postmaster in its communication with frontends.It
97102
* contains all state information needed during this communication before the

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp