1041176 - Store securityFlags as one unsigned long in nsILoadInfo.idl

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb]]

We should store securityFlags in nsILoadInfo as *one* unsigned long in the idl and 'or' all the different security flags into that unsigned long, similar to the loadFlags.

We are about to store more securityFlags in that loadInfo soon:
https://etherpad.mozilla.org/BetterNeckoSecurityHooks

'Or'ing all the different flags has the advantage that we don't need to extend/change the constructor all the time including all the callsites. In addition we save space in every nsLoadInfo object.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Once bug 1038756 has landed we can start executing this.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1041176.patch

We should land this patch on top of the patches from bug 1038756 and land them at the same time. I don't like the leading 'e' (enum) for eInheritPrincipal, eSandbox, which was left behind in the patches from bug 1038756. This patch introduces securityFlags and collapses the two flags into those securityFlags.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8480165 [details] [diff] [review]
bug_1041176.patch

>+    rv = loadInfo->GetSecurityFlags(&securityFlags);
>+    NS_ENSURE_SUCCESS(rv, rv);

Please add an infallible getter in a %C++ block in the IDL, so consumers don't have to jump through this silly hoop.  Then all these consumers can just write loadInfo->GetSecurityFlags().

>+++ b/docshell/base/LoadInfo.cpp
>+  *aInheritPrincipal = (mSecurityFlags & nsILoadInfo::SEC_INHERIT_PRINCIPAL);

This looks wrong to me.  It should also be checking that the SEC_SANDBOXED flag is not set, no?

Alternately, to make GetSecurityFlags less of a footgun, we could clear the SEC_INHERIT_PRINCIPAL flag in the constructor of SEC_SANDBOXED is set.

Might be worth documenting in the IDL that these two flags can't both be set at the same time.

The rest is good.  r=me with the above fixed.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Since we slightly modified the API of NS_NewChannel and such before landing Bug 1038756, we had to revisit callsites. Hence we folded that patch into the patches in Bug 1038756 which landed on mc yesterday. Marking as resolved.