71394 - Need security checks for SOAPCall and other new XMLExtras

Status: VERIFIED FIXED

(Core :: Security, defect, P1)

Description

[Mitchell Stoltz (not reading bugmail)]

I think the SOAPCall object, as well as any other interfaces in XMLExtras that
involve data transfer, need a hook for the security manager. When the DOM gets
XPConnected, this will be easy; if we're planning to put SOAP or any other
interfaces to use before then, we'll need a special-case security solution like
I did for XMLHttpRequest.

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Vidur is still in charge of the SOAP stuff, so he can probably tell you when
this is needed. As far as I know, there is no hurry...

Comment 2

[Mitchell Stoltz (not reading bugmail)]

OK, we don't need this right away, since XMLHttpRequest is still the only
interface exposed. Heikki, is this correct?

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

SOAP is not built by default. And I believe Ray Whitmer is now in charge of our
SOAP story, he should be rewriting this stuff anyway (supposing he has the time).

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Ray, let me know if and when the SOAP interfaces will be added to the product,
so you and I can work out the security.

Comment 5

[Ray Whitmer]

OK.  I'll let you know quite a bit before I am ready to throw the switch.

Comment 6

[Mitchell Stoltz (not reading bugmail)]

Target is now 0.9.5, Priority P1.

Comment 7

[Mitchell Stoltz (not reading bugmail)]

time marches on. Retargeting to 0.9.6.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Moving the most time-critical bugs and minor security fixes to 0.9.7

Comment 9

[Ray Whitmer]

Changed target milestone.  There is something there to look at now for milestone
0.9.8

Comment 10

[Ray Whitmer]

Retargetting.

Comment 11

[Ray Whitmer]

For the record, we looked at the SOAP call, and it misses the security check for
xml request so we clearly need to decide on what check to add.

Comment 12

[Mitchell Stoltz (not reading bugmail)]

Ray, have you located any reference material on possible SOAP security schemes?

Supposing I'm inside a firewall, and I am authorized to use a SOAP server that's
within my firewall, a malicious script running on my machine could also access
that SOAP server, and the server has no way to tell that it isn't me doing the
accessing. So, some sort of client-side security policy will probably be necessary.

Comment 13

[Ray Whitmer]

I think the solution should be in two parts, and we can log a bug for the second
part of the solution:

1.  Add the normal security check, and fail if the security check fails.

2.  If the security check fails, instead of failing, add a SOAP header packet
called "UntrustedSource" which contains the URI of the page which made the call.
 SOAP services which do not understand the packet will fail, but a service
(especially those intended to execute outside of the firewall) can choose to
ignore the packet and accept all calls from untrusted sources or can choose to
look at the domain of the source and decide whether it should be trusted or not.

We need to work with the SOAP community for proper standardization and support
of this untrusted source security packet.

So I am ready to do part 1 now, so we can get on with the show, and part 2 will
come along as soon as we can get enough people outside to agree on the packet. 
I have sent a message to the soapbuilders list on Yahoo seeing if anyone else is
interested, and I won't stop there.

Comment 14

[Mitchell Stoltz (not reading bugmail)]

Regarding part (1), IE apparently disables SOAP calls from pages loaded from the
"Internet Zone," meaning general untrusted content, and allows SOAP calls with
user confirmation from pages in the "intranet zone," and I'm not sure how that
one is defined. 

I've just filed bug 124281 to add UI for enabling and disabling out-of-band XML
data loading, which will cover SOAP, XMLHttpRequest, and XMLDocument.load.

Three issues to work out:
- What should the default security setting be - Enabled, Disabled, or Ask User?
- Should unsigned scripts be allowed to use these calls cross-host?
- Should we eliminate the requirement that the script call enablePrivilege
before initiating the SOAP call?

Comment 15

[Ray Whitmer]

Here is my take on the issue now, which I am going to code:

1.  Keep the status-quo enabling / disabling of this feature through
SOAPCall.invoke and UniversalBrowserRead.  If this checks out OK, then just send
the packet.

2.  If (1) fails, then perform a similar check on SOAPCall.invokeUntrusted.  If
this checks out OK, then make the call with a special mustUnderstand=true header
that identifies the source URI and possibly the company if the script is signed.

I will assume that this is well-enough aligned with what everyone else is doing
(UI-wise) until I hear otherwise.  I have raised the suggestion of a new header
for untrusted calls on soapbuilders and axis-dev, and as of yet, no one has
chosen to respond.

Comment 16

[Ray Whitmer]

Attachment: Patch to add CheckConnect and verify source header.

This adds a new property verifySource to SOAPCall.  If false, CheckConnect is
called on the property SOAPCall.invoke and if permitted, an unverified
(unaltered) SOAP call is made.	If true, CheckConnect is called on the property
SOAPCall.invokeVerifySource and if permitted, the call is made after adding a
header element of namespace "urn:inet:www.mozilla.org:user-agent" type
"verifySource", which contains a "uri" element,as a string in standard SOAP
encoding citing the URI which contained the executing script
(mustUnderstand="1").

With Vidur's more-complete description of SOAP settings, simple calls to
CheckConnect may have to be replaced.  As this patch was written, it was
intended that "invokeVerifySource" should be enabled by default.  Whatever is
actually landed for the deadline at the end of the week, the SOAP examples will
need to be modified to stand a reasonably chance of successfully executing once
the security checks are in place.  At the same time, it would be a good idea to
remove tests that don't work such as SOAPCall.

Comment 17

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 68924 [details] [diff] [review]
Patch to add CheckConnect and verify source header.

r=mstoltz. This patch covers most of Vidur's proposal and will work fine for
now.

Comment 18

[Ray Whitmer]

The patch has been committed.

Comment 19

[Mitchell Stoltz (not reading bugmail)]

The bug owner has been committed :)

I'll leave this bug open to cover the remaining work for adding UI for this
feature. Some backend changes may be required if we want to do a "remember this
decision for this target service" option.

Comment 20

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Could you rather create a new bug for the UI + pref work? After all, this bug
has been fixed...

Comment 21

[Ray Whitmer]

I believe there already is such a bug.

Comment 22

[Ray Whitmer]

Bug 124281 is the continuation of this one.  Sorry I closed this one, but I
forgot it didn't belong to me since I committed the patch.  You can reopen it if
there is some reason to.

Comment 23

[bsharma]

Marking verified as per above developer comments.