On Fri, Mar 27, 2015 at 7:19 PM, Erik Tkal <etks...@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 <emi...@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 <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 > > > > _______________________________________________ > 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