From 2916a5d0f5807506a822badddc5cd0f0a23be296 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Thu, 14 Nov 2024 20:40:24 -0800 Subject: [PATCH] addressing pr comments --- tool/client.cc | 70 ++++++++++++++++++++-------------------- tool/internal.h | 2 +- tool/transport_common.cc | 2 +- tool/transport_common.h | 4 +-- 4 files changed, 39 insertions(+), 39 deletions(-) diff --git a/tool/client.cc b/tool/client.cc index fe6f06a30a..e459aa83a5 100644 --- a/tool/client.cc +++ b/tool/client.cc @@ -256,21 +256,23 @@ static void print_verify_details(SSL *s) { } } -static void PrintOpenSSLConnectionInfo(SSL *ssl, int show_certs) { +static void PrintOpenSSLConnectionInfo(SSL *ssl, bool show_certs) { STACK_OF(X509) *sk; - X509 *peer; - int i; sk = SSL_get_peer_cert_chain(ssl); if (sk != NULL) { fprintf(stdout, "---\nCertificate chain\n"); - for (i = 0; i < (int) sk_X509_num(sk); i++) { - fprintf(stdout, "%2d s:", i); - X509_NAME_print_ex_fp(stdout, X509_get_subject_name(sk_X509_value(sk, i)), - 0, XN_FLAG_ONELINE); + for (size_t i = 0; i < sk_X509_num(sk); i++) { + fprintf(stdout, "%2zd s:", i); + if (X509_NAME_print_ex_fp(stdout, X509_get_subject_name(sk_X509_value(sk, i)), + 0, XN_FLAG_ONELINE) < 0) { + fprintf(stderr, "Error: Printing subject name failed"); + } fprintf(stdout, "\n i:"); - X509_NAME_print_ex_fp(stdout, X509_get_issuer_name(sk_X509_value(sk, i)), - 0, XN_FLAG_ONELINE); + if (X509_NAME_print_ex_fp(stdout, X509_get_issuer_name(sk_X509_value(sk, i)), + 0, XN_FLAG_ONELINE) < 0) { + fprintf(stderr, "Error: Printing issuer name failed"); + } fprintf(stdout, "\n"); if (show_certs) { PEM_write_X509(stdout, sk_X509_value(sk, i)); @@ -279,18 +281,20 @@ static void PrintOpenSSLConnectionInfo(SSL *ssl, int show_certs) { } fprintf(stdout, "---\n"); - peer = SSL_get_peer_certificate(ssl); - if (peer != NULL) { + bssl::UniquePtr peer(SSL_get_peer_certificate(ssl)); + if (peer != NULL && peer.get() != NULL) { fprintf(stdout, "Server certificate\n"); - PEM_write_X509(stdout, peer); + PEM_write_X509(stdout, peer.get()); fprintf(stdout, "subject="); - X509_NAME_print_ex_fp(stdout, - X509_get_subject_name(peer), - 0, XN_FLAG_ONELINE); + if (X509_NAME_print_ex_fp(stdout, X509_get_subject_name(peer.get()), + 0, XN_FLAG_ONELINE) < 0) { + fprintf(stderr, "Error: Printing subject name failed"); + } fprintf(stdout, "\n\nissuer="); - X509_NAME_print_ex_fp(stdout, - X509_get_issuer_name(peer), - 0, XN_FLAG_ONELINE); + if (X509_NAME_print_ex_fp(stdout, X509_get_issuer_name(peer.get()), + 0, XN_FLAG_ONELINE) < 0) { + fprintf(stderr, "Error: Printing issuer name failed"); + } fprintf(stdout, "\n\n---\n"); } else { fprintf(stdout, "no peer certificate available\n"); @@ -307,24 +311,23 @@ static void PrintOpenSSLConnectionInfo(SSL *ssl, int show_certs) { (int)BIO_number_read(SSL_get_rbio(ssl)), (int)BIO_number_written(SSL_get_wbio(ssl))); print_verify_details(ssl); - X509_free(peer); } static bool DoConnection(SSL_CTX *ctx, std::map args_map, - bool (*cb)(SSL *ssl, int sock), int tool) { + bool (*cb)(SSL *ssl, int sock), bool is_openssl_s_client) { int sock = -1; if (args_map.count("-http-tunnel") != 0) { - if (!Connect(&sock, args_map["-http-tunnel"], tool) || + if (!Connect(&sock, args_map["-http-tunnel"], is_openssl_s_client) || !DoHTTPTunnel(sock, args_map["-connect"])) { return false; } - } else if (!Connect(&sock, args_map["-connect"], tool)) { + } else if (!Connect(&sock, args_map["-connect"], is_openssl_s_client)) { return false; } // print for openssl tool - if (tool) { + if (is_openssl_s_client) { fprintf(stdout, "CONNECTED(%08d)\n", sock); } @@ -428,7 +431,7 @@ static bool DoConnection(SSL_CTX *ctx, } // print for bssl - if (!tool) { + if (!is_openssl_s_client) { fprintf(stderr, "Connected.\n"); bssl::UniquePtr bio_stderr(BIO_new_fp(stderr, BIO_NOCLOSE)); PrintConnectionInfo(bio_stderr.get(), ssl.get()); @@ -455,13 +458,10 @@ static void InfoCallback(const SSL *ssl, int type, int value) { static int verify_cb(int ok, X509_STORE_CTX *ctx) { - X509 *err_cert; - int err, depth; - BIO* bio_err = BIO_new_fp(stdout, BIO_CLOSE); - - err_cert = X509_STORE_CTX_get_current_cert(ctx); - err = X509_STORE_CTX_get_error(ctx); - depth = X509_STORE_CTX_get_error_depth(ctx); + X509 *err_cert = X509_STORE_CTX_get_current_cert(ctx); + int err = X509_STORE_CTX_get_error(ctx); + int depth = X509_STORE_CTX_get_error_depth(ctx); + BIO* bio_err = BIO_new_fp(stderr, BIO_CLOSE); BIO_printf(bio_err, "depth=%d ", depth); if (err_cert != NULL) { @@ -528,7 +528,7 @@ bool Client(const std::vector &args) { return DoClient(args_map, 0); } -bool DoClient(std::map args_map, int tool) { +bool DoClient(std::map args_map, bool is_openssl_s_client) { if (!InitSocketLibrary()) { return false; } @@ -731,7 +731,7 @@ bool DoClient(std::map args_map, int tool) { verify = SSL_VERIFY_PEER; } - if (tool) { // openssl tool + if (is_openssl_s_client) { // openssl tool SSL_CTX_set_verify(ctx.get(), verify, verify_cb); } else { SSL_CTX_set_verify(ctx.get(), verify, nullptr); @@ -752,10 +752,10 @@ bool DoClient(std::map args_map, int tool) { return false; } - if (!DoConnection(ctx.get(), args_map, &WaitForSession, tool)) { + if (!DoConnection(ctx.get(), args_map, &WaitForSession, is_openssl_s_client)) { return false; } } - return DoConnection(ctx.get(), args_map, &TransferData, tool); + return DoConnection(ctx.get(), args_map, &TransferData, is_openssl_s_client); } diff --git a/tool/internal.h b/tool/internal.h index 53ac0c37e9..229841b4c7 100644 --- a/tool/internal.h +++ b/tool/internal.h @@ -141,7 +141,7 @@ bool Client(const std::vector &args); // bssl and openssl tools. It takes an additional parameter |tool| to indicate // which tool's s_client is being invoked. 1 indicates openssl and 0 indicates // bssl. -bool DoClient(std::map args_map, int tool); +bool DoClient(std::map args_map, bool is_openssl_s_client); bool DoPKCS12(const std::vector &args); bool GenerateECH(const std::vector &args); bool GenerateEd25519Key(const std::vector &args); diff --git a/tool/transport_common.cc b/tool/transport_common.cc index 034570da39..80c8d91030 100644 --- a/tool/transport_common.cc +++ b/tool/transport_common.cc @@ -137,7 +137,7 @@ static void PrintSocketError(const char *function) { // Connect sets |*out_sock| to be a socket connected to the destination given // in |hostname_and_port|, which should be of the form "www.example.com:123". // It returns true on success and false otherwise. -bool Connect(int *out_sock, const std::string &hostname_and_port, int quiet) { +bool Connect(int *out_sock, const std::string &hostname_and_port, bool quiet) { std::string hostname, port; SplitHostPort(&hostname, &port, hostname_and_port); diff --git a/tool/transport_common.h b/tool/transport_common.h index 0c4b031099..c23e856e50 100644 --- a/tool/transport_common.h +++ b/tool/transport_common.h @@ -25,10 +25,10 @@ bool InitSocketLibrary(); // Connect sets |*out_sock| to be a socket connected to the destination given // in |hostname_and_port|, which should be of the form "www.example.com:123". -// |quiet| when set to 1 suppresses any output to stdout or stderr from this +// |quiet| when set to true suppresses any output to stdout or stderr from this // function. // It returns true on success and false otherwise. -bool Connect(int *out_sock, const std::string &hostname_and_port, int quiet); +bool Connect(int *out_sock, const std::string &hostname_and_port, bool quiet); class Listener { public: