Bug 1686828 - Handle the case that we have an empty https rr before deciding whether to do https upgrade r=necko-reviewers,dragana

Differential Revision: https://phabricator.services.mozilla.com/D102144
This commit is contained in:
Kershaw Chang 2021-01-21 22:08:27 +00:00
Родитель 4362da24e9
Коммит a14601368c
3 изменённых файлов: 123 добавлений и 71 удалений

Просмотреть файл

@ -621,11 +621,13 @@ nsresult nsHttpChannel::MaybeUseHTTPSRRForUpgrade(bool aShouldUpgrade,
return ContinueOnBeforeConnect(aShouldUpgrade, aStatus);
}
if (mHTTPSSVCRecord) {
LOG(("nsHttpChannel::MaybeUseHTTPSRRForUpgrade [%p] already got HTTPS RR",
if (mHTTPSSVCRecord.isSome()) {
LOG((
"nsHttpChannel::MaybeUseHTTPSRRForUpgrade [%p] mHTTPSSVCRecord is some",
this));
StoreWaitHTTPSSVCRecord(false);
return ContinueOnBeforeConnect(true, aStatus);
bool hasHTTPSRR = (mHTTPSSVCRecord.ref() != nullptr);
return ContinueOnBeforeConnect(hasHTTPSRR, aStatus);
}
LOG(("nsHttpChannel::MaybeUseHTTPSRRForUpgrade [%p] wait for HTTPS RR",
@ -690,29 +692,6 @@ nsresult nsHttpChannel::ContinueOnBeforeConnect(bool aShouldUpgrade,
mConnectionInfo->SetIPv4Disabled(mCaps & NS_HTTP_DISABLE_IPV4);
mConnectionInfo->SetIPv6Disabled(mCaps & NS_HTTP_DISABLE_IPV6);
if (mHTTPSSVCRecord) {
MOZ_ASSERT(mURI->SchemeIs("https"));
LOG((" Using connection info with HTTPSSVC record"));
nsCOMPtr<nsIDNSHTTPSSVCRecord> rec;
mHTTPSSVCRecord.swap(rec);
bool http3Allowed = !mUpgradeProtocolCallback && !mProxyInfo &&
!(mCaps & NS_HTTP_BE_CONSERVATIVE) &&
!LoadBeConservative();
nsCOMPtr<nsISVCBRecord> record;
if (NS_SUCCEEDED(rec->GetServiceModeRecord(mCaps & NS_HTTP_DISALLOW_SPDY,
!http3Allowed,
getter_AddRefs(record)))) {
MOZ_ASSERT(record);
RefPtr<nsHttpConnectionInfo> newConnInfo =
mConnectionInfo->CloneAndAdoptHTTPSSVCRecord(record);
mConnectionInfo = std::move(newConnInfo);
}
}
// notify "http-on-before-connect" observers
gHttpHandler->OnBeforeConnect(this);
@ -2843,9 +2822,12 @@ nsresult nsHttpChannel::ProxyFailover() {
return AsyncDoReplaceWithProxy(pi);
}
void nsHttpChannel::SetHTTPSSVCRecord(nsIDNSHTTPSSVCRecord* aRecord) {
void nsHttpChannel::SetHTTPSSVCRecord(
already_AddRefed<nsIDNSHTTPSSVCRecord>&& aRecord) {
LOG(("nsHttpChannel::SetHTTPSSVCRecord [this=%p]\n", this));
mHTTPSSVCRecord = aRecord;
nsCOMPtr<nsIDNSHTTPSSVCRecord> record = aRecord;
MOZ_ASSERT(!mHTTPSSVCRecord);
mHTTPSSVCRecord.emplace(std::move(record));
}
void nsHttpChannel::HandleAsyncRedirectChannelToHttps() {
@ -2933,10 +2915,9 @@ nsresult nsHttpChannel::StartRedirectChannelToURI(nsIURI* upgradedURI,
if (mHTTPSSVCRecord) {
RefPtr<nsHttpChannel> httpChan = do_QueryObject(newChannel);
if (httpChan) {
nsCOMPtr<nsIDNSHTTPSSVCRecord> rec;
mHTTPSSVCRecord.swap(rec);
httpChan->SetHTTPSSVCRecord(rec);
nsCOMPtr<nsIDNSHTTPSSVCRecord> rec = mHTTPSSVCRecord.ref();
if (httpChan && rec) {
httpChan->SetHTTPSSVCRecord(rec.forget());
}
}
@ -6613,9 +6594,10 @@ nsresult nsHttpChannel::BeginConnect() {
!(mCaps & NS_HTTP_BE_CONSERVATIVE) &&
!LoadBeConservative() && LoadAllowHttp3();
// No need to lookup HTTPSSVC record if we already have one.
// No need to lookup HTTPSSVC record if mHTTPSSVCRecord already contains a
// value.
StoreUseHTTPSSVC(StaticPrefs::network_dns_upgrade_with_https_rr() &&
!mHTTPSSVCRecord);
mHTTPSSVCRecord.isNothing());
RefPtr<AltSvcMapping> mapping;
if (!mConnectionInfo && LoadAllowAltSvc() && // per channel
@ -9037,25 +9019,27 @@ void nsHttpChannel::OnHTTPSRRAvailable(nsIDNSHTTPSSVCRecord* aRecord) {
LOG(("nsHttpChannel::OnHTTPSRRAvailable [this=%p, aRecord=%p]\n", this,
aRecord));
// This record will be used in the new redirect channel.
MOZ_ASSERT(!mHTTPSSVCRecord);
mHTTPSSVCRecord = aRecord;
nsCOMPtr<nsIDNSHTTPSSVCRecord> record = aRecord;
mHTTPSSVCRecord.emplace(std::move(record));
const nsCOMPtr<nsIDNSHTTPSSVCRecord>& httprr = mHTTPSSVCRecord.ref();
if (LoadWaitHTTPSSVCRecord()) {
MOZ_ASSERT(mURI->SchemeIs("http"));
StoreWaitHTTPSSVCRecord(false);
nsresult rv = ContinueOnBeforeConnect(!!mHTTPSSVCRecord, mStatus);
nsresult rv = ContinueOnBeforeConnect(!!httprr, mStatus);
if (NS_FAILED(rv)) {
CloseCacheEntry(false);
Unused << AsyncAbort(rv);
}
} else {
// This channel is not canceled and the transaction is not created.
if (mHTTPSSVCRecord && NS_SUCCEEDED(mStatus) && !mTransaction &&
if (httprr && NS_SUCCEEDED(mStatus) && !mTransaction &&
(mFirstResponseSource != RESPONSE_FROM_CACHE)) {
bool hasIPAddress = false;
Unused << mHTTPSSVCRecord->GetHasIPAddresses(&hasIPAddress);
Unused << httprr->GetHasIPAddresses(&hasIPAddress);
Telemetry::Accumulate(Telemetry::DNS_HTTPSSVC_RECORD_RECEIVING_STAGE,
hasIPAddress
? HTTPSSVC_WITH_IPHINT_RECEIVED_STAGE_0

Просмотреть файл

@ -824,7 +824,7 @@ class nsHttpChannel final : public HttpBaseChannel,
nsresult TriggerNetwork();
void CancelNetworkRequest(nsresult aStatus);
void SetHTTPSSVCRecord(nsIDNSHTTPSSVCRecord* aRecord);
void SetHTTPSSVCRecord(already_AddRefed<nsIDNSHTTPSSVCRecord>&& aRecord);
// Timer used to delay the network request, or to trigger the network
// request if retrieving the cache entry takes too long.
@ -859,9 +859,10 @@ class nsHttpChannel final : public HttpBaseChannel,
// called and reset the value when we switch to another failover proxy.
int32_t mProxyConnectResponseCode;
// If this is not null, this will be used to update the connection info in
// nsHttpChannel::BeginConnect().
nsCOMPtr<nsIDNSHTTPSSVCRecord> mHTTPSSVCRecord;
// If mHTTPSSVCRecord has value, it means OnHTTPSRRAvailable() is called and
// we got the result of HTTPS RR query. Otherwise, it means we are still
// waiting for the result or the query is not performed.
Maybe<nsCOMPtr<nsIDNSHTTPSSVCRecord>> mHTTPSSVCRecord;
protected:
virtual void DoNotifyListenerCleanup() override;

Просмотреть файл

@ -19,6 +19,7 @@ const certOverrideService = Cc[
const threadManager = Cc["@mozilla.org/thread-manager;1"].getService(
Ci.nsIThreadManager
);
const { HttpServer } = ChromeUtils.import("resource://testing-common/httpd.js");
const mainThread = threadManager.currentThread;
const defaultOriginAttributes = {};
@ -116,7 +117,9 @@ function makeChan(url) {
return chan;
}
function channelOpenPromise(chan, flags) {
// When observer is specified, the channel will be suspended when receiving
// "http-on-modify-request".
function channelOpenPromise(chan, flags, observer) {
return new Promise(resolve => {
function finish(req, buffer) {
certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
@ -127,6 +130,11 @@ function channelOpenPromise(chan, flags) {
certOverrideService.setDisableAllSecurityChecksAndLetAttackersInterceptMyData(
true
);
if (observer) {
let topic = "http-on-modify-request";
Services.obs.addObserver(observer, topic);
}
chan.asyncOpen(new ChannelListener(finish, null, flags));
});
}
@ -193,13 +201,70 @@ add_task(async function testUseHTTPSSVCAsHSTS() {
Assert.equal(req.getResponseHeader("x-connection-http2"), "yes");
});
// Test the case that we got an invalid DNS response.
// Test the case that we got an invalid DNS response. In this case,
// nsHttpChannel::OnHTTPSRRAvailable is called after
// nsHttpChannel::MaybeUseHTTPSRRForUpgrade.
add_task(async function testInvalidDNSResult() {
let dnsListener = new DNSListener();
dns.clearCache(true);
// Do DNS resolution before creating the channel, so the HTTPSSVC record will
// be resolved from the cache.
let request = dns.asyncResolve(
let httpserv = new HttpServer();
let content = "ok";
httpserv.registerPathHandler("/", function handler(metadata, response) {
response.setHeader("Content-Length", `${content.length}`);
response.bodyOutputStream.write(content, content.length);
});
httpserv.start(-1);
httpserv.identity.setPrimary(
"http",
"foo.notexisted.com",
httpserv.identity.primaryPort
);
let chan = makeChan(
`http://foo.notexisted.com:${httpserv.identity.primaryPort}/`
);
let [, response] = await channelOpenPromise(chan);
Assert.equal(response, content);
await new Promise(resolve => httpserv.stop(resolve));
});
// The same test as above, but nsHttpChannel::MaybeUseHTTPSRRForUpgrade is
// called after nsHttpChannel::OnHTTPSRRAvailable.
add_task(async function testInvalidDNSResult1() {
dns.clearCache(true);
let httpserv = new HttpServer();
let content = "ok";
httpserv.registerPathHandler("/", function handler(metadata, response) {
response.setHeader("Content-Length", `${content.length}`);
response.bodyOutputStream.write(content, content.length);
});
httpserv.start(-1);
httpserv.identity.setPrimary(
"http",
"foo.notexisted.com",
httpserv.identity.primaryPort
);
let chan = makeChan(
`http://foo.notexisted.com:${httpserv.identity.primaryPort}/`
);
let topic = "http-on-modify-request";
let observer = {
QueryInterface: ChromeUtils.generateQI(["nsIObserver"]),
observe(aSubject, aTopic, aData) {
if (aTopic == topic) {
Services.obs.removeObserver(observer, topic);
let channel = aSubject.QueryInterface(Ci.nsIChannel);
channel.suspend();
let dnsListener = {
QueryInterface: ChromeUtils.generateQI(["nsIDNSListener"]),
onLookupComplete(inRequest, inRecord, inStatus) {
channel.resume();
},
};
dns.asyncResolve(
"foo.notexisted.com",
dns.RESOLVE_TYPE_HTTPSSVC,
0,
@ -208,24 +273,26 @@ add_task(async function testInvalidDNSResult() {
mainThread,
defaultOriginAttributes
);
}
},
};
let [inRequest, , inStatus] = await dnsListener;
Assert.equal(inRequest, request, "correct request was used");
Assert.equal(inStatus, Cr.NS_ERROR_UNKNOWN_HOST, "status error");
let chan = makeChan(`http://foo.notexisted.com:8888/server-timing`);
let [req] = await channelOpenPromise(
chan,
CL_EXPECT_LATE_FAILURE | CL_ALLOW_UNKNOWN_CL
);
Assert.equal(req.status, Cr.NS_ERROR_CONNECTION_REFUSED);
let [, response] = await channelOpenPromise(chan, 0, observer);
Assert.equal(response, content);
await new Promise(resolve => httpserv.stop(resolve));
});
add_task(async function testLiteralIP() {
let chan = makeChan(`http://127.0.0.1:8888/server-timing`);
let [req] = await channelOpenPromise(
chan,
CL_EXPECT_LATE_FAILURE | CL_ALLOW_UNKNOWN_CL
);
Assert.equal(req.status, Cr.NS_ERROR_CONNECTION_REFUSED);
let httpserv = new HttpServer();
let content = "ok";
httpserv.registerPathHandler("/", function handler(metadata, response) {
response.setHeader("Content-Length", `${content.length}`);
response.bodyOutputStream.write(content, content.length);
});
httpserv.start(-1);
let chan = makeChan(`http://127.0.0.1:${httpserv.identity.primaryPort}/`);
let [, response] = await channelOpenPromise(chan);
Assert.equal(response, content);
await new Promise(resolve => httpserv.stop(resolve));
});