From c181c81fb01592ad1d49ebf7afa9676c41a32aaf Mon Sep 17 00:00:00 2001
From: tb <>
Date: Sat, 27 Mar 2021 17:56:28 +0000
Subject: Garbage collect s->internal->type
This variable is used in the legacy stack to decide whether we are
a server or a client. That's what s->server is for...
The new TLSv1.3 stack failed to set s->internal->type, which resulted
in hilarious mishandling of previous_{client,server}_finished. Indeed,
both client and server would first store the client's verify_data in
previous_server_finished and later overwrite it with the server's
verify_data. Consequently, renegotiation has been completely broken
for more than a year. In fact, server side renegotiation was broken
during the 6.5 release cycle. Clearly, no-one uses this.
This commit fixes client side renegotiation and restores the previous
behavior of SSL_get_client_CA_list(). Server side renegotiation will
be fixed in a later commit.
ok jsing
---
src/lib/libssl/ssl_both.c | 6 +++---
src/lib/libssl/ssl_cert.c | 4 ++--
src/lib/libssl/ssl_clnt.c | 4 +---
src/lib/libssl/ssl_lib.c | 5 +----
src/lib/libssl/ssl_locl.h | 4 +---
src/lib/libssl/ssl_srvr.c | 4 +---
6 files changed, 9 insertions(+), 18 deletions(-)
(limited to 'src/lib')
diff --git a/src/lib/libssl/ssl_both.c b/src/lib/libssl/ssl_both.c
index 6625286daf..789ab01213 100644
--- a/src/lib/libssl/ssl_both.c
+++ b/src/lib/libssl/ssl_both.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_both.c,v 1.25 2021/03/24 18:44:00 jsing Exp $ */
+/* $OpenBSD: ssl_both.c,v 1.26 2021/03/27 17:56:28 tb Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@@ -181,7 +181,7 @@ ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen)
S3I(s)->tmp.finish_md_len = md_len;
/* Copy finished so we can use it for renegotiation checks. */
- if (s->internal->type == SSL_ST_CONNECT) {
+ if (!s->server) {
memcpy(S3I(s)->previous_client_finished,
S3I(s)->tmp.finish_md, md_len);
S3I(s)->previous_client_finished_len = md_len;
@@ -285,7 +285,7 @@ ssl3_get_finished(SSL *s, int a, int b)
/* Copy finished so we can use it for renegotiation checks. */
OPENSSL_assert(md_len <= EVP_MAX_MD_SIZE);
- if (s->internal->type == SSL_ST_ACCEPT) {
+ if (s->server) {
memcpy(S3I(s)->previous_client_finished,
S3I(s)->tmp.peer_finish_md, md_len);
S3I(s)->previous_client_finished_len = md_len;
diff --git a/src/lib/libssl/ssl_cert.c b/src/lib/libssl/ssl_cert.c
index 2e0dca58ea..03ef8565ac 100644
--- a/src/lib/libssl/ssl_cert.c
+++ b/src/lib/libssl/ssl_cert.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_cert.c,v 1.80 2020/11/20 08:08:02 tb Exp $ */
+/* $OpenBSD: ssl_cert.c,v 1.81 2021/03/27 17:56:28 tb Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@@ -505,7 +505,7 @@ SSL_CTX_get_client_CA_list(const SSL_CTX *ctx)
STACK_OF(X509_NAME) *
SSL_get_client_CA_list(const SSL *s)
{
- if (s->internal->type == SSL_ST_CONNECT) {
+ if (!s->server) {
/* We are in the client. */
if ((s->version >> 8) == SSL3_VERSION_MAJOR)
return (S3I(s)->tmp.ca_names);
diff --git a/src/lib/libssl/ssl_clnt.c b/src/lib/libssl/ssl_clnt.c
index 984ade0957..63adacd9cf 100644
--- a/src/lib/libssl/ssl_clnt.c
+++ b/src/lib/libssl/ssl_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_clnt.c,v 1.88 2021/03/24 18:44:00 jsing Exp $ */
+/* $OpenBSD: ssl_clnt.c,v 1.89 2021/03/27 17:56:28 tb Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@@ -226,8 +226,6 @@ ssl3_connect(SSL *s)
goto end;
}
- s->internal->type = SSL_ST_CONNECT;
-
if (!ssl3_setup_init_buffer(s)) {
ret = -1;
goto end;
diff --git a/src/lib/libssl/ssl_lib.c b/src/lib/libssl/ssl_lib.c
index e3e0c974af..c77fdd77e9 100644
--- a/src/lib/libssl/ssl_lib.c
+++ b/src/lib/libssl/ssl_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_lib.c,v 1.252 2021/03/24 18:44:00 jsing Exp $ */
+/* $OpenBSD: ssl_lib.c,v 1.253 2021/03/27 17:56:28 tb Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@@ -186,8 +186,6 @@ SSL_clear(SSL *s)
return (0);
}
- s->internal->type = 0;
-
s->version = s->method->internal->version;
s->client_version = s->version;
s->internal->rwstate = SSL_NOTHING;
@@ -2494,7 +2492,6 @@ SSL_dup(SSL *s)
goto err;
ret->version = s->version;
- ret->internal->type = s->internal->type;
ret->method = s->method;
if (s->session != NULL) {
diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index c8c7ca5472..7f197bbcdf 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.330 2021/03/24 18:44:00 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.331 2021/03/27 17:56:28 tb Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@@ -755,8 +755,6 @@ typedef struct ssl_internal_st {
/* XXX non-callback */
- int type; /* SSL_ST_CONNECT or SSL_ST_ACCEPT */
-
/* This holds a variable that indicates what we were doing
* when a 0 or -1 is returned. This is needed for
* non-blocking IO so we know what request needs re-doing when
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index 047087c1c9..aea8d67260 100644
--- a/src/lib/libssl/ssl_srvr.c
+++ b/src/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.99 2021/03/24 18:44:00 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.100 2021/03/27 17:56:28 tb Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@@ -227,8 +227,6 @@ ssl3_accept(SSL *s)
goto end;
}
- s->internal->type = SSL_ST_ACCEPT;
-
if (!ssl3_setup_init_buffer(s)) {
ret = -1;
goto end;
--
cgit v1.2.3-55-g6feb