Bug 1792858 - Part 1: Remove the IDN allowlist code from nsIDNService, r=necko-reviewers,kershaw

This pref was disabled in 2015, and hasn't been re-enabled since. Removing it
will allow simplifying this service significantly, and remove some
thread-safety hazards which would appear if the pref is enabled.

Differential Revision: https://phabricator.services.mozilla.com/D158520
This commit is contained in:
Nika Layzell 2022-10-04 15:28:40 +00:00
parent 4ff2281d03
commit 298e72ba8f
9 changed files with 14 additions and 211 deletions

View File

@ -1428,11 +1428,9 @@ pref("dom.server-events.default-reconnection-time", 5000); // in milliseconds
// generate them from punycode.
pref("network.IDN_show_punycode", false);
// If "network.IDN.use_whitelist" is set to true, TLDs with
// "network.IDN.whitelist.tld" explicitly set to true are treated as
// IDN-safe. Otherwise, they're treated as unsafe and punycode will be used
// for displaying them in the UI (e.g. URL bar), unless they conform to one of
// the profiles specified in
// TLDs are treated as IDN-unsafe and punycode will be used for displaying them
// in the UI (e.g. URL bar), unless they conform to one of the profiles
// specified in
// https://www.unicode.org/reports/tr39/#Restriction_Level_Detection
// If "network.IDN.restriction_profile" is "high", the Highly Restrictive
// profile is used.
@ -1443,122 +1441,9 @@ pref("network.IDN_show_punycode", false);
// "network.IDN_show_punycode" is false. In other words, all IDNs will be shown
// in punycode if "network.IDN_show_punycode" is true.
pref("network.IDN.restriction_profile", "high");
pref("network.IDN.use_whitelist", false);
// ccTLDs
pref("network.IDN.whitelist.ac", true);
pref("network.IDN.whitelist.ar", true);
pref("network.IDN.whitelist.at", true);
pref("network.IDN.whitelist.br", true);
pref("network.IDN.whitelist.ca", true);
pref("network.IDN.whitelist.ch", true);
pref("network.IDN.whitelist.cl", true);
pref("network.IDN.whitelist.cn", true);
pref("network.IDN.whitelist.de", true);
pref("network.IDN.whitelist.dk", true);
pref("network.IDN.whitelist.ee", true);
pref("network.IDN.whitelist.es", true);
pref("network.IDN.whitelist.fi", true);
pref("network.IDN.whitelist.fr", true);
pref("network.IDN.whitelist.gr", true);
pref("network.IDN.whitelist.gt", true);
pref("network.IDN.whitelist.hu", true);
pref("network.IDN.whitelist.il", true);
pref("network.IDN.whitelist.io", true);
pref("network.IDN.whitelist.ir", true);
pref("network.IDN.whitelist.is", true);
pref("network.IDN.whitelist.jp", true);
pref("network.IDN.whitelist.kr", true);
pref("network.IDN.whitelist.li", true);
pref("network.IDN.whitelist.lt", true);
pref("network.IDN.whitelist.lu", true);
pref("network.IDN.whitelist.lv", true);
pref("network.IDN.whitelist.no", true);
pref("network.IDN.whitelist.nu", true);
pref("network.IDN.whitelist.nz", true);
pref("network.IDN.whitelist.pl", true);
pref("network.IDN.whitelist.pm", true);
pref("network.IDN.whitelist.pr", true);
pref("network.IDN.whitelist.re", true);
pref("network.IDN.whitelist.se", true);
pref("network.IDN.whitelist.sh", true);
pref("network.IDN.whitelist.si", true);
pref("network.IDN.whitelist.tf", true);
pref("network.IDN.whitelist.th", true);
pref("network.IDN.whitelist.tm", true);
pref("network.IDN.whitelist.tw", true);
pref("network.IDN.whitelist.ua", true);
pref("network.IDN.whitelist.vn", true);
pref("network.IDN.whitelist.wf", true);
pref("network.IDN.whitelist.yt", true);
// IDN ccTLDs
// ae, UAE, .<Emarat>
pref("network.IDN.whitelist.xn--mgbaam7a8h", true);
// cn, China, .<China> with variants
pref("network.IDN.whitelist.xn--fiqz9s", true); // Traditional
pref("network.IDN.whitelist.xn--fiqs8s", true); // Simplified
// eg, Egypt, .<Masr>
pref("network.IDN.whitelist.xn--wgbh1c", true);
// hk, Hong Kong, .<Hong Kong>
pref("network.IDN.whitelist.xn--j6w193g", true);
// ir, Iran, <.Iran> with variants
pref("network.IDN.whitelist.xn--mgba3a4f16a", true);
pref("network.IDN.whitelist.xn--mgba3a4fra", true);
// jo, Jordan, .<Al-Ordon>
pref("network.IDN.whitelist.xn--mgbayh7gpa", true);
// lk, Sri Lanka, .<Lanka> and .<Ilangai>
pref("network.IDN.whitelist.xn--fzc2c9e2c", true);
pref("network.IDN.whitelist.xn--xkc2al3hye2a", true);
// qa, Qatar, .<Qatar>
pref("network.IDN.whitelist.xn--wgbl6a", true);
// rs, Serbia, .<Srb>
pref("network.IDN.whitelist.xn--90a3ac", true);
// ru, Russian Federation, .<RF>
pref("network.IDN.whitelist.xn--p1ai", true);
// sa, Saudi Arabia, .<al-Saudiah> with variants
pref("network.IDN.whitelist.xn--mgberp4a5d4ar", true);
pref("network.IDN.whitelist.xn--mgberp4a5d4a87g", true);
pref("network.IDN.whitelist.xn--mgbqly7c0a67fbc", true);
pref("network.IDN.whitelist.xn--mgbqly7cvafr", true);
// sy, Syria, .<Souria>
pref("network.IDN.whitelist.xn--ogbpf8fl", true);
// th, Thailand, .<Thai>
pref("network.IDN.whitelist.xn--o3cw4h", true);
// tw, Taiwan, <.Taiwan> with variants
pref("network.IDN.whitelist.xn--kpry57d", true); // Traditional
pref("network.IDN.whitelist.xn--kprw13d", true); // Simplified
// gTLDs
pref("network.IDN.whitelist.asia", true);
pref("network.IDN.whitelist.biz", true);
pref("network.IDN.whitelist.cat", true);
pref("network.IDN.whitelist.info", true);
pref("network.IDN.whitelist.museum", true);
pref("network.IDN.whitelist.org", true);
pref("network.IDN.whitelist.tel", true);
// NOTE: Before these can be removed, one of bug 414812's tests must be updated
// or it will likely fail! Please CC jwalden+bmo on the bug associated
// with removing these so he can provide a patch to make the necessary
// changes to avoid bustage.
// ".test" localised TLDs for ICANN's top-level IDN trial
pref("network.IDN.whitelist.xn--0zwm56d", true);
pref("network.IDN.whitelist.xn--11b5bs3a9aj6g", true);
pref("network.IDN.whitelist.xn--80akhbyknj4f", true);
pref("network.IDN.whitelist.xn--9t4b11yi5a", true);
pref("network.IDN.whitelist.xn--deba0ad", true);
pref("network.IDN.whitelist.xn--g6w251d", true);
pref("network.IDN.whitelist.xn--hgbk6aj7f53bba", true);
pref("network.IDN.whitelist.xn--hlcj6aya9esc7a", true);
pref("network.IDN.whitelist.xn--jxalpdlp", true);
pref("network.IDN.whitelist.xn--kgbechtv", true);
pref("network.IDN.whitelist.xn--zckzah", true);
// If a domain includes any of the blocklist characters, it may be a spoof
// attempt and so we always display the domain name as punycode. This would
// override the settings "network.IDN_show_punycode" and
// "network.IDN.whitelist.*".
// attempt and so we always display the domain name as punycode.
// For a complete list of the blocked IDN characters see:
// netwerk/dns/IDNCharacterBlocklist.inc

View File

@ -52,8 +52,6 @@ static const char kACEPrefix[] = "xn--";
#define NS_NET_PREF_EXTRAALLOWED "network.IDN.extra_allowed_chars"
#define NS_NET_PREF_EXTRABLOCKED "network.IDN.extra_blocked_chars"
#define NS_NET_PREF_SHOWPUNYCODE "network.IDN_show_punycode"
#define NS_NET_PREF_IDNWHITELIST "network.IDN.whitelist."
#define NS_NET_PREF_IDNUSEWHITELIST "network.IDN.use_whitelist"
#define NS_NET_PREF_IDNRESTRICTION "network.IDN.restriction_profile"
static inline bool isOnlySafeChars(const nsString& in,
@ -80,21 +78,17 @@ static inline bool isOnlySafeChars(const nsString& in,
NS_IMPL_ISUPPORTS(nsIDNService, nsIIDNService, nsISupportsWeakReference)
static const char* gCallbackPrefs[] = {
NS_NET_PREF_EXTRAALLOWED, NS_NET_PREF_EXTRABLOCKED,
NS_NET_PREF_SHOWPUNYCODE, NS_NET_PREF_IDNRESTRICTION,
NS_NET_PREF_IDNUSEWHITELIST, nullptr,
NS_NET_PREF_EXTRAALLOWED,
NS_NET_PREF_EXTRABLOCKED,
NS_NET_PREF_SHOWPUNYCODE,
NS_NET_PREF_IDNRESTRICTION,
nullptr,
};
nsresult nsIDNService::Init() {
MOZ_ASSERT(NS_IsMainThread());
MutexSingleWriterAutoLock lock(mLock);
nsCOMPtr<nsIPrefService> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID));
if (prefs) {
prefs->GetBranch(NS_NET_PREF_IDNWHITELIST,
getter_AddRefs(mIDNWhitelistPrefBranch));
}
Preferences::RegisterPrefixCallbacks(PrefChanged, gCallbackPrefs, this);
prefsChanged(nullptr);
InitializeBlocklist(mIDNBlocklist);
@ -118,12 +112,6 @@ void nsIDNService::prefsChanged(const char* pref) {
mShowPunycode = val;
}
}
if (!pref || nsLiteralCString(NS_NET_PREF_IDNUSEWHITELIST).Equals(pref)) {
bool val;
if (NS_SUCCEEDED(Preferences::GetBool(NS_NET_PREF_IDNUSEWHITELIST, &val))) {
mIDNUseWhitelist = val;
}
}
if (!pref || nsLiteralCString(NS_NET_PREF_IDNRESTRICTION).Equals(pref)) {
nsAutoCString profile;
if (NS_FAILED(
@ -451,12 +439,9 @@ NS_IMETHODIMP nsIDNService::ConvertToDisplayIDN(
if (isACE && !mShowPunycode) {
// ACEtoUTF8() can't fail, but might return the original ACE string
nsAutoCString temp(_retval);
// If the domain is in the whitelist, return the host in UTF-8.
// Otherwise convert from ACE to UTF8 only those labels which are
// considered safe for display
ACEtoUTF8(
temp, _retval,
isInWhitelist(temp) ? eStringPrepIgnoreErrors : eStringPrepForUI);
// Convert from ACE to UTF8 only those labels which are considered safe
// for display
ACEtoUTF8(temp, _retval, eStringPrepForUI);
*_isASCII = IsAscii(_retval);
} else {
*_isASCII = true;
@ -490,7 +475,7 @@ NS_IMETHODIMP nsIDNService::ConvertToDisplayIDN(
// unsafe characters, so leave it ACE encoded. see bug 283016, bug 301694,
// and bug 309311.
*_isASCII = IsAscii(_retval);
if (!*_isASCII && !isInWhitelist(_retval)) {
if (!*_isASCII) {
// UTF8toACE with eStringPrepForUI may return a domain name where
// some labels are in UTF-8 and some are in ACE, depending on
// whether they are considered safe for display
@ -678,39 +663,6 @@ nsresult nsIDNService::decodeACE(const nsACString& in, nsACString& out,
return NS_OK;
}
bool nsIDNService::isInWhitelist(const nsACString& host) {
if (!NS_IsMainThread()) {
mLock.AssertCurrentThreadOwns();
} else {
mLock.AssertOnWritingThread();
}
if (mIDNUseWhitelist && mIDNWhitelistPrefBranch) {
nsAutoCString tld(host);
// make sure the host is ACE for lookup and check that there are no
// unassigned codepoints
if (!IsAscii(tld) && NS_FAILED(UTF8toACE(tld, tld, eStringPrepForDNS))) {
return false;
}
// truncate trailing dots first
tld.Trim(".");
int32_t pos = tld.RFind(".");
if (pos == kNotFound) {
return false;
}
tld.Cut(0, pos + 1);
bool safe;
if (NS_SUCCEEDED(mIDNWhitelistPrefBranch->GetBoolPref(tld.get(), &safe))) {
return safe;
}
}
return false;
}
bool nsIDNService::isLabelSafe(const nsAString& label) {
if (!NS_IsMainThread()) {
mLock.AssertCurrentThreadOwns();

View File

@ -99,7 +99,6 @@ class nsIDNService final : public nsIIDNService,
nsresult ACEtoUTF8(const nsACString& input, nsACString& _retval,
stringPrepFlag flag);
bool isInWhitelist(const nsACString& host);
void prefsChanged(const char* pref);
static void PrefChanged(const char* aPref, void* aSelf) {
@ -167,8 +166,7 @@ class nsIDNService final : public nsIIDNService,
mozilla::UniquePtr<mozilla::intl::IDNA> mIDNA;
// We use this mutex to guard access to:
// |mIDNBlocklist|, |mShowPunycode|, |mRestrictionProfile|,
// |mIDNUseWhitelist|, |mIDNWhitelistPrefBranch|.
// |mIDNBlocklist|, |mShowPunycode|, |mRestrictionProfile|
//
// These members can only be updated on the main thread and
// read on any thread. Therefore, acquiring the mutex is required
@ -200,10 +198,6 @@ class nsIDNService final : public nsIIDNService,
// guarded by mLock;
restrictionProfile mRestrictionProfile MOZ_GUARDED_BY(mLock){
eASCIIOnlyProfile};
// guarded by mLock;
nsCOMPtr<nsIPrefBranch> mIDNWhitelistPrefBranch MOZ_GUARDED_BY(mLock);
// guarded by mLock
bool mIDNUseWhitelist MOZ_GUARDED_BY(mLock) = false;
};
#endif // nsIDNService_h__

View File

@ -22,10 +22,6 @@ var prefData = [
name: "network.IDN_show_punycode",
newVal: false,
},
{
name: "network.IDN.whitelist.ch",
newVal: true,
},
];
function run_test() {

View File

@ -24,9 +24,6 @@ function expected_fail(inputIDN) {
}
function run_test() {
// add an IDN whitelist pref
Services.prefs.setBoolPref("network.IDN.whitelist.com", true);
idnService = Cc["@mozilla.org/network/idn-service;1"].getService(
Ci.nsIIDNService
);

View File

@ -26,10 +26,6 @@ let prefData = [
name: "network.IDN_show_punycode",
newVal: false,
},
{
name: "network.IDN.whitelist.org",
newVal: true,
},
];
let prefIdnBlackList = {

View File

@ -24,12 +24,6 @@ function expected_fail(inputIDN) {
}
function run_test() {
// add an IDN whitelist pref
var pbi = Services.prefs;
var whitelistPref = "network.IDN.whitelist.com";
pbi.setBoolPref(whitelistPref, true);
idnService = Cc["@mozilla.org/network/idn-service;1"].getService(
Ci.nsIIDNService
);
@ -51,9 +45,4 @@ function run_test() {
// code point assigned since Unicode 3.0
// XXX This test will unexpectedly pass when we update to IDNAbis
expected_fail("foo\u0370bar.com");
// reset the pref
if (pbi.prefHasUserValue(whitelistPref)) {
pbi.clearUserPref(whitelistPref);
}
}

View File

@ -126,13 +126,11 @@ function run_test() {
"network.IDN.restriction_profile",
"moderate"
);
var oldWhitelistCom = pbi.getBoolPref("network.IDN.whitelist.com", false);
var idnService = Cc["@mozilla.org/network/idn-service;1"].getService(
Ci.nsIIDNService
);
pbi.setCharPref("network.IDN.restriction_profile", "moderate");
pbi.setBoolPref("network.IDN.whitelist.com", false);
for (var j = 0; j < testcases.length; ++j) {
var test = testcases[j];
@ -166,6 +164,5 @@ function run_test() {
}
}
}
pbi.setBoolPref("network.IDN.whitelist.com", oldWhitelistCom);
pbi.setCharPref("network.IDN.restriction_profile", oldProfile);
}

View File

@ -385,14 +385,12 @@ function run_test() {
"network.IDN.restriction_profile",
"moderate"
);
var oldWhitelistCom = pbi.getBoolPref("network.IDN.whitelist.com", false);
var idnService = Cc["@mozilla.org/network/idn-service;1"].getService(
Ci.nsIIDNService
);
for (var i = 0; i < profiles.length; ++i) {
pbi.setCharPref("network.IDN.restriction_profile", profiles[i]);
pbi.setBoolPref("network.IDN.whitelist.com", false);
dump("testing " + profiles[i] + " profile");
@ -434,6 +432,5 @@ function run_test() {
}
}
}
pbi.setBoolPref("network.IDN.whitelist.com", oldWhitelistCom);
pbi.setCharPref("network.IDN.restriction_profile", oldProfile);
}