GrapheneOS / os-issue-tracker

Issue tracker for GrapheneOS Android Open Source Project hardening work. Standalone projects like Auditor, AttestationServer and hardened_malloc have their own dedicated trackers.
https://grapheneos.org/
348 stars 19 forks source link

[Feature request] Allow more DNS-over-HTTPS providers #3827

Open kovdan01 opened 1 month ago

kovdan01 commented 1 month ago

It looks like that currently only certain DoH providers are allowed: see the following chunk of code in https://cs.android.com/android/platform/superproject/+/master:packages/modules/DnsResolver/PrivateDnsConfiguration.h;drc=3aea8db351212126ee1420598090057c258f8335;l=259

    // TODO: Move below DoH relevant stuff into Rust implementation.
    std::map<unsigned, DohIdentity> mDohTracker GUARDED_BY(mPrivateDnsLock);
    std::array<DohProviderEntry, 5> mAvailableDoHProviders = {{
            {"Google",
             {"2001:4860:4860::8888", "2001:4860:4860::8844", "8.8.8.8", "8.8.4.4"},
             "dns.google",
             "https://dns.google/dns-query",
             false},
            {"Google DNS64",
             {"2001:4860:4860::64", "2001:4860:4860::6464"},
             "dns64.dns.google",
             "https://dns64.dns.google/dns-query",
             false},
            {"Cloudflare",
             {"2606:4700::6810:f8f9", "2606:4700::6810:f9f9", "104.16.248.249", "104.16.249.249"},
             "cloudflare-dns.com",
             "https://cloudflare-dns.com/dns-query",
             false},

            // The DoH providers for testing only.
            // Using ResolverTestProvider requires that the DnsResolver is configured by someone
            // who has root permission, which should be run by tests only.
            {"ResolverTestProvider",
             {"127.0.0.3", "::1"},
             "example.com",
             "https://example.com/dns-query",
             true},
            {"AndroidTesting",
             {"192.0.2.100"},
             "dns.androidtesting.org",
             "https://dns.androidtesting.org/dns-query",
             false},
    }};

It would be nice to also allow other DoH providers, like NextDNS.

The logic behind choosing between DoH and DoTLS does not actually look trivial (see calls to setDot and setDoh from PrivateDnsConfiguration::set in https://cs.android.com/android/platform/superproject/+/master:packages/modules/DnsResolver/PrivateDnsConfiguration.cpp;drc=bf52841c57f05b9caff97c80d2d86d59ecf609e4;l=115), and maybe it's even better to just have two separate options in private DNS configuration: one for those who want to use DoT, and one for DoH.

Actually, one field (as right now) should be enough, and choice between DoH/DoT can be made just because resolver addresses look different (in case of NextDNS, https://dns.nextdns.io/username vs username.dns.nextdns.io). I'm not sure why it's not implemented this way right now since it seems pretty straightforward. This way, probably there would be no need for "white list" of allowed DoH resolvers, and usage of any resolver would be possible (if I'm not missing smth).

Thanks for your great work on GrapheneOS!

kovdan01 commented 1 month ago

It looks like that a similar issue was opened some time ago #1376. As far as I understood, it was not resolved yet.

This new issue can be used to track work on allowing any DoH provider and getting rid of whitelist at all. I suppose it might not be the highest priority, and I'll be happy to try to implement that by myself if I have enough time.