On Thu, Jul 30, 2015 at 11:00:45AM +0100, Matt Caswell wrote: > On 28/07/15 15:09, Jouni Malinen wrote: > > The remaining issue for EAP-FAST server is in the > > SSL_set_session_secret_cb() callback not having access to the correct > > server_random through SSL_get_server_random().
> Is this still a problem? From looking at the code it seems to me that > the server random is set prior to calling the callback: It is a problem with the current master branch snapshot, but not with older versions, i.e., a regression of some sort.. > /* > * Check if we want to use external pre-shared secret for this handshake > * for not reused session only. We need to generate server_random before > * calling tls_session_secret_cb in order to allow SessionTicket > * processing to use it in key derivation. > */ > { > unsigned char *pos; > pos = s->s3->server_random; > if (ssl_fill_hello_random(s, 1, pos, SSL3_RANDOM_SIZE) <= 0) { This is indeed the server_random I see in the tls_session_secret_cb, but that is not the server_random that gets used in the negotiation.. > Checking the commit logs this seems to have been put in by this commit > responding to one of your tickets! > > commit 12bf56c017a34bd0d5fc6d817564ae49d0a9e861 It was indeed and that commit worked.. Please note that it uses #ifdef OPENSSL_NO_TLSEXT to avoid having ssl3_send_server_hello() override the previously selected server_random. > You seem to imply that you can get the server_random through > ssl->s3->server_random but not through SSL_get_server_random(). Looking > at the code I can't see an obvious reason why that would be the case. > Here is SSL_get_server_random(): > > size_t SSL_get_server_random(const SSL *ssl, unsigned char *out, size_t > outlen) > { > if (outlen == 0) > return sizeof(ssl->s3->server_random); > if (outlen > sizeof(ssl->s3->server_random)) > outlen = sizeof(ssl->s3->server_random); > memcpy(out, ssl->s3->server_random, outlen); > return outlen; > } I had not checked what this does, but yes, that's indeed identical to the mechanism I use with older OpenSSL version. In other words, the issue is in ssl3_send_server_hello(). It looks like commit e481f9b90b164fd1053015d1c4e0a0d92076d7a8 ("Remove support for OPENSSL_NO_TLSEXT") broke this. It is deleting number of "#ifndef OPENSSL_NO_TLSEXT" lines correctly, but it is also deleting one "#ifdef OPENSSL_NO_TLSEXT" without removing the block of code that should have also been removed from ssl3_send_server_hello(). Because of that, server_random gets replaced after the call to tls_session_secret_cb which breaks the EAP-FAST use case. This is the relevant part of that commit: @@ -1602,13 +1585,13 @@ int ssl3_send_server_hello(SSL *s) if (s->state == SSL3_ST_SW_SRVR_HELLO_A) { buf = (unsigned char *)s->init_buf->data; -#ifdef OPENSSL_NO_TLSEXT + p = s->s3->server_random; if (ssl_fill_hello_random(s, 1, p, SSL3_RANDOM_SIZE) <= 0) { s->state = SSL_ST_ERR; return -1; } -#endif + /* Do the message type and length last */ d = p = ssl_handshake_start(s); That ssl_fill_hello_random() call needs to be deleted to fix this issue. Based on a quick test, that does indeed fix the EAP-FAST server issue I saw. -- Jouni Malinen PGP id EFC895FA _______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev