Resolve swallowed returns codes

The recent updates to libssl to enforce stricter return code checking, left
a small number of instances behind where return codes were being swallowed
(typically because the function they were being called from was declared as
void). This commit fixes those instances to handle the return codes more
appropriately.

Reviewed-by: Richard Levitte <levitte@openssl.org>

7 files changed, 26 insertions, 20 deletions

diff --git a/doc/ssl/ssl.pod b/doc/ssl/ssl.pod
index 3634fa9a3e..5af0fc63c6 100644
--- a/doc/ssl/ssl.pod
+++ b/doc/ssl/ssl.pod
@@ -453,7 +453,10 @@ connection defined in the B<SSL> structure.
 
 =item int B<SSL_connect>(SSL *ssl);
 
-=item void B<SSL_copy_session_id>(SSL *t, const SSL *f);
+=item int B<SSL_copy_session_id>(SSL *t, const SSL *f);
+
+Sets the session details for B<t> to be the same as in B<f>. Returns 1 on
+success or 0 on failure.
 
 =item long B<SSL_ctrl>(SSL *ssl, int cmd, long larg, char *parg);
 
@@ -756,5 +759,8 @@ The L<ssl(3)|ssl(3)> document appeared in OpenSSL 0.9.2
 B<SSLv2_client_method>, B<SSLv2_server_method> and B<SSLv2_method> where removed
 in OpenSSL 1.1.0.
 
+The return type of B<SSL_copy_session_id> was changed from void to int in
+OpenSSL 1.1.0.
+
 =cut
 
diff --git a/ssl/bio_ssl.c b/ssl/bio_ssl.c
index 11a2aadc5f..0344b7e35b 100644
--- a/ssl/bio_ssl.c
+++ b/ssl/bio_ssl.c
@@ -556,7 +556,8 @@ int 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/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 5463acfe7f..19e60b7889 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -1249,8 +1249,7 @@ int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
             if (dtls1_check_timeout_num(s) < 0)
                 return -1;
 
-            /* Ignore retransmit failures - swallow return code */
-            if(dtls1_retransmit_buffered_messages(s));
+            dtls1_retransmit_buffered_messages(s);
             rr->length = 0;
             goto start;
         }
diff --git a/ssl/ssl.h b/ssl/ssl.h
index c0a368b8f5..55aa6a6c94 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -1467,7 +1467,7 @@ __owur int SSL_SESSION_has_ticket(const SSL_SESSION *s);
 __owur unsigned long SSL_SESSION_get_ticket_lifetime_hint(const SSL_SESSION *s);
 void SSL_SESSION_get0_ticket(const SSL_SESSION *s, unsigned char **tick,
                             size_t *len);
-void SSL_copy_session_id(SSL *to, const SSL *from);
+__owur int SSL_copy_session_id(SSL *to, const SSL *from);
 __owur X509 *SSL_SESSION_get0_peer(SSL_SESSION *s);
 __owur int SSL_SESSION_set1_id_context(SSL_SESSION *s, const unsigned char *sid_ctx,
                                 unsigned int sid_ctx_len);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 1e9ad91c46..0b2d8de612 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -880,12 +880,11 @@ STACK_OF(X509) *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 SSL_copy_session_id(SSL *t, const SSL *f)
+int SSL_copy_session_id(SSL *t, const SSL *f)
 {
     /* Do we need to to SSL locking? */
     if(!SSL_set_session(t, SSL_get_session(f))) {
-        /* How do we handle this!! void function */
-        return;
+        return 0;
     }
 
     /*
@@ -901,9 +900,10 @@ void SSL_copy_session_id(SSL *t, const SSL *f)
     ssl_cert_free(t->cert);
     t->cert = f->cert;
     if(!SSL_set_session_id_context(t, f->sid_ctx, f->sid_ctx_length)) {
-        /* Really should do something about this..but void function - ignore */
-        ;
+        return 0;
     }
+
+    return 1;
 }
 
 /* Fix this so it checks all the valid key/cert options */
@@ -2757,7 +2757,8 @@ SSL *SSL_dup(SSL *s)
 
     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 that
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 3b3f298a69..40c42a8f49 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2215,7 +2215,7 @@ __owur int dtls1_buffer_message(SSL *s, int ccs);
 __owur int dtls1_retransmit_message(SSL *s, unsigned short seq,
                              unsigned long frag_off, int *found);
 __owur int dtls1_get_queue_priority(unsigned short seq, int is_ccs);
-__owur int dtls1_retransmit_buffered_messages(SSL *s);
+int dtls1_retransmit_buffered_messages(SSL *s);
 void dtls1_clear_record_buffer(SSL *s);
 void dtls1_get_message_header(unsigned char *data,
                               struct hm_header_st *msg_hdr);
@@ -2369,7 +2369,7 @@ void ssl_set_sig_mask(unsigned long *pmask_a, SSL *s, int op);
 
 __owur int tls1_set_sigalgs_list(CERT *c, const char *str, int client);
 __owur int tls1_set_sigalgs(CERT *c, const int *salg, size_t salglen, int client);
-__owur int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain,
+int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain,
                      int idx);
 void tls1_set_cert_validity(SSL *s);
 
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index b5eb4bfadc..40d64904d8 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -4157,13 +4157,12 @@ int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain,
 /* Set validity of certificates in an SSL structure */
 void tls1_set_cert_validity(SSL *s)
 {
-    /* Deliberately ignore all return values */
-    if(tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_RSA_ENC)
-       || tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_RSA_SIGN)
-       || tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_DSA_SIGN)
-       || tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_DH_RSA)
-       || tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_DH_DSA)
-       || tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_ECC));
+    tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_RSA_ENC);
+    tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_RSA_SIGN);
+    tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_DSA_SIGN);
+    tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_DH_RSA);
+    tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_DH_DSA);
+    tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_ECC);
 }
 
 /* User level utiity function to check a chain is suitable */