774949 - Do not validate JARs by default, provide async APIs to do so

Status: RESOLVED DUPLICATE of bug 726125

(Core :: Networking: JAR, defect)

Description

[(dormant account)]

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

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I agree. It may be difficult to determine what security-critical components are relying on these implicit validation checks.

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 4

[(dormant account)]

(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.

Comment 5

[Nicolás Chaim]

Attachment: Preliminary patch

Comment 6

[Nicolás Chaim]

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

Comment 7

[Nicolás Chaim]

(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.

Comment 8

[Nicolás Chaim]

We already have 
- bug 726125, where validation in nsJARChannel is being removed
- bug 776060 - Provide Async API for validating JAR signatures