From ac59d70553723cd8c7c1558071a2e1672d80daef Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 6 Mar 2015 14:39:46 +0000 Subject: [PATCH] apps return value checks Ensure that all libssl functions called from within the apps have their return values checked where appropriate. Reviewed-by: Richard Levitte --- apps/s_cb.c | 6 ++-- apps/s_client.c | 36 ++++++++++++++++++------ apps/s_server.c | 73 +++++++++++++++++++++++++++++++++++++------------ apps/s_time.c | 12 +++++--- apps/sess_id.c | 5 +++- 5 files changed, 99 insertions(+), 33 deletions(-) diff --git a/apps/s_cb.c b/apps/s_cb.c index 8a66c9a7dc..8bc4b81710 100644 --- a/apps/s_cb.c +++ b/apps/s_cb.c @@ -1181,8 +1181,10 @@ static int set_cert_cb(SSL *ssl, void *arg) print_chain_flags(bio_err, ssl, rv); if (rv & CERT_PKEY_VALID) { - SSL_use_certificate(ssl, exc->cert); - SSL_use_PrivateKey(ssl, exc->key); + if(!SSL_use_certificate(ssl, exc->cert) + || !SSL_use_PrivateKey(ssl, exc->key)) { + return 0; + } /* * NB: we wouldn't normally do this as it is not efficient * building chains on each connection better to cache the chain diff --git a/apps/s_client.c b/apps/s_client.c index 3ec754f346..c02ed3c0e5 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -1259,8 +1259,11 @@ int MAIN(int argc, char **argv) if (sdebug) ssl_ctx_security_debug(ctx, bio_err, sdebug); - if (vpm) - SSL_CTX_set1_param(ctx, vpm); + if (vpm && !SSL_CTX_set1_param(ctx, vpm)) { + BIO_printf(bio_err, "Error setting verify params\n"); + ERR_print_errors(bio_err); + goto end; + } if (!args_ssl_call(ctx, bio_err, cctx, ssl_args, 1, no_jpake)) { ERR_print_errors(bio_err); @@ -1299,8 +1302,14 @@ int MAIN(int argc, char **argv) } #endif #ifndef OPENSSL_NO_SRTP - if (srtp_profiles != NULL) - SSL_CTX_set_tlsext_use_srtp(ctx, srtp_profiles); + if (srtp_profiles != NULL) { + /* Returns 0 on success!! */ + if(SSL_CTX_set_tlsext_use_srtp(ctx, srtp_profiles)) { + BIO_printf(bio_err, "Error setting SRTP profile\n"); + ERR_print_errors(bio_err); + goto end; + } + } #endif if (exc) ssl_ctx_set_excert(ctx, exc); @@ -1318,16 +1327,23 @@ int MAIN(int argc, char **argv) BIO_printf(bio_err, "Error parsing -alpn argument\n"); goto end; } - SSL_CTX_set_alpn_protos(ctx, alpn, alpn_len); + /* Returns 0 on success!! */ + if(SSL_CTX_set_alpn_protos(ctx, alpn, alpn_len)) { + BIO_printf(bio_err, "Error setting ALPN\n"); + goto end; + } OPENSSL_free(alpn); } #endif #ifndef OPENSSL_NO_TLSEXT for (i = 0; i < serverinfo_types_count; i++) { - SSL_CTX_add_client_custom_ext(ctx, + if(!SSL_CTX_add_client_custom_ext(ctx, serverinfo_types[i], NULL, NULL, NULL, - serverinfo_cli_parse_cb, NULL); + serverinfo_cli_parse_cb, NULL)) { + BIO_printf(bio_err, "Warning: Unable to add custom extension %u. " + "Skipping\n", serverinfo_types[i]); + } } #endif @@ -1390,7 +1406,11 @@ int MAIN(int argc, char **argv) ERR_print_errors(bio_err); goto end; } - SSL_set_session(con, sess); + if(!SSL_set_session(con, sess)) { + BIO_printf(bio_err, "Can't set session\n"); + ERR_print_errors(bio_err); + goto end; + } SSL_SESSION_free(sess); } diff --git a/apps/s_server.c b/apps/s_server.c index ec2fe6fd57..298e665d97 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -1723,8 +1723,14 @@ int MAIN(int argc, char *argv[]) SSL_CTX_sess_set_cache_size(ctx, 128); #ifndef OPENSSL_NO_SRTP - if (srtp_profiles != NULL) - SSL_CTX_set_tlsext_use_srtp(ctx, srtp_profiles); + if (srtp_profiles != NULL) { + /* Returns 0 on success!! */ + if(SSL_CTX_set_tlsext_use_srtp(ctx, srtp_profiles)) { + BIO_printf(bio_err, "Error setting SRTP profile\n"); + ERR_print_errors(bio_err); + goto end; + } + } #endif if ((!SSL_CTX_load_verify_locations(ctx, CAfile, CApath)) || @@ -1733,8 +1739,11 @@ int MAIN(int argc, char *argv[]) ERR_print_errors(bio_err); /* goto end; */ } - if (vpm) - SSL_CTX_set1_param(ctx, vpm); + if (vpm && !SSL_CTX_set1_param(ctx, vpm)) { + BIO_printf(bio_err, "Error setting X509 params\n"); + ERR_print_errors(bio_err); + goto end; + } ssl_ctx_add_crls(ctx, crls, 0); if (!args_ssl_call(ctx, bio_err, cctx, ssl_args, no_ecdhe, no_jpake)) @@ -1790,8 +1799,11 @@ int MAIN(int argc, char *argv[]) (!SSL_CTX_set_default_verify_paths(ctx2))) { ERR_print_errors(bio_err); } - if (vpm) - SSL_CTX_set1_param(ctx2, vpm); + if (vpm && !SSL_CTX_set1_param(ctx2, vpm)) { + BIO_printf(bio_err, "Error setting X509 params\n"); + ERR_print_errors(bio_err); + goto end; + } ssl_ctx_add_crls(ctx2, crls, 0); if (!args_ssl_call(ctx2, bio_err, cctx, ssl_args, no_ecdhe, no_jpake)) @@ -1913,8 +1925,13 @@ int MAIN(int argc, char *argv[]) #endif SSL_CTX_set_verify(ctx, s_server_verify, verify_callback); - SSL_CTX_set_session_id_context(ctx, (void *)&s_server_session_id_context, - sizeof s_server_session_id_context); + if(!SSL_CTX_set_session_id_context(ctx, + (void *)&s_server_session_id_context, + sizeof s_server_session_id_context)) { + BIO_printf(bio_err, "error setting session id context\n"); + ERR_print_errors(bio_err); + goto end; + } /* Set DTLS cookie generation and verification callbacks */ SSL_CTX_set_cookie_generate_cb(ctx, generate_cookie_callback); @@ -1923,9 +1940,13 @@ int MAIN(int argc, char *argv[]) #ifndef OPENSSL_NO_TLSEXT if (ctx2) { SSL_CTX_set_verify(ctx2, s_server_verify, verify_callback); - SSL_CTX_set_session_id_context(ctx2, + if(!SSL_CTX_set_session_id_context(ctx2, (void *)&s_server_session_id_context, - sizeof s_server_session_id_context); + sizeof s_server_session_id_context)) { + BIO_printf(bio_err, "error setting session id context\n"); + ERR_print_errors(bio_err); + goto end; + } tlsextcbp.biodebug = bio_s_out; SSL_CTX_set_tlsext_servername_callback(ctx2, ssl_servername_cb); @@ -2130,10 +2151,18 @@ static int sv_body(char *hostname, int s, int stype, unsigned char *context) kssl_ctx_setstring(kctx, KSSL_KEYTAB, KRB5KEYTAB); } #endif /* OPENSSL_NO_KRB5 */ - if (context) - SSL_set_session_id_context(con, context, strlen((char *)context)); + if (context && !SSL_set_session_id_context(con, context, + strlen((char *)context))) { + BIO_printf(bio_err, "Error setting session id context\n"); + ret = -1; + goto err; + } + } + if(!SSL_clear(con)) { + BIO_printf(bio_err, "Error clearing SSL connection\n"); + ret = -1; + goto err; } - SSL_clear(con); if (stype == SOCK_DGRAM) { @@ -2687,8 +2716,10 @@ static int www_body(char *hostname, int s, int stype, unsigned char *context) kssl_ctx_setstring(kctx, KSSL_KEYTAB, KRB5KEYTAB); } #endif /* OPENSSL_NO_KRB5 */ - if (context) - SSL_set_session_id_context(con, context, strlen((char *)context)); + if (context && !SSL_set_session_id_context(con, context, + strlen((char *)context))) { + goto err; + } sbio = BIO_new_socket(s, BIO_NOCLOSE); if (s_nbio_test) { @@ -3033,8 +3064,11 @@ static int rev_body(char *hostname, int s, int stype, unsigned char *context) kssl_ctx_setstring(kctx, KSSL_KEYTAB, KRB5KEYTAB); } #endif /* OPENSSL_NO_KRB5 */ - if (context) - SSL_set_session_id_context(con, context, strlen((char *)context)); + if (context && !SSL_set_session_id_context(con, context, + strlen((char *)context))) { + ERR_print_errors(bio_err); + goto err; + } sbio = BIO_new_socket(s, BIO_NOCLOSE); SSL_set_bio(con, sbio, sbio); @@ -3230,7 +3264,10 @@ static int add_session(SSL *ssl, SSL_SESSION *session) return 0; } p = sess->der; - i2d_SSL_SESSION(session, &p); + if(i2d_SSL_SESSION(session, &p) < 0) { + BIO_printf(bio_err, "Error encoding session\n"); + return 0; + } sess->next = first; first = sess; diff --git a/apps/s_time.c b/apps/s_time.c index 96e39aa90d..5b94634a53 100644 --- a/apps/s_time.c +++ b/apps/s_time.c @@ -356,7 +356,8 @@ int MAIN(int argc, char **argv) if (st_bugs) SSL_CTX_set_options(tm_ctx, SSL_OP_ALL); - SSL_CTX_set_cipher_list(tm_ctx, tm_cipher); + if(!SSL_CTX_set_cipher_list(tm_ctx, tm_cipher)) + goto end; if (!set_cert_stuff(tm_ctx, t_cert_file, t_key_file)) goto end; @@ -405,7 +406,8 @@ int MAIN(int argc, char **argv) if (s_www_path != NULL) { BIO_snprintf(buf, sizeof buf, "GET %s HTTP/1.0\r\n\r\n", s_www_path); - SSL_write(scon, buf, strlen(buf)); + if(SSL_write(scon, buf, strlen(buf)) <= 0) + goto end; while ((i = SSL_read(scon, buf, sizeof(buf))) > 0) bytes_read += i; } @@ -461,7 +463,8 @@ int MAIN(int argc, char **argv) if (s_www_path != NULL) { BIO_snprintf(buf, sizeof buf, "GET %s HTTP/1.0\r\n\r\n", s_www_path); - SSL_write(scon, buf, strlen(buf)); + if(SSL_write(scon, buf, strlen(buf)) <= 0) + goto end; while (SSL_read(scon, buf, sizeof(buf)) > 0) ; } #ifdef NO_SHUTDOWN @@ -498,7 +501,8 @@ int MAIN(int argc, char **argv) if (s_www_path) { BIO_snprintf(buf, sizeof buf, "GET %s HTTP/1.0\r\n\r\n", s_www_path); - SSL_write(scon, buf, strlen(buf)); + if(SSL_write(scon, buf, strlen(buf)) <= 0) + goto end; while ((i = SSL_read(scon, buf, sizeof(buf))) > 0) bytes_read += i; } diff --git a/apps/sess_id.c b/apps/sess_id.c index fcb09118a6..9400af964e 100644 --- a/apps/sess_id.c +++ b/apps/sess_id.c @@ -166,7 +166,10 @@ int MAIN(int argc, char **argv) BIO_printf(bio_err, "Context too long\n"); goto end; } - SSL_SESSION_set1_id_context(x, (unsigned char *)context, ctx_len); + if(!SSL_SESSION_set1_id_context(x, (unsigned char *)context, ctx_len)) { + BIO_printf(bio_err, "Error setting id context\n"); + goto end; + } } if (!noout || text) { -- 2.40.0