Move MAC computations for Finished from ssl3_read_bytes into

ssl3_get_message, which is more logical (and avoids a bug, in addition to the one that I introduced yesterday :-) and makes Microsoft "fast SGC" less special. MS SGC should still work now without an extra state of its own (it goes directly to SSL3_ST_SR_CLNT_HELLO_C, which is the usual state for reading the body of a Client Hello message), however this should be tested to make sure, and I don't have a MS SGC client.
openssl · Feb 21, 2000 · 745c70e · 745c70e
1 parent 9dd2b2a
commit 745c70e
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 25 deletions.
diff --git a/CHANGES b/CHANGES
@@ -4,6 +4,22 @@
 
  Changes between 0.9.4 and 0.9.5  [xx XXX 2000]
 
+  *) ..._ctrl functions now have corresponding ..._callback_ctrl functions
+     where the 'void *' argument is replaced by a function pointer argument.
+     Previously 'void *' was abused to point to functions, which works on
+     many platforms, but is not correct.  As these functions are usually
+     called by macros defined in OpenSSL header files, most source code
+     should work without changes.
+
+  *) <openssl/opensslconf.h> (which is created by Configure) now contains
+     sections with information on -D... compiler switches used for
+     compiling the library so that applications can see them.  To enable
+     one of these sections, a pre-processor symbol OPENSSL_..._DEFINES
+     must be defined.  E.g.,
+        #define OPENSSL_ALGORITHM_DEFINES
+        #include <openssl/opensslconf.h>
+     defines all pertinent NO_<algo> symbols, such as NO_IDEA, NO_RSA, etc.
+
   *) Bugfix: Tolerate fragmentation and interleaving in the SSL 3/TLS
      record layer.
      [Bodo Moeller]
@@ -341,8 +357,7 @@
 
      To get OpenSSL to support MS SGC we have to permit a second client
      hello message after we have sent server done. In addition we have to
-     reset the MAC if we do get this second client hello and include the
-     data just received.
+     reset the MAC if we do get this second client hello.
      [Steve Henson]
 
   *) Add a function 'd2i_AutoPrivateKey()' this will automatically decide

diff --git a/Configure b/Configure
@@ -422,7 +422,7 @@ foreach (@ARGV)
 	 	{
 		$no_asm=1;
 		$flags .= "-DNO_ASM ";
-		$openssl_algorithm_defines .= "#define NO_ASM\n";
+		$openssl_other_defines .= "#define NO_ASM\n";
 		}
 	elsif (/^no-threads$/)
 		{ $no_threads=1; }

diff --git a/ssl/s3_both.c b/ssl/s3_both.c
@@ -292,7 +292,7 @@ long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
 			while (s->init_num < 4)
 				{
 				i=ssl3_read_bytes(s,SSL3_RT_HANDSHAKE,&p[s->init_num],
-					4-s->init_num);
+					4 - s->init_num);
 				if (i <= 0)
 					{
 					s->rwstate=SSL_READING;
@@ -307,12 +307,15 @@ long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
 				if (p[0] == SSL3_MT_HELLO_REQUEST)
 					/* The server may always send 'Hello Request' messages --
 					 * we are doing a handshake anyway now, so ignore them
-					 * if their format is correct */
+					 * if their format is correct. Does not count for
+					 * 'Finished' MAC. */
 					if (p[1] == 0 && p[2] == 0 &&p[3] == 0)
 						skip_message = 1;
 			}
 		while (skip_message);
 
+		/* s->init_num == 4 */
+
 		if ((mt >= 0) && (*p != mt))
 			{
 			al=SSL_AD_UNEXPECTED_MESSAGE;
@@ -324,12 +327,13 @@ long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
 					(stn == SSL3_ST_SR_CERT_B))
 			{
 			/* At this point we have got an MS SGC second client
-			 * hello. We need to restart the mac and mac the data
-			 * currently received.
+			 * hello (maybe we should always allow the client to
+			 * start a new handshake?). We need to restart the mac.
 			 */
 			ssl3_init_finished_mac(s);
-			ssl3_finish_mac(s, p + s->init_num, i);
 			}
+
+		ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, 4);
 
 		s->s3->tmp.message_type= *(p++);
 
@@ -366,6 +370,7 @@ long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok)
 		s->init_num += i;
 		n -= i;
 		}
+	ssl3_finish_mac(s, (unsigned char *)s->init_buf->data, s->init_num);
 	*ok=1;
 	return s->init_num;
 f_err:

diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
@@ -507,9 +507,6 @@ int ssl3_write_bytes(SSL *s, int type, const void *buf_, int len)
 			return(i);
 			}
 
-		if (type == SSL3_RT_HANDSHAKE)
-			ssl3_finish_mac(s,&(buf[tot]),i);
-
 		if ((i == (int)n) ||
 			(type == SSL3_RT_APPLICATION_DATA &&
 			 (s->mode & SSL_MODE_ENABLE_PARTIAL_WRITE)))
@@ -740,7 +737,6 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len)
 		/* move any remaining fragment bytes: */
 		for (i = 0; i < s->s3->handshake_fragment_len; i++)
 			s->s3->handshake_fragment[i] = *src++;
-		ssl3_finish_mac(s, buf, n);
 		return n;
 	}
 
@@ -820,9 +816,6 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len)
 			s->rstate=SSL_ST_READ_HEADER;
 			rr->off=0;
 			}
-
-		if (type == SSL3_RT_HANDSHAKE)
-			ssl3_finish_mac(s,buf,n);
 		return(n);
 		}
 
@@ -1130,10 +1123,15 @@ int ssl3_do_write(SSL *s, int type)
 	int ret;
 
 	ret=ssl3_write_bytes(s,type,&s->init_buf->data[s->init_off],
-			     s->init_num);
+	                     s->init_num);
+	if (ret < 0) return(-1);
+	if (type == SSL3_RT_HANDSHAKE)
+		/* should not be done for 'Hello Request's, but in that case
+		 * we'll ignore the result anyway */
+		ssl3_finish_mac(s,&s->init_buf->data[s->init_off],ret);
+
 	if (ret == s->init_num)
 		return(1);
-	if (ret < 0) return(-1);
 	s->init_off+=ret;
 	s->init_num-=ret;
 	return(0);

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
@@ -144,7 +144,6 @@ int ssl3_accept(SSL *s)
 			s->new_session=1;
 			/* s->state=SSL_ST_ACCEPT; */
 
-		case SSL3_ST_SR_MS_SGC:
 		case SSL_ST_BEFORE:
 		case SSL_ST_ACCEPT:
 		case SSL_ST_BEFORE|SSL_ST_ACCEPT:
@@ -188,7 +187,7 @@ int ssl3_accept(SSL *s)
 
 			if (s->state != SSL_ST_RENEGOTIATE)
 				{
-				if(s->state != SSL3_ST_SR_MS_SGC) ssl3_init_finished_mac(s);
+				ssl3_init_finished_mac(s);
 				s->state=SSL3_ST_SR_CLNT_HELLO_A;
 				s->ctx->stats.sess_accept++;
 				}
@@ -350,10 +349,12 @@ int ssl3_accept(SSL *s)
 
 		case SSL3_ST_SR_CERT_A:
 		case SSL3_ST_SR_CERT_B:
-			/* Check for second client hello if MS SGC */
+			/* Check for second client hello (MS SGC) */
 			ret = ssl3_check_client_hello(s);
-			if(ret <= 0) goto end;
-			if(ret == 2) s->state = SSL3_ST_SR_MS_SGC;
+			if (ret <= 0)
+				goto end;
+			if (ret == 2)
+				s->state = SSL3_ST_SR_CLNT_HELLO_C;
 			else {
 				/* could be sent for a DH cert, even if we
 				 * have not asked for it :-) */

diff --git a/ssl/ssl3.h b/ssl/ssl3.h
@@ -365,7 +365,6 @@ typedef struct ssl3_state_st
 #define SSL3_ST_SR_CLNT_HELLO_A		(0x110|SSL_ST_ACCEPT)
 #define SSL3_ST_SR_CLNT_HELLO_B		(0x111|SSL_ST_ACCEPT)
 #define SSL3_ST_SR_CLNT_HELLO_C		(0x112|SSL_ST_ACCEPT)
-#define SSL3_ST_SR_MS_SGC			(0x113|SSL_ST_ACCEPT)
 /* write to client */
 #define SSL3_ST_SW_HELLO_REQ_A		(0x120|SSL_ST_ACCEPT)
 #define SSL3_ST_SW_HELLO_REQ_B		(0x121|SSL_ST_ACCEPT)

diff --git a/ssl/ssl_stat.c b/ssl/ssl_stat.c
@@ -161,7 +161,6 @@ case SSL3_ST_SW_FLUSH:		str="SSLv3 flush data"; break;
 case SSL3_ST_SR_CLNT_HELLO_A:	str="SSLv3 read client hello A"; break;
 case SSL3_ST_SR_CLNT_HELLO_B:	str="SSLv3 read client hello B"; break;
 case SSL3_ST_SR_CLNT_HELLO_C:	str="SSLv3 read client hello C"; break;
-case SSL3_ST_SR_MS_SGC:		str="SSLv3 read second client hello (MS SGC)"; break;
 case SSL3_ST_SW_HELLO_REQ_A:	str="SSLv3 write hello request A"; break;
 case SSL3_ST_SW_HELLO_REQ_B:	str="SSLv3 write hello request B"; break;
 case SSL3_ST_SW_HELLO_REQ_C:	str="SSLv3 write hello request C"; break;
@@ -313,7 +312,6 @@ case SSL3_ST_SW_HELLO_REQ_C:			str="3WHR_C"; break;
 case SSL3_ST_SR_CLNT_HELLO_A:			str="3RCH_A"; break;
 case SSL3_ST_SR_CLNT_HELLO_B:			str="3RCH_B"; break;
 case SSL3_ST_SR_CLNT_HELLO_C:			str="3RCH_C"; break;
-case SSL3_ST_SR_MS_SGC:				str="3RMSSG"; break;
 case SSL3_ST_SW_SRVR_HELLO_A:			str="3WSH_A"; break;
 case SSL3_ST_SW_SRVR_HELLO_B:			str="3WSH_B"; break;
 case SSL3_ST_SW_CERT_A:				str="3WSC_A"; break;