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

resolved: Mitigate DVE-2018-0001, by retrying NXDOMAIN without EDNS0. #8608

Open
wants to merge 1 commit into
base: master
from

Conversation

@xnox
Copy link
Member

xnox commented Mar 28, 2018

Some captive portals, lie and do not respond with the captive portal IP
address, if the query is with EDNS0 enabled and D0 bit set to zero. Thus retry
"secure" domain name look ups with less secure methods, upon NXDOMAIN.

Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/systemd/+bug/1727237
Bug-DNS: https://github.com/dns-violations/dns-violations/blob/master/2018/DVE-2018-0001.md

@yuwata yuwata added the resolve label Mar 29, 2018
@xnox xnox force-pushed the xnox:DVE-2018-0001 branch 2 times, most recently from 7abcaa9 to 26a6c6b May 31, 2018
@xnox

This comment has been minimized.

Copy link
Member Author

xnox commented May 31, 2018

So without this, one cannot resolve and auth with the captive portal at Starbucks in the US. They appear to be deploying aruba networks captive portals at the moment.

Between asserting broken DNS behavior, and coffee.... coffee wins.

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jun 8, 2018

Urks, so this is of course frickin ugly, but I guess if this is what we need to do this is what we need to do...

Copy link
Member

poettering left a comment

OK, sounds conceptually OK, but please make the two indicated changes: jump down to UDP mode in one jump please.

And add a check t->scope->dnssec_mode != DNSSEC_YES so that we don't second guess DNSSEC replies in strict DNSSEC mode.

It kinda sucks that this means in permissive DNSSEC mode we'll never be able do do DNSSEC for NXDOMAIN anymore though

@xnox

This comment has been minimized.

Copy link
Member Author

xnox commented Jun 13, 2018

I've tested this against a DNSSEC signed domain, in permissive mode, and it was asserted correctly. The captive portals in question, simply do not know how to rewrite ends0/dnssec queries and pass them through unmodified.

Thus to access the captive portal, one must request/resolve something in a non-dnssec-signed domain, e.g. like start.ubuntu.com or the whatever network-manager uses, and with this fallback alone one experiences correct behaviour.

Thus i'm not sure the extra check t->scope->dnssec_mode != DNSSEC_YES is needed. Unless I am missing something... Does that flip to NO, for the given transaction, if the domain in question is not DNSSEC signed at all? Cause if the domain in question is not dnssec enabled, it should be fine to second guess it, no?

Imho we should still assert DNSSEC for NXDOMAIN in permissive mode for dnssec signed domains, and the users simply must access something non-dnssec-signed to get to the captive portal. Thoughts?

@poettering

This comment has been minimized.

Copy link
Member

poettering commented Jun 13, 2018

If people enable DNSSEC strict mode they basically say "fuck captive portals". It's a no-compromise mode, enabled by folks who do not want to compromise on security, but captive portals by their nature really are a compromise on security since they generally mean rewriting DNS and/or HTTP.

I don't think anyone is helped if we'd second guess NXDOMAIN in strict DNSSEC mode. It sounds like something the security nerds who care about strict DNSSEC mode would just be pissed about, and hence not worth doing. I mean, if you pick DNSSEC strict mode you are in for a hard time anyway... And regular people would never pick DNSSEC strict mode in the first place...

In an ideal world, NetworkManager's captive portal detection would tell resolved to turn off DNSSEC on that interface until connectivity is verified at which point it should be turned on (if the user said so on that interface).

@jrb0001

This comment has been minimized.

Copy link

jrb0001 commented Oct 6, 2018

@xnox: That check is definitely required. Ubuntu bionic seems to use the same patch and it produces SERVFAIL responses if DNSSEC=yes and the upstream returns a NXDOMAIN. The log is spammed with those retry messages followed by "DNSSEC validation failed for question test.asdf IN SOA: incompatible-server" for the SOA and DS of every segment which resulted in a NXDOMAIN.

See also https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1796501.

@ofosos

This comment has been minimized.

Copy link

ofosos commented Dec 19, 2018

Apparently Ubuntu 18 pulled this patch into their build. As far as I can tell this patch will lead to a reported DNS violation for every request that gets answered with NXDOMAIN and it will then downgrade from EDNS to plain DNS and try it again. Is this correct?

I wish there was a solution that is less obnoxious in production, i.e. something that doesn't dump a lot of messages in a log for a failed dns query.

@jrb0001

This comment has been minimized.

Copy link

jrb0001 commented Dec 19, 2018

@ofosos yes, it will retry with reduced "feature level" until plain DNS and then fail with SERVFAIL because it can't do DNSSEC at that level anymore.

@xnox xnox force-pushed the xnox:DVE-2018-0001 branch 2 times, most recently from a1a35a3 to fa0f317 Jan 25, 2019
Some captive portals, lie and do not respond with the captive portal IP
address, if the query is with EDNS0 enabled and DO bit set to zero. Thus retry
all domain name look ups with less secure methods, upon NXDOMAIN. Unless strict
DNSSEC validation is enabled.

Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/systemd/+bug/1766969
Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/systemd/+bug/1727237
Bug-DNS: https://github.com/dns-violations/dns-violations/blob/master/2018/DVE-2018-0001.md
(cherry picked from commit cc0a0eb)
@xnox xnox force-pushed the xnox:DVE-2018-0001 branch from fa0f317 to 50b9974 Jan 26, 2019
@xnox

This comment has been minimized.

Copy link
Member Author

xnox commented Jan 26, 2019

@poettering requested changes are now done. Drops straight to UDP, and doesn't do anything if script DNSSEC mode is on.

@BryanQuigley

This comment has been minimized.

Copy link

BryanQuigley commented Mar 2, 2019

+1 to updated patch - definite improvement.

I am curious if we could restrict how often it runs more - say only on a new network, or special case something else about arubanetworks.

To that end, was arubanetworks.com contacted? Possible fixes on their end? Happy to reach out if not.

@BryanQuigley

This comment has been minimized.

Copy link

BryanQuigley commented Apr 23, 2019

Can anyone rerun the autopkgtests - I'm pretty sure the failure was unrelated to the PR.

@BryanQuigley

This comment has been minimized.

Copy link

BryanQuigley commented Sep 19, 2019

I've been running this updated patch in Ubuntu 19.10 Dev release with my PPA: https://launchpad.net/~bryanquigley/+archive/ubuntu/1796501/+packages Everything seems to be working fine (DNSSEC=yes for me).

I did report the issue to one of Starbuck's vendors, but have not heard back yet.

@ddstreet

This comment has been minimized.

Copy link
Contributor

ddstreet commented Dec 11, 2019

This patch has been included in Ubuntu for a while, and the log messages have widely been reported as annoying:
https://www.google.com/search?q=mitigating+potential+dns+violation

And since its assumption (dns violation) is actually incorrect in the vast majority of cases (i.e. for everyone not using a broken captive portal), it's highly misleading.

Additionally, the dns retry appears to be causing other problems with delays in dns resolution:
https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1785383

I haven't yet had time to look at a proper way of working around broken captive portals, but I would not recommend applying this patch in its current form.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.