Closed
Bug 774949
Opened 12 years ago
Closed 12 years ago
Do not validate JARs by default, provide async APIs to do so
Categories
(Core :: Networking: JAR, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 726125
People
(Reporter: taras.mozilla, Assigned: nicolas)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
It's silly to do extra validation just because a jar contains a manifest...every single time everything in it is accessed. It's even more silly to do so on the main thread, especially because validation involves network IO.
Thus we should change existing behavior to not do any manifest validation by default and to provide an opt-in api to do so any consumers that care can still make use of this functionality in a way that is sane.
Comment 1•12 years ago
|
||
I agree. It may be difficult to determine what security-critical components are relying on these implicit validation checks.
Comment 2•12 years ago
|
||
Actually, I remember why this is done this way. The idea is that the JAR file may be very large, and many of the files may go unused or may not be used right away (probably very true), so rather than verifying the whole JAR file once, this is supposed to lazily verify the contents of each file within the JAR as needed.
There are two parts of the verification:
1. The verification of the digital signature of some manifest file. This only needs to happen once per JAR file load. If this happens more than once then that's a bug, AFAICT. This is what will do the disk I/O and/or network I/O because it involves certificate processing.
2. Verifying the hash of each JAR entry as it's accessed. If this happens more than once per entry (i.e. we're not memoizing the results) then this is probably something that should be fixed. It shouldn't be nearly as bad of a problem as #1 because it's just SHA-1 over the entry, which shouldn't be a big deal except maybe for large entries.
There's another bug about doing #1 on the main thread and causing jank. We must definitely fix that.
Updated•12 years ago
|
Comment 3•12 years ago
|
||
Nicolás, I mentioned that I was working on part of this, but I've since changed the approach I'm using for signed B2G apps and I'm not working on anything related to this now.
Reporter | ||
Comment 4•12 years ago
|
||
(In reply to Brian Smith (:bsmith) from comment #1)
> I agree. It may be difficult to determine what security-critical components
> are relying on these implicit validation checks.
Security-critical and 'implicit' look a bit weird together. Given how we only do these checks for some files in the jar, but not others I have very little faith in the check-by-some-side-effect approach to security here.
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #648030 -
Flags: review?(vdjeric)
Assignee | ||
Comment 6•12 years ago
|
||
Example case (suggested by Taras and Vlad):
Install the "Element Hiding Helper for Ad Block Plus" and "Ad Block Plus" add-ons. Since the Element Hiding Helper add-on has a manifest and is signed, every time Firefox is started and the add-on is loaded, the certificated is verified. This generates a significant delay, since the status of the certificate is checked over the network via OCSP.
Call stacks (Main Thread):
ocsp_GetOCSPStatusFromNetwork: https://gist.github.com/9c03fdd5217dd556519b
Followed by fetchOcspHttpClientV1: https://gist.github.com/88dd4f1f831d4fdf27b7
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Brian Smith (:bsmith) from comment #2)
I saw both parts of the verification process work as expected while debugging with a jar file. Results per entry/manifest/certificate are cached and processing is only done once.
Assignee | ||
Updated•12 years ago
|
Attachment #648030 -
Flags: review?(vdjeric)
Assignee | ||
Comment 8•12 years ago
|
||
We already have
- bug 726125, where validation in nsJARChannel is being removed
- bug 776060 - Provide Async API for validating JAR signatures
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Updated•12 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•