Bug 1779110 - Skip HTTPS AliasMode record if the RR type is not HTTPS r=necko-reviewers,kershaw

According to https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https#section-2.4.2
> Unlike CNAME, AliasMode
>   records do not affect the resolution of other RR types, and apply
>   only to a specific service, not an entire domain name.

As such, we should skip the AliasMode response if the RR type is not HTTPS.

Additionally, when an AliasMode record is present in the response, we must
ignore any ServiceMode records present in the response.

https://datatracker.ietf.org/doc/html/draft-ietf-dnsop-svcb-https#section-2.4.1
> Within a SVCB RRSet, all RRs SHOULD have the same Mode.  If an RRSet
>   contains a record in AliasMode, the recipient MUST ignore any
>   ServiceMode records in the set.

Differential Revision: https://phabricator.services.mozilla.com/D153511
This commit is contained in:
Valentin Gosu 2022-08-09 12:16:33 +00:00
Родитель f5cb2931f9
Коммит b4bd0c45a0
2 изменённых файлов: 92 добавлений и 20 удалений

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

@ -809,6 +809,11 @@ nsresult DNSPacket::DecodeInternal(
parsed.mSvcFieldValue.AppendElement(value);
}
if (aType != TRRTYPE_HTTPSSVC) {
// Ignore the entry that we just parsed if we didn't ask for it.
break;
}
// Check for AliasForm
if (aCname.IsEmpty() && parsed.mSvcFieldPriority == 0) {
// Alias form SvcDomainName must not have the "." value (empty)
@ -816,17 +821,14 @@ nsresult DNSPacket::DecodeInternal(
return NS_ERROR_UNEXPECTED;
}
aCname = parsed.mSvcDomainName;
// If aliasForm is present, Service form must be ignored.
aTypeResult = mozilla::AsVariant(Nothing());
ToLowerCase(aCname);
LOG(("DNSPacket::DohDecode HTTPSSVC AliasForm host %s => %s\n",
host.get(), aCname.get()));
break;
}
if (aType != TRRTYPE_HTTPSSVC) {
// Ignore the entry that we just parsed if we didn't ask for it.
break;
}
if (!aTypeResult.is<TypeRecordHTTPSSVC>()) {
aTypeResult = mozilla::AsVariant(CopyableTArray<SVCB>());
}

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

@ -161,7 +161,7 @@ add_task(async function test_aliasform() {
);
}
// Test that HTTPS priority = 0 (AliasForm) behaves like a CNAME
// Make sure that HTTPS AliasForm is only treated as a CNAME for HTTPS requests
await trrServer.registerDoHAnswers("test.com", "A", {
answers: [
{
@ -189,10 +189,62 @@ add_task(async function test_aliasform() {
],
});
await new TRRDNSListener("test.com", { expectedAnswer: "1.2.3.4" });
{
let { inStatus } = await new TRRDNSListener("test.com", {
expectedSuccess: false,
});
Assert.ok(
!Components.isSuccessCode(inStatus),
`${inStatus} should be an error code`
);
}
dns.clearCache(true);
// Test that HTTPS priority = 0 (AliasForm) behaves like a CNAME
await trrServer.registerDoHAnswers("test.com", "HTTPS", {
answers: [
{
name: "test.com",
ttl: 55,
type: "HTTPS",
flush: false,
data: {
priority: 0,
name: "something.com",
values: [],
},
},
],
});
await trrServer.registerDoHAnswers("something.com", "HTTPS", {
answers: [
{
name: "something.com",
ttl: 55,
type: "HTTPS",
flush: false,
data: {
priority: 1,
name: "h3pool",
values: [{ key: "alpn", value: ["h2", "h3"] }],
},
},
],
});
{
let { inStatus, inRecord } = await new TRRDNSListener("test.com", {
type: dns.RESOLVE_TYPE_HTTPSSVC,
expectedSuccess: false,
});
Assert.ok(Components.isSuccessCode(inStatus), `${inStatus} should succeed`);
let answer = inRecord.QueryInterface(Ci.nsIDNSHTTPSSVCRecord).records;
Assert.equal(answer[0].priority, 1);
Assert.equal(answer[0].name, "h3pool");
}
// Test a chain of HTTPSSVC AliasForm and CNAMEs
await trrServer.registerDoHAnswers("x.com", "A", {
await trrServer.registerDoHAnswers("x.com", "HTTPS", {
answers: [
{
name: "x.com",
@ -207,7 +259,7 @@ add_task(async function test_aliasform() {
},
],
});
await trrServer.registerDoHAnswers("y.com", "A", {
await trrServer.registerDoHAnswers("y.com", "HTTPS", {
answers: [
{
name: "y.com",
@ -219,7 +271,7 @@ add_task(async function test_aliasform() {
},
],
});
await trrServer.registerDoHAnswers("z.com", "A", {
await trrServer.registerDoHAnswers("z.com", "HTTPS", {
answers: [
{
name: "z.com",
@ -234,19 +286,30 @@ add_task(async function test_aliasform() {
},
],
});
await trrServer.registerDoHAnswers("target.com", "A", {
await trrServer.registerDoHAnswers("target.com", "HTTPS", {
answers: [
{
name: "target.com",
ttl: 55,
type: "A",
type: "HTTPS",
flush: false,
data: "4.3.2.1",
data: {
priority: 1,
name: "h3pool",
values: [{ key: "alpn", value: ["h2", "h3"] }],
},
},
],
});
await new TRRDNSListener("x.com", { expectedAnswer: "4.3.2.1" });
let { inStatus, inRecord } = await new TRRDNSListener("x.com", {
type: dns.RESOLVE_TYPE_HTTPSSVC,
expectedSuccess: false,
});
Assert.ok(Components.isSuccessCode(inStatus), `${inStatus} should succeed`);
let answer = inRecord.QueryInterface(Ci.nsIDNSHTTPSSVCRecord).records;
Assert.equal(answer[0].priority, 1);
Assert.equal(answer[0].name, "h3pool");
// We get a ServiceForm instead of a A answer, CNAME or AliasForm
await trrServer.registerDoHAnswers("no-ip-host.com", "A", {
@ -272,16 +335,16 @@ add_task(async function test_aliasform() {
],
});
let { inStatus } = await new TRRDNSListener("no-ip-host.com", {
({ inStatus } = await new TRRDNSListener("no-ip-host.com", {
expectedSuccess: false,
});
}));
Assert.ok(
!Components.isSuccessCode(inStatus),
`${inStatus} should be an error code`
);
// Test CNAME/AliasForm loop
await trrServer.registerDoHAnswers("loop.com", "A", {
await trrServer.registerDoHAnswers("loop.com", "HTTPS", {
answers: [
{
name: "loop.com",
@ -293,7 +356,7 @@ add_task(async function test_aliasform() {
},
],
});
await trrServer.registerDoHAnswers("loop2.com", "A", {
await trrServer.registerDoHAnswers("loop2.com", "HTTPS", {
answers: [
{
name: "loop2.com",
@ -309,13 +372,21 @@ add_task(async function test_aliasform() {
],
});
// Make sure these are the first requests
Assert.equal(await trrServer.requestCount("loop.com", "HTTPS"), 0);
Assert.equal(await trrServer.requestCount("loop2.com", "HTTPS"), 0);
({ inStatus } = await new TRRDNSListener("loop.com", {
type: dns.RESOLVE_TYPE_HTTPSSVC,
expectedSuccess: false,
}));
Assert.ok(
!Components.isSuccessCode(inStatus),
`${inStatus} should be an error code`
);
// Make sure the error was actually triggered by a loop.
Assert.greater(await trrServer.requestCount("loop.com", "HTTPS"), 2);
Assert.greater(await trrServer.requestCount("loop2.com", "HTTPS"), 2);
// Alias form for .
await trrServer.registerDoHAnswers("empty.com", "A", {
@ -557,12 +628,11 @@ add_task(async function test_aliasform() {
],
});
let inRecord;
({ inRecord, inStatus: inStatus2 } = await new TRRDNSListener("service.com", {
type: dns.RESOLVE_TYPE_HTTPSSVC,
}));
Assert.ok(Components.isSuccessCode(inStatus2), `${inStatus2} should work`);
let answer = inRecord.QueryInterface(Ci.nsIDNSHTTPSSVCRecord).records;
answer = inRecord.QueryInterface(Ci.nsIDNSHTTPSSVCRecord).records;
Assert.equal(answer[0].priority, 1);
Assert.equal(answer[0].name, "service.com");
});