1164977 - Consolidate subsumes checks and attribute handling onto BasePrincipal

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Bobby Holley (:bholley)]

This is another significant unit of prep work I found myself doing on the road to bug 1163254. Splitting it out.

Comment 1

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5160708d0b7

Comment 2

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=37d52c6001e9

Comment 3

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7101421b98af

Comment 4

[Bobby Holley (:bholley)]

Attachment: Part 1 - Prevent MediaStreamAudioSourceNode from passing null to nsIPrincipal::subsumes. v1

Comment 5

[Bobby Holley (:bholley)]

Attachment: Part 2 - Hoist all the app attribute handling into BasePrincipal. v1

Comment 6

[Bobby Holley (:bholley)]

Attachment: Part 3 - Unify subsumes/equals logic on BasePrincipal and reduce duplicated code. v1

Comment 7

[Bobby Holley (:bholley)]

Attachment: Part 4 - Hoist app attributes into a struct on BasePrincipal and refer to them as 'origin attributes'. v1

This sets the stage for the upcoming work for signed apps.

Comment 8

[Bobby Holley (:bholley)]

Attachment: Part 5 - Hoist attribute serialization into BasePrincipal. v1

Comment 9

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac1da236ec70

Comment 10

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8606443 [details] [diff] [review]
Part 3 - Unify subsumes/equals logic on BasePrincipal and reduce duplicated code. v1

Review of attachment 8606443 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/nsPrincipal.cpp
@@ +186,5 @@
> +    nsCOMPtr<nsIURI> thisDomain, otherDomain;
> +    rv = GetDomain(getter_AddRefs(thisDomain));
> +    NS_ENSURE_SUCCESS(rv, false);
> +    rv = aOther->GetDomain(getter_AddRefs(otherDomain));
> +    NS_ENSURE_SUCCESS(rv, false);

I don't really see the point of rv here. There are other consumers of GetDomain where the return value is ignored. So I guess we either fix all of them, ignoring it here too, or if you want to check it here I think we should just crash instead of returning false.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/10dbc3300287
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2bb0908e1f9
https://hg.mozilla.org/integration/mozilla-inbound/rev/2493c405317f
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4e6af40c5e8
https://hg.mozilla.org/integration/mozilla-inbound/rev/e18c56c6ade5

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/10dbc3300287
https://hg.mozilla.org/mozilla-central/rev/f2bb0908e1f9
https://hg.mozilla.org/mozilla-central/rev/2493c405317f
https://hg.mozilla.org/mozilla-central/rev/d4e6af40c5e8
https://hg.mozilla.org/mozilla-central/rev/e18c56c6ade5