369201 - [FIX]nsPrincipal::Equals seems odd for certificate principals

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Boris Zbarsky [:bzbarsky]]

For one thing, we don't do the cert check symmetrically...  And I think we should be calling CheckSameOriginPrincipal() even if the certs match.

The thing is, we don't store the codebase for certificate principals in prefs.  So if you grant something signed by a cert some sort of permissions right now, everything signed by that cert (even if it's on a different site) has those permissions.  Is that the behavior we want?

Comment 1

[Ben Bucksch (:BenB)]

You write on the newsgroup:
"Right now we treat them as equal if they have the same fingerprint and subject name.  Which means that if you sign jars with the same certificate and put them up on different hosts we'll treat the principals as being equal.  That seems wrong to me, frankly.  I've filed bug 369201 [this bug] on that."

I don't see what's wrong with it. If 2 objects are signed by exactly the same cert, that means they come from the same author. Which implies they have the same trust. I think signatures are even stronger base for trust entity identification than domains - we merely use domains because we usually don't have anything better.

I'm assuming the whole PKI stuff doesn't even enter the picture there, it's merely individual certs.

Comment 2

[Boris Zbarsky [:bzbarsky]]

This isn't about "the same trust".  Or rather, in the current implementation it is.  But in the current implementation we use the cert to determine which capabilities are enabled and then turn around and use the domain when doing all other security checks.  Which can lead to privilege escalation if unsigned code can get signed code from the same domain to do something, of course.

So at the very least we need _some_ function that compares both domains and certs and claims that the trust labels are equal if and only if both are equal.  The only question is whether it's used when reading in stored prefs.

I could buy that stored permissions should be shared across sites (and in fact that would mean the least code changes for me), but personally, as a user, I am much more likely to trust code signed by the same person in some contexts than others and it bothers me a _lot_ that granting expanded access to some particular web app grants it to all other apps by the same guy.  I thing the privilege grants we make should be as narrow in scope as possible.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: As a bare minimum we should do this.

I'd like to do this as a start so we can start moving to calling subsumes() as needed....  I still think we should save codebases for cert principals, though.

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 254774 [details] [diff] [review]
As a bare minimum we should do this.

r=dveditz

I agree with you, we ought to be saving the codebase when we save the permission grant, but it is an improvement to at least check in the non-saved case.

This may break some signed intranet apps which relied on the equality we're taking away here to avoid having to explicitly ask for UniversalBrowserRead/Write. I don't know if this is more than just a hypothetical situation.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Checked in.  Thanks for the reviews!