Factor out change cipher spec handing code in the legacy stack.

Factor out the code that handles the processing of a change cipher spec
message that has been read in the legacy stack, deduplicating code in the
DTLS stack.

ok inoguchi@ tb@

diff --git a/lib/libssl/d1_pkt.c b/lib/libssl/d1_pkt.c
index e07fc7e..6ed0439 100644 (file)
--- a/lib/libssl/d1_pkt.c
+++ b/lib/libssl/d1_pkt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: d1_pkt.c,v 1.118 2022/02/21 18:22:20 jsing Exp $ */
+/* $OpenBSD: d1_pkt.c,v 1.119 2022/03/12 12:53:03 jsing Exp $ */
 /*
  * DTLS implementation written by Nagendra Modadugu
  * (nagendra@cs.stanford.edu) for the OpenSSL project 2005.
@@ -748,33 +748,8 @@ dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
        }
 
        if (rr->type == SSL3_RT_CHANGE_CIPHER_SPEC) {
-               /* 'Change Cipher Spec' is just a single byte, so we know
-                * exactly what the record payload has to look like */
-               /* XDTLS: check that epoch is consistent */
-               if ((rr->length != DTLS1_CCS_HEADER_LENGTH) ||
-                   (rr->off != 0) || (rr->data[0] != SSL3_MT_CCS)) {
-                       al = SSL_AD_DECODE_ERROR;
-                       SSLerror(s, SSL_R_BAD_CHANGE_CIPHER_SPEC);
-                       goto fatal_err;
-               }
-
-               ssl_msg_callback(s, 0, SSL3_RT_CHANGE_CIPHER_SPEC, rr->data, 1);
-
-               /* We can't process a CCS now, because previous handshake
-                * messages are still missing, so just drop it.
-                */
-               if (!s->d1->change_cipher_spec_ok) {
-                       rr->length = 0;
-                       goto start;
-               }
-
-               s->d1->change_cipher_spec_ok = 0;
-
-               s->s3->change_cipher_spec = 1;
-               if (!ssl3_do_change_cipher_spec(s))
-                       goto err;
-
-               rr->length = 0;
+               if ((ret = ssl3_read_change_cipher_spec(s)) <= 0)
+                       return ret;
                goto start;
        }
 
@@ -872,7 +847,7 @@ dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
 
  fatal_err:
        ssl3_send_alert(s, SSL3_AL_FATAL, al);
- err:
+
        return (-1);
 }
 

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index ada9949..8a2f69f 100644 (file)
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.386 2022/02/21 18:22:20 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.387 2022/03/12 12:53:03 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1334,6 +1334,7 @@ void ssl_force_want_read(SSL *s);
 
 int ssl3_dispatch_alert(SSL *s);
 int ssl3_read_alert(SSL *s);
+int ssl3_read_change_cipher_spec(SSL *s);
 int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek);
 int ssl3_write_bytes(SSL *s, int type, const void *buf, int len);
 int ssl3_output_cert_chain(SSL *s, CBB *cbb, SSL_CERT_PKEY *cpk);

diff --git a/lib/libssl/ssl_pkt.c b/lib/libssl/ssl_pkt.c
index e3b2034..33bb4b6 100644 (file)
--- a/lib/libssl/ssl_pkt.c
+++ b/lib/libssl/ssl_pkt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_pkt.c,v 1.54 2022/02/21 18:22:20 jsing Exp $ */
+/* $OpenBSD: ssl_pkt.c,v 1.55 2022/03/12 12:53:03 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -752,6 +752,68 @@ ssl3_read_alert(SSL *s)
        return 1;
 }
 
+int
+ssl3_read_change_cipher_spec(SSL *s)
+{
+       SSL3_RECORD_INTERNAL *rr = &s->s3->rrec;
+
+       /*
+        * 'Change Cipher Spec' is just a single byte, so we know exactly what
+        * the record payload has to look like.
+        */
+       if (rr->length != 1 || rr->off != 0) {
+               SSLerror(s, SSL_R_BAD_CHANGE_CIPHER_SPEC);
+               ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+               return -1;
+       }
+       if (rr->data[0] != SSL3_MT_CCS) {
+               SSLerror(s, SSL_R_BAD_CHANGE_CIPHER_SPEC);
+               ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+               return -1;
+       }
+
+       /* XDTLS: check that epoch is consistent */
+
+       ssl_msg_callback(s, 0, SSL3_RT_CHANGE_CIPHER_SPEC, rr->data, 1);
+
+       /* Check that we have a cipher to change to. */
+       if (s->s3->hs.cipher == NULL) {
+               SSLerror(s, SSL_R_CCS_RECEIVED_EARLY);
+               ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
+               return -1;
+       }
+
+       /* Check that we should be receiving a Change Cipher Spec. */
+       if (SSL_is_dtls(s)) {
+               if (!s->d1->change_cipher_spec_ok) {
+                       /*
+                        * We can't process a CCS now, because previous
+                        * handshake messages are still missing, so just
+                        * drop it.
+                        */
+                       rr->length = 0;
+                       return 1;
+               }
+               s->d1->change_cipher_spec_ok = 0;
+       } else {
+               if ((s->s3->flags & SSL3_FLAGS_CCS_OK) == 0) {
+                       SSLerror(s, SSL_R_CCS_RECEIVED_EARLY);
+                       ssl3_send_alert(s, SSL3_AL_FATAL,
+                           SSL_AD_UNEXPECTED_MESSAGE);
+                       return -1;
+               }
+               s->s3->flags &= ~SSL3_FLAGS_CCS_OK;
+       }
+
+       rr->length = 0;
+
+       s->s3->change_cipher_spec = 1;
+       if (!ssl3_do_change_cipher_spec(s))
+               return -1;
+
+       return 1;
+}
+
 /* Return up to 'len' payload bytes received in 'type' records.
  * 'type' is one of the following:
  *
@@ -1044,39 +1106,9 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
        }
 
        if (rr->type == SSL3_RT_CHANGE_CIPHER_SPEC) {
-               /* 'Change Cipher Spec' is just a single byte, so we know
-                * exactly what the record payload has to look like */
-               if ((rr->length != 1) || (rr->off != 0) ||
-                       (rr->data[0] != SSL3_MT_CCS)) {
-                       al = SSL_AD_ILLEGAL_PARAMETER;
-                       SSLerror(s, SSL_R_BAD_CHANGE_CIPHER_SPEC);
-                       goto fatal_err;
-               }
-
-               /* Check we have a cipher to change to */
-               if (s->s3->hs.cipher == NULL) {
-                       al = SSL_AD_UNEXPECTED_MESSAGE;
-                       SSLerror(s, SSL_R_CCS_RECEIVED_EARLY);
-                       goto fatal_err;
-               }
-
-               /* Check that we should be receiving a Change Cipher Spec. */
-               if (!(s->s3->flags & SSL3_FLAGS_CCS_OK)) {
-                       al = SSL_AD_UNEXPECTED_MESSAGE;
-                       SSLerror(s, SSL_R_CCS_RECEIVED_EARLY);
-                       goto fatal_err;
-               }
-               s->s3->flags &= ~SSL3_FLAGS_CCS_OK;
-
-               rr->length = 0;
-
-               ssl_msg_callback(s, 0, SSL3_RT_CHANGE_CIPHER_SPEC, rr->data, 1);
-
-               s->s3->change_cipher_spec = 1;
-               if (!ssl3_do_change_cipher_spec(s))
-                       goto err;
-               else
-                       goto start;
+               if ((ret = ssl3_read_change_cipher_spec(s)) <= 0)
+                       return ret;
+               goto start;
        }
 
        /* Unexpected handshake message (Client Hello, or protocol violation) */
@@ -1155,7 +1187,7 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
 
  fatal_err:
        ssl3_send_alert(s, SSL3_AL_FATAL, al);
- err:
+
        return (-1);
 }