Windows: Fix ssl_socket_test (#13264)

- SmallReadsIntoSameSlice test: use write method on connection instead
  of transport socket so SSL_write failures will be properly retried,
  also otherwise empty writes from connection will trigger failed
  assertions in buffer::linearize
- When SSL shutdown is not appropriate, perform basic socket shutdown;
  relevant when client certificate is invalid and connection should be
  dropped by server, however we still need to ensure TLS alerts are
  able to be read by client, shutdown on socket ensures this can happen
- Open files for test in text mode to take care of line ending issues

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>

source/common/filesystem/win32/filesystem_impl.cc

@@ -129,7 +129,7 @@ std::string InstanceImplWin32::fileReadToEnd(const std::string& path) {
 
   // On Windows, we need to explicitly set the file mode as binary. Otherwise,
   // 0x1a will be treated as EOF
-  std::ifstream file(path, std::ios_base::binary);
+  std::ifstream file(path, std::ios::binary);
   if (file.fail()) {
     auto last_error = ::GetLastError();
     if (last_error == ERROR_FILE_NOT_FOUND) {

source/extensions/transport_sockets/tls/ssl_socket.cc

@@ -291,6 +291,15 @@ void SslSocket::shutdownSsl() {
   }
 }
 
+void SslSocket::shutdownBasic() {
+  if (info_->state() != Ssl::SocketState::ShutdownSent &&
+      callbacks_->connection().state() != Network::Connection::State::Closed) {
+    callbacks_->ioHandle().shutdown(ENVOY_SHUT_WR);
+    drainErrorQueue();
+    info_->setState(Ssl::SocketState::ShutdownSent);
+  }
+}
+
 void SslSocket::closeSocket(Network::ConnectionEvent) {
   // Unregister the SSL connection object from private key method providers.
   for (auto const& provider : ctx_->getPrivateKeyMethodProviders()) {
@@ -303,6 +312,10 @@ void SslSocket::closeSocket(Network::ConnectionEvent) {
   if (info_->state() == Ssl::SocketState::HandshakeInProgress ||
       info_->state() == Ssl::SocketState::HandshakeComplete) {
     shutdownSsl();
+  } else {
+    // We're not in a state to do the full SSL shutdown so perform a basic shutdown to flush any
+    // outstanding alerts
+    shutdownBasic();
   }
 }
 

source/extensions/transport_sockets/tls/ssl_socket.h

@@ -85,6 +85,7 @@ class SslSocket : public Network::TransportSocket,
   Network::PostIoAction doHandshake();
   void drainErrorQueue();
   void shutdownSsl();
+  void shutdownBasic();
   bool isThreadSafe() const {
     return callbacks_ != nullptr && callbacks_->connection().dispatcher().isThreadSafe();
   }

test/extensions/transport_sockets/tls/BUILD

@@ -24,8 +24,6 @@ envoy_cc_test(
     ],
     external_deps = ["ssl"],
     shard_count = 4,
-    # TODO(wrowe): Diagnose timeout error on Windows (skipped for the moment)
-    tags = ["fails_on_windows"],
     deps = [
         ":test_private_key_method_provider_test_lib",
         "//include/envoy/network:transport_socket_interface",

test/extensions/transport_sockets/tls/ssl_socket_test.cc

@@ -398,17 +398,23 @@ void testUtil(const TestUtilOptions& options) {
                   server_connection->ssl()->subjectLocalCertificate());
       }
       if (!options.expectedPeerCert().empty()) {
-        std::string urlencoded = absl::StrReplaceAll(
-            options.expectedPeerCert(),
-            {{"\n", "%0A"}, {" ", "%20"}, {"+", "%2B"}, {"/", "%2F"}, {"=", "%3D"}});
+        std::string urlencoded =
+            absl::StrReplaceAll(options.expectedPeerCert(), {{TestEnvironment::newLine, "%0A"},
+                                                             {" ", "%20"},
+                                                             {"+", "%2B"},
+                                                             {"/", "%2F"},
+                                                             {"=", "%3D"}});
         // Assert twice to ensure a cached value is returned and still valid.
         EXPECT_EQ(urlencoded, server_connection->ssl()->urlEncodedPemEncodedPeerCertificate());
         EXPECT_EQ(urlencoded, server_connection->ssl()->urlEncodedPemEncodedPeerCertificate());
       }
       if (!options.expectedPeerCertChain().empty()) {
-        std::string cert_chain = absl::StrReplaceAll(
-            options.expectedPeerCertChain(),
-            {{"\n", "%0A"}, {" ", "%20"}, {"+", "%2B"}, {"/", "%2F"}, {"=", "%3D"}});
+        std::string cert_chain =
+            absl::StrReplaceAll(options.expectedPeerCertChain(), {{TestEnvironment::newLine, "%0A"},
+                                                                  {" ", "%20"},
+                                                                  {"+", "%2B"},
+                                                                  {"/", "%2F"},
+                                                                  {"=", "%3D"}});
         // Assert twice to ensure a cached value is returned and still valid.
         EXPECT_EQ(cert_chain, server_connection->ssl()->urlEncodedPemEncodedPeerCertificateChain());
         EXPECT_EQ(cert_chain, server_connection->ssl()->urlEncodedPemEncodedPeerCertificateChain());
@@ -4873,7 +4879,7 @@ TEST_P(SslReadBufferLimitTest, SmallReadsIntoSameSlice) {
 
   for (uint32_t i = 0; i < num_writes; i++) {
     Buffer::OwnedImpl data(std::string(write_size, 'a'));
-    client_transport_socket_->doWrite(data, false);
+    client_connection_->write(data, false);
   }
 
   dispatcher_->run(Event::Dispatcher::RunType::Block);