Closed
Bug 1209843
Opened 9 years ago
Closed 9 years ago
Throw in the towel on killing UNKNOWN_APP_ID
Categories
(Core :: Security: CAPS, defect)
Core
Security: CAPS
Tracking
()
RESOLVED
FIXED
mozilla44
People
(Reporter: bholley, Assigned: bholley)
Details
Attachments
(1 file)
(deleted),
patch
|
sicking
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
UNKNOWN_APP_ID indicates that the caller created a principal without having a good idea of what the appId should be. The idea was that we'd eventually stamp it out by incrementally making more of the core machinery reject it.
When I added the OriginAttributes machinery, the possibility of encountering UNKNOWN_APP_ID seemed like an annoying reason to make everything fallible, so I tried to just release-assert against that case, figuring that we could fix any crash reports that came in.
In retrospect, this was clearly a mistake. UNKNOWN_APP_ID is too easy to generate from script (which addons seem to do copiously), and checking for it in all the API entry points is too error prone to get right. Bug 1209587, bug 1171432, bug 1205456, bug 1182610, and bug 1172542 are all testaments to the failure of this strategy.
Given that the new security model involves eliminating appId in the long run, this doesn't feel like the right battle to fight. In the modern world, we _really_ want to battle the consumers that create arbitrary OriginAttributes without inheriting them from the proper thing. We need a strategy for that, but this isn't it, so let's stop paying the price for it.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8667678 -
Flags: review?(jonas)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8667678 -
Flags: review?(jonas) → review+
Comment 4•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8667678 [details] [diff] [review]
Stop checking for UNKNOWN_APP_ID in all places except those where AppId() is explicitly queried. v1
Approval Request Comment
[Feature/regressing bug #]: bug 1165162
[User impact if declined]: A bunch of unnecessary release-mode crashes (See all the bugs about crashing in CreateSuffix).
[Describe test coverage new/current, TreeHerder]: None
[Risks and why]: Low risk - this is basically just removing a bunch of manual crashes.
[String/UUID change made/needed]: None
Attachment #8667678 -
Flags: approval-mozilla-beta?
Attachment #8667678 -
Flags: approval-mozilla-aurora?
Comment on attachment 8667678 [details] [diff] [review]
Stop checking for UNKNOWN_APP_ID in all places except those where AppId() is explicitly queried. v1
Let's take this on aurora and beta. Improving the crash rate sounds good.
Attachment #8667678 -
Flags: approval-mozilla-beta?
Attachment #8667678 -
Flags: approval-mozilla-beta+
Attachment #8667678 -
Flags: approval-mozilla-aurora?
Attachment #8667678 -
Flags: approval-mozilla-aurora+
Comment 7•9 years ago
|
||
status-firefox43:
--- → fixed
Comment 8•9 years ago
|
||
status-firefox42:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•