From: tb <tb@openbsd.org>
Date: Fri, 24 Aug 2018 20:30:21 +0000 (+0000)
Subject: Let SSL_copy_session_id() return an int for error checking.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=1e7a28f35c78b2a96f391bbdce6e59bf4b46d07c;p=openbsd

Let SSL_copy_session_id() return an int for error checking.

Accordingly, add some error checking to SSL_copy_session_id(),
BIO_ssl_copy_session_id(), and SSL_dup().
Prompted by OpenSSL commit 17dd65e6e1f

Tested in a bulk build by sthen

ok jsing
---

diff --git a/lib/libssl/bio_ssl.c b/lib/libssl/bio_ssl.c
index d68e011c620..93cfa0d2a42 100644
--- a/lib/libssl/bio_ssl.c
+++ b/lib/libssl/bio_ssl.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: bio_ssl.c,v 1.28 2018/05/01 13:30:24 tb Exp $ */
+/* $OpenBSD: bio_ssl.c,v 1.29 2018/08/24 20:30:21 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -568,7 +568,9 @@ BIO_ssl_copy_session_id(BIO *t, BIO *f)
 	if ((((BIO_SSL *)t->ptr)->ssl == NULL) ||
 	    (((BIO_SSL *)f->ptr)->ssl == NULL))
 		return (0);
-	SSL_copy_session_id(((BIO_SSL *)t->ptr)->ssl, ((BIO_SSL *)f->ptr)->ssl);
+	if (!SSL_copy_session_id(((BIO_SSL *)t->ptr)->ssl,
+	    ((BIO_SSL *)f->ptr)->ssl))
+		return (0);
 	return (1);
 }
 
diff --git a/lib/libssl/ssl.h b/lib/libssl/ssl.h
index c3b553fa2f6..324691485bc 100644
--- a/lib/libssl/ssl.h
+++ b/lib/libssl/ssl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl.h,v 1.158 2018/05/01 13:30:24 tb Exp $ */
+/* $OpenBSD: ssl.h,v 1.159 2018/08/24 20:30:21 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1311,7 +1311,7 @@ long	SSL_SESSION_get_time(const SSL_SESSION *s);
 long	SSL_SESSION_set_time(SSL_SESSION *s, long t);
 long	SSL_SESSION_get_timeout(const SSL_SESSION *s);
 long	SSL_SESSION_set_timeout(SSL_SESSION *s, long t);
-void	SSL_copy_session_id(SSL *to, const SSL *from);
+int	SSL_copy_session_id(SSL *to, const SSL *from);
 X509	*SSL_SESSION_get0_peer(SSL_SESSION *s);
 int	SSL_SESSION_set1_id(SSL_SESSION *s, const unsigned char *sid,
 	    unsigned int sid_len);
diff --git a/lib/libssl/ssl_lib.c b/lib/libssl/ssl_lib.c
index 4f1eb5bf0af..0dbc7b37071 100644
--- a/lib/libssl/ssl_lib.c
+++ b/lib/libssl/ssl_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_lib.c,v 1.185 2018/04/25 07:10:39 tb Exp $ */
+/* $OpenBSD: ssl_lib.c,v 1.186 2018/08/24 20:30:21 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -853,22 +853,21 @@ SSL_get_peer_cert_chain(const SSL *s)
  * Now in theory, since the calling process own 't' it should be safe to
  * modify.  We need to be able to read f without being hassled
  */
-void
+int
 SSL_copy_session_id(SSL *t, const SSL *f)
 {
 	CERT	*tmp;
 
-	/* Do we need to to SSL locking? */
-	SSL_set_session(t, SSL_get_session(f));
+	/* Do we need to do SSL locking? */
+	if (!SSL_set_session(t, SSL_get_session(f)))
+		return 0;
 
-	/*
-	 * What if we are setup as SSLv2 but want to talk SSLv3 or
-	 * vice-versa.
-	 */
+	/* What if we are set up for one protocol but want to talk another? */
 	if (t->method != f->method) {
-		t->method->internal->ssl_free(t);	/* cleanup current */
-		t->method = f->method;	/* change method */
-		t->method->internal->ssl_new(t);	/* setup new */
+		t->method->internal->ssl_free(t);
+		t->method = f->method;
+		if (!t->method->internal->ssl_new(t))
+			return 0;
 	}
 
 	tmp = t->cert;
@@ -878,7 +877,11 @@ SSL_copy_session_id(SSL *t, const SSL *f)
 	} else
 		t->cert = NULL;
 	ssl_cert_free(tmp);
-	SSL_set_session_id_context(t, f->sid_ctx, f->sid_ctx_length);
+
+	if (!SSL_set_session_id_context(t, f->sid_ctx, f->sid_ctx_length))
+		return 0;
+
+	return 1;
 }
 
 /* Fix this so it checks all the valid key/cert options */
@@ -2500,15 +2503,15 @@ SSL_dup(SSL *s)
 	int i;
 
 	if ((ret = SSL_new(SSL_get_SSL_CTX(s))) == NULL)
-		return (NULL);
+		goto err;
 
 	ret->version = s->version;
 	ret->internal->type = s->internal->type;
 	ret->method = s->method;
 
 	if (s->session != NULL) {
-		/* This copies session-id, SSL_METHOD, sid_ctx, and 'cert' */
-		SSL_copy_session_id(ret, s);
+		if (!SSL_copy_session_id(ret, s))
+			goto err;
 	} else {
 		/*
 		 * No session has been established yet, so we have to expect
@@ -2528,8 +2531,9 @@ SSL_dup(SSL *s)
 				goto err;
 		}
 
-		SSL_set_session_id_context(ret,
-		s->sid_ctx, s->sid_ctx_length);
+		if (!SSL_set_session_id_context(ret, s->sid_ctx,
+		    s->sid_ctx_length))
+			goto err;
 	}
 
 	ret->internal->options = s->internal->options;
@@ -2612,13 +2616,10 @@ SSL_dup(SSL *s)
 		}
 	}
 
-	if (0) {
-err:
-		if (ret != NULL)
-			SSL_free(ret);
-		ret = NULL;
-	}
-	return (ret);
+	return ret;
+ err:
+	SSL_free(ret);
+	return NULL;
 }
 
 void