Hi Tor Browser devs,
I've been working on uplift this week and I have updated the uplift tracker table at https://torpat.ch .
There are bugzilla bugs for most of our eligible patches, but there are still about 15 "untriaged" patches in gray, because I couldn't be sure if they were appropriate to uplift. If you have time to have a look at the untriaged ones you are familiar with and let me know if you think any are available for uplift, I would be grateful. My goal is to mark all patches either red, yellow, or green so that no untriaged patches remain.
Thanks, Arthur
Whooo! I think I've told you before but this is SO helpful to me.
If I could nitpit - I think not-yet uplifted should be in red and no-uplift in yellow. (The latter is 'warning' the former is 'error'.)
21862 could be mapped to 1376621 13612 - can't find a reference but I believe we either disabled or removed this by default
Any reason we can't upstream some/all of those regression tests?
-tom
On 26 January 2018 at 19:06, Arthur D. Edelstein arthuredelstein@gmail.com wrote:
Hi Tor Browser devs,
I've been working on uplift this week and I have updated the uplift tracker table at https://torpat.ch .
There are bugzilla bugs for most of our eligible patches, but there are still about 15 "untriaged" patches in gray, because I couldn't be sure if they were appropriate to uplift. If you have time to have a look at the untriaged ones you are familiar with and let me know if you think any are available for uplift, I would be grateful. My goal is to mark all patches either red, yellow, or green so that no untriaged patches remain.
Thanks, Arthur _______________________________________________ tbb-dev mailing list tbb-dev@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tbb-dev
On Sat, Jan 27, 2018 at 9:46 AM, Tom Ritter tom@ritter.vg wrote:
Whooo! I think I've told you before but this is SO helpful to me.
If I could nitpit - I think not-yet uplifted should be in red and no-uplift in yellow. (The latter is 'warning' the former is 'error'.)
Thanks for the feedback. You're right, except I'm too used to yellow meaning uplift. I made no-uplift purple -- maybe that's better?
21862 could be mapped to 1376621
Done.
13612 - can't find a reference but I believe we either disabled or removed this by default
You're right -- I found at least some of the bugs and have listed them.
Any reason we can't upstream some/all of those regression tests?
I think you may be right about 18995 -- I have set it to untriaged. The other regression test patches are no-uplift because they are testing things I don't imagine we will uplift anyway. I could be wrong somewhere, though.
Arthur
Arthur D. Edelstein:
Hi Tor Browser devs,
I've been working on uplift this week and I have updated the uplift tracker table at https://torpat.ch .
Wow, thanks. FWIW something like that did I have in mind when asking about a list of patches and how they map to tickets etc. Although, this is way fancier than I imagined. Very helpful!
I did not look at the priorities set right now.
Mcs/brade: I might have got some of your update or profile dir related patches wrong. Please have a look especially at those if you have some time.
There are bugzilla bugs for most of our eligible patches, but there are still about 15 "untriaged" patches in gray, because I couldn't be sure if they were appropriate to uplift. If you have time to have a look at the untriaged ones you are familiar with and let me know if you think any are available for uplift, I would be grateful. My goal is to mark all patches either red, yellow, or green so that no untriaged patches remain.
Some comments regarding the patches after I went over the whole list:
1) According to Richard the patch for #23016 does not need uplift.
2) I am not sure whether we want to uplift #21431. I think we can leave it as "no uplift" for now.
3) f4dd994 could be "no uplift" right now.
5) #21907: "no uplift". There won't be an option to build with GTK2 anymore for ESR 60.
6) #21849: "no uplift" for now I think. See: bugs 1183318 and 1188657 for the Mozilla discussion.
7) Not sure if "uplifted" means "we are done here". If so, the patch for #5741 is in the wrong category. See: https://trac.torproject.org/projects/tor/ticket/21611#comment:5
8) #14970: "no uplift".
9) 5e2ac8a is already uplifted.
10) #13252: I am inclined to say "no uplift" as this patch only exists because we need to ship an own bundle (which we don't want to do if we are done with our Firefox fork).
11) #13379: probably "no uplift" (for now at least). At any rate we'd need to investigate first what we still need to carry over to ESR 60 here, given that https://bugzilla.mozilla.org/show_bug.cgi?id=1105689 got fixed.
12) #21724: "no uplift", the reasoning is the same as in 10).
13) #18912: seems worth trying to uplift.
14) #19121: it seems this is WONTFIX for Mozilla right now? I guess we keep it and make our argument later again? Or we argue along the lines of 10) and bite the bullet.
15) #18900: "uplift" according to https://bugzilla.mozilla.org/show_bug.cgi?id=1159090#c4 I guess.
16) #11641: "no uplift", the reasoning is the same as in 10).
17) #9173: I am not sure but after skimming over it, it seems like "no uplift" with reasoning like in 10)?
18) #18995: Seems worth uplifting to me (in case Mozilla does not have similar test already).
19) #3875: "no uplift" right now. I am more and more convinced we need a new patch for this idea (if at all) as the current one seems to be wrong. See: #19910 and above all dcf's amazing https://trac.torproject.org/projects/tor/ticket/24432#comment:19.
20) #5282: "no uplift". The whole pipelining code is gone and Mike is fine having our patch removed in that wake, too.
21) #16488: "no uplift" yet at least. We don't even have a proper patch ready right now: see #22564 for instance.
Do all new patches somehow block the META Tor uplift bug? (e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1432905? Shouldn't that block 1433504 at least?)
Georg
On 1/29/18 6:47 AM, Georg Koppen wrote:
Mcs/brade: I might have got some of your update or profile dir related patches wrong. Please have a look especially at those if you have some time.
See my comments below.
- #13252: I am inclined to say "no uplift" as this patch only exists
because we need to ship an own bundle (which we don't want to do if we are done with our Firefox fork).
Kathy and I agree this one should be "no uplift."
- #13379: probably "no uplift" (for now at least). At any rate we'd
need to investigate first what we still need to carry over to ESR 60 here, given that https://bugzilla.mozilla.org/show_bug.cgi?id=1105689 got fixed.
Agreed on "no uplift" but note that in 1105689 Mozilla went with SHA384 instead of SHA512 (it seems there are security reasons for that choice). We could switch to SHA384 but we will need to think through the migration issues.
- #18912: seems worth trying to uplift.
Agreed.
- #19121: it seems this is WONTFIX for Mozilla right now? I guess we
keep it and make our argument later again? Or we argue along the lines of 10) and bite the bullet.
This is the one we discussed yesterday.
- #18900: "uplift" according to
https://bugzilla.mozilla.org/show_bug.cgi?id=1159090#c4 I guess.
Kathy and I agree this should be uplifted but the patch may need some work (I think what we did regresses 1159090).
- #11641: "no uplift", the reasoning is the same as in 10).
Agreed.
- #9173: I am not sure but after skimming over it, it seems like "no
uplift" with reasoning like in 10)?
Agreed — no uplift.
-Mark
On 30 January 2018 at 10:16, Mark Smith mcs@pearlcrescent.com wrote:
- #13379: probably "no uplift" (for now at least). At any rate we'd
need to investigate first what we still need to carry over to ESR 60 here, given that https://bugzilla.mozilla.org/show_bug.cgi?id=1105689 got fixed.
Agreed on "no uplift" but note that in 1105689 Mozilla went with SHA384 instead of SHA512 (it seems there are security reasons for that choice). We could switch to SHA384 but we will need to think through the migration issues.
There is no real security benefit to 384 over 512; the folks who picked 384 figured "Well maybe length-extension-resistance is valuable" - but it's not.
-tom
On Mon, Jan 29, 2018 at 3:47 AM, Georg Koppen gk@torproject.org wrote:
Hi Georg,
Thanks for looking through the list! I addressed each of your points below. (Please see my questions below about 14 and 20.)
- According to Richard the patch for #23016 does not need uplift.
Oops, I remember he said that now. Fixed.
- I am not sure whether we want to uplift #21431. I think we can leave
it as "no uplift" for now.
OK, tagged is tbb-no-uplift-60
- f4dd994 could be "no uplift" right now.
Ditto.
- #21907: "no uplift". There won't be an option to build with GTK2
anymore for ESR 60.
Tagged as tbb-no-uplift.
- #21849: "no uplift" for now I think. See: bugs 1183318 and 1188657
for the Mozilla discussion.
Tagged as tbb-no-uplift-60.
- Not sure if "uplifted" means "we are done here". If so, the patch for
#5741 is in the wrong category. See: https://trac.torproject.org/projects/tor/ticket/21611#comment:5
I opened a new ticket for that.
- #14970: "no uplift".
Tagged as tbb-no-uplift.
- 5e2ac8a is already uplifted.
Actually, I'm not 100% sure the fix they made is the same fix. So I'm slightly nervous about marking it as uplifted. I have a NEEDINFO on bmo to look at it more closely.
- #13252: I am inclined to say "no uplift" as this patch only exists
because we need to ship an own bundle (which we don't want to do if we are done with our Firefox fork).
Tagged as tbb-no-uplift.
- #13379: probably "no uplift" (for now at least). At any rate we'd
need to investigate first what we still need to carry over to ESR 60 here, given that https://bugzilla.mozilla.org/show_bug.cgi?id=1105689 got fixed.
Tagged as tbb-no-uplift-60.
- #21724: "no uplift", the reasoning is the same as in 10).
Tagged as tbb-no-uplift.
- #18912: seems worth trying to uplift.
https://bugzilla.mozilla.org/show_bug.cgi?id=1434660
- #19121: it seems this is WONTFIX for Mozilla right now? I guess we
keep it and make our argument later again? Or we argue along the lines of 10) and bite the bullet.
I looked at our discussion yesterday but I don't really understand the what our patch is fixing. What's the advantage in doing a separate hash check if there is a signature verification (which presumably includes a hash check anyway)?
Tagging as tbb-no-uplift-60
- #18900: "uplift" according to
https://bugzilla.mozilla.org/show_bug.cgi?id=1159090#c4 I guess.
I opened https://bugzilla.mozilla.org/show_bug.cgi?id=1434666
- #11641: "no uplift", the reasoning is the same as in 10).
Tagged as tbb-no-uplift.
- #9173: I am not sure but after skimming over it, it seems like "no
uplift" with reasoning like in 10)?
Tagged as tbb-no-uplift.
- #18995: Seems worth uplifting to me (in case Mozilla does not have
similar test already).
I opened https://bugzilla.mozilla.org/show_bug.cgi?id=1434772
- #3875: "no uplift" right now. I am more and more convinced we need a
new patch for this idea (if at all) as the current one seems to be wrong. See: #19910 and above all dcf's amazing https://trac.torproject.org/projects/tor/ticket/24432#comment:19.
Tagged as tbb-no-uplift.
- #5282: "no uplift". The whole pipelining code is gone and Mike is
fine having our patch removed in that wake, too.
OK! Shall I remove it from my TBB-ESR60 branch? Also tagged as tbb-no-uplift.
- #16488: "no uplift" yet at least. We don't even have a proper patch
ready right now: see #22564 for instance.
I took another look at #22564 and I think there is some hope of uplifting after we complete it. So I opened https://bugzilla.mozilla.org/show_bug.cgi?id=1434706 for now.
Do all new patches somehow block the META Tor uplift bug? (e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1432905? Shouldn't that block 1433504 at least?)
Good point -- I'll work on blocking the meta_tor bug. And you're right about the ProxyBypass depending on 1432905 as well.
Arthur
Arthur D. Edelstein:
On Mon, Jan 29, 2018 at 3:47 AM, Georg Koppen gk@torproject.org wrote:
Hi Georg,
Thanks for looking through the list! I addressed each of your points below. (Please see my questions below about 14 and 20.)
[snip]
- #19121: it seems this is WONTFIX for Mozilla right now? I guess we
keep it and make our argument later again? Or we argue along the lines of 10) and bite the bullet.
I looked at our discussion yesterday but I don't really understand the what our patch is fixing. What's the advantage in doing a separate hash check if there is a signature verification (which presumably includes a hash check anyway)?
Okay, after a lot of digging I found what I was looking for. FWIW: it's not the same hash check that you do when verifying a signature. What is meant in our case is that the hash that you got via the update.xml file check is matching the hash of the acual MAR file.
The context for that discussion was:
https://trac.torproject.org/projects/tor/ticket/17442#comment:4
which was kind of a reply to https://bugzilla.mozilla.org/show_bug.cgi?id=1063111#c3 which argued that, especially due to legitimate MitM, only signature based verification should be used. However, we want to have at least two independent means that need to get compromised before fake updates can get applied. That's especially true in our current setup where we host the update.xml ourselves and Fastly holds all the actual update files. tjr made this point in the last meeting.
(Note, though, that we might want to think about strengthening both pillars we currently rely on for our update security but that is orthogonal to the question whether we want to enable the hash check or not)
[snip]
- #5282: "no uplift". The whole pipelining code is gone and Mike is
fine having our patch removed in that wake, too.
OK! Shall I remove it from my TBB-ESR60 branch?
Yes, please.
[snip]
Georg
On 5 February 2018 at 06:31, Georg Koppen gk@torproject.org wrote:
only signature based verification should be used. However, we want to have at least two independent means that need to get compromised before fake updates can get applied. That's especially true in our current setup where we host the update.xml ourselves and Fastly holds all the actual update files. tjr made this point in the last meeting.
That makes sense!
Since Tor Browser should never have to worry about Cert MITM, you could (maybe) pin Fastly's CA (if they support that) and get part-of-a-third.
(Note, though, that we might want to think about strengthening both pillars we currently rely on for our update security but that is orthogonal to the question whether we want to enable the hash check or not)
We're working on Binary Transparency! =)
-tom
-
Arthur D. Edelstein -
Georg Koppen -
Mark Smith -
Tom Ritter