New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 825357 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

anonymizer_tool is too aggressive

Project Member Reported by cernekee@chromium.org, Mar 23 2018

Issue description

In 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?
 
Cc: r...@chromium.org
Owner: afakhry@chromium.org
Status: Assigned (was: Untriaged)
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"
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.
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.
Project Member

Comment 5 by bugdroid1@chromium.org, 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

Project Member

Comment 6 by bugdroid1@chromium.org, 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

Cc: afakhry@chromium.org
Owner: cernekee@chromium.org
Status: Started (was: Assigned)
cernekee@ Thanks for improving this tool. Please take ownership of this issue.
Cc: cernekee@chromium.org
Owner: garrick@chromium.org
Related: b/111048642.

Sign in to add a comment