On Tue, Jun 10, 2014 at 10:33:32PM +0200, Kurt Roeckx wrote: > On Tue, Jun 10, 2014 at 01:03:17PM -0700, Kyle Hamilton wrote: > > http://opensslrampage.org/post/88383880093 > > > > I don't know if this has in fact been given to the OpenSSL team yet. I > > am not jsing, and I am not involved in the OpenBSD audit. > > > > However, this is important. If MD5 passes, but SHA1 fails, then the MAC > > verification will pass. This reduces the security of the handshake to MD5. > > > > I don't know where ssl3_final_finish_mac() is called from, if it's > > limited to SSLv3 or if it's also called from TLS. > > As far as I can see this is SSLv3 only, and only about the Finish > message. > > So it seems that function return the length of the digest, and in > some error cases 0. We'll end up with a wrong value in > (peer_)finish_md_len. > > It should then result in this error: > if (i != n) > { > al=SSL_AD_DECODE_ERROR; > SSLerr(SSL_F_SSL3_GET_FINISHED,SSL_R_BAD_DIGEST_LENGTH); > goto f_err; > } > > So at first look there doesn't seem to be anything wrong with the > current code. But their patch doesn't do anything wrong either.
So to clarify this a little more. ssl3_final_finish_mac() returns 0 on an internal error, or the length of the digest. In case of SSLv3 it's both an MD5 and SHA1. In ssl3_final_finish_mac() they only get calculated and the length is returned. The check that they are correct happens just after the if I quoted above. Kurt ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
