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.
Erik
> On 27 Mar 2015, at 12:33 PM, Emilia Käsper <[email protected]> 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 <[email protected]
> <mailto:[email protected]>> 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 <[email protected]
>> <mailto:[email protected]>> 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) <[email protected]
>>> <mailto:[email protected]>> 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 [[email protected]
>>> <mailto:[email protected]>] on behalf of Dr. Stephen Henson
>>> [[email protected] <mailto:[email protected]>]
>>> Sent: Thursday, March 19, 2015 11:49 AM
>>> To: [email protected] <mailto:[email protected]>
>>> 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
_______________________________________________
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
