From 1e7a28f35c78b2a96f391bbdce6e59bf4b46d07c Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 24 Aug 2018 20:30:21 +0000 Subject: [PATCH] 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 --- lib/libssl/bio_ssl.c | 6 ++++-- lib/libssl/ssl.h | 4 ++-- lib/libssl/ssl_lib.c | 49 ++++++++++++++++++++++---------------------- 3 files changed, 31 insertions(+), 28 deletions(-) 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 -- 2.20.1