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>

diff --git a/doc/ssl/ssl.pod b/doc/ssl/ssl.pod
index 3634fa9a3ee7dd0b788e0e4b358b8852d4d0bb8c..5af0fc63c6776dc5d91e119bff37b449ec22d998 100644 (file)
--- 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 11a2aadc5fdc97e3e1463201539c707753249424..0344b7e35b9bafa1908e41332693e3466d924d8f 100644 (file)
--- 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 5463acfe7fccc7608df3cbdb2ebe482e1d10f5b7..19e60b7889602d8f7dfb56ac0619931a24d57637 100644 (file)
--- 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 c0a368b8f5187222e35ad4b4c46ca514c510e26a..55aa6a6c94a211072b538059cf909cf02ec10c39 100644 (file)
--- 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 1e9ad91c468abc97941da16a5b695ca99b21f530..0b2d8de6121783eabea44c825440910324b95d63 100644 (file)
--- 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 3b3f298a691fc79d143b07e15c828e55f13531df..40c42a8f499033049d7f6d1085815b98d21cdf83 100644 (file)
--- 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 b5eb4bfadc14dda224bf53cc5c77a196a4ab3699..40d64904d87f602188bbaa3af8ffec5183d73357 100644 (file)
--- 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 */