Bug 1382886 - Use HandshakeDone to signal that a handshake is done r=necko-reviewers,keeler,kershaw

HandshakeDone will be called after a handshake is finished and also after the certificate verifications are done.
The code relies on HandshakeDone to signal that the handshake is done. When early-data is not available HandshakeDone is responsible for setting up a Http2 session if needed. There are 2 outcomes when early-data is used:
1) early-data is accepted and transaction continues polling for read,
2) early-data is rejected. In this case, the transaction is restarted as well as polling flags, i.e. the connection will stop polling for read and start polling for write.
Another difference is that a transaction that is started during the early-data period will behave as a normal transaction, i.e. it will write data and continue polling for read to receive response. The special cases during early-data(mWaitingFor0RTTResponse==true) are removed from nsHttpConnection::OnSocketWritable().

EnsureNPNComplete is only responsible for driving handshake and checking the early-data availability. All logic for finishing a handshake (i.e. checking whether early-data is accepted and checking alpn value) has been moved to HandshakeDone.
The patch also extracts FinishNPNSetup that is responsible for the bookkeeping after a handshake is done or fails, e.g. resetting transactions if 0Rtt is used but handshake fails, updating timings and sending telemetry.

HandshakeDone needs to be dispatched so that it is not called inside nss locks. The side effect of this is that nsHttpConnection::OnSocketWritable() may be called in between  HandshakeDone being dispatched and executed. Therefore we still need to keep CheckCanWrite0RTTData(). This can be fixed in a follow up patch.

Side cleanups:
Remove mNotTrustedMitmDetected - his was used for ESNI, but it is not used anymore

Differential Revision: https://phabricator.services.mozilla.com/D123824
This commit is contained in:
Dragana Damjanovic 2021-09-22 09:19:31 +00:00
parent 18800f98eb
commit 985c02c2f1
5 changed files with 174 additions and 148 deletions

View File

@ -1605,7 +1605,6 @@ nsSocketTransportService::Observe(nsISupports* subject, const char* topic,
ShutdownThread();
} else if (!strcmp(topic, NS_NETWORK_LINK_TOPIC)) {
mLastNetworkLinkChangeTime = PR_IntervalNow();
mNotTrustedMitmDetected = false;
}
return NS_OK;

View File

@ -126,8 +126,6 @@ class nsSocketTransportService final : public nsPISocketTransportService,
bool IsTelemetryEnabledAndNotSleepPhase();
PRIntervalTime MaxTimeForPrClosePref() { return mMaxTimeForPrClosePref; }
void SetNotTrustedMitmDetected() { mNotTrustedMitmDetected = true; }
// According the preference value of `network.socket.forcePort` this method
// possibly remaps the port number passed as the arg.
void ApplyPortRemap(uint16_t* aPort);
@ -344,8 +342,6 @@ class nsSocketTransportService final : public nsPISocketTransportService,
void TryRepairPollableEvent();
bool mNotTrustedMitmDetected{false};
CopyableTArray<nsCOMPtr<nsISTSShutdownObserver>> mShutdownObservers;
};

View File

@ -436,10 +436,8 @@ void nsHttpConnection::EarlyDataTelemetry(int16_t tlsVersion,
}
}
// Checks if TLS handshake is needed and it is responsible to move it forward.
bool nsHttpConnection::EnsureNPNComplete() {
// If for some reason the components to check on NPN aren't available,
// this function will just return true to continue on and disable SPDY
MOZ_ASSERT(mSocketTransport);
if (!mSocketTransport) {
// this cannot happen
@ -453,22 +451,16 @@ bool nsHttpConnection::EnsureNPNComplete() {
nsresult rv = NS_OK;
nsCOMPtr<nsISupports> securityInfo;
nsCOMPtr<nsITransportSecurityInfo> info;
nsCOMPtr<nsISSLSocketControl> ssl;
nsAutoCString negotiatedNPN;
// This is needed for telemetry
bool handshakeSucceeded = false;
GetSecurityInfo(getter_AddRefs(securityInfo));
if (!securityInfo) {
goto npnComplete;
FinishNPNSetup(false, false);
return true;
}
ssl = do_QueryInterface(securityInfo, &rv);
if (NS_FAILED(rv)) goto npnComplete;
info = do_QueryInterface(securityInfo, &rv);
if (NS_FAILED(rv)) goto npnComplete;
nsCOMPtr<nsISSLSocketControl> ssl = do_QueryInterface(securityInfo, &rv);
if (NS_FAILED(rv)) {
FinishNPNSetup(false, false);
}
if (!m0RTTChecked) {
// We reuse m0RTTChecked. We want to send this status only once.
@ -476,92 +468,18 @@ bool nsHttpConnection::EnsureNPNComplete() {
NS_NET_STATUS_TLS_HANDSHAKE_STARTING, 0);
}
rv = info->GetNegotiatedNPN(negotiatedNPN);
if (rv == NS_ERROR_NOT_CONNECTED) {
rv = ssl->DriveHandshake();
if (NS_FAILED(rv) && rv != NS_BASE_STREAM_WOULD_BLOCK) {
goto npnComplete;
}
// Check again, maybe the handshake is done now.
rv = info->GetNegotiatedNPN(negotiatedNPN);
}
if (rv == NS_ERROR_NOT_CONNECTED) {
Check0RttEnabled(ssl);
return mWaitingFor0RTTResponse;
}
if (NS_SUCCEEDED(rv)) {
LOG1(("nsHttpConnection::EnsureNPNComplete %p [%s] negotiated to '%s'%s\n",
this, mConnInfo->HashKey().get(), negotiatedNPN.get(),
mTLSFilter ? " [Double Tunnel]" : ""));
handshakeSucceeded = true;
int16_t tlsVersion;
ssl->GetSSLVersionUsed(&tlsVersion);
mConnInfo->SetLessThanTls13(
(tlsVersion < nsISSLSocketControl::TLS_VERSION_1_3) &&
(tlsVersion != nsISSLSocketControl::SSL_VERSION_UNKNOWN));
bool earlyDataAccepted = false;
if (mWaitingFor0RTTResponse) {
// Check if early data has been accepted.
nsresult rvEarlyData = ssl->GetEarlyDataAccepted(&earlyDataAccepted);
LOG(
("nsHttpConnection::EnsureNPNComplete [this=%p] - early data "
"that was sent during 0RTT %s been accepted [rv=%" PRIx32 "].",
this, earlyDataAccepted ? "has" : "has not",
static_cast<uint32_t>(rv)));
if (NS_FAILED(rvEarlyData) ||
NS_FAILED(mTransaction->Finish0RTT(
!earlyDataAccepted, negotiatedNPN != mEarlyNegotiatedALPN))) {
LOG(
("nsHttpConection::EnsureNPNComplete [this=%p] closing transaction "
"%p",
this, mTransaction.get()));
mTransaction->Close(NS_ERROR_NET_RESET);
goto npnComplete;
}
}
EarlyDataTelemetry(tlsVersion, earlyDataAccepted);
mWaitingFor0RTTResponse = false;
if (!earlyDataAccepted) {
LOG(
("nsHttpConnection::EnsureNPNComplete [this=%p] early data not "
"accepted",
this));
uint32_t infoIndex;
const SpdyInformation* info = gHttpHandler->SpdyInfo();
if (NS_SUCCEEDED(info->GetNPNIndex(negotiatedNPN, &infoIndex))) {
StartSpdy(ssl, info->Version[infoIndex]);
}
} else {
LOG(("nsHttpConnection::EnsureNPNComplete [this=%p] - %" PRId64 " bytes "
"has been sent during 0RTT.",
this, mContentBytesWritten0RTT));
mContentBytesWritten = mContentBytesWritten0RTT;
if (mSpdySession) {
// We had already started 0RTT-spdy, now we need to fully set up
// spdy, since we know we're sticking with it.
LOG(
("nsHttpConnection::EnsureNPNComplete [this=%p] - finishing "
"StartSpdy for 0rtt spdy session %p",
this, mSpdySession.get()));
StartSpdy(ssl, mSpdySession->SpdyVersion());
}
}
Telemetry::Accumulate(Telemetry::SPDY_NPN_CONNECT, UsingSpdy());
}
npnComplete:
LOG(("nsHttpConnection::EnsureNPNComplete [this=%p] setting complete to true",
LOG(("nsHttpConnection::EnsureNPNComplete [this=%p] drive TLS handshake",
this));
rv = ssl->DriveHandshake();
if (NS_FAILED(rv) && rv != NS_BASE_STREAM_WOULD_BLOCK) {
FinishNPNSetup(false, true);
}
Check0RttEnabled(ssl);
return false;
}
void nsHttpConnection::FinishNPNSetup(bool handshakeSucceeded, bool hasSecurityInfo) {
mNPNComplete = true;
mTransaction->OnTransportStatus(mSocketTransport,
@ -578,35 +496,23 @@ npnComplete:
mTransaction->QueryNullTransaction()->GetTcpConnectEnd();
}
if (securityInfo) {
if (hasSecurityInfo) {
mBootstrappedTimings.connectEnd = TimeStamp::Now();
}
if (mWaitingFor0RTTResponse) {
// Didn't get 0RTT OK, back out of the "attempting 0RTT" state
mWaitingFor0RTTResponse = false;
LOG(("nsHttpConnection::EnsureNPNComplete [this=%p] 0rtt failed", this));
LOG(("nsHttpConnection::FinishNPNSetup [this=%p] 0rtt failed", this));
if (NS_FAILED(mTransaction->Finish0RTT(
true, negotiatedNPN != mEarlyNegotiatedALPN))) {
true, true))) {
mTransaction->Close(NS_ERROR_NET_RESET);
}
mContentBytesWritten0RTT = 0;
Reset0RttForSpdy();
}
if (mDid0RTTSpdy && negotiatedNPN != mEarlyNegotiatedALPN) {
// Reset the work done by Start0RTTSpdy
LOG((
"nsHttpConnection::EnsureNPNComplete [this=%p] resetting Start0RTTSpdy",
this));
mUsingSpdyVersion = SpdyVersion::NONE;
mTransaction = nullptr;
mSpdySession = nullptr;
// We have to reset this here, just in case we end up starting spdy again,
// so it can actually do everything it needs to do.
mDid0RTTSpdy = false;
}
if (ssl) {
if (hasSecurityInfo) {
// Telemetry for tls failure rate with and without esni;
bool echConfigUsed = false;
mSocketTransport->GetEchConfigUsed(&echConfigUsed);
@ -618,12 +524,16 @@ npnComplete:
: TlsHandshakeResult::NoEchConfigFailed);
Telemetry::Accumulate(Telemetry::ECHCONFIG_SUCCESS_RATE, result);
}
}
if (rv == psm::GetXPCOMFromNSSError(
mozilla::pkix::MOZILLA_PKIX_ERROR_MITM_DETECTED)) {
gSocketTransportService->SetNotTrustedMitmDetected();
}
return true;
void nsHttpConnection::Reset0RttForSpdy() {
// Reset the work done by Start0RTTSpdy
mUsingSpdyVersion = SpdyVersion::NONE;
mTransaction = nullptr;
mSpdySession = nullptr;
// We have to reset this here, just in case we end up starting spdy again,
// so it can actually do everything it needs to do.
mDid0RTTSpdy = false;
}
nsresult nsHttpConnection::OnTunnelNudged(TLSFilterTransaction* trans) {
@ -916,6 +826,16 @@ void nsHttpConnection::Close(nsresult reason, bool aIsShutdown) {
}
}
nsCOMPtr<nsISupports> securityInfo;
GetSecurityInfo(getter_AddRefs(securityInfo));
if (securityInfo) {
nsresult rv;
nsCOMPtr<nsISSLSocketControl> ssl = do_QueryInterface(securityInfo, &rv);
if (NS_SUCCEEDED(rv)) {
ssl->SetHandshakeCallbackListener(nullptr);
}
}
if (NS_FAILED(reason)) {
if (mIdleMonitoring) EndIdleMonitoring();
@ -988,7 +908,8 @@ nsresult nsHttpConnection::InitSSLParams(bool connectingToProxy,
}
}
if (NS_SUCCEEDED(SetupNPNList(ssl, mTransactionCaps))) {
if (NS_SUCCEEDED(SetupNPNList(ssl, mTransactionCaps)) &&
NS_SUCCEEDED(ssl->SetHandshakeCallbackListener(this))) {
LOG(("InitSSLParams Setting up SPDY Negotiation OK"));
mNPNComplete = false;
}
@ -1850,9 +1771,9 @@ nsresult nsHttpConnection::OnSocketWritable() {
// transaction->readsegments() processing can proceed because we need to
// know how to format the request differently for http/1, http/2, spdy,
// etc.. and that is negotiated with NPN/ALPN in the SSL handshake.
if (mConnInfo->UsingHttpsProxy() &&
!EnsureNPNComplete()) {
MOZ_DIAGNOSTIC_ASSERT(!mWaitingFor0RTTResponse);
mSocketOutCondition = NS_BASE_STREAM_WOULD_BLOCK;
} else if (mProxyConnectStream) {
// If we're need an HTTP/1 CONNECT tunnel through a proxy
@ -1861,7 +1782,7 @@ nsresult nsHttpConnection::OnSocketWritable() {
rv = mProxyConnectStream->ReadSegments(ReadFromStream, this,
nsIOService::gDefaultSegmentSize,
&transactionBytes);
} else if (!EnsureNPNComplete()) {
} else if (!EnsureNPNComplete() && !mWaitingFor0RTTResponse) {
mSocketOutCondition = NS_BASE_STREAM_WOULD_BLOCK;
} else if (!mTransaction) {
rv = NS_ERROR_FAILURE;
@ -1881,6 +1802,11 @@ nsresult nsHttpConnection::OnSocketWritable() {
this, nsIOService::gDefaultSegmentSize, &transactionBytes, &again);
if (mWaitingFor0RTTResponse) {
mContentBytesWritten0RTT += transactionBytes;
if (NS_FAILED(rv) && rv != NS_BASE_STREAM_WOULD_BLOCK) {
// If an error happens while writting 0RTT data, restart
// the transactiions without 0RTT.
FinishNPNSetup(false, true);
}
} else {
mContentBytesWritten += transactionBytes;
}
@ -1899,19 +1825,11 @@ nsresult nsHttpConnection::OnSocketWritable() {
transactionBytes = 0;
}
if (!again && mWaitingFor0RTTResponse) {
// Continue waiting;
rv = mSocketOut->AsyncWait(this, 0, 0, nullptr);
}
if (NS_FAILED(rv)) {
// if the transaction didn't want to write any more data, then
// wait for the transaction to call ResumeSend.
if (rv == NS_BASE_STREAM_WOULD_BLOCK) {
rv = NS_OK;
if (mWaitingFor0RTTResponse) {
// Continue waiting;
rv = mSocketOut->AsyncWait(this, 0, 0, nullptr);
}
}
again = false;
} else if (NS_FAILED(mSocketOutCondition)) {
@ -1929,11 +1847,8 @@ nsresult nsHttpConnection::OnSocketWritable() {
} else if (!transactionBytes) {
rv = NS_OK;
if (mWaitingFor0RTTResponse) {
// Wait for tls handshake to finish or waiting for connect.
rv = mSocketOut->AsyncWait(this, 0, 0, nullptr);
} else if (mTransaction) { // in case the ReadSegments stack called
// CloseTransaction()
if (mTransaction) { // in case the ReadSegments stack called
// CloseTransaction()
//
// at this point we've written out the entire transaction, and now we
// must wait for the server's response. we manufacture a status message
@ -2020,7 +1935,7 @@ nsresult nsHttpConnection::OnSocketReadable() {
bool again = true;
do {
if (!mProxyConnectInProgress && !mNPNComplete) {
if (!mProxyConnectInProgress && !EnsureNPNComplete()) {
// Unless we are setting up a tunnel via CONNECT, prevent reading
// from the socket until the results of NPN
// negotiation are known (which is determined from the write path).
@ -2032,7 +1947,7 @@ nsresult nsHttpConnection::OnSocketReadable() {
("nsHttpConnection::OnSocketReadable %p return due to inactive "
"tunnel setup but incomplete NPN state\n",
this));
rv = NS_OK;
rv = ResumeRecv();
break;
}
@ -2582,6 +2497,112 @@ bool nsHttpConnection::GetEchConfigUsed() {
NS_IMETHODIMP
nsHttpConnection::HandshakeDone() {
if (mNPNComplete) {
return NS_OK;
}
nsresult rv = NS_OK;
nsCOMPtr<nsISupports> securityInfo;
nsCOMPtr<nsITransportSecurityInfo> info;
nsCOMPtr<nsISSLSocketControl> ssl;
nsAutoCString negotiatedNPN;
GetSecurityInfo(getter_AddRefs(securityInfo));
if (!securityInfo) {
FinishNPNSetup(false, false);
return NS_OK;
}
ssl = do_QueryInterface(securityInfo, &rv);
if (NS_FAILED(rv)) {
FinishNPNSetup(false, false);
return NS_OK;
}
info = do_QueryInterface(securityInfo, &rv);
if (NS_FAILED(rv)) {
FinishNPNSetup(false, false);
return NS_OK;
}
DebugOnly<nsresult> rvDebug = info->GetNegotiatedNPN(negotiatedNPN);
MOZ_ASSERT(NS_SUCCEEDED(rvDebug));
bool earlyDataAccepted = false;
if (mWaitingFor0RTTResponse) {
// Check if early data has been accepted.
nsresult rvEarlyData = ssl->GetEarlyDataAccepted(&earlyDataAccepted);
LOG(
("nsHttpConnection::HandshakeDone [this=%p] - early data "
"that was sent during 0RTT %s been accepted [rv=%" PRIx32 "].",
this, earlyDataAccepted ? "has" : "has not",
static_cast<uint32_t>(rv)));
if (NS_FAILED(rvEarlyData) ||
NS_FAILED(mTransaction->Finish0RTT(
!earlyDataAccepted, negotiatedNPN != mEarlyNegotiatedALPN))) {
LOG(
("nsHttpConection::HandshakeDone [this=%p] closing transaction "
"%p",
this, mTransaction.get()));
mTransaction->Close(NS_ERROR_NET_RESET);
FinishNPNSetup(false, true);
return NS_OK;
}
if (mDid0RTTSpdy && (negotiatedNPN != mEarlyNegotiatedALPN)) {
Reset0RttForSpdy();
}
if (!earlyDataAccepted) {
// We need to start from the begining here and start writing
// the request again.
if (mSocketIn) {
mSocketIn->AsyncWait(nullptr, 0, 0, nullptr);
}
Unused << ResumeSend();
}
}
int16_t tlsVersion;
ssl->GetSSLVersionUsed(&tlsVersion);
mConnInfo->SetLessThanTls13(
(tlsVersion < nsISSLSocketControl::TLS_VERSION_1_3) &&
(tlsVersion != nsISSLSocketControl::SSL_VERSION_UNKNOWN));
EarlyDataTelemetry(tlsVersion, earlyDataAccepted);
if (!earlyDataAccepted) {
LOG(
("nsHttpConnection::HandshakeDone [this=%p] early data not "
"accepted or early data were not used",
this));
uint32_t infoIndex;
const SpdyInformation* info = gHttpHandler->SpdyInfo();
if (NS_SUCCEEDED(info->GetNPNIndex(negotiatedNPN, &infoIndex))) {
StartSpdy(ssl, info->Version[infoIndex]);
}
} else {
LOG(("nsHttpConnection::HandshakeDone [this=%p] - %" PRId64 " bytes "
"has been sent during 0RTT.",
this, mContentBytesWritten0RTT));
mContentBytesWritten = mContentBytesWritten0RTT;
if (mSpdySession) {
// We had already started 0RTT-spdy, now we need to fully set up
// spdy, since we know we're sticking with it.
LOG(
("nsHttpConnection::HandshakeDone [this=%p] - finishing "
"StartSpdy for 0rtt spdy session %p",
this, mSpdySession.get()));
StartSpdy(ssl, mSpdySession->SpdyVersion());
}
}
Telemetry::Accumulate(Telemetry::SPDY_NPN_CONNECT, UsingSpdy());
mWaitingFor0RTTResponse = false;
FinishNPNSetup(true, true);
return NS_OK;
}

View File

@ -247,6 +247,8 @@ class nsHttpConnection final : public HttpConnectionBase,
bool CheckCanWrite0RTTData();
void Check0RttEnabled(nsISSLSocketControl* ssl);
void EarlyDataTelemetry(int16_t tlsVersion, bool earlyDataAccepted);
void FinishNPNSetup(bool handshakeSucceeded, bool hasSecurityInfo);
void Reset0RttForSpdy();
private:
// mTransaction only points to the HTTP Transaction callbacks if the

View File

@ -251,9 +251,17 @@ void nsNSSSocketInfo::SetHandshakeCompleted() {
mIsFullHandshake = false; // reset for next handshake on this connection
if (mTlsHandshakeCallback) {
Unused << mTlsHandshakeCallback->HandshakeDone();
}
// HandshakeDone needs to be dispatched so that it is not called inside
// nss locks.
RefPtr<nsNSSSocketInfo> self(this);
NS_DispatchToCurrentThread(NS_NewRunnableFunction(
"nsNSSSocketInfo::HandshakeDone",
[self{std::move(self)}]() {
if (self->mTlsHandshakeCallback) {
auto callback = std::move(self->mTlsHandshakeCallback);
Unused << callback->HandshakeDone();
}
}));
}
void nsNSSSocketInfo::SetNegotiatedNPN(const char* value, uint32_t length) {