summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--gtests/ssl_gtest/ssl_loopback_unittest.cc79
-rw-r--r--gtests/ssl_gtest/test_io.cc28
-rw-r--r--gtests/ssl_gtest/tls_agent.cc49
-rw-r--r--lib/ssl/sslsecur.c23
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);
}