Fix DTLS transcript handling for HelloVerifyRequest.

If DTLS sees a HelloVerifyRequest the transcript is reset - the previous
tls1_init_finished_mac() function could be called multiple times and would
discard any existing state. The replacement tls1_transcript_init() is more
strict and fails if a transcript already exists.

Provide an explicit tls1_transcript_reset() function and call it from the
appropriate places. This also lets us make DTLS less of a special snowflake
and call tls1_transcript_init() in the same place as used for TLS.

ok beck@ tb@

4 files changed, 26 insertions, 18 deletions

diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c
index 35df70f2f00..65277ef4eff 100644
--- a/lib/libssl/ssl_clnt.c
+++ b/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.49 2018/11/19 15:07:29 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.50 2018/11/21 15:13:29 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -244,11 +244,9 @@ ssl3_connect(SSL *s)
 
 			/* don't push the buffering BIO quite yet */
 
-			if (!SSL_IS_DTLS(s)) {
-				if (!tls1_transcript_init(s)) {
-					ret = -1;
-					goto end;
-				}
+			if (!tls1_transcript_init(s)) {
+				ret = -1;
+				goto end;
 			}
 
 			S3I(s)->hs.state = SSL3_ST_CW_CLNT_HELLO_A;
@@ -270,10 +268,7 @@ ssl3_connect(SSL *s)
 
 			if (SSL_IS_DTLS(s)) {
 				/* every DTLS ClientHello resets Finished MAC */
-				if (!tls1_transcript_init(s)) {
-					ret = -1;
-					goto end;
-				}
+				tls1_transcript_reset(s);
 
 				dtls1_start_timer(s);
 			}
diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 50806d1b187..94bb76eca32 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.224 2018/11/10 01:19:09 beck Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.225 2018/11/21 15:13:29 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1242,6 +1242,7 @@ void tls1_handshake_hash_free(SSL *s);
 
 int tls1_transcript_init(SSL *s);
 void tls1_transcript_free(SSL *s);
+void tls1_transcript_reset(SSL *s);
 int tls1_transcript_append(SSL *s, const unsigned char *buf, size_t len);
 int tls1_transcript_data(SSL *s, const unsigned char **data, size_t *len);
 void tls1_transcript_freeze(SSL *s);
diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index 27024be856a..0667ac8da3f 100644
--- a/lib/libssl/ssl_srvr.c
+++ b/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.60 2018/11/11 21:54:47 beck Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.61 2018/11/21 15:13:29 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -368,10 +368,7 @@ ssl3_accept(SSL *s)
 			S3I(s)->hs.next_state = SSL3_ST_SR_CLNT_HELLO_A;
 
 			/* HelloVerifyRequest resets Finished MAC. */
-			if (!tls1_transcript_init(s)) {
-				ret = -1;
-				goto end;
-			}
+			tls1_transcript_reset(s);
 			break;
 
 		case SSL3_ST_SW_SRVR_HELLO_A:
diff --git a/lib/libssl/t1_hash.c b/lib/libssl/t1_hash.c
index f514c5290ed..50e0ad3ca06 100644
--- a/lib/libssl/t1_hash.c
+++ b/lib/libssl/t1_hash.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_hash.c,v 1.4 2018/11/08 22:28:52 jsing Exp $ */
+/* $OpenBSD: t1_hash.c,v 1.5 2018/11/21 15:13:29 jsing Exp $ */
 /*
  * Copyright (c) 2017 Joel Sing <jsing@openbsd.org>
  *
@@ -118,7 +118,7 @@ tls1_transcript_init(SSL *s)
 	if ((S3I(s)->handshake_transcript = BUF_MEM_new()) == NULL)
 		return 0;
 
-	s->s3->flags &= ~TLS1_FLAGS_FREEZE_TRANSCRIPT;
+	tls1_transcript_reset(s);
 
 	return 1;
 }
@@ -130,6 +130,21 @@ tls1_transcript_free(SSL *s)
 	S3I(s)->handshake_transcript = NULL;
 }
 
+void
+tls1_transcript_reset(SSL *s)
+{
+	/*
+	 * We should check the return value of BUF_MEM_grow_clean(), however
+	 * due to yet another bad API design, when called with a length of zero
+	 * it is impossible to tell if it succeeded (returning a length of zero)
+	 * or if it failed (and returned zero)... our implementation never
+	 * fails with a length of zero, so we trust all is okay...
+	 */ 
+	(void)BUF_MEM_grow_clean(S3I(s)->handshake_transcript, 0);
+
+	s->s3->flags &= ~TLS1_FLAGS_FREEZE_TRANSCRIPT;
+}
+
 int
 tls1_transcript_append(SSL *s, const unsigned char *buf, size_t len)
 {