anonymizer_tool is too aggressive |
||
Issue descriptionIn feedback report 85213778257 I see: 2018-03-21T12:33:21.103975+00:00 ERR shill[1175]: [ERROR:l2tp_ipsec_driver.cc(149)] Not implemented reached in virtual bool shill<IPv6: 1>L2TPIPSecDriver<IPv6: 10>laimInterface(const st<IPv6: 5>string &, int) I think it is a mistake to assume that every valid character sequence that matches the IPv6 regexp is an IPv6 address, because that will catch "::", "::C", "d::", and many other harmless strings. Suggest checking to see if the suspected IPv6 address has at least (say) 4 unique bytes before redacting it. Also, can we exempt 0.0.0.0, 255.255.255.255, and 224.x.x.x-239.x.x.x from the filters too?
,
Mar 28 2018
The other thing I see (on IPv4) is: 03-28 13:40:59.784 113 162 D ConnectivityService: Setting DNS servers for network 101 to [/<IPv4: 13>, /<IPv4: 13>53, /<IPv4: 14>3, /<IPv6: 18>, /<IPv6: 19>, /<IPv6: 20>] I can't tell for sure, but I think we are getting strange results if one IP address is a substring of another. e.g. "172.16.255.1, 172.16.255.153" -> "<IPv4: 1>, <IPv4: 1>53"
,
Apr 16 2018
In https://cs.chromium.org/chromium/src/components/feedback/anonymizer_tool.cc?sq=package:chromium #define DEC_OCTET NCG("[0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-9]") #define IPV4ADDRESS DEC_OCTET "\\." DEC_OCTET "\\." DEC_OCTET "\\." DEC_OCTET If DEC_OCTET is only supposed to match 0-255, should "25[0-9]" be "25[0-5]" instead? It looks like the final DEC_OCTET is only matching the first digit. anonymizer_tool_unittest.cc says that "255.255.155.255" is supposed to translate to "<IPv4: 1>55". This seems surprising to me; I would be inclined to test for a 3-digit match first, then fall back to 2-digit, then to 1-digit.
,
Apr 16 2018
Here's my first attempt at fixing a couple of the issues: https://chromium-review.googlesource.com/q/topic:%22anontool-825357%22+(status:open%20OR%20status:merged) Not sure on the best approach for the IPv4 whitelist. Options might include: - Try to mangle the IPV4ADDRESS regexp to exclude the interesting addresses. - Add a callback or an explicit whitelist to the kCustomPatternsWithoutContext array. - Add a whitelist somewhere else.
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e3328208d2f48621f74162d9a09fb2f3a12511b1 commit e3328208d2f48621f74162d9a09fb2f3a12511b1 Author: Kevin Cernekee <cernekee@chromium.org> Date: Thu Apr 19 03:51:29 2018 AnonymizerTool: Match longer IPv4 octets before shorter ones The current implementation of DEC_OCTET, given an input of "255", would only match "2". This means that an IPv4 address of "255.255.255.255" would be turned into "<IPv4: 1>55". Change the regexp so that it prefers the longer match. BUG=825357 TEST=components_unittests Change-Id: Ie86f10f75990a552b94d31cb8c1a59bfcaf580a2 Reviewed-on: https://chromium-review.googlesource.com/1013233 Reviewed-by: Dominic Battré <battre@chromium.org> Commit-Queue: Kevin Cernekee <cernekee@chromium.org> Cr-Commit-Position: refs/heads/master@{#551939} [modify] https://crrev.com/e3328208d2f48621f74162d9a09fb2f3a12511b1/components/feedback/anonymizer_tool.cc [modify] https://crrev.com/e3328208d2f48621f74162d9a09fb2f3a12511b1/components/feedback/anonymizer_tool_unittest.cc
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/70b0dfbeb8256287ec8294fa821a7f752cbe3547 commit 70b0dfbeb8256287ec8294fa821a7f752cbe3547 Author: Kevin Cernekee <cernekee@chromium.org> Date: Thu Apr 19 05:40:29 2018 AnonymizerTool: Don't match invalid IPv4 octets Currently the regexp matches 0-259. Change it to match 0-255. BUG=825357 TEST=components_unittests Change-Id: Ia0ef4440185b03fa5e0fd4dae484da66b81c7b5e Reviewed-on: https://chromium-review.googlesource.com/1013234 Reviewed-by: Dominic Battré <battre@chromium.org> Commit-Queue: Kevin Cernekee <cernekee@chromium.org> Cr-Commit-Position: refs/heads/master@{#551952} [modify] https://crrev.com/70b0dfbeb8256287ec8294fa821a7f752cbe3547/components/feedback/anonymizer_tool.cc [modify] https://crrev.com/70b0dfbeb8256287ec8294fa821a7f752cbe3547/components/feedback/anonymizer_tool_unittest.cc
,
Apr 19 2018
cernekee@ Thanks for improving this tool. Please take ownership of this issue.
,
Nov 20
|
||
►
Sign in to add a comment |
||
Comment 1 by afakhry@chromium.org
, Mar 28 2018Owner: afakhry@chromium.org
Status: Assigned (was: Untriaged)