209560 - (jar:https) Secure JS doesn't work from https JARs

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[Mike Kaply [:mkaply]]

Unfortunately I don't have an external site for this, but here is the basics:

jar:http://minardi.yamato.ibm.com/GeckoSignedScripts.jar!/GeckoSignedScripts.html

works

jar:https://minardi.yamato.ibm.com/GeckoSignedScripts.jar!/GeckoSignedScripts.html

doesn't.

This is a HUGE issue for an IBM product.

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Mike, can you describe the results you see? Does the page load at all? Does it
load but the signed script doesn't work?

Comment 2

[Doron Rosenberg (IBM)]

the contents of the html file is shown as an empty html file.  My secure server
however is down too now, trying to see if I can get it up.

My signed jar example (none secure site, nscp internal):

jar:http://elwood.mcom.com/doron2/icbc/icbcsignedjs.jar!/icbcsignedjs.html

Comment 3

[Mike Kaply [:mkaply]]

The error necko is simply a "NS_ERROR_NOT_AVAILABLE"

I'm going to do some logging.

Comment 4

[Mike Kaply [:mkaply]]

Attachment: nsHttp:5,nsSocketTransport:5 log

Here's a log of when the problem occurs

Comment 5

[Darin Fisher]

actually, the problem is that HTTPS content is not written to the disk cache,
and jar:http: only works when the content is written to the disk cache.  the
problem is that we need to first download the jar file to disk, and then we go
and load the jar file as if it were a normal file:// URL.  fixing this problem
will be very tricky.  ideally we'd need support for loading jar files from a
data stream.  or we'd have to bite-the-bullet and write HTTPS content out to
disk (which we generally avoid for privacy reasons).  this problem is very much
related to bug 172279.

Comment 6

[Darin Fisher]

NS_ERROR_NOT_AVAILABLE is generated from nsHttpChannel.cpp:3392.

Comment 7

[Mike Kaply [:mkaply]]

So signed Javascript and secure http are mutually exclusive? That seems really
bad...

Comment 8

[Darin Fisher]

it is... seems to me that the feature was just hacked together.  jar:http(s)://
really needs to be redesigned from the ground up because anyways the disk cache
might not be present in an embedding build.  if you're in a pinch and need a
quick solution, you might want to go the route of allowing HTTPS content to be
written out to the disk cache.  IE allows HTTPS content in its disk cache, so
its not exactly a crazy idea.

Comment 9

[Darin Fisher]

bug 205921 unfortunately needs to be fixed before caching of HTTPS content to
disk can be enabled.

Comment 10

[Darin Fisher]

Attachment: v0 patch

here's a possible patch for this bug.  what this does is make it so that we
will fall back on writing the data stream to a temporary file when the channel
cannot provide us with a disk cache file.  the temporary file is deleted once
the JAR channel finishes loading the contents of the JAR file.	obviously, this
will have poor performance if multiple items need to be extracted from the JAR
file since the entire JAR file will be written out to disk once for each item
that is loaded.  improving that will require a bit more work, but i suspect
this won't be that big of an issue in practice (knock-on-wood).

the patch seems to have some issues.  namely, the temporary files aren't
getting deleted.  but for the most part this patch seems to do the trick.

Comment 11

[Mike Kaply [:mkaply]]

Darin, I know this will never fly with ADT or drivers, but is there any 
possibility of a safe fix that I could argue into 1.4?

This particular bug is going to cause some major IBM applications to not be 
supported on Netscape or Mozilla.

Unfortunately, they didn't inform me of the problems until very recently.

Comment 12

[Darin Fisher]

mike: yeah, maybe.  i have something simpler in mind that might be good for the
1.4 branch.  in fact, i could probably just massage
nsHttpChannel::SetCacheAsFile(PR_TRUE) to not fail if SSL.  hmm....

Comment 13

[Mitchell Stoltz (not reading bugmail)]

Darin, thanks for working on this. The current cache-stream-as-file mechanism
made sense at the time it was written, but that was before embedding.

Alternatively, we could get signed scripts working with the old syntax (bug
37333), though that's not feasible for 1.4. That way we wouldn't have to use the
jar: protocol for signed scripts.

Comment 14

[Darin Fisher]

mitch,

no problem.  sorry for pointing fingers at stream-as-file... i think too many
things changed between its original inception and now, and along the way this
was simply broken :-/

i think the solution that i'm going for with this patch is probably the best and
safest solution.  though the patch is large, the changes are purely confined to
the domain of jar:http(s), so the risk is minimal to the overall product.  so,
it might have a chance of making it onto the 1.4 branch.  i just need to shake
out a remaining issue... actually, i think i may have uncovered a file
descriptor leak in libjar.

Comment 15

[Darin Fisher]

Attachment: v1 patch

ok, same patch cleaned up and working completely.  as i mentioned, there was a
file descriptor leak in the jar cache (or at least it seems that way to me). 
in current builds this also means that we would leak disk cache files if the
cache tried to delete a file on disk after libjar had read from it.  over time,
i think the cache would just grow in size as a result of this.	of course few
people load jar:http, so probably not a big deal in that case.

Comment 16

[Darin Fisher]

-> me

Comment 17

[Darin Fisher]

.

Comment 18

[Darin Fisher]

Attachment: v1.1 patch (revised per comments from dougt)

minor changes to the API.  now, if an explicit download location is specified,
the resulting file will not be deleted when the downloader is destroyed.  this
allows the consumer to take ownership of the downloaded file.

Comment 19

[Doug Turner (:dougt)]

Comment on attachment 126140 [details] [diff] [review]
v1.1 patch (revised per comments from dougt)

r=dougt.

this interface is great.  you should think about marking it UNDER_REVIEW.  I
would love to have something like this exposed.

Comment 20

[Mike Kaply [:mkaply]]

I might still try to sneak this in 1.4. How risky is this?

Comment 21

[Darin Fisher]

mkaply: not too risky in my book since this code is only exercised when loading
remote jar files.

Comment 22

[Alec Flett]

Comment on attachment 126140 [details] [diff] [review]
v1.1 patch (revised per comments from dougt)

sr=alecf

Comment 23

[Darin Fisher]

Comment on attachment 126140 [details] [diff] [review]
v1.1 patch (revised per comments from dougt)

seeking drivers approval for 1.4...

this looks scarier than it is.	the risk is low because the code is only
executed when loading remote jar files, but without this jar:https:// won't
work at all.

Comment 24

[Darin Fisher]

fixed-on-trunk

Comment 25

[Asa Dotzler [:asa]]

Comment on attachment 126140 [details] [diff] [review]
v1.1 patch (revised per comments from dougt)

moving approval request forward.

Comment 26

[Mike Kaply [:mkaply]]

Comment on attachment 126140 [details] [diff] [review]
v1.1 patch (revised per comments from dougt)

I haven't seen any mentions of refressions on this one and we want it for IBM.

a=mkaply

Comment 27

[Mike Kaply [:mkaply]]

I want this on the branch.

Comment 28

[Darin Fisher]

fixed1.4

Comment 29

[Darin Fisher]

ok, minor tweak to the keyword... fixed1.4.1 instead of fixed1.4 (thanks kaie!)

Comment 30

[Darin Fisher]

*** Bug 126344 has been marked as a duplicate of this bug. ***