diff options
-rw-r--r-- | gtests/ssl_gtest/ssl_loopback_unittest.cc | 79 | ||||
-rw-r--r-- | gtests/ssl_gtest/test_io.cc | 28 | ||||
-rw-r--r-- | gtests/ssl_gtest/tls_agent.cc | 49 | ||||
-rw-r--r-- | lib/ssl/sslsecur.c | 23 |
4 files changed, 138 insertions, 41 deletions
diff --git a/gtests/ssl_gtest/ssl_loopback_unittest.cc b/gtests/ssl_gtest/ssl_loopback_unittest.cc index b4a74b99e..68b104908 100644 --- a/gtests/ssl_gtest/ssl_loopback_unittest.cc +++ b/gtests/ssl_gtest/ssl_loopback_unittest.cc @@ -491,6 +491,85 @@ TEST_F(TlsConnectTest, OneNRecordSplitting) { EXPECT_EQ(ExpectedCbcLen(20), records->record(2).buffer.len()); } +void FailOnCloseNotify(const PRFileDesc* fd, void* arg, const SSLAlert* alert) { + ADD_FAILURE() << "received alert " << alert->description; +} + +void CheckCloseNotify(const PRFileDesc* fd, void* arg, const SSLAlert* alert) { + *reinterpret_cast<bool*>(arg) = true; + EXPECT_EQ(close_notify, alert->description); + EXPECT_EQ(alert_warning, alert->level); +} + +TEST_P(TlsConnectGeneric, ShutdownOneSide) { + Connect(); + + // Setup to check alerts. + EXPECT_EQ(SECSuccess, SSL_AlertSentCallback(server_->ssl_fd(), + FailOnCloseNotify, nullptr)); + EXPECT_EQ(SECSuccess, SSL_AlertReceivedCallback(client_->ssl_fd(), + FailOnCloseNotify, nullptr)); + + bool client_sent = false; + EXPECT_EQ(SECSuccess, SSL_AlertSentCallback(client_->ssl_fd(), + CheckCloseNotify, &client_sent)); + bool server_received = false; + EXPECT_EQ(SECSuccess, + SSL_AlertReceivedCallback(server_->ssl_fd(), CheckCloseNotify, + &server_received)); + EXPECT_EQ(PR_SUCCESS, PR_Shutdown(client_->ssl_fd(), PR_SHUTDOWN_SEND)); + + // Make sure that the server reads out the close_notify. + uint8_t buf[10]; + EXPECT_EQ(0, PR_Read(server_->ssl_fd(), buf, sizeof(buf))); + + // Reading and writing should still work in the one open direction. + EXPECT_TRUE(client_sent); + EXPECT_TRUE(server_received); + server_->SendData(10, 10); + client_->ReadBytes(10); + + // Now close the other side and do the same checks. + bool server_sent = false; + EXPECT_EQ(SECSuccess, SSL_AlertSentCallback(server_->ssl_fd(), + CheckCloseNotify, &server_sent)); + bool client_received = false; + EXPECT_EQ(SECSuccess, + SSL_AlertReceivedCallback(client_->ssl_fd(), CheckCloseNotify, + &client_received)); + EXPECT_EQ(PR_SUCCESS, PR_Shutdown(server_->ssl_fd(), PR_SHUTDOWN_SEND)); + + EXPECT_EQ(0, PR_Read(client_->ssl_fd(), buf, sizeof(buf))); + EXPECT_TRUE(server_sent); + EXPECT_TRUE(client_received); +} + +TEST_P(TlsConnectGeneric, ShutdownOneSideThenCloseTcp) { + Connect(); + + bool client_sent = false; + EXPECT_EQ(SECSuccess, SSL_AlertSentCallback(client_->ssl_fd(), + CheckCloseNotify, &client_sent)); + bool server_received = false; + EXPECT_EQ(SECSuccess, + SSL_AlertReceivedCallback(server_->ssl_fd(), CheckCloseNotify, + &server_received)); + EXPECT_EQ(PR_SUCCESS, PR_Shutdown(client_->ssl_fd(), PR_SHUTDOWN_SEND)); + + // Make sure that the server reads out the close_notify. + uint8_t buf[10]; + EXPECT_EQ(0, PR_Read(server_->ssl_fd(), buf, sizeof(buf))); + + // Now simulate the underlying connection closing. + client_->adapter()->Reset(); + + // Now close the other side and see that things don't explode. + EXPECT_EQ(PR_SUCCESS, PR_Shutdown(server_->ssl_fd(), PR_SHUTDOWN_SEND)); + + EXPECT_GT(0, PR_Read(client_->ssl_fd(), buf, sizeof(buf))); + EXPECT_EQ(PR_NOT_CONNECTED_ERROR, PR_GetError()); +} + INSTANTIATE_TEST_CASE_P( GenericStream, TlsConnectGeneric, ::testing::Combine(TlsConnectTestBase::kTlsVariantsStream, diff --git a/gtests/ssl_gtest/test_io.cc b/gtests/ssl_gtest/test_io.cc index d76b3526c..6d792c520 100644 --- a/gtests/ssl_gtest/test_io.cc +++ b/gtests/ssl_gtest/test_io.cc @@ -31,6 +31,20 @@ ScopedPRFileDesc DummyPrSocket::CreateFD() { return DummyIOLayerMethods::CreateFD(test_fd_identity, this); } +void DummyPrSocket::Reset() { + auto p = peer_.lock(); + peer_.reset(); + if (p) { + p->peer_.reset(); + p->Reset(); + } + while (!input_.empty()) { + input_.pop(); + } + filter_ = nullptr; + write_error_ = 0; +} + void DummyPrSocket::PacketReceived(const DataBuffer &packet) { input_.push(Packet(packet)); } @@ -42,6 +56,12 @@ int32_t DummyPrSocket::Read(PRFileDesc *f, void *data, int32_t len) { return -1; } + auto dst = peer_.lock(); + if (!dst) { + PR_SetError(PR_NOT_CONNECTED_ERROR, 0); + return -1; + } + if (input_.empty()) { LOGV("Read --> wouldblock " << len); PR_SetError(PR_WOULD_BLOCK_ERROR, 0); @@ -74,6 +94,12 @@ int32_t DummyPrSocket::Recv(PRFileDesc *f, void *buf, int32_t buflen, return Read(f, buf, buflen); } + auto dst = peer_.lock(); + if (!dst) { + PR_SetError(PR_NOT_CONNECTED_ERROR, 0); + return -1; + } + if (input_.empty()) { PR_SetError(PR_WOULD_BLOCK_ERROR, 0); return -1; @@ -101,7 +127,7 @@ int32_t DummyPrSocket::Write(PRFileDesc *f, const void *buf, int32_t length) { auto dst = peer_.lock(); if (!dst) { - PR_SetError(PR_IO_ERROR, 0); + PR_SetError(PR_NOT_CONNECTED_ERROR, 0); return -1; } diff --git a/gtests/ssl_gtest/tls_agent.cc b/gtests/ssl_gtest/tls_agent.cc index 3ee0b68f4..a27da6c5a 100644 --- a/gtests/ssl_gtest/tls_agent.cc +++ b/gtests/ssl_gtest/tls_agent.cc @@ -921,8 +921,8 @@ void TlsAgent::SendRecordDirect(const TlsRecord& record) { SendDirect(buf); } -static bool ErrorIsNonFatal(PRErrorCode code) { - return code == PR_WOULD_BLOCK_ERROR || code == SSL_ERROR_RX_SHORT_DTLS_READ; +static bool ErrorIsFatal(PRErrorCode code) { + return code != PR_WOULD_BLOCK_ERROR && code != SSL_ERROR_RX_SHORT_DTLS_READ; } void TlsAgent::SendData(size_t bytes, size_t blocksize) { @@ -981,28 +981,39 @@ bool TlsAgent::SendEncryptedRecord(const std::shared_ptr<TlsCipherSpec>& spec, void TlsAgent::ReadBytes(size_t amount) { uint8_t block[16384]; - int32_t rv = PR_Read(ssl_fd(), block, (std::min)(amount, sizeof(block))); - LOGV("ReadBytes " << rv); - int32_t err; + size_t remaining = amount; + while (remaining > 0) { + int32_t rv = PR_Read(ssl_fd(), block, (std::min)(amount, sizeof(block))); + LOGV("ReadBytes " << rv); - if (rv >= 0) { - size_t count = static_cast<size_t>(rv); - for (size_t i = 0; i < count; ++i) { - ASSERT_EQ(recv_ctr_ & 0xff, block[i]); - recv_ctr_++; - } - } else { - err = PR_GetError(); - LOG("Read error " << PORT_ErrorToName(err) << ": " - << PORT_ErrorToString(err)); - if (err != PR_WOULD_BLOCK_ERROR && expect_readwrite_error_) { - error_code_ = err; - expect_readwrite_error_ = false; + if (rv > 0) { + size_t count = static_cast<size_t>(rv); + for (size_t i = 0; i < count; ++i) { + ASSERT_EQ(recv_ctr_ & 0xff, block[i]); + recv_ctr_++; + } + remaining -= rv; + } else { + PRErrorCode err = 0; + if (rv < 0) { + err = PR_GetError(); + LOG("Read error " << PORT_ErrorToName(err) << ": " + << PORT_ErrorToString(err)); + if (err != PR_WOULD_BLOCK_ERROR && expect_readwrite_error_) { + error_code_ = err; + expect_readwrite_error_ = false; + } + } + if (err != 0 && ErrorIsFatal(err)) { + // If we hit a fatal error, we're done. + remaining = 0; + } + break; } } // If closed, then don't bother waiting around. - if (rv > 0 || (rv < 0 && ErrorIsNonFatal(err))) { + if (remaining) { LOGV("Re-arming"); Poller::Instance()->Wait(READABLE_EVENT, adapter_, this, &TlsAgent::ReadableCallback); diff --git a/lib/ssl/sslsecur.c b/lib/ssl/sslsecur.c index d3424a7ad..b98969b95 100644 --- a/lib/ssl/sslsecur.c +++ b/lib/ssl/sslsecur.c @@ -685,23 +685,6 @@ ssl_SecureConnect(sslSocket *ss, const PRNetAddr *sa) } /* - * The TLS 1.2 RFC 5246, Section 7.2.1 says: - * - * Unless some other fatal alert has been transmitted, each party is - * required to send a close_notify alert before closing the write side - * of the connection. The other party MUST respond with a close_notify - * alert of its own and close down the connection immediately, - * discarding any pending writes. It is not required for the initiator - * of the close to wait for the responding close_notify alert before - * closing the read side of the connection. - * - * The second sentence requires that we send a close_notify alert when we - * have received a close_notify alert. In practice, all SSL implementations - * close the socket immediately after sending a close_notify alert (which is - * allowed by the third sentence), so responding with a close_notify alert - * would result in a write failure with the ECONNRESET error. This is why - * we don't respond with a close_notify alert. - * * Also, in the unlikely event that the TCP pipe is full and the peer stops * reading, the SSL3_SendAlert call in ssl_SecureClose and ssl_SecureShutdown * may block indefinitely in blocking mode, and may fail (without retrying) @@ -714,8 +697,7 @@ ssl_SecureClose(sslSocket *ss) int rv; if (!(ss->shutdownHow & ssl_SHUTDOWN_SEND) && - ss->firstHsDone && - !ss->recvdCloseNotify) { + ss->firstHsDone) { /* We don't want the final alert to be Nagle delayed. */ if (!ss->delayDisabled) { @@ -744,8 +726,7 @@ ssl_SecureShutdown(sslSocket *ss, int nsprHow) if ((sslHow & ssl_SHUTDOWN_SEND) != 0 && !(ss->shutdownHow & ssl_SHUTDOWN_SEND) && - ss->firstHsDone && - !ss->recvdCloseNotify) { + ss->firstHsDone) { (void)SSL3_SendAlert(ss, alert_warning, close_notify); } |