[openssl-dev] Extended master secret goober in s3_srvr.c

[Dr. Stephen Henson]

On Fri, Jun 12, 2015, Bill Cox wrote:

> Here's some code in master starting at line 594 in s3_srvr.c:
> 
>                 if (!s->s3->handshake_buffer) {
>                     SSLerr(SSL_F_SSL3_ACCEPT, ERR_R_INTERNAL_ERROR);
>                     return -1;
>                 }
>                 /*
>                  * For sigalgs freeze the handshake buffer. If we support
>                  * extms we've done this already.
>                  */
>                 if (!(s->s3->flags & SSL_SESS_FLAG_EXTMS)) {
>                     s->s3->flags |= TLS1_FLAGS_KEEP_HANDSHAKE;
>                     if (!ssl3_digest_cached_records(s))
>                         return -1;
>                 }
> 
> The goober is that s->s3->flags does not have a flag for
> SSL_SESS_FLAG_EXTMS.  This flag is defined for s->session->flags, not
> s->s3->flags.  What happens is that s->s3->flags generally has bit 0 clear,
> because this iis the flag for SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS.
> Therefore, this generally runs, and we set TLS1_FLAGS_KEEP_HANDSHAKE even
> if we have extended master secret support enabled.
> 
> I haven't figured out yet what this does in the code.  If it were really
> bad, we would have heard about it already.  What was this code supposed to
> do, and how should it get fixed?
> 

Nice catch. That code only gets called when TLS 1.2 client authentication is
used: I think it just means we digest cached records twice instead of once.

Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org