1691547 - (CVE-2021-4127) Angle Security Backports for ESR-78

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[Tom Ritter [:tjr]]

Attachment: concerning-commits.txt

I'm trying to figure out what (if any, but seems likely some) issues we should consider backporting to ESR or Beta.

I ran a script that looks at all commits since origin/chromium/3865 (7cf862c9fcd2068a444eef6d4e8a0c1c2170d7f1) and got the following statistics:

Summary (4453 Commits Reviewed)
-----------------
Commit Message Keyword: crash: 110 issues identified
Bugtracker: Restricted Bug : 64 issues identified
Commit Message Keyword: ASAN: 15 issues identified
Commit Message Keyword: TSAN: 14 issues identified
Commit Message Keyword: overflow: 13 issues identified
Commit Message Keyword: fuzz: 12 issues identified
Bugtracker Keyword: Security_Severity-High: 4 issues identified
Bugtracker Keyword: Security_Severity-Medium: 3 issues identified
Commit Message Keyword: security: 2 issues identified
Bugtracker Keyword: Security_Impact: 1 issues identified
Commit Message Keyword: MSAN: 1 issues identified
Commit Message Keyword: UBSAN: 1 issues identified
Total: 240

I compared it with beta/esr78's cherry-picks file and there were no overlaps (AFAICT - it's possible the sha commit id's may not map exactly because we have our github repo?) This includes Bug 1676636 because there's nothing that identifies it as a security fix on angle's side :-/

I have to run for the day, but getting this bug on file before I go.

Comment 1

[Daniel Veditz [:dveditz]]

Our ANGLE has been updated to current as-of 87 nightly in bug 1690349 so I'll call this "fixed" for 87. I believe this is a bug about identifying the most important of those fixes to protect ESR-78 and Beta 86. ESR is really the biggest issue -- if we wait 86 turns into 87 and inherits the fixes, but ESR is just hanging out there.

Given the number of missing patches that we potentially have to backport it might be less work to upgrade the older ANGLE. On the other hand, in bug 1676636 comment 24 Jeff noted that we are affected by only a subset of ANGLE sec issues. I don't know how to identify that subset :-(. Is it anything simple we could incorporate into Tom's scanner, like "we don't care about patches in directory A, B, and F" ?

Comment 2

[Kelsey Gilbert [:jgilbert]]

It's difficult to tell whether an ANGLE issue is also a Firefox issue. These would require individual triage and investigation by graphics engineers most of the time. I think most of them are not Firefox issues, because we don't rely on ANGLE for (most) validation. There are some directories we can ignore, but ANGLE's still under active development in the areas we care about. (mostly perf work, some correctness fixes, mixed in with validation fixes that we do not care about)

There are on the order of 1000 commits between the ANGLE on 86 (and ESR78) and what's now on 87.

I don't think there's any good answer for robust sec backporting for ANGLE for ESR.

Options: (least to most work)

• Status quo: Generally treat sec issues found in esr reactively, not proactively
  • Backport obvious stuff that we do come across
• Whole-library update: Backport all of ANGLE to each ESR
  • Risks of regressions on ESR
  • Sort of the opposite of what ESR promises
  • We use ANGLE through its standardized OpenGL API though, so this should be easier than it sounds
• Chart a course to move ESR off of ANGLE, instead onto system drivers (that are presumably satisfactorily up-to-date, as much as the user cares to be)
  • We would need a software rendering library for Windows, which may put us in a similar predicament as with ANGLE (though probably fewer sec issues)
  • This would need one-time staffing
• Proactively evaluate ANGLE's commit log for sec issues we need to investigate and pick up
  • This would need ongoing staffing

For 86, all of the sec issues 86 has accrued prior to the update in 87 are already in 85, which is wontfix. If we care a lot, we should try taking the 87 update in 86 as well. We are probably OK just leaving 86 as-is so long as ESR is left as-is. If the 87 update has no issues as it rolls out to release, we can try to update ANGLE in esr, since that's the easiest way to catch up on fixes, though it's not without risks. If we change anything about ESR's WebGL behavior on accident, people using ESR in long-term deployments will be sad. We definitely get issues like this occasionally, and we have to tell people to fix their websites to not rely on the old behavior. This is probably less palatable for ESR?

That said, the status quo may not be awful: There is bounty incentive for people to do the work of finding fixed ANGLE vulns and hitting us in our ESR pinata for sec bounty payouts. As long as this covers the most egregious of the issues, it's probably a tolerable (if messy and unsatisfying) compromise. (but it's where we've ended up right now)

Comment 3

[Bobby Holley (:bholley)]

(In reply to Jeff Gilbert [:jgilbert] from comment #2)

For 86, all of the sec issues 86 has accrued prior to the update in 87 are already in 85, which is wontfix. If we care a lot, we should try taking the 87 update in 86 as well. We are probably OK just leaving 86 as-is so long as ESR is left as-is. If the 87 update has no issues as it rolls out to release, we can try to update ANGLE in esr, since that's the easiest way to catch up on fixes, though it's not without risks. If we change anything about ESR's WebGL behavior on accident, people using ESR in long-term deployments will be sad. We definitely get issues like this occasionally, and we have to tell people to fix their websites to not rely on the old behavior. This is probably less palatable for ESR?

I think it's fine to update ANGLE wholesale on ESR, for two reasons.

First, I think the chance of problematic breakage is low. The situations where entities relies on ESR to keep consistent behavior tend to be business-y use-cases, which are unlikely to use WebGL at all, let alone in a manner that would be broken by an ANGLE update.

Second, the goal of ESR is to trade new features for stability while maintaining security and a reasonable engineering cost envelope. To the extent that we have security fixes which we cannot deploy in a cost-effective way without risking compat, we're going to have to accept the compat risk.

Ted, as the current product owner for enterprise, do you have any concerns with the above?

Comment 4

[Ryan VanderMeulen [:RyanVM]]

I agree that we should leave 86 alone at this point since next week is RC week (bug 1690349 does graft cleanly, however). I took a quick look at the ESR backport and most of the conflicts are in moz.build files, which I guess isn't surprising since the last update was done in Fx72. I would assume Jeff could work through those conflicts without too much trouble if push came to shove.

One thing to note is that ESR is also our vehicle for EOL platforms, so it's not entirely an enterprise-only situation at this point.

Comment 5

[Ted Schuh]

From what I'm gleaning here - it seems Bobby's proposal is the path we should take (upgrade ANGLE).

Have we had any Enterprise customers complain about the security issues (as in FF doesn't pass their security audit and now it's at risk of being de-certified for their environment)?

Comment 6

[Kelsey Gilbert [:jgilbert]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #4)

I agree that we should leave 86 alone at this point since next week is RC week (bug 1690349 does graft cleanly, however). I took a quick look at the ESR backport and most of the conflicts are in moz.build files, which I guess isn't surprising since the last update was done in Fx72. I would assume Jeff could work through those conflicts without too much trouble if push came to shove.

One thing to note is that ESR is also our vehicle for EOL platforms, so it's not entirely an enterprise-only situation at this point.

All those build files are generated, so I can make an esr-specific patch largely just by re-running the import script. (nice and easy)

It sounds like that might be what we want, though we might want to coordinate, and wait to land an ANGLE update for ESR when our mainline update in 87 hits release?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

(In reply to Jeff Gilbert [:jgilbert] from comment #6)

It sounds like that might be what we want, though we might want to coordinate, and wait to land an ANGLE update for ESR when our mainline update in 87 hits release?

Yeah, if we're going to go the route of a full-scale update, let's aim to land it towards the end of the next cycle when the ANGLE update on 87 has had as much bake time as we can give it first. Obviously we can create the patches whenever in the mean time :)

Comment 8

[Daniel Veditz [:dveditz]]

setting the tracking flags to represent 87 per the above conversation. I'm a bit worried someone will pull out a year-old ANGLE bug to win Pwn2Own so I guess we can look through the bugs and cherry-pick a few into beta/ESR

Comment 9

[Kelsey Gilbert [:jgilbert]]

86 is out-of-process WebGL on Windows at least. They'll have to work for it in 86.
They also have to realize that we're that far behind and also do all the work of finding a vuln in angle that still applies.
It's a concatenation of a lot of "probably possible" that makes it increasingly unlikely, vs yet another jit/decoder vuln.
I'm not saying it can't happen, but this isn't the first time we've been behind on ANGLE.
I think we're just focused on it/worried right now because we've visibly realized it internally, but from a pragmatic standpoint we've been here before.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Fx87 will be the active release during Pwn2Own.

Comment 11

[Bobby Holley (:bholley)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #4)

One thing to note is that ESR is also our vehicle for EOL platforms, so it's not entirely an enterprise-only situation at this point.

Yes, but I think enterprise users are the only category of customers that might have unique cause to be upset if a hypothetical site-breaking change from Fx-Released were also shipped to ESR.

(In reply to Ted Schuh from comment #5)

Have we had any Enterprise customers complain about the security issues (as in FF doesn't pass their security audit and now it's at risk of being de-certified for their environment)?

Not to my knowledge, but that's really a question for your team. My operating model is that security is a higher constituency than edge-case compat for ESR, both because I think that's what customers want and also because I think that's the right thing to do.

Comment 12

[Ted Schuh]

Yes - in general I agree with you. Security issues are one of those things that can get enterprises to finally address those compatibility issues by updating old apps. It's better for us to address security issues before enterprises complain because when they do - it turns into a fire drill.

Comment 13

[Julien Cristau [:jcristau]]

If we want to do something for 78.9 here, that should happen soon. Jeff, AIUI the plan was to run the import script to update angle to the same version as 87?

Comment 14

[Kelsey Gilbert [:jgilbert]]

That's right.
I'll get that run.

Comment 15

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1691547 - Update ESR78's gfx/angle to match FF87.

Comment 16

[Kelsey Gilbert [:jgilbert]]

If bug 1692832 is a regression from the ANGLE update in 87, we'll need to take that fix at the same time.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR78 approval ASAP.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

As a quick update here, we had to take a couple MinGW toolchain updates on ESR78 to avoid breaking those builds, but we seem to be in the clear now. There's also macOS build bustage which appears to be caused by a missing header in the 10.11 SDK we build with on ESR78:
https://treeherder.mozilla.org/logviewer?job_id=332732685&repo=try&lineNumber=21570

Unfortunately, backporting the 10.12 SDK update seems like an awfully big change to take on the ESR branch, so Jeff is going to figure out a workaround for that one.

Comment 19

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1691547 - Don't assume macOS 10.12+ in gfx/angle/checkout/src/common/debug.cpp.

Comment 20

[Kelsey Gilbert [:jgilbert]]

I also needed bug 1672940 to build locally, else I got python errors because that old lib doesn't support python 3.9 on fully-up-to-date macs.

Comment 21

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 9207665 [details]
Bug 1691547 - Update ESR78's gfx/angle to match FF87.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: We've decided to update ANGLE wholesale to match our Release channel for ESR, rather than attempt to backport targeted changes to a many-months-old snapshot.
• User impact if declined: Probably security bugs from an increasingly out-of-date ANGLE vendoring on ESR.
• Fix Landed on Version: 87
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Some risk here. We do have tests for most aspects of the API, but it's a complicated API, and we can and do miss things. This means there's regression risk here.
  We decided that it's more important to prioritize security for ESR, rather than absolute compatibility/no-regressions.
• String or UUID changes made by this patch: none

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9207665 [details]
Bug 1691547 - Update ESR78's gfx/angle to match FF87.

Approved for 78.9esr.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/bc15dee26430
https://hg.mozilla.org/releases/mozilla-esr78/rev/e7cbd1f49221
https://hg.mozilla.org/releases/mozilla-esr78/rev/00a5d02fc431

Comment 24

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.