commit b082fbe79d885dfc00795c6c1fcfffdf47e197f1
author Brad Hess <bdhess@users.noreply.github.com> Thu Sep 02 18:55:16 2021 -0400
committer GitHub <noreply@github.com> Thu Sep 02 15:55:16 2021 -0700
tree 6f1a25859405718d02e183774b061315128aa7b7
parent 999e36fb31ca13ef467f3b7327a282a1bc4353e1

Clear SSL error stacks before performing SSL I/O. (#26834)

* Clear SSL error stacks before performing SSL I/O.

src/core/tsi/ssl_transport_security.cc

diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc
index 4241b6f..938421e 100644
--- a/src/core/tsi/ssl_transport_security.cc
+++ b/src/core/tsi/ssl_transport_security.cc
@@ -503,10 +503,10 @@
 /* Performs an SSL_read and handle errors. */
 static tsi_result do_ssl_read(SSL* ssl, unsigned char* unprotected_bytes,
                               size_t* unprotected_bytes_size) {
-  int read_from_ssl;
   GPR_ASSERT(*unprotected_bytes_size <= INT_MAX);
-  read_from_ssl = SSL_read(ssl, unprotected_bytes,
-                           static_cast<int>(*unprotected_bytes_size));
+  ERR_clear_error();
+  int read_from_ssl = SSL_read(ssl, unprotected_bytes,
+                               static_cast<int>(*unprotected_bytes_size));
   if (read_from_ssl <= 0) {
     read_from_ssl = SSL_get_error(ssl, read_from_ssl);
     switch (read_from_ssl) {
@@ -536,10 +536,10 @@
 /* Performs an SSL_write and handle errors. */
 static tsi_result do_ssl_write(SSL* ssl, unsigned char* unprotected_bytes,
                                size_t unprotected_bytes_size) {
-  int ssl_write_result;
   GPR_ASSERT(unprotected_bytes_size <= INT_MAX);
-  ssl_write_result = SSL_write(ssl, unprotected_bytes,
-                               static_cast<int>(unprotected_bytes_size));
+  ERR_clear_error();
+  int ssl_write_result = SSL_write(ssl, unprotected_bytes,
+                                   static_cast<int>(unprotected_bytes_size));
   if (ssl_write_result < 0) {
     ssl_write_result = SSL_get_error(ssl, ssl_write_result);
     if (ssl_write_result == SSL_ERROR_WANT_READ) {
@@ -1449,6 +1449,7 @@
     impl->result = TSI_OK;
     return impl->result;
   } else {
+    ERR_clear_error();
     /* Get ready to get some bytes from SSL. */
     int ssl_result = SSL_do_handshake(impl->ssl);
     ssl_result = SSL_get_error(impl->ssl, ssl_result);
@@ -1645,6 +1646,7 @@
       tsi_ssl_handshaker_resume_session(ssl,
                                         client_factory->session_cache.get());
     }
+    ERR_clear_error();
     ssl_result = SSL_do_handshake(ssl);
     ssl_result = SSL_get_error(ssl, ssl_result);
     if (ssl_result != SSL_ERROR_WANT_READ) {

test/core/tsi/ssl_transport_security_test.cc

diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc
index 87f8dfb..53bd59e 100644
--- a/test/core/tsi/ssl_transport_security_test.cc
+++ b/test/core/tsi/ssl_transport_security_test.cc
@@ -35,6 +35,7 @@
 
 extern "C" {
 #include <openssl/crypto.h>
+#include <openssl/err.h>
 #include <openssl/pem.h>
 }
 
@@ -705,6 +706,17 @@
   gpr_free(bit_array);
 }
 
+void ssl_tsi_test_do_round_trip_with_error_on_stack() {
+  gpr_log(GPR_INFO, "ssl_tsi_test_do_round_trip_with_error_on_stack");
+  // Invoke an SSL function that causes an error, and ensure the error
+  // makes it to the stack.
+  GPR_ASSERT(!EC_KEY_new_by_curve_name(NID_rsa));
+  GPR_ASSERT(ERR_peek_error() != 0);
+  tsi_test_fixture* fixture = ssl_tsi_test_fixture_create();
+  tsi_test_do_round_trip(fixture);
+  tsi_test_fixture_destroy(fixture);
+}
+
 static bool is_slow_build() {
 #if defined(GPR_ARCH_32) || defined(__APPLE__)
   return true;
@@ -1035,6 +1047,7 @@
     ssl_tsi_test_do_handshake_alpn_client_server_ok();
     ssl_tsi_test_do_handshake_session_cache();
     ssl_tsi_test_do_round_trip_for_all_configs();
+    ssl_tsi_test_do_round_trip_with_error_on_stack();
     ssl_tsi_test_do_round_trip_odd_buffer_size();
     ssl_tsi_test_handshaker_factory_internals();
     ssl_tsi_test_duplicate_root_certificates();