Network traffic annotation added to goolge_apis/gcm/engine.

Network traffic annotation added to goolge_apis/gcm/engine.

Network traffic annotation is added to network request of
google_apis/gcm/engine.

BUG=656607
Review-Url: https://codereview.chromium.org/2737433002
Cr-Commit-Position: refs/heads/master@{#458270}
Committed: https://chromium.googlesource.com/chromium/src/+/19ae4d04c84aa9453c35e3aa8a470045a3f675d1

[Ramin Halavati, 2017-03-06 08:17:01]

rhalavati@chromium.org changed reviewers:
+ dimich@chromium.org

[Ramin Halavati, 2017-03-06 08:17:02]

We are annotating all network requests in Chromium with a new
NetworkTrafficAnnotation scheme. This allows enterprise users of Chrome to audit
the requests and configure Chrome in a way that meets their security policies
and expectations. Furthermore, it allows us to generate better debugging data in
chrome://net-internals and measure bandwidth consumption on a per-request-type
basis.

I've modified 3 files related to gcm/engine and added annotation template to
them. Please review them and suggest the required answers for the empty parts
(marked with '...'). Please note that the claims should be thorough and covering
all cases. In case you believe that annotation should be passed to the modified
function instead of originating from it, please tell me to change the CL
accordingly.

Please take a look at the protobuf scheme in:
https://cs.chromium.org/chromium/src/tools/traffic_annotation/traffic_annotat...
for the definition of the annotations.

You can find a sample annotation in:
https://cs.chromium.org/chromium/src/components/spellcheck/browser/spelling_s...

Entriprise policy options are here:
https://cs.chromium.org/chromium/src/out/Debug/gen/components/policy/proto/ch...

And more description on enterprise policy settings is here:
http://dev.chromium.org/administrators/policy-list-3

Please tell me if you need any clarifications from my side. If you want to learn
more, see:
https://docs.google.com/document/d/1FWEFTHzdqIA1wHUo_DuSpJjppW22otCMPCOA5NumhJU.

[Dmitry Titov, 2017-03-08 00:15:12]

I see Peter Beverloo already did some of this for gcm... I think he might be a
better reviewer here, the annotations should be consistent...

[Ramin Halavati, 2017-03-08 09:20:21]

rhalavati@chromium.org changed reviewers:
+ peter@chromium.org

[Ramin Halavati, 2017-03-08 09:20:21]

Thank you Dimitry.


Peter,
Could you please suggest these Annotations as well? I repeat the resources here:

Please take a look at the protobuf scheme in:
https://cs.chromium.org/chromium/src/tools/traffic_annotation/traffic_annotat...
for the definition of the annotations.

You can find a sample annotation in:
https://cs.chromium.org/chromium/src/components/spellcheck/browser/spelling_s...

Entriprise policy options are here:
https://cs.chromium.org/chromium/src/out/Debug/gen/components/policy/proto/ch...

And more description on enterprise policy settings is here:
http://dev.chromium.org/administrators/policy-list-3

Please tell me if you need any clarifications from my side. If you want to learn
more, see:
https://docs.google.com/document/d/1FWEFTHzdqIA1wHUo_DuSpJjppW22otCMPCOA5NumhJU.

[Peter Beverloo, 2017-03-09 15:41:48]

Happy to fill these in - I'll reply tomorrow. (Yay perf.)

[Ramin Halavati, 2017-03-14 11:46:36]

On 2017/03/09 15:41:48, Peter Beverloo wrote:
> Happy to fill these in - I'll reply tomorrow. (Yay perf.)

Hi,

A friendly reminder on this.

[Peter Beverloo, 2017-03-15 18:58:04]

There's a lot of duplication here with gcm_channel_status_request.cc.

+zea for review since I don't own this code (though I probably should?)

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/chec...
File google_apis/gcm/engine/checkin_request.cc (right):

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/chec...
google_apis/gcm/engine/checkin_request.cc:148: net::NetworkTrafficAnnotationTag
traffic_annotation =
Proposal (gcm_checkin):

semantics {
  sender: "GCM Driver"
  description:
      "Google Chrome interacts with Google Cloud Messaging to receive "
      "push messages for various browser features, as well as on behalf "
      "of websites and extensions. The check-in periodically verifies "
      "the client's validity with Google servers, and receive updates "
      "to configuration regarding interacting with Google services."
  trigger:
      "Immediately after a feature creates the first Google Cloud "
      "Messaging registration. By default, Google Chrome will check in "
      "with Google Cloud Messaging every two days. Google can adjust "
      "this interval when it deems necessary."
  data:
      "The profile-bound Android ID and associated secret and account "
      "tokens. A structure containing the Google Chrome version, channel "
      "and platform of the host operating system."
  destination: GOOGLE_OWNED_SERVICE
}
policy {
  cookies_allowed: false
  setting:
      "Support for interacting with Google Cloud Messaging is enabled by "
      "default, and there is no configuration option to completely "
      "disable it. Websites wishing to receive push messages must "
      "acquire express permission from the user for the 'Notification' "
      "permission."
  policy_exception_justification:
      "Not implemented, considered not useful."
}

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/regi...
File google_apis/gcm/engine/registration_request.cc (right):

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/regi...
google_apis/gcm/engine/registration_request.cc:148:
net::NetworkTrafficAnnotationTag traffic_annotation =
Proposal (gcm_registration):

semantics {
  sender: "GCM Driver"
  description:
      "Google Chrome interacts with Google Cloud Messaging to receive "
      "push messages for various browser features, as well as on behalf "
      "of websites and extensions. This requests Google Cloud Messaging "
      "to create a new subscription through which messages can be sent "
      "to the registering entity, through Google Chrome."
  trigger:
      "Immediately after a feature, website or extension creates a new "
      "registration with the GCM Driver."
  data:
      "The profile-bound Android ID and associated secret, and the "
      "identifiers for the feature, website or extension that is "
      "creating the registration."
  destination: GOOGLE_OWNED_SERVICE
}
policy {
  cookies_allowed: false
  setting:
      "Support for interacting with Google Cloud Messaging is enabled by "
      "default, and there is no configuration option to completely "
      "disable it."
  policy_exception_justification:
      "Not implemented, considered not useful."
}

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/unre...
File google_apis/gcm/engine/unregistration_request.cc (right):

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/unre...
google_apis/gcm/engine/unregistration_request.cc:131:
net::NetworkTrafficAnnotationTag traffic_annotation =
Proposal (gcm_unregistration):

semantics {
  sender: "GCM Driver"
  description:
      "Google Chrome interacts with Google Cloud Messaging to receive "
      "push messages for various browser features, as well as on behalf "
      "of websites and extensions. This requests Google Cloud Messaging "
      "to invalidate the included registration so that it can no longer "
      "be used to distribute messages to Google Chrome."
  trigger:
      "Immediately after a feature, website or extension removes a "
      "registration they previously created with the GCM Driver."
  data:
      "The profile-bound Android ID and associated secret, and the "
      "identifiers for the feature, website or extension that is "
      "removing the registration."
  destination: GOOGLE_OWNED_SERVICE
}
policy {
  cookies_allowed: false
  setting:
      "Support for interacting with Google Cloud Messaging is enabled by "
      "default, and there is no configuration option to completely "
      "disable it."
  policy_exception_justification:
      "Not implemented, considered not useful."
}

[Peter Beverloo, 2017-03-15 19:00:30]

peter@chromium.org changed reviewers:
+ zea@chromium.org

[Peter Beverloo, 2017-03-15 19:00:30]

actually +zea

[Ramin Halavati, 2017-03-16 08:43:52]

Annotations updated, please review.

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/chec...
File google_apis/gcm/engine/checkin_request.cc (right):

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/chec...
google_apis/gcm/engine/checkin_request.cc:148: net::NetworkTrafficAnnotationTag
traffic_annotation =
On 2017/03/15 18:58:04, Peter Beverloo wrote:
> Proposal (gcm_checkin):
> 
> semantics {
>   sender: "GCM Driver"
>   description:
>       "Google Chrome interacts with Google Cloud Messaging to receive "
>       "push messages for various browser features, as well as on behalf "
>       "of websites and extensions. The check-in periodically verifies "
>       "the client's validity with Google servers, and receive updates "
>       "to configuration regarding interacting with Google services."
>   trigger:
>       "Immediately after a feature creates the first Google Cloud "
>       "Messaging registration. By default, Google Chrome will check in "
>       "with Google Cloud Messaging every two days. Google can adjust "
>       "this interval when it deems necessary."
>   data:
>       "The profile-bound Android ID and associated secret and account "
>       "tokens. A structure containing the Google Chrome version, channel "
>       "and platform of the host operating system."
>   destination: GOOGLE_OWNED_SERVICE
> }
> policy {
>   cookies_allowed: false
>   setting:
>       "Support for interacting with Google Cloud Messaging is enabled by "
>       "default, and there is no configuration option to completely "
>       "disable it. Websites wishing to receive push messages must "
>       "acquire express permission from the user for the 'Notification' "
>       "permission."
>   policy_exception_justification:
>       "Not implemented, considered not useful."
> }

Done.

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/regi...
File google_apis/gcm/engine/registration_request.cc (right):

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/regi...
google_apis/gcm/engine/registration_request.cc:148:
net::NetworkTrafficAnnotationTag traffic_annotation =
On 2017/03/15 18:58:04, Peter Beverloo wrote:
> Proposal (gcm_registration):
> 
> semantics {
>   sender: "GCM Driver"
>   description:
>       "Google Chrome interacts with Google Cloud Messaging to receive "
>       "push messages for various browser features, as well as on behalf "
>       "of websites and extensions. This requests Google Cloud Messaging "
>       "to create a new subscription through which messages can be sent "
>       "to the registering entity, through Google Chrome."
>   trigger:
>       "Immediately after a feature, website or extension creates a new "
>       "registration with the GCM Driver."
>   data:
>       "The profile-bound Android ID and associated secret, and the "
>       "identifiers for the feature, website or extension that is "
>       "creating the registration."
>   destination: GOOGLE_OWNED_SERVICE
> }
> policy {
>   cookies_allowed: false
>   setting:
>       "Support for interacting with Google Cloud Messaging is enabled by "
>       "default, and there is no configuration option to completely "
>       "disable it."
>   policy_exception_justification:
>       "Not implemented, considered not useful."
> }

Done.

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/unre...
File google_apis/gcm/engine/unregistration_request.cc (right):

https://codereview.chromium.org/2737433002/diff/1/google_apis/gcm/engine/unre...
google_apis/gcm/engine/unregistration_request.cc:131:
net::NetworkTrafficAnnotationTag traffic_annotation =
On 2017/03/15 18:58:04, Peter Beverloo wrote:
> Proposal (gcm_unregistration):
> 
> semantics {
>   sender: "GCM Driver"
>   description:
>       "Google Chrome interacts with Google Cloud Messaging to receive "
>       "push messages for various browser features, as well as on behalf "
>       "of websites and extensions. This requests Google Cloud Messaging "
>       "to invalidate the included registration so that it can no longer "
>       "be used to distribute messages to Google Chrome."
>   trigger:
>       "Immediately after a feature, website or extension removes a "
>       "registration they previously created with the GCM Driver."
>   data:
>       "The profile-bound Android ID and associated secret, and the "
>       "identifiers for the feature, website or extension that is "
>       "removing the registration."
>   destination: GOOGLE_OWNED_SERVICE
> }
> policy {
>   cookies_allowed: false
>   setting:
>       "Support for interacting with Google Cloud Messaging is enabled by "
>       "default, and there is no configuration option to completely "
>       "disable it."
>   policy_exception_justification:
>       "Not implemented, considered not useful."
> }

Done.

[Nicolas Zea, 2017-03-17 22:01:41]

lgtm

[Dmitry Titov, 2017-03-18 01:13:48]

lgtm

[Dmitry Titov, 2017-03-18 01:27:12]

https://codereview.chromium.org/2737433002/diff/40001/google_apis/gcm/engine/...
File google_apis/gcm/engine/registration_request.cc (right):

https://codereview.chromium.org/2737433002/diff/40001/google_apis/gcm/engine/...
google_apis/gcm/engine/registration_request.cc:160: "registration with the GCM
Driver."
Not sure if it makes any difference, but it can be said that the site/extensions
may not trigger multiple network requests here because once received, the
registration is cached locally and simply served locally even if site/extension
request it many times...

[Peter Beverloo, 2017-03-21 00:18:46]

https://codereview.chromium.org/2737433002/diff/40001/google_apis/gcm/engine/...
File google_apis/gcm/engine/registration_request.cc (right):

https://codereview.chromium.org/2737433002/diff/40001/google_apis/gcm/engine/...
google_apis/gcm/engine/registration_request.cc:160: "registration with the GCM
Driver."
On 2017/03/18 01:27:12, Dmitry Titov wrote:
> Not sure if it makes any difference, but it can be said that the
site/extensions
> may not trigger multiple network requests here because once received, the
> registration is cached locally and simply served locally even if
site/extension
> request it many times...

I think that'd be a good addition. Let me send you a separate CL tomorrow since
Ramin is OOO for another three weeks - that way we don't have to block this on
that.

[Peter Beverloo, 2017-03-21 00:19:10]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2017-03-21 00:20:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 01:37:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490055550955940,
"parent_rev": "1622188c142fb8f4564a3a8d35e9e734bc0c80e8", "commit_rev":
"19ae4d04c84aa9453c35e3aa8a470045a3f675d1"}

[commit-bot: I haz the power, 2017-03-21 01:38:11]

Description was changed from

==========
Network traffic annotation added to goolge_apis/gcm/engine.

Network traffic annotation is added to network request of
google_apis/gcm/engine.

BUG=656607
==========

to

==========
Network traffic annotation added to goolge_apis/gcm/engine.

Network traffic annotation is added to network request of
google_apis/gcm/engine.

BUG=656607

Review-Url: https://codereview.chromium.org/2737433002
Cr-Commit-Position: refs/heads/master@{#458270}
Committed:
https://chromium.googlesource.com/chromium/src/+/19ae4d04c84aa9453c35e3aa8a47...
==========

[commit-bot: I haz the power, 2017-03-21 01:38:12]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/19ae4d04c84aa9453c35e3aa8a47...