Re: SSL renegotiation

Поиск
Список
Период
Сортировка
От Troels Nielsen
Тема Re: SSL renegotiation
Дата
Msg-id CAOdE5WSyb4Y5sCwe1CqBYmLoK7ZAON3R7aBEa8OEX0LVnff_Og@mail.gmail.com
обсуждение исходный текст
Ответ на Re: SSL renegotiation  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Ответы Re: SSL renegotiation  (Sean Chittenden <sean@chittenden.org>)
Re: SSL renegotiation  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers
Дерево обсуждения
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.
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@2ndquadrant.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Josh Berkus
Дата:
Сообщение: Re: [PATCH] Add transforms feature
Следующее
От: "Prabakaran, Vaishnavi"
Дата:
Сообщение: Re: Differences in WHERE clause of SELECT