1353148 - Extend Telemetry Histograms for COOKIE_SCHEME_SECURITY, FAMILY_SAFETY, MIXED_CONTENT_OBJECT_SUBREQUEST, , WEBRTC_GET_USER_MEDIA_SECURE_ORIGIN

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb]]

The following histograms will be expiring on 2017-04-17, and should be removed from the codebase, or have their expiry versions updated:

* COOKIE_SCHEME_SECURITY expires in version 55.0a1 (watched by [e-mail]) - How often are secure cookies set from non-secure origins, and vice-versa? 0=nonsecure/http, 1=nonsecure/https, 2=secure/http, 3=secure/https
* FAMILY_SAFETY expires in version 55.0a1 (watched by [e-mail]) - Status of Family Safety detection and remediation. See nsNSSComponent.cpp.
* MIXED_CONTENT_OBJECT_SUBREQUEST expires in version 55.0a1 (watched by [e-mail]) - How often objects load insecure content on secure pages (counting pages, not objects). 0=pages with no mixed object subrequests, 1=pages with mixed object subrequests
* WEBRTC_GET_USER_MEDIA_SECURE_ORIGIN expires in version 55.0a1 (watched by [e-mail]) - Origins for getUserMedia calls (0=other, 1=HTTPS, 2=file, 3=app, 4=localhost, 5=loop, 6=privileged)

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1353148_histogram_extension.patch

Dan, looking at the histograms that are about to expire, I think we want to renew all of those 4. Agreed?

Btw, do I need review from someone else besides you to get those histogram extensions landed?

Comment 2

[Daniel Veditz [:dveditz]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #1)
> Dan, looking at the histograms that are about to expire, I think we want to
> renew all of those 4. Agreed?

Agreed.

> Btw, do I need review from someone else besides you to get those histogram
> extensions landed?

No, just to add them in the first place (and maybe to change expiration to "never"?)

There are several CSP_ probes that are expiring at the same time:

CSP_DOCUMENTS_COUNT
CSP_UNSAFE_INLINE_DOCUMENTS_COUNT
CSP_UNSAFE_EVAL_DOCUMENTS_COUNT
CSP_REFERRER_DIRECTIVE (expires in 56)

Do you want to extend those as well? I'm not entirely sure what they count. Number of CSP page loads in a session?

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 8854136 [details] [diff] [review]
bug_1353148_histogram_extension.patch

Review of attachment 8854136 [details] [diff] [review]:
-----------------------------------------------------------------

Is extending these three releases (another 4.5 months) sufficient? While we're doing this let's grab another quarter and just push it all to 60. Either way please also push out COOKIE_LEAVE_SECURE_ALONE to match. r=dveditz

::: toolkit/components/telemetry/Histograms.json
@@ -8757,5 @@
>      "n_values": 10,
>      "releaseChannelCollection": "opt-out",
>      "description": "How often are secure cookies set from non-secure origins, and vice-versa? 0=nonsecure/http, 1=nonsecure/https, 2=secure/http, 3=secure/https"
>    },
>    "COOKIE_LEAVE_SECURE_ALONE": {

Please push COOKIE_LEAVE_SECURE_ALONE out to match the others

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1353148_histogram_extension.patch

Good catch Dan, I extended the CSP_* probes as well. I think someone told me that we should not use 'never' but I can't recall who it was. Anyway, I am using "60" now as a tradeoff. Extending thoses probes only takes minimum effort, so I think we can extend it again in FF60 if needed.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1353148_histogram_extension.patch

Ah sorry, for some reason I missed comment 3. Updated everything you requested. Carrying over your r+ (no need for additonal review).

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bfa8d0189e50
Extend Telemetry Histograms for COOKIE_*, FAMILY_SAFETY, MIXED_CONTENT_OBJECT_SUBREQUEST, WEBRTC_GET_USER_MEDIA_SECURE_ORIGIN and CSP_*. r=dveditz

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1353148_followup.patch

Comment 8

[François Marier [:francois]]

Comment on attachment 8854559 [details] [diff] [review]
bug_1353148_followup.patch

Review of attachment 8854559 [details] [diff] [review]:
-----------------------------------------------------------------

datareview+ for both patches

Comment 9

[Christoph Kerschbaumer [:ckerschb]]

(In reply to François Marier [:francois] from comment #8)
> datareview+ for both patches

Can someone please check in this second patch (the follow up?). I just chatted with francois over IRC and figured that I actually need a datareview and he pointed out that we should use '[e-mail]' as the email address - thanks!

Please note that the first patch already got checked in; see comment 6.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d46d033148b
Change email from seceng@ to seceng-telemetry@. r=francois

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bfa8d0189e50
https://hg.mozilla.org/mozilla-central/rev/3d46d033148b