1019493 - IAC: setting the |manifestURLs| rules doesn't make sense

Status: RESOLVED INCOMPLETE

(Core :: DOM: Device Interfaces, defect)

Description

[Gene Lian (I already quit Mozilla)]

Alive used to point out this to me. We thought putting the manifest URLs as one of the rules doesn't make sense. For example, the System App can allow only the "foo" app to do the IAC connection through the following |manifestURLs| rule in the manifest:

  'connections': {
    'keyword': {
      'rules': {
        'manifestURLs': [app://foo.gaiamobile.org/manifest.webapp]
      }
    }
  }

This works on the mobile device but doesn't work on the desktop build (simulator) because the manifest URL of the foo app will be translated to:

  http://foo.gaiamobile.org:8080/manifest.webapp

which is based on the http protocol, so the |manifestURLs| rule won't match to take effect. This issue also occurs in the connect(..., rules) from the publisher side.

One alternative might be adding another manifest URL based on the http protocol:

  'connections': {
    'keyword': {
      'rules': {
        'manifestURLs': [app://foo.gaiamobile.org/manifest.webapp,
                         http://foo.gaiamobile.org:8080/manifest.webapp]
      }
    }
  }

but it's going to be an endless task when we want to take new protocols into consideration, which doesn't sound reasonable either.

Since IAC is only exposed certified apps for now, using the rules to restrict connections doesn't help a lot because no apps are malicious. So, we tend to remove the |manifestURLs| checking logic from Gecko and
remove its usage from the API spec (Alive already did this at https://wiki.mozilla.org/WebAPI/Inter_App_Communication_Alt_proposal).

Does this make sense to you guys overall?

Comment 1

[(no longer active)]

I think manifestURLs was added as a provision for when we decide to expose the IAC API to less privileged contexts, and because as you probably know I'm not really supportive of doing that for now, this change looks good to me.  :-)

Comment 2

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Attachment: Patch Gaia v1

Fixing correspondent Gaia UI testing.

Comment 3

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Attachment: Patch

Comment 4

[Sean Lin [:seanlin] (inactive from Mar 2016)]

Attachment: Patch Gecko v1

Patch for the core logic in Gecko.

Comment 5

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8439744 [details]
Patch Gaia v1

Nice! Note that we have to land the Gaia patch first.

Comment 6

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8439746 [details] [diff] [review]
Patch Gecko v1

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

Looks good to me. Note that we have to land Gaia first. Please refer to how to work at bug 1009408, comment #5.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/9f6e945b3ba88afb21e0213278fe300ded4e650e

Thanks for the whiteboard note :). We don't have a great way of tracking when a Gaia patch has made it over to m-c (it's not impossible, just not simple). Let's just assume that no news is good news if you haven't heard anything here by tomorrow and set checkin-needed again for the Gecko patch.

Comment 8

[Antonio Manuel Amaya Calvo (:amac)]

Hmm... Besides what Ehsan points out in comment 1, there's another reason why restricting the access via IAC to the apps that should really access was a good idea: it made the attack surface smaller. It's the same reason why even though certified apps are not malicious, we still have to explicitly declare what permissions they use, and why their processes don't run as root: when they're compromised down the line (and note that I say when and not if ;)) they still won't be able to use anything that's not on their manifest (assuming what's compromised it's not system). 

With this change, any app that's compromised gets access to all the IAC ports declared on the system. I for one don't think that's a good thing. At least the security review of the apps that expose ports should be revised with this change in mind. Need-infoing Paul to get his thoughts on this also.

Comment 9

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #8)
> Hmm... Besides what Ehsan points out in comment 1, there's another reason
> why restricting the access via IAC to the apps that should really access was
> a good idea: it made the attack surface smaller. It's the same reason why
> even though certified apps are not malicious, we still have to explicitly
> declare what permissions they use, and why their processes don't run as
> root: when they're compromised down the line (and note that I say when and
> not if ;)) they still won't be able to use anything that's not on their
> manifest (assuming what's compromised it's not system). 
> 
> With this change, any app that's compromised gets access to all the IAC
> ports declared on the system. I for one don't think that's a good thing. At
> least the security review of the apps that expose ports should be revised
> with this change in mind. Need-infoing Paul to get his thoughts on this also.

+1 to what Antonio said. I haven't audited what is possible with the IAC API - this would be a major effort, and would likely result in further hardening required.

How about we just disable the checking on debug builds, instead of removing it entirely? Does that provide a happy middle ground?

Comment 10

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Paul Theriault [:pauljt] from comment #9)
> (In reply to Antonio Manuel Amaya Calvo (:amac) from comment #8)
> > Hmm... Besides what Ehsan points out in comment 1, there's another reason
> > why restricting the access via IAC to the apps that should really access was
> > a good idea: it made the attack surface smaller. It's the same reason why
> > even though certified apps are not malicious, we still have to explicitly
> > declare what permissions they use, and why their processes don't run as
> > root: when they're compromised down the line (and note that I say when and
> > not if ;)) they still won't be able to use anything that's not on their
> > manifest (assuming what's compromised it's not system). 
> > 
> > With this change, any app that's compromised gets access to all the IAC
> > ports declared on the system. I for one don't think that's a good thing. At
> > least the security review of the apps that expose ports should be revised
> > with this change in mind. Need-infoing Paul to get his thoughts on this also.
> 
> +1 to what Antonio said. I haven't audited what is possible with the IAC API
> - this would be a major effort, and would likely result in further hardening
> required.
> 
> How about we just disable the checking on debug builds, instead of removing
> it entirely? Does that provide a happy middle ground?

Actually another thought - users can also sideload certified apps, which could then be used to try to comrpomise the system locally. Again, I havent audited IAC protocols so I dont know if this is a big risk, but this is another reason not to do this.

Comment 11

[(no longer active)]

(In reply to comment #10)
> (In reply to Paul Theriault [:pauljt] from comment #9)
> > (In reply to Antonio Manuel Amaya Calvo (:amac) from comment #8)
> > > Hmm... Besides what Ehsan points out in comment 1, there's another reason
> > > why restricting the access via IAC to the apps that should really access was
> > > a good idea: it made the attack surface smaller. It's the same reason why
> > > even though certified apps are not malicious, we still have to explicitly
> > > declare what permissions they use, and why their processes don't run as
> > > root: when they're compromised down the line (and note that I say when and
> > > not if ;)) they still won't be able to use anything that's not on their
> > > manifest (assuming what's compromised it's not system). 
> > > 
> > > With this change, any app that's compromised gets access to all the IAC
> > > ports declared on the system. I for one don't think that's a good thing. At
> > > least the security review of the apps that expose ports should be revised
> > > with this change in mind. Need-infoing Paul to get his thoughts on this also.
> > 
> > +1 to what Antonio said. I haven't audited what is possible with the IAC API
> > - this would be a major effort, and would likely result in further hardening
> > required.
> > 
> > How about we just disable the checking on debug builds, instead of removing
> > it entirely? Does that provide a happy middle ground?
> 
> Actually another thought - users can also sideload certified apps, which could
> then be used to try to comrpomise the system locally. Again, I havent audited
> IAC protocols so I dont know if this is a big risk, but this is another reason
> not to do this.

If you manage to sideload a malicious certified app, you could create much more damage than just abusing IAC, though, right?

Comment 12

[Paul Theriault [:pauljt] (no longer reading bugmail)]

True - so its really just the issue with providing an attack surface in the case of vulnerabilities.

Comment 13

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I am also confused as to why some IAC rules specify "minimumAccessLevel": "certified" - but only some of them? Does that imply that at some point in the future, some of the these IAC endpoints will be exposed to privileged/web apps?

Comment 14

[Sean Lin [:seanlin] (inactive from Mar 2016)]

In response to comment 13 above, we'll eventually expose IAC to non-certified apps, as stated in bug 907068.

Comment 15

[Gene Lian (I already quit Mozilla)]

(In reply to Sean Lin [:seanlin] from comment #14)
> In response to comment 13 above, we'll eventually expose IAC to
> non-certified apps, as stated in bug 907068.

Actually, no, we won't attempt to expose IAC to non-certified apps. We hope to find better alternatives like XHR to do this.

Comment 16

[Andrew Overholt [:overholt]]

Will we need to uplift whatever lands here to aurora so FxOS 2.0 gets it?

Comment 17

[Sean Lin [:seanlin] (inactive from Mar 2016)]

(In reply to Andrew Overholt [:overholt] from comment #16)
> Will we need to uplift whatever lands here to aurora so FxOS 2.0 gets it?
So far only the Gaia patch has landed, and it won't affect anything unless the Gecko patch lands, which is pending for the discussion above and might need to be revised. So please feel free to do whichever is convenient for you.

Comment 18

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Cleaning up Device Interfaces component, and mass-marking old FxOS bugs as incomplete.

If any of these bugs are still valid, please let me know.