878886 - TestPermissionFromPrincipal() using an unknown app id principal asserts

Status: RESOLVED INVALID

(Core :: Permission Manager, defect)

Description

[Patrick McManus [:mcmanus]]

I have a principal I got from the security manager. It turns out that it has an unknown app id.. in this case it is the background channel that is created to fetch the safe browsing database.

When I call TestPermissionFromPrincipal with it I get an assert and a crash :(. It's a valid nsIPrinicipal so this really isn't ideal error handling..

I've got a workaround in place to test it from unknown app id right now.

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff28856af in nsPrincipal::GetAppId (this=0x7fffd05bf940, aAppId=0x7fffffffc478) at ../../../scratch/caps/src/nsPrincipal.cpp:531
531        MOZ_ASSERT(false);
(gdb) bt
#0  0x00007ffff28856af in nsPrincipal::GetAppId (this=0x7fffd05bf940, aAppId=0x7fffffffc478) at ../../../scratch/caps/src/nsPrincipal.cpp:531
#1  0x00007ffff2f33cc3 in nsPermissionManager::CommonTestPermission (this=0x7fffd8017320, aPrincipal=0x7fffd05bf940, aType=0x7ffff4b1e781 "falsestart-rsa", aPermission=0x7fffffffc618, aExactHostMatch=false, aIncludingSession=true)
    at ../../../scratch/extensions/cookie/nsPermissionManager.cpp:1044
#2  0x00007ffff2f33ac7 in nsPermissionManager::TestPermissionFromPrincipal (this=0x7fffd8017320, aPrincipal=0x7fffd05bf940, aType=0x7ffff4b1e781 "falsestart-rsa", aPermission=0x7fffffffc618) at ../../../scratch/extensions/cookie/nsPermissionManager.cpp:1018

Comment 1

[Mounir Lamouri (:mounir)]

Where is that principal coming from? Unknown app id principals should be very uncommon and the idea is that when you get one of them we actually can't make any assumption regarding the app from which it came from.

Comment 2

[Patrick McManus [:mcmanus]]

>(In reply to Mounir Lamouri (:mounir) from comment #1)
> Where is that principal coming from? 

The abstract answer is that it comes from 
nsIScriptSecurityManager::GetChannelPrincipal() at some point in time when all I've got is the nsIChannel.

The lookup is NS_OK and there is a valid nsIPrinicipal pointer returned. It seems really strange for that to then ASSERT when passed to TestPermissionFromPrinicipal. It would be one thing if an unknown app id couldn't exist - but if they're just uncommon I don't understand asserting it as an invariant.

more concretely the channel is created by the safe browsing code that downloads the hashes for the safe browsing database. I _think_ that's this code: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js

> Unknown app id principals should be
> very uncommon and the idea is that when you get one of them we actually
> can't make any assumption regarding the app from which it came from.

I'm totally fine with it failing the permission check.

Comment 3

[Mounir Lamouri (:mounir)]

It is asserting because the principals with unknown app id shouldn't be used to test permissions because we have no information on them. This might be the case here or it has an unknown app id while it should have one set.

Jonas, any idea?

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

So the problem is basically here:

http://hg.mozilla.org/mozilla-central/file/bb4f883bfa10/caps/src/nsScriptSecurityManager.cpp#l312

We ideally would want to create NO_APP_ID codebase principals here somehow, but since we don't have a docshell we always fall into the UNKNOWN_APP_ID code path.

It would be great if we had a way to signal to a channel that it doesn't belongs to NO_APP_ID without using a docshell. I don't have any great ideas for how to do that though.

Comment 5

[Justin Dolske [:Dolske]]

Appears I'm seeing a related issue in bug 881996. :(

Comment 6

[[:fabrice] Fabrice Desré]

(In reply to Jonas Sicking (:sicking) (vacation until 9/9. In norway until 9/16) from comment #4)
> So the problem is basically here:
> 
> http://hg.mozilla.org/mozilla-central/file/bb4f883bfa10/caps/src/
> nsScriptSecurityManager.cpp#l312
> 
> We ideally would want to create NO_APP_ID codebase principals here somehow,
> but since we don't have a docshell we always fall into the UNKNOWN_APP_ID
> code path.
> 
> It would be great if we had a way to signal to a channel that it doesn't
> belongs to NO_APP_ID without using a docshell. I don't have any great ideas
> for how to do that though.

Shouldn't we try to get a load context instead of the docshell and get the app id from there?

Comment 7

[:Gavin Sharp [email: [e-mail]]]

(In reply to Patrick McManus [:mcmanus] from comment #0)
> When I call TestPermissionFromPrincipal with it I get an assert and a crash
> :(. It's a valid nsIPrinicipal so this really isn't ideal error handling..
> 
> I've got a workaround in place to test it from unknown app id right now.

Where is this workaround? From context it seems like it'd be in nsHttpChannel::RetrieveSSLOptions, but I don't see one there.

Comment 8

[Patrick McManus [:mcmanus]]

(In reply to :Gavin Sharp (email [e-mail]) from comment #7)

> Where is this workaround? From context it seems like it'd be in
> nsHttpChannel::RetrieveSSLOptions, but I don't see one there.

https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#6149

Comment 9

[Johann Hofmann [:johannh]]

appid is long gone.