Add UMA metric for DNS domain name length |
|||
Issue descriptionIt appears that DnsRecordParser::ReadName() does not enforce a size limit on incoming data. This may have reared its head in issue 811618 -- my working theory is that pathological input of the form "a.a.a.a..." cause O(N) string append operations, resulting in O(N^2) behavior. A malicious DNS server could potentially stall Chromium's IO thread. The relevant RFCs (1034, 1035) specify a max size of 255 octets for the dotted representation of a domain name. I would like to enforce this limit, or something based on it (e.g. 4*255). The first step towards fixing this is to add a UMA metric that tracks the domain name lengths that Chrome sees in the wild. More discussion: https://groups.google.com/a/chromium.org/forum/#!topic/net-dev/h5FM5kR70IM
,
Nov 21
I added a UMA metric to DnsRecordParser::ReadRecord(), but I don't see this new entry appearing in chrome://histograms.
Mysteries to resolve:
(1) chrome://net-internals is very bare compared to my officially-branded Google Chrome browser.
(2) I am looking for a way to force the internal DNS resolver.
See https://cs.chromium.org/chromium/src/services/network/network_context.cc?sq=package:chromium&g=0&l=1224
,
Nov 21
Mystery (1) resolved. Upon closer inspection, the page actually says, "The net-internals events viewer and related functionality has been removed."
,
Nov 26
WRT (2) in comment 2: the internal resolver defaults to enabled on Android, Mac OSX, and ChromeOS: https://cs.chromium.org/chromium/src/chrome/common/chrome_features.cc?type=cs&q=asyncdns+-file:third_party+-file:net/&sq=package:chromium&g=0&l=80 I'd be happy to help you build and use Chrome on Android.
,
Nov 26
Aha! Thank you! I know how to build for Android and run unit tests on a device. I will stop by sometime in case you have some other tricks to share :)
,
Nov 28
For future reference, this is how you enable the internal DNS resolver in desktop Chrome: out/Default/chrome --enable-features="AsyncDns"
,
Nov 29
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/27d04cc4b253a82f7f3f0fb71de122d4e051bbf1 commit 27d04cc4b253a82f7f3f0fb71de122d4e051bbf1 Author: Daniel McArdle <dmcardle@chromium.org> Date: Fri Nov 30 15:46:41 2018 Add UMA metric for domain name length New metric is Net.DNS.RecordParser.DomainNameLength Bug: 905823 Change-Id: I910ea0bf2629ee2e4663acf998a50debdbed5445 Reviewed-on: https://chromium-review.googlesource.com/c/1340819 Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: David Benjamin <davidben@chromium.org> Commit-Queue: Daniel McArdle <dmcardle@chromium.org> Cr-Commit-Position: refs/heads/master@{#612657} [modify] https://crrev.com/27d04cc4b253a82f7f3f0fb71de122d4e051bbf1/net/dns/dns_response.cc [modify] https://crrev.com/27d04cc4b253a82f7f3f0fb71de122d4e051bbf1/tools/metrics/histograms/histograms.xml
,
Nov 30
|
|||
►
Sign in to add a comment |
|||
Comment 1 by dmcardle@chromium.org
, Nov 15