[openssl-dev] s3_clnt.c changes regarding external pre-shared secret seem to break EAP-FAST

[Emilia Käsper]

On Tue, Mar 24, 2015 at 2:01 PM, John Foley <foleyj at cisco.com> wrote:

>  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.
>

But only in case we are actually resuming - no? Does the client always have
a PAC available - I would guess not? Seems the legacy logic is such that it
"happens to work", but I'd like to clear it up.


>   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) <foleyj at 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-bounces at openssl.org] on behalf of Dr.
>> Stephen Henson [steve at openssl.org]
>> Sent: Thursday, March 19, 2015 11:49 AM
>> To: openssl-dev at 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20150326/a777c9bf/attachment-0001.html>