Clean up tls1_change_cipher_state().

Replace flag gymnastics at call sites with separate read and write,
functions which call the common code. Condition on s->server instead of
using SSL_ST_ACCEPT, for consistency and more readable code.

ok inoguchi@ tb@

diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c
index 022efd8..1874d22 100644 (file)
--- a/lib/libssl/ssl_clnt.c
+++ b/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.95 2021/05/02 17:18:10 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.96 2021/05/02 17:46:58 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -474,15 +474,13 @@ ssl3_connect(SSL *s)
 
                        S3I(s)->hs.state = SSL3_ST_CW_FINISHED_A;
                        s->internal->init_num = 0;
-
                        s->session->cipher = S3I(s)->hs.cipher;
+
                        if (!tls1_setup_key_block(s)) {
                                ret = -1;
                                goto end;
                        }
-
-                       if (!tls1_change_cipher_state(s,
-                           SSL3_CHANGE_CIPHER_CLIENT_WRITE)) {
+                       if (!tls1_change_write_cipher_state(s)) {
                                ret = -1;
                                goto end;
                        }

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 38b6838..9dfa124 100644 (file)
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.340 2021/05/02 17:18:10 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.341 2021/05/02 17:46:58 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1378,7 +1378,8 @@ int tls1_PRF(SSL *s, const unsigned char *secret, size_t secret_len,
     const void *seed5, size_t seed5_len, unsigned char *out, size_t out_len);
 
 void tls1_cleanup_key_block(SSL *s);
-int tls1_change_cipher_state(SSL *s, int which);
+int tls1_change_read_cipher_state(SSL *s);
+int tls1_change_write_cipher_state(SSL *s);
 int tls1_setup_key_block(SSL *s);
 int tls1_export_keying_material(SSL *s, unsigned char *out, size_t olen,
     const char *label, size_t llen, const unsigned char *p, size_t plen,

diff --git a/lib/libssl/ssl_pkt.c b/lib/libssl/ssl_pkt.c
index 6e0cfe2..ba59aa3 100644 (file)
--- a/lib/libssl/ssl_pkt.c
+++ b/lib/libssl/ssl_pkt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_pkt.c,v 1.41 2021/04/25 13:15:22 jsing Exp $ */
+/* $OpenBSD: ssl_pkt.c,v 1.42 2021/05/02 17:46:58 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1154,8 +1154,6 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
 int
 ssl3_do_change_cipher_spec(SSL *s)
 {
-       int i;
-
        if (S3I(s)->hs.tls12.key_block == NULL) {
                if (s->session == NULL || s->session->master_key_length == 0) {
                        /* might happen if dtls1_read_bytes() calls this */
@@ -1168,12 +1166,7 @@ ssl3_do_change_cipher_spec(SSL *s)
                        return (0);
        }
 
-       if (S3I(s)->hs.state & SSL_ST_ACCEPT)
-               i = SSL3_CHANGE_CIPHER_SERVER_READ;
-       else
-               i = SSL3_CHANGE_CIPHER_CLIENT_READ;
-
-       if (!tls1_change_cipher_state(s, i))
+       if (!tls1_change_read_cipher_state(s))
                return (0);
 
        /*

diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index 8e6a185..f884ea3 100644 (file)
--- a/lib/libssl/ssl_srvr.c
+++ b/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.107 2021/05/02 17:28:33 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.108 2021/05/02 17:46:58 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -647,15 +647,13 @@ ssl3_accept(SSL *s)
                                goto end;
                        S3I(s)->hs.state = SSL3_ST_SW_FINISHED_A;
                        s->internal->init_num = 0;
-
                        s->session->cipher = S3I(s)->hs.cipher;
+
                        if (!tls1_setup_key_block(s)) {
                                ret = -1;
                                goto end;
                        }
-
-                       if (!tls1_change_cipher_state(s,
-                           SSL3_CHANGE_CIPHER_SERVER_WRITE)) {
+                       if (!tls1_change_write_cipher_state(s)) {
                                ret = -1;
                                goto end;
                        }

diff --git a/lib/libssl/t1_enc.c b/lib/libssl/t1_enc.c
index 6cdae0c..e3cdcc1 100644 (file)
--- a/lib/libssl/t1_enc.c
+++ b/lib/libssl/t1_enc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_enc.c,v 1.141 2021/05/02 17:18:10 jsing Exp $ */
+/* $OpenBSD: t1_enc.c,v 1.142 2021/05/02 17:46:58 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -294,8 +294,8 @@ tls1_generate_key_block(SSL *s, uint8_t *key_block, size_t key_block_len)
            NULL, 0, NULL, 0, key_block, key_block_len);
 }
 
-int
-tls1_change_cipher_state(SSL *s, int which)
+static int
+tls1_change_cipher_state(SSL *s, int is_write)
 {
        const unsigned char *client_write_mac_secret, *server_write_mac_secret;
        const unsigned char *client_write_key, *server_write_key;
@@ -305,26 +305,10 @@ tls1_change_cipher_state(SSL *s, int which)
        unsigned char *key_block;
        const EVP_CIPHER *cipher;
        const EVP_AEAD *aead;
-       char is_read, use_client_keys;
 
        aead = tls12_record_layer_aead(s->internal->rl);
        cipher = tls12_record_layer_cipher(s->internal->rl);
 
-       /*
-        * is_read is true if we have just read a ChangeCipherSpec message,
-        * that is we need to update the read cipherspec. Otherwise we have
-        * just written one.
-        */
-       is_read = (which & SSL3_CC_READ) != 0;
-
-       /*
-        * use_client_keys is true if we wish to use the keys for the "client
-        * write" direction. This is the case if we're a client sending a
-        * ChangeCipherSpec, or a server reading a client's ChangeCipherSpec.
-        */
-       use_client_keys = ((which == SSL3_CHANGE_CIPHER_CLIENT_WRITE) ||
-           (which == SSL3_CHANGE_CIPHER_SERVER_READ));
-
        if (aead != NULL) {
                key_len = EVP_AEAD_key_length(aead);
                iv_len = SSL_CIPHER_AEAD_FIXED_NONCE_LEN(S3I(s)->hs.cipher);
@@ -349,7 +333,8 @@ tls1_change_cipher_state(SSL *s, int which)
        server_write_iv = key_block;
        key_block += iv_len;
 
-       if (use_client_keys) {
+       /* Use client write keys on client write and server read. */
+       if ((!s->server && is_write) || (s->server && !is_write)) {
                mac_secret = client_write_mac_secret;
                key = client_write_key;
                iv = client_write_iv;
@@ -365,7 +350,7 @@ tls1_change_cipher_state(SSL *s, int which)
                goto err;
        }
 
-       if (is_read) {
+       if (!is_write) {
                if (!tls12_record_layer_change_read_cipher_state(s->internal->rl,
                    mac_secret, mac_secret_size, key, key_len, iv, iv_len))
                        goto err;
@@ -386,6 +371,18 @@ tls1_change_cipher_state(SSL *s, int which)
        return (0);
 }
 
+int
+tls1_change_read_cipher_state(SSL *s)
+{
+       return tls1_change_cipher_state(s, 0);
+}
+
+int
+tls1_change_write_cipher_state(SSL *s)
+{
+       return tls1_change_cipher_state(s, 1);
+}
+
 int
 tls1_setup_key_block(SSL *s)
 {