Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dns/server: blacklist rfc2606 domains in the resolver #544

Closed

Conversation

turbomaze
Copy link
Contributor

Reference: https://tools.ietf.org/id/draft-chapin-rfc2606bis-00.html#registry

There are a small handful of domains that are commonly used for internal DNS that ICANN has never delegated. They make up a significant portion of DNS traffic because all those internal DNS queries leak to resolvers they're not supposed to. Handshake has already blacklisted a few of these domains from being registered (see covenants/rules.js), but there are others that have not been blacklisted.

Perhaps this was an intentional choice, and if so, I'm curious to discuss it. But assuming it was just an oversight, this PR blacklists the remaining names in the referenced RFC draft.

@coveralls
Copy link

coveralls commented Jan 27, 2021

Pull Request Test Coverage Report for Build 516470833

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 5 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.02%) to 59.586%

Files with Coverage Reduction New Missed Lines %
lib/covenants/rules.js 1 67.45%
lib/protocol/consensus.js 1 82.79%
lib/net/pool.js 3 32.49%
Totals Coverage Status
Change from base Build 512602537: -0.02%
Covered Lines: 19508
Relevant Lines: 30511

💛 - Coveralls

@pinheadmz
Copy link
Member

pinheadmz commented Jan 28, 2021

some of these strings are already blacklisted at the consensus level:

rules.blacklist = new Set([
'example', // ICANN reserved
'invalid', // ICANN reserved
'local', // mDNS
'localhost', // ICANN reserved
'test' // ICANN reserved
]);

The strings in the dns server can actually still be acquired on chain (I think they are all reserved though) whereas the names in the rules.js blacklist are invalid and can not be opened for auction or claimed, ever.

We could add the remaining names to the list in the resolver though like you have in this PR. We couldn't add anything else in rules.js because I think that would be a hard fork.

@turbomaze
Copy link
Contributor Author

Yep definitely can't touch the consensus/rules.js file. I think that the ideal version of this is an additional DNS blacklist that gets enabled by switching on a config flag. This way people that run resolvers don't need to get their logs flooded with all the constant errors caused by requests to these names (the linked draft estimates that it's a considerable % of DNS traffic).

@pinheadmz
Copy link
Member

Out of curiosity I wanted to determine who and how much this change would impact Handshake users. Of the names from the RFC, half are already blacklisted. Two have been sold, three are reserved and one... fantastically... has been assigned by ICANN despite being on this list! Maybe @dnsguru has some insight to that.

name status
example blacklisted in rules.js
invalid blacklisted in rules.js
local blacklisted in rules.js
localhost blacklisted in rules.js
test blacklisted in rules.js
corp sold for 10,000 HNS
localdomain sold for 111 HNS, has HNS root zone records but they go nowhere
domain reserved for domain.com.au.
home reserved for home.pl.
lan reserved for lan.com.
host reserved for ICANN TLD host.

@brandondees
Copy link

I think I might have captured localdomain, can't verify atm, but I've been making a little effort to grab these kinds of things when possible to prevent them from becoming vehicles for abuse and to be able to e.g. comment here and say "I am the affected party, block them" or "I have a sane use case to justify unblocking" (I don't but it was a thought)

@dnsguru
Copy link

dnsguru commented Feb 3, 2021

Out of curiosity I wanted to determine who and how much this change would impact Handshake users. Of the names from the RFC, half are already blacklisted. Two have been sold, three are reserved and one... fantastically... has been assigned by ICANN despite being on this list! Maybe @dnsguru has some insight to that.

RFC2606 from the year 1999 was replaced by RFC6761 in the year 2013, so if this project is going to work with RFCs on names, it is important to use the most recent versions of them.

As TLDs that had been approved in the 2012 round were starting to be added to the root nameservers, ICANN (intended to mean the whole of the ICANN stakeholder community + ICANN staff) paid close attention to the potential for things that might break stuff.

One of the areas that had some attention was called 'Name Collision'. In plain terms this was careful measurement, assessment and planning for where existing devices and networks might have some namespaces being used before a TLD that matched it gets lit up on the root.

There was significant work done by ICANN.org and the ICANN community on Name Collision, and they assembled a humbling pantheon of ivory tower-worthy experts from accross a diversity of perspectives, including security professionals and firms.

Without relitigating that stuff, they took some random sources of historic root zone traffic logs from a few of the root name servers to see what kinds of TLD traffic was hitting those nameservers and getting an NXD.

.host was found to fall below a threshold

@pinheadmz
Copy link
Member

@dnsguru thanks - so looking at RFC6761, the only names I see specified are:
test, localhost, invalid, and example -- which are already on the consensus blacklist (along with a bonus local, perhaps from a previous draft?)

Jothan what is your opinion on this pull request?

@dnsguru
Copy link

dnsguru commented Feb 4, 2021

This should consider #455 #208 and #255 from this repo and would want to also wrap in
handshake-org/hs-names#6 & handshake-org/hs-names#8

@turbomaze
Copy link
Contributor Author

This should consider #455 #208 and #255 from this repo and would want to also wrap in
handshake-org/hs-names#6 & handshake-org/hs-names#8

Thanks for those thoughts above + all these references to relevant issues! I think a broader refactor of the dns server is warranted that adds plugin functionality + more fine grained control of the DNS blacklist. I don't have capacity for that at the moment but maybe soon

@pinheadmz
Copy link
Member

I think this goes in a larger category earmarked in #541
There's all kinds of resolver settings that can be set with a plugin, if we just make server.js more flexible. All the issues @dnsguru mentioned plus some other greatest hits like "resolve ICANN root before HNS root". Then we can issue a standard child-proof plugin that hackers can easily disable. I'd feel better about that than just expanding the blacklist for everyone using hsd.

@dnsguru
Copy link

dnsguru commented Feb 5, 2021 via email

@brandondees
Copy link

I concur with the consensus here that plugins/configurability are gonna be necessary longer term and this kind of thing might as well be deferred until it can be resolved with that kind of approach. We definitely have a spectrum of users with different preferences in terms of prioritizing HNS or ICANN or other potentially conflicting systems like ENS, Butterfly, etc. or using greylisting for individual names, and those options will only multiply.

@pinheadmz
Copy link
Member

Please see https://github.com/pinheadmz/holdmyhand which requires update to hsd (review requested!) #558

@pinheadmz
Copy link
Member

I'm going to close this now because, as the mathematicians like to say "the solution exists". #558 is merged, and the holdmyhand plugin has been released. Users or services interested in applying extra protection can do so in the applicaiton layer (i.e. with a plugin) and so I don't think this is a relevant issue to hsd anymore.

@pinheadmz pinheadmz closed this Mar 13, 2021
@nodech nodech deleted the anthony-blacklist-rfc2606 branch April 20, 2023 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants