[openssl-dev] s3_clnt.c changes regarding external pre-shared secret seem to break EAP-FAST
Emilia Käsper
emilia at openssl.org
Mon Mar 30 11:20:24 UTC 2015
On Fri, Mar 27, 2015 at 7:19 PM, Erik Tkal <etksubs at gmail.com> wrote:
> Hi Emelia,
>
> I’m not sure that will work as presently designed, as it keys off of the
> session object:
>
> - if (s->version >= TLS1_VERSION && s->tls_session_secret_cb) {
> + if (s->version >= TLS1_VERSION && s->tls_session_secret_cb &&
> + s->session->tlsext_tick) {
>
> Our client uses the public API SSL_set_session_ticket_ext(), which
> populates the SSL->tls_session_ticket, and not the SSL->session copy.
>
Yes, I know - but that would get copied to the session when the extension
is first sent.
Do you have a way of testing the patch?
>
> Erik
>
>
> On 27 Mar 2015, at 12:33 PM, Emilia Käsper <emilia at openssl.org> wrote:
>
> John, Erik,
>
> 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> 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> 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
>>>
>>>
>>
>>
> _______________________________________________
> 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/20150330/84b62b12/attachment-0001.html>
More information about the openssl-dev
mailing list