Robert Helmer [:rhelmer] Assignee
	Description • 6 years ago

We want to do a gradual roll-out of TLS 1.3 to the release channel.

This is controlled using a "security.tls.version.max" pref, which is currently 3 (TLS 1.2) on release and 4 (TLS 1.3) on beta/nightly.

The plan is to use a system add-on (SAO) update, and to initially roll out to 10% of users. Due to limitations around the throttling feature on the (balrog) server-side, the easiest route is to make the determination of whether or not to enable TLS 1.3 inside the add-on.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 1 • 6 years ago

A couple more issues here.  rhelmer, Can you make a PI request (https://mana.mozilla.org/wiki/display/PI/PI+Request) for QE to test once we have the add-on and balrog configuration?  

ekr:  will there be a blog post or anything else (say, on MDN) about the change that we can link to from release notes? I'd like to include this in release notes even though we're rolling out the change gradually. It helps to have some deeper explanation available.

	Robert Helmer [:rhelmer] Assignee
	Comment 2 • 6 years ago

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #1)
> A couple more issues here.  rhelmer, Can you make a PI request
> (https://mana.mozilla.org/wiki/display/PI/PI+Request) for QE to test once we
> have the add-on and balrog configuration?  


Done.

	Robert Helmer [:rhelmer] Assignee
	Comment 3 • 6 years ago

Jason, could you please sign this system add-on update? Thanks!

	:wezhou
	Comment 4 • 6 years ago

Signed file attached. Please test.

	Paul Theriault [:pauljt] (no longer reading bugmail)
	Comment 5 • 6 years ago

(In reply to :wezhou from comment #4)
> Created attachment 8956175 [details]
> signed.8956147.xpi
> 
> Signed file attached. Please test.

Quick question: With the System Addon Update (SAO) process, does bootstrap.js run once ? I assume so - otherwise this code seems to be a 10% chance to enable TLS1.3 every time you startup, which doesn't sound like what we want from comment 0. Just checking...

	Robert Helmer [:rhelmer] Assignee
	Comment 6 • 6 years ago

(In reply to Paul Theriault [:pauljt] from comment #5)
> (In reply to :wezhou from comment #4)
> > Created attachment 8956175 [details]
> > signed.8956147.xpi
> > 
> > Signed file attached. Please test.
> 
> Quick question: With the System Addon Update (SAO) process, does
> bootstrap.js run once ? I assume so - otherwise this code seems to be a 10%
> chance to enable TLS1.3 every time you startup, which doesn't sound like
> what we want from comment 0. Just checking...

This does run on every startup, but the seed used is always the string (client ID + the addon ID), both of which persist across browser sessions.

That said, I don't think there's any reason we couldn't move this to the install method so it just runs once. The decision shouldn't ever change across different startups unless we change the addon ID, in which case it'll require an install of a new add-on anyway.

	Paul Theriault [:pauljt] (no longer reading bugmail)
	Comment 7 • 6 years ago

(In reply to Robert Helmer [:rhelmer] from comment #6)
> (In reply to Paul Theriault [:pauljt] from comment #5)
> > (In reply to :wezhou from comment #4)
> > > Created attachment 8956175 [details]
> > > signed.8956147.xpi
> > > 
> > > Signed file attached. Please test.
> > 
> > Quick question: With the System Addon Update (SAO) process, does
> > bootstrap.js run once ? I assume so - otherwise this code seems to be a 10%
> > chance to enable TLS1.3 every time you startup, which doesn't sound like
> > what we want from comment 0. Just checking...
> 
> This does run on every startup, but the seed used is always the string
> (client ID + the addon ID), both of which persist across browser sessions.
> 
> That said, I don't think there's any reason we couldn't move this to the
> install method so it just runs once. The decision shouldn't ever change
> across different startups unless we change the addon ID, in which case it'll
> require an install of a new add-on anyway.

Ah ok, makes sense. Whatever think is a fair thing - I didn't realize the seed was persistent.

	Robert Helmer [:rhelmer] Assignee
	Comment 8 • 6 years ago

(In reply to Paul Theriault [:pauljt] from comment #7)
> (In reply to Robert Helmer [:rhelmer] from comment #6)
> > (In reply to Paul Theriault [:pauljt] from comment #5)
> > > (In reply to :wezhou from comment #4)
> > > > Created attachment 8956175 [details]
> > > > signed.8956147.xpi
> > > > 
> > > > Signed file attached. Please test.
> > > 
> > > Quick question: With the System Addon Update (SAO) process, does
> > > bootstrap.js run once ? I assume so - otherwise this code seems to be a 10%
> > > chance to enable TLS1.3 every time you startup, which doesn't sound like
> > > what we want from comment 0. Just checking...
> > 
> > This does run on every startup, but the seed used is always the string
> > (client ID + the addon ID), both of which persist across browser sessions.
> > 
> > That said, I don't think there's any reason we couldn't move this to the
> > install method so it just runs once. The decision shouldn't ever change
> > across different startups unless we change the addon ID, in which case it'll
> > require an install of a new add-on anyway.
> 
> Ah ok, makes sense. Whatever think is a fair thing - I didn't realize the
> seed was persistent.

ekr just reminded me that we *do* want to use startup() here, because the pref doesn't stick around across restarts (it's using the "default" branch, so if the add-on doesn't work for any reason, or is removed by external software etc, the pref will revert to the same as we shipped for 59)

So, that means the XPI on this bug is still good to go.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 9 • 6 years ago

Noting that we landed the patch to disable TLS 1.3 by default on release for the RC2 build, in bug 1442033.

	Robert Helmer [:rhelmer] Assignee
	Comment 12 • 6 years ago

New version of the TLS 1.3 roll-out system add-on for signing. Thanks!

	Robert Helmer [:rhelmer] Assignee
	Comment 14 • 6 years ago

QA found a problem when testing this - ClientID.getClientID() returns a Promise which wasn't being `await`ed (it's used as part of the seed for determining whether or not to activate TLS 1.3 for the desired percentage of users)

This is fixed in the latest version of the add-on (2.0)

	:wezhou
	Comment 15 • 6 years ago

:rhelmer, do you need attachment 8958864 [details] to be signed? I'm a little confused because it says v1 but there is also saying that something is fixed in 2.0.

	Robert Helmer [:rhelmer] Assignee
	Comment 16 • 6 years ago

After discussing w/ ekr, we've decided to roll this out to 1% of users first.

Since this decision is being made inside the add-on, we'll need to re-sign. I've bumped the version # as well, to 3.0

Thanks!

	Robert Helmer [:rhelmer] Assignee
	Comment 17 • 6 years ago

Sorry, one last minor change to disable debug logging.

	:wezhou
	Comment 18 • 6 years ago

Signed file attached. Please test.

	Robert Helmer [:rhelmer] Assignee
	Comment 19 • 6 years ago

Rehan, could you please host attachment 8959646 [details] on the system add-on test channel? Thanks!

	Rehan Dalal (Inactive)
	Comment 20 • 6 years ago

This has been shipped on the test channel (release-sysaddon).

	Rehan Dalal (Inactive)
	Comment 21 • 6 years ago

The rules were also created for the release channel and are waiting for sign off from relman.

rhelmer has confirmed it worked as expected on the test channel.

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 22 • 6 years ago

Does this have a QA sign-off? Without that it's hard for relman to give a green signal.

	Hani Yacoub, Desktop QA
	Comment 23 • 6 years ago

Following the steps from comment 10 I tested on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12 on Beta 59.0 build 5(10-Mar-2018) and everything went as expected, in "about:support" the add-on is displayed under "Firefox Features" and in "about:config" perf "security.tls.version.max" is set to 4.

Note: I could not verify that the add-on is active only for 1% of profiles.
Thanks.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 24 • 6 years ago

Let's go with it, since we've used this same add-on before in test rollouts.  However, I think that in general, testing should check the functionality of what the add-on does (for example we had a system add-on that changed a pref value from one number to another, tested by looking at that value, but the value turned out to be a string not an int and we had to re-release) 

I'll sign off on the update rules in balrog now, though.

	Robert Helmer [:rhelmer] Assignee
	Comment 25 • 6 years ago

Telemetry query showing the roll-out of this add-on using main_summary. We expect ~1% of clients with the add-on installed to actually be making TLS 1.3 connections, which is what this is showing so far.

I've set the query to update every 12 hours.

	Robert Helmer [:rhelmer] Assignee
	Comment 26 • 6 years ago

Please sign this system add-on update XPI.

Changes from the previous version are:

* activates for 5% of clients (previously 1%)
* bump version number to 5.0

	:wezhou
	Comment 27 • 6 years ago

Signed file attached. Please test.

	Robert Helmer [:rhelmer] Assignee
	Comment 28 • 6 years ago

Comment on attachment 8960826 [details]
TLS 1.3 roll-out SAO v5 (signed)

Rehan, could you please stage this and put on the test channel? Thanks!

	Rehan Dalal (Inactive)
	Comment 29 • 6 years ago

This is now on the test channel.

	Robert Helmer [:rhelmer] Assignee
	Comment 30 • 6 years ago

(In reply to Rehan Dalal [:rehan, :rdalal] from comment #29)
> This is now on the test channel.

Thanks! Tested this locally on latest 59 (macOS) and inspected the hosted XPI versus the previous one we shipped (excluding the signature files):

diff -r tmp1/bootstrap.js tmp2/bootstrap.js
18c18
< const ENABLE_PROB = 0.01;
---
> const ENABLE_PROB = 0.05;
diff -r tmp1/install.rdf tmp2/install.rdf
5c5
<     <em:version>4.0</em:version>
---
>     <em:version>5.0</em:version>

Thanks Rehan! Can you also set this up on the release channel so it's pending relman approval?

Liz, the only change here is we've updated the probability that TLS 1.3 will be active, from 1% -> 5%, and we're watching it in telemetry (per comment 25). If you want us to do any other testing please let me know, given the above I think we're probably good to go.

	Rehan Dalal (Inactive)
	Comment 31 • 6 years ago

I've set up the rules on release and it's pending relman signoff.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 32 • 6 years ago

I signed off on this earlier in balrog, so we can go to 5%.

	Robert Helmer [:rhelmer] Assignee
	Comment 33 • 6 years ago

Version 6.0, enable TLS 1.3 for 10% of release population.

Please sign as a system add-on update, thanks!

	:wezhou
	Comment 34 • 6 years ago

Signed file attached. Please test.

	Robert Helmer [:rhelmer] Assignee
	Comment 35 • 6 years ago

Comment on attachment 8961623 [details]
TLS 1.3 roll-out SAO v6 (signed)

Could you please set this up on the test channel? Thanks!

	Rehan Dalal (Inactive)
	Comment 36 • 6 years ago

This is on the test channel and pending sign off on release.

	Robert Helmer [:rhelmer] Assignee
	Comment 37 • 6 years ago

(In reply to Rehan Dalal [:rehan, :rdalal] from comment #36)
> This is on the test channel and pending sign off on release.

Liz, can you please sign-off? As before I verified that the only changes are the probability and version #:

diff -r tmp1/bootstrap.js tmp2/bootstrap.js
18c18
< const ENABLE_PROB = 0.05;
---
> const ENABLE_PROB = 0.10;
diff -r tmp1/install.rdf tmp2/install.rdf
5c5
<     <em:version>5.0</em:version>
---
>     <em:version>6.0</em:version>

	Julien Cristau [:jcristau] (back April 22)
	Comment 38 • 6 years ago

I signed off so we will be going at 10%, but probably only after bug 1448377 is resolved.

	Robert Helmer [:rhelmer] Assignee
	Comment 39 • 6 years ago

Version 7.0, which sets the enable probability to 20%

Could you please sign this as a system add-on update? Thanks!

	:wezhou
	Comment 40 • 6 years ago

Signed file attached. Please test.

	Robert Helmer [:rhelmer] Assignee
	Comment 41 • 6 years ago

Comment on attachment 8962527 [details]
TLS 1.3 roll-out SAO v7 (signed)

Rehan, could you please host on test channel and set up on release for relman to sign-off? Thanks!

	Rehan Dalal (Inactive)
	Comment 42 • 6 years ago

This is on the test channel and pending sign off from relman on release.

	Rehan Dalal (Inactive)
	Comment 43 • 6 years ago

I ni'ed :lizzard but I'm gonna cancel since rhelmer should probably confirm it is all good on the test channel before sign off

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 44 • 6 years ago

Since we are just bumping the % and re-signing on the same channel, I went ahead and signed off.

	Robert Helmer [:rhelmer] Assignee
	Comment 45 • 6 years ago

Could you please sign this as a system add-on update? Thanks!

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 46 • 6 years ago

Just let me know when you have this on balrog for me to sign off again.

	:wezhou
	Comment 47 • 6 years ago

Signed file attached. Please test.

This file is signed using the autograph service. Please let us and Julien V. know if there are issues.

	Rehan Dalal (Inactive)
	Comment 48 • 6 years ago

This is on the test channel (release-sysaddon) and pending sign off on release.

	Robert Helmer [:rhelmer] Assignee
	Comment 49 • 6 years ago

(In reply to Rehan Dalal [:rehan, :rdalal] from comment #48)
> This is on the test channel (release-sysaddon) and pending sign off on
> release.

Liz, I've checked and the only difference between the previous extension are the version number (7 -> 8) and the chance of enabling (20% -> 50%). ekr and I have been watching telemetry and everything is going as expected so far.

Please let me know if there's anything I'm missing, otherwise it's ready for your sign-off. Thanks!

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 50 • 6 years ago

I signed off in balrog just now, so this should be rolling out today.

	Robert Helmer [:rhelmer] Assignee
	Comment 51 • 6 years ago

Could you please sign this system add-on update? Thanks!

	:wezhou
	Comment 52 • 6 years ago

Signed file attached. Please test.

	Robert Helmer [:rhelmer] Assignee
	Comment 53 • 6 years ago

Comment on attachment 8969127 [details]
TLS 1.3 roll-out SAO v9 (signed) - 95%

Rehan, could you please put this up on the release-sysaddon test channel and also stage for release? Thanks!

	Rehan Dalal (Inactive)
	Comment 54 • 6 years ago

This is live on release-sysaddon and pending sign off on release.

	Eric Rescorla (:ekr)
	Comment 55 • 6 years ago

I'm double-checking the statistics now to make sure we are good. I should know later today, but please don't pull the trigger till then.

	Rehan Dalal (Inactive)
	Comment 56 • 6 years ago

I've already queued it up on the release channel. But it will not go out till relman signs off, which won't happen unless specifically requested. Is that okay?

	Eric Rescorla (:ekr)
	Comment 57 • 6 years ago

WFM

	Bill his name
	Comment 58 • 6 years ago

FYI

The "TLS 1.3 gradual roll-out" add-on briefly appeared here in my ff AMO, then vanished again.

Intriguingly, it is stamped [Legacy] (shurely some mistake? - given the dates)

It has no author link, no further-info links, no comments, no stars and is presumably hidden most of the time (not in AMO search)
but from recall there iS an option to remove it.  (I wonder how many 'cleaners' would remove it anyway?)


So, now I know. Isn't obscurity wonderful.

I often lament that there seems to be no mechanism for mozilla to impose useful information on the user, perhaps some restrained broadcast via tweak on the start-up pages.

	Eric Rescorla (:ekr)
	Comment 59 • 6 years ago

Bill, thanks for the heads-up.

It actually is a legacy add-on (it's not a Web Extension), but it's not something we expect people to install. This sort of clumsy way of rolling out pref flips will be going away in the next few releases though.

	Eric Rescorla (:ekr)
	Comment 60 • 6 years ago

Rehan, please deploy

	Rehan Dalal (Inactive)
	Comment 61 • 6 years ago

Liz, could you please provide relman sign off on scheduled change 880 (rule id: 789). Thanks!

	Julien Cristau [:jcristau] (back April 22)
	Comment 62 • 6 years ago

Signed off now so this is live for 59.x.

ekr, for 60 we've built release candidates with security.tls.version.max=4 and security.tls.version.fallback-limit=3, is that ok or do we need a last minute change there?

	Eric Rescorla (:ekr)
	Comment 63 • 6 years ago

Julien: Good question. Given the time frame, and the fact that we want to dial up fallback-limit on release anyway, I feel like it might be easier to just do this with add-ons, but I'm open to other people's opinions.

	Martin Thomson [:mt:]
	Comment 64 • 6 years ago

max=4, fallback=3 is ideal.  We're 100% on max, but want to progressively work up to 100% on fallback (meaning that we want to eliminate it, of course).  The RC seems right.

	Sylvestre Ledru [:Sylvestre]
	Comment 65 • 6 years ago

Moving to p3 because no activity for at least 24 weeks.

	Eric Rescorla (:ekr)
	Comment 66 • 6 years ago

Release already has fallback disabled.