[openssl-dev] [openssl.org #3712] TLS Renegotiation with Java is broken

[Matt Caswell via RT]

On 16/10/15 10:56, Hubert Kario via RT wrote:
> On Friday 16 October 2015 08:53:06 Matt Caswell via RT wrote:
>> So now I really don't know what the "right" way forward is. Should we
>> be applying the patch or not?
> 
> I can't think of a way to exploit it if two assumptions hold:
>  1). we have secure renegotiation
>  2). API calls return metadata (certificates especially) from *active* 
>      context, not one currently negotiated

But here is where we come unstuck I think. I've given this some thought
and I think there is an exploit if we were to apply the patch as is.

Consider a reneg where the server has requested a client certificate.
The client responds with a valid certificate and chain....but where they
do not possess the private key, i.e. they are attempting to impersonate
someone else. The client sends the Certificate message and the server
extracts it and verifies it successfully.

On the server side the Certificate message is processed via the function
"ssl3_get_client_certificate()". At the end of this function there are
these two lines of code:

    s->session->peer = sk_X509_shift(sk);
    s->session->verify_result = s->verify_result;

|s->session->peer| stores the newly received Certificate away for future
use. Note that at this point the result of the verification (which is
successful) is stored in both |s->session->verify_result| and
|s->verify_result|.

Now, before sending the CertificateVerify message, the client sends
application data. Maybe that application data causes the application to
try to do something privileged that only authorised users are allowed to
do, so the application calls SSL_get_peer_certificate():

X509 *SSL_get_peer_certificate(const SSL *s)
{
    X509 *r;

    if ((s == NULL) || (s->session == NULL))
        r = NULL;
    else
        r = s->session->peer;

    if (r == NULL)
        return (r);

    X509_up_ref(r);

    return (r);
}

And SSL_get_verify_result():

long SSL_get_verify_result(const SSL *ssl)
{
    return (ssl->verify_result);
}

So these API calls will return the *new* certificate and verification
result *before* a CertificateVerify has been received.

Fixing this sort of problem is going to be *hard* and probably require
quite a lot of non-trivial changes - definitely not the sort of the
thing I want to be doing in a stable branch. Fixing this is an example
of what I meant by "onerous mitigations", but I now realise it is
absolutely necessary if we wanted to pursue this.

I think we should be marking this as a "won't fix" for all released
versions. The question is whether we should even attempt to fix it for
1.1.0 or not.

Matt