Re: SSL renegotiation

Hi,

These are the relevant bits from Apache2.4's mod_ssl.

SSL_renegotiate(ssl);
SSL_do_handshake(ssl);

if (SSL_get_state(ssl) != SSL_ST_OK) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02225)
"Re-negotiation request failed");
ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, r->server);

r->connection->keepalive = AP_CONN_CLOSE;
return HTTP_FORBIDDEN;
}

ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02226)
"Awaiting re-negotiation handshake");

/* XXX: Should replace setting state with SSL_renegotiate(ssl);
* However, this causes failures in perl-framework currently,
* perhaps pre-test if we have already negotiated?
*/
#ifdef OPENSSL_NO_SSL_INTERN
SSL_set_state(ssl, SSL_ST_ACCEPT);
#else
ssl->state = SSL_ST_ACCEPT;
#endif
SSL_do_handshake(ssl);

sslconn->reneg_state = RENEG_REJECT;

if (SSL_get_state(ssl) != SSL_ST_OK) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02261)
"Re-negotiation handshake failed: "
"Not accepted by client!?");

r->connection->keepalive = AP_CONN_CLOSE;
return HTTP_FORBIDDEN;
}

That code supports at least OpenSSL 0.9.7 and later.

Some explanation for it can be found here:

http://books.google.dk/books?id=IIqwAy4qEl0C&pg=PT184&lpg=PT184&dq=SSL_do_handshake&source=bl&ots=ma632U4vWv&sig=d2qqS0svhu4EwIZZaONdHoTulVI&hl=en&sa=X&ei=xdPdUczoDuf34QSzmoDQDg&ved=0CIIDEOgBMCo

The snippet there is probably the textbook implementation.

So the original code looks OK. Perhaps the check
on the return code of the first SSL_do_handshake (and SSL_renegotiate)
is unnecessary and may lead to unwarranted error messages, though.
And it may be wrong to continue the renegotiation if
the state is not SSL_ST_OK after the first SSL_do_handshake.

If the renegotiation fails, I suppose two things can be done:
1. Quit the connection
2. Carry on pretending nothing happened.

I think 2. is correct in the vast majority of cases (as it looks like is
being done now).
And in that case: not resetting port->count will make for a very slow
and awkward connection as new handshakes will be attempted again and again,
even if the other party persistently refuse to shake hands.

Kind Regards
Troels Nielsen

On Thu, Jul 11, 2013 at 12:34 AM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com>wrote:

> I think this block is better written as:
>
> 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 in
> renegotiate")));
> else
> {
> int handshake;
>
> do {
> handshake = SSL_do_handshake(port->ssl);
> if (handshake <= 0)
> ereport(COMMERROR,
> (errcode(ERRCODE_PROTOCOL_VIOLATION),
> errmsg("SSL renegotiation failure in
> handshake, retrying")));
> } while (handshake <= 0);
>
> if (port->ssl->state != SSL_ST_OK)
> ereport(COMMERROR,
> (errcode(ERRCODE_PROTOCOL_VIOLATION),
> errmsg("SSL failed to send renegotiation
> request")));
> else
> port->count = 0;
> }
> }
>
> In other words, retry the SSL_do_handshake() until it reports OK, but
> not more than that; this seems to give better results in my (admittedly
> crude) experiments. I am unsure about checking port->ssl->state after
> the handshake; it's probably pointless, really.
>
> In any case, the old code was calling SSL_do_handshake() even if
> SSL_renegotiate() failed; and it was resetting the port->count even if
> the handshake failed. Both of these smell like bugs to me.
>
> --
> Álvaro Herrera http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>