Trying again w/o PGP...  :-)

Thanks for taking a look at this problem. Regarding how to handle a failure in the session secret callback, the legacy logic would likely result in a "bad record mac" error because the master secrets on the client/server do not match. It would be good to trigger an internal error to aid with troubleshooting. Maybe something like:

        SSLerr(SSL_F_SSL3_GET_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
        goto err;

It's debatable whether the alert needs to be sent to the server. Since this is an internal error, it should be safe to send the alert. Therefore, maybe you would actually want to do something like:

        SSLerr(SSL_F_SSL3_GET_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
        al = SSL_AD_INTERNAL_ERROR;
        goto f_err;



On 03/23/2015 09:17 PM, Emilia Käsper wrote:


On Tue, Mar 24, 2015 at 1:20 AM, John Foley (foleyj) <fol...@cisco.com <mailto:fol...@cisco.com>> wrote:

    We've found a way to recreate the scenario using
    s_client/s_server.  We're using the -no_ticket option on the
    server.  Therefore, the ServerHello doesn't contain the session
    ticket extension.  It also doesn't send the NewSessionTicket message.

    To summarize the problem, when the client side is using
    SSL_set_session_secret_cb() and including a valid ticket in the
    ClintHello, then the logic in ssl3_get_server_hello() assumes the
    server is doing session resumption.  This puts the client-side
    state machine into the SSL3_ST_CR_FINISHED_A.  However, since the
    server side is configured to not do resumption via the -no_ticket
    option, the server continues with a normal handshake by sending
    the Certificate message.  The client aborts the handshake when it
    receives the Certificate message while in the
    SSL3_ST_CR_FINISHED_A state.


    As Erik identified earlier in the thread, the cause of this
    appears to be the addition of setting s->hit in the following code:

        if (s->version >= TLS1_VERSION && s->tls_session_secret_cb) {
            SSL_CIPHER *pref_cipher = NULL;
            s->session->master_key_length =
    sizeof(s->session->master_key);
            if (s->tls_session_secret_cb(s, s->session->master_key,
     &s->session->master_key_length,
                                         NULL, &pref_cipher,
     s->tls_session_secret_cb_arg)) {
                s->session->cipher = pref_cipher ?
                    pref_cipher : ssl_get_cipher_by_char(s, p + j);
                s->hit = 1;
            }
        }

    Why does the client-side now assume the server is doing session
    resumption simply because the session secret callback facility is
    being used?


Because a developer (me) introduced a bug. With OpenSSL client behaviour, peeking ahead is only required for EAP-FAST. I got rid of the peeking while tightening up the ChangeCipherSpec handling and in the process, got it wrong for EAP-FAST. Anyway, apologies, I see the problem and am working on a patch.

While we're at it, you may be able to help me with the following question: how should the client handle callback failure? The old code (pre my refactoring which introduced the bug) did this

#ifndef OPENSSL_NO_TLSEXT
        /* check if we want to resume the session based on external pre-shared 
secret */
        if (s->version >= TLS1_VERSION && s->tls_session_secret_cb)
                {
                SSL_CIPHER *pref_cipher=NULL;
                s->session->master_key_length=sizeof(s->session->master_key);
                if (s->tls_session_secret_cb(s, s->session->master_key,
                                             &s->session->master_key_length,
                                             NULL, &pref_cipher,
                                             s->tls_session_secret_cb_arg))
                        {
                        s->session->cipher = pref_cipher ?
                                pref_cipher : ssl_get_cipher_by_char(s, p+j);
                        }
                }
#endif /* OPENSSL_NO_TLSEXT */
This is surely wrong as it's just ignoring the failure?
Thanks,
Emilia

    ________________________________________
    From: openssl-dev [openssl-dev-boun...@openssl.org
    <mailto:openssl-dev-boun...@openssl.org>] on behalf of Dr. Stephen
    Henson [st...@openssl.org <mailto:st...@openssl.org>]
    Sent: Thursday, March 19, 2015 11:49 AM
    To: openssl-dev@openssl.org <mailto:openssl-dev@openssl.org>
    Subject: Re: [openssl-dev] s3_clnt.c changes regarding external
    pre-shared secret seem to break EAP-FAST

    On Thu, Mar 19, 2015, Erik Tkal wrote:

    >
    > If I do not send a sessionID in the clientHello but do send a valid
    > sessionTicket extension, the server goes straight to
    changeCipherSpec and
    > the client generates an UnexpectedMessage alert.
    >

    Does the server send back an empty session ticket extension?

    Steve.
    --
    Dr Stephen N. Henson. OpenSSL project core developer.
    Commercial tech support now available see: http://www.openssl.org
    _______________________________________________
    openssl-dev mailing list
    To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
    _______________________________________________
    openssl-dev mailing list
    To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev




_______________________________________________
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

_______________________________________________
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Reply via email to