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

[John Foley]

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) <foleyj at cisco.com 
> <mailto: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
>     <mailto:openssl-dev-bounces at openssl.org>] on behalf of Dr. Stephen
>     Henson [steve at openssl.org <mailto:steve at openssl.org>]
>     Sent: Thursday, March 19, 2015 11:49 AM
>     To: openssl-dev at openssl.org <mailto: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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20150324/6a0f130e/attachment.html>