New issue
Advanced search Search tips

Issue 914402 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 16
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 846423



Sign in to add a comment

support querying non-address types in HostResolver

Project Member Reported by ericorth@chromium.org, Dec 12

Issue description

Breaking off as a sub-bug of crbug.com/846423 as that bug is fairly overloaded at this point.  But a lot of the work for this has already been done there.

Primary motivation is servicification.  Design doc:
https://docs.google.com/document/d/1NmADJX00oRe9TxFCJTWl8ofdfyR22pbKT8-5Ad5lBsU/edit#heading=h.7nki9mck5t64

Summary relevant to this issue:
For servicification, we decided to create a single unified mojo API to handle all host resolution (anything querying a hostname for DNS-style results), rather than mirroring the multiple inconsistent APIs previously available in the C++ API in //net/dns. General strategy is to create the new and improved API in HostResolver and mirror in mojo rather than completely diverging mojo and C++ APIs.

Specific to non-address results, various DNS/MDNS usage (currently directly interacting with private instances of DnsClient and MdnsClient) in Chrome that need to be migrated to calling network service need support for TXT, PTR, and SRV results.

Secondary motivation: Some upcoming security work (certificate transparency and ESNI) is expected to need stronger non-address DNS support, so it makes further sense to improve our general support for it rather than do some mojo-API-specific hack.

General plan:
*Fix and generalize various assumptions within HostResolver that previously assume results are always AddressList (work already complete in crbug.com/846423).
*Add non-address support to the HostResolver interface with a DnsQueryType parameter and non-address Getters on the request object (work already complete in crbug.com/846423).
*Add handling and DnsQueryType values for specific non-address types that we need, TXT, PTR, and SRV, and lay the groundwork to make it easy to add any additional types in the future (work in-progress).
 
How are PTR records currently fetched?
Current method for querying non-address types: Caller creates its own private DnsClient or MdnsClient instance (along with all the redundant logic to make that work outside HostResolverImpl) and has its own code to parse the DnsResponse result.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9db8d232067ce7a5df5dbef28072ab18f80442a

commit e9db8d232067ce7a5df5dbef28072ab18f80442a
Author: Eric Orth <ericorth@chromium.org>
Date: Mon Jan 14 21:24:45 2019

Add PTR query support to HostResolver.

Follows the same pattern as the recent addition of TXT support.

Filters root domain from results because it is not well-handled by
our host resolution stack (eg, we do not allow querying the root
domain), and there is no expected Chrome usecase where we would need
such results.

Bug:  914402 
Change-Id: I14ab2f97e12270c866148329d32bc5660c401e8c
Reviewed-on: https://chromium-review.googlesource.com/c/1370719
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622608}
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/net/dns/dns_test_util.cc
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/net/dns/dns_test_util.h
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/net/dns/dns_util.cc
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/net/dns/host_cache.h
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/net/dns/host_resolver_impl_unittest.cc
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/net/dns/host_resolver_mdns_task.cc
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/net/dns/public/dns_query_type.h
[modify] https://crrev.com/e9db8d232067ce7a5df5dbef28072ab18f80442a/services/network/public/cpp/host_resolver_mojom_traits.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a625b045dd4a13d26b6c10374f53a83e310c7e04

commit a625b045dd4a13d26b6c10374f53a83e310c7e04
Author: Eric Orth <ericorth@chromium.org>
Date: Wed Jan 16 01:14:45 2019

Add SRV query support to HostResolver.

Follows the same pattern as the recent addition of TXT and PTR support.

Like PTR support, filters root domain from results because it is not
well-handled by our host resolution stack (eg, we do not allow querying
the root domain), and there is no expected Chrome usecase where we
would need such results.

Bug:  914402 
Change-Id: Ie4ff8ffc3d79a81fa5c1e9a3b3868415401b09d2
Reviewed-on: https://chromium-review.googlesource.com/c/1374729
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622958}
[modify] https://crrev.com/a625b045dd4a13d26b6c10374f53a83e310c7e04/net/dns/dns_test_util.cc
[modify] https://crrev.com/a625b045dd4a13d26b6c10374f53a83e310c7e04/net/dns/dns_test_util.h
[modify] https://crrev.com/a625b045dd4a13d26b6c10374f53a83e310c7e04/net/dns/dns_util.cc
[modify] https://crrev.com/a625b045dd4a13d26b6c10374f53a83e310c7e04/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/a625b045dd4a13d26b6c10374f53a83e310c7e04/net/dns/host_resolver_impl_unittest.cc
[modify] https://crrev.com/a625b045dd4a13d26b6c10374f53a83e310c7e04/net/dns/host_resolver_mdns_task.cc
[modify] https://crrev.com/a625b045dd4a13d26b6c10374f53a83e310c7e04/net/dns/public/dns_query_type.h
[modify] https://crrev.com/a625b045dd4a13d26b6c10374f53a83e310c7e04/services/network/public/cpp/host_resolver_mojom_traits.cc

Comment 5 by ericorth@chromium.org, Jan 16 (6 days ago)

Status: Fixed (was: Started)

Sign in to add a comment