[openssl-dev] s3_clnt.c changes regarding external pre-shared secret seem to break EAP-FAST
Erik Tkal
etksubs at gmail.com
Tue Mar 31 21:23:45 UTC 2015
Hi Emilia,
I was able to test the patch and it does successfully now allow the server to fall back to a Certificate message if it does not accept the SessionTicket sent by the client.
Thanks for your help,
Erik
On 27 Mar2015, at 12:33, Emilia Käsper <emilia at openssl.org> wrote:
John, Erik,
https://github.com/openssl/openssl/pull/250 <https://github.com/openssl/openssl/pull/250>
Can you verify whether this resolves the problem? (And also, does not create any new problems.)
Note this is pending team review so is not a definitive fix. But since we're maintaining this feature more or less blind, we'd appreciate your help testing the fix.
Thanks,
Emilia
On Thu, Mar 26, 2015 at 9:02 PM, John Foley <foleyj at cisco.com <mailto:foleyj at cisco.com>> wrote:
Someone that understands EAP better than myself should probably provide input. But my limited understand of EAP-FAST is it contributes to the master secret calculation used for the TLS session. See section RFC 4851 Section 5.1. My understanding is this logic applies to both new and resumed sessions. Hence, tls_session_secret_cb() is always in play for EAP-FAST.
On 03/26/2015 02:13 PM, Emilia Käsper wrote:
>
>
> On Tue, Mar 24, 2015 at 2:01 PM, John Foley <foleyj at cisco.com <mailto: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 <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 <http://www.openssl.org/>
>> _______________________________________________
>> openssl-dev mailing list
>> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev <https://mta.openssl.org/mailman/listinfo/openssl-dev>
>> _______________________________________________
>> openssl-dev mailing list
>> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev <https://mta.openssl.org/mailman/listinfo/openssl-dev>
>>
>>
>>
>> _______________________________________________
>> openssl-dev mailing list
>> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev <https://mta.openssl.org/mailman/listinfo/openssl-dev>
>
>
> _______________________________________________
> openssl-dev mailing list
> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev <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/20150331/3b12d00b/attachment-0001.html>
More information about the openssl-dev
mailing list