Bug 1734579 - Make block size of encrypted edns padding configurable r=necko-reviewers,valentin

Additionally changing the paddding block size to the recommended default
of 128 bytes by RFC 8467 Sec 4.1

Differential Revision: https://phabricator.services.mozilla.com/D127957
This commit is contained in:
Manuel Bucher 2021-10-20 07:51:14 +00:00
parent 567d5bdab3
commit 90522eeeca
4 changed files with 117 additions and 29 deletions

View File

@ -9780,6 +9780,14 @@
value: true
mirror: always
# The block size to pad to. Capped at 1024 bytes.
# Setting it to 0 doesn't add additional padding, but allows the server to
# respond with padding (RFC7930 Sec 4)
- name: network.trr.padding.length
type: RelaxedAtomicUint32
value: 128
mirror: always
# Whether to skip the NS check for the blocked host.
# Note this is used for test only.
- name: network.trr.skip-check-for-blocked-host

View File

@ -411,7 +411,8 @@ nsresult DNSPacket::EncodeRequest(nsCString& aBody, const nsACString& aHost,
// calculate padding length
unsigned int paddingLen = 0;
unsigned int rdlen = 0;
if (StaticPrefs::network_trr_padding()) {
bool padding = StaticPrefs::network_trr_padding();
if (padding) {
// always add padding specified in rfc 7830 when this config is enabled
// to allow the reponse to be padded as well
@ -421,8 +422,17 @@ nsresult DNSPacket::EncodeRequest(nsCString& aBody, const nsACString& aHost,
// 8 bytes for disabling ecs
packetLen += 8;
}
// pad to 16 byte
paddingLen = (16 - (packetLen & 15)) & 15;
// clamp the padding length, because the padding extension only allows up
// to 2^16 - 1 bytes padding and adding too much padding wastes resources
uint32_t padTo = std::clamp<uint32_t>(
StaticPrefs::network_trr_padding_length(), 0, 1024);
// Calculate number of padding bytes. The second '%'-operator is necessary
// because we prefer to add 0 bytes padding rather than padTo bytes
if (padTo > 0) {
paddingLen = (padTo - (packetLen % padTo)) % padTo;
}
// padding header + padding length
rdlen += 4 + paddingLen;
}
@ -454,7 +464,7 @@ nsresult DNSPacket::EncodeRequest(nsCString& aBody, const nsACString& aHost,
// ADDRESS, minimum number of octets == nothing because zero bits
}
if (StaticPrefs::network_trr_padding()) {
if (padding) {
aBody += '\0'; // upper 8 bit option OPTION-CODE PADDING
aBody += 12; // OPTION-CODE, 2 octets, for PADDING is 12

View File

@ -4,44 +4,64 @@
using namespace mozilla::net;
void AssertDnsPadding(unsigned int WithPadding, unsigned int WithoutPadding,
bool DisableEcn, const nsCString& host) {
void AssertDnsPadding(uint32_t PaddingLength, unsigned int WithPadding,
unsigned int WithoutPadding, bool DisableEcn,
const nsCString& host) {
DNSPacket encoder;
nsCString buf;
Preferences::SetBool("network.trr.padding", true);
ASSERT_EQ(Preferences::SetUint("network.trr.padding.length", PaddingLength),
NS_OK);
ASSERT_EQ(Preferences::SetBool("network.trr.padding", true), NS_OK);
ASSERT_EQ(encoder.EncodeRequest(buf, host, 1, DisableEcn), NS_OK);
ASSERT_EQ(buf.Length(), WithPadding);
Preferences::SetBool("network.trr.padding", false);
ASSERT_EQ(Preferences::SetBool("network.trr.padding", false), NS_OK);
ASSERT_EQ(encoder.EncodeRequest(buf, host, 1, DisableEcn), NS_OK);
ASSERT_EQ(buf.Length(), WithoutPadding);
}
TEST(TestDNSPacket, PaddingLenEcn)
{
AssertDnsPadding(48, 41, true, "a.de"_ns);
AssertDnsPadding(48, 42, true, "ab.de"_ns);
AssertDnsPadding(48, 43, true, "abc.de"_ns);
AssertDnsPadding(48, 44, true, "abcd.de"_ns);
AssertDnsPadding(64, 45, true, "abcde.de"_ns);
AssertDnsPadding(64, 46, true, "abcdef.de"_ns);
AssertDnsPadding(64, 47, true, "abcdefg.de"_ns);
AssertDnsPadding(64, 48, true, "abcdefgh.de"_ns);
AssertDnsPadding(16, 48, 41, true, "a.de"_ns);
AssertDnsPadding(16, 48, 42, true, "ab.de"_ns);
AssertDnsPadding(16, 48, 43, true, "abc.de"_ns);
AssertDnsPadding(16, 48, 44, true, "abcd.de"_ns);
AssertDnsPadding(16, 64, 45, true, "abcde.de"_ns);
AssertDnsPadding(16, 64, 46, true, "abcdef.de"_ns);
AssertDnsPadding(16, 64, 47, true, "abcdefg.de"_ns);
AssertDnsPadding(16, 64, 48, true, "abcdefgh.de"_ns);
}
TEST(TestDNSPacket, PaddingLenDisableEcn)
{
AssertDnsPadding(48, 22, false, "a.de"_ns);
AssertDnsPadding(48, 23, false, "ab.de"_ns);
AssertDnsPadding(48, 24, false, "abc.de"_ns);
AssertDnsPadding(48, 25, false, "abcd.de"_ns);
AssertDnsPadding(48, 26, false, "abcde.de"_ns);
AssertDnsPadding(48, 27, false, "abcdef.de"_ns);
AssertDnsPadding(48, 32, false, "abcdefghijk.de"_ns);
AssertDnsPadding(48, 33, false, "abcdefghijkl.de"_ns);
AssertDnsPadding(64, 34, false, "abcdefghijklm.de"_ns);
AssertDnsPadding(64, 35, false, "abcdefghijklmn.de"_ns);
AssertDnsPadding(16, 48, 22, false, "a.de"_ns);
AssertDnsPadding(16, 48, 23, false, "ab.de"_ns);
AssertDnsPadding(16, 48, 24, false, "abc.de"_ns);
AssertDnsPadding(16, 48, 25, false, "abcd.de"_ns);
AssertDnsPadding(16, 48, 26, false, "abcde.de"_ns);
AssertDnsPadding(16, 48, 27, false, "abcdef.de"_ns);
AssertDnsPadding(16, 48, 32, false, "abcdefghijk.de"_ns);
AssertDnsPadding(16, 48, 33, false, "abcdefghijkl.de"_ns);
AssertDnsPadding(16, 64, 34, false, "abcdefghijklm.de"_ns);
AssertDnsPadding(16, 64, 35, false, "abcdefghijklmn.de"_ns);
}
TEST(TestDNSPacket, PaddingLengths)
{
AssertDnsPadding(0, 45, 41, true, "a.de"_ns);
AssertDnsPadding(1, 45, 41, true, "a.de"_ns);
AssertDnsPadding(2, 46, 41, true, "a.de"_ns);
AssertDnsPadding(3, 45, 41, true, "a.de"_ns);
AssertDnsPadding(4, 48, 41, true, "a.de"_ns);
AssertDnsPadding(16, 48, 41, true, "a.de"_ns);
AssertDnsPadding(32, 64, 41, true, "a.de"_ns);
AssertDnsPadding(42, 84, 41, true, "a.de"_ns);
AssertDnsPadding(52, 52, 41, true, "a.de"_ns);
AssertDnsPadding(80, 80, 41, true, "a.de"_ns);
AssertDnsPadding(128, 128, 41, true, "a.de"_ns);
AssertDnsPadding(256, 256, 41, true, "a.de"_ns);
AssertDnsPadding(1024, 1024, 41, true, "a.de"_ns);
AssertDnsPadding(1025, 1024, 41, true, "a.de"_ns);
}

View File

@ -798,7 +798,15 @@ add_task(async function test_old_bootstrap_pref() {
add_task(async function test_padding() {
setModeAndURI(Ci.nsIDNSService.MODE_TRRONLY, `doh`);
async function CheckPadding(request, none, ecs, padding, ecsPadding) {
async function CheckPadding(
pad_length,
request,
none,
ecs,
padding,
ecsPadding
) {
Services.prefs.setIntPref("network.trr.padding.length", pad_length);
dns.clearCache(true);
Services.prefs.setBoolPref("network.trr.padding", false);
Services.prefs.setBoolPref("network.trr.disable-ECS", false);
@ -821,19 +829,61 @@ add_task(async function test_padding() {
}
// short domain name
await CheckPadding("a.pd", "2.2.0.22", "2.2.0.41", "1.1.0.48", "1.1.0.48");
await CheckPadding(
16,
"a.pd",
"2.2.0.22",
"2.2.0.41",
"1.1.0.48",
"1.1.0.48"
);
await CheckPadding(256, "a.pd", "2.2.0.22", "2.2.0.41", "1.1.1.0", "1.1.1.0");
// medium domain name
await CheckPadding(
16,
"has-padding.pd",
"2.2.0.32",
"2.2.0.51",
"1.1.0.48",
"1.1.0.64"
);
await CheckPadding(
128,
"has-padding.pd",
"2.2.0.32",
"2.2.0.51",
"1.1.0.128",
"1.1.0.128"
);
await CheckPadding(
80,
"has-padding.pd",
"2.2.0.32",
"2.2.0.51",
"1.1.0.80",
"1.1.0.80"
);
// long domain name
await CheckPadding(
16,
"abcdefghijklmnopqrstuvwxyz0123456789.abcdefghijklmnopqrstuvwxyz0123456789.abcdefghijklmnopqrstuvwxyz0123456789.pd",
"2.2.0.131",
"2.2.0.150",
"1.1.0.160",
"1.1.0.160"
);
await CheckPadding(
128,
"abcdefghijklmnopqrstuvwxyz0123456789.abcdefghijklmnopqrstuvwxyz0123456789.abcdefghijklmnopqrstuvwxyz0123456789.pd",
"2.2.0.131",
"2.2.0.150",
"1.1.1.0",
"1.1.1.0"
);
await CheckPadding(
80,
"abcdefghijklmnopqrstuvwxyz0123456789.abcdefghijklmnopqrstuvwxyz0123456789.abcdefghijklmnopqrstuvwxyz0123456789.pd",
"2.2.0.131",
"2.2.0.150",