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 <fol...@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 <fol...@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) <fol...@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-boun...@openssl.org] on behalf of Dr. >>> Stephen Henson [st...@openssl.org] >>> Sent: Thursday, March 19, 2015 11:49 AM >>> To: openssl-dev@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