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