Add a secure registry to F2

[ilinkuo]

Add a secure registry to F2 so that the Container can get at the list of _apps on line 8 of https://github.com/OpenF2/F2/blob/master/sdk/src/container.js

F2.extend('', (function() {
  var _apps = {};
  var _config = false;
  var _bUsesAppHandlers = false;

and Apps can query the registry securely as well.

By "securely", I mean that only the Container can mutate the registry, and that the Apps receive only a safely filtered copy of what's in the registry. I envision that line 8 above would be replaced by

      var registry= pluginRegistry(F2); // formerly var_apps = {};

I've created a jsfiddle to illustrate the technique of creating a secure private mutatable accessor and a public non-mutatable accessor. We are putting this registry code into production in our next release, but without replacing line 8 above.

[montlebalm]

This sounds like it would be a security concern for app developers. The container would have access to any property on any app. Currently, only F2 controls the app instances as a firewall between containers and apps.

Is the purpose to allow more powerful plugin development?

[ilinkuo]

The purpose is not for more powerful plugin development -- it's for the Container to do its job conveniently. Currently, the Container has to keep track of the AppConfigs it passes in in its own private registry. This proposal would remove that duplication.

There is no security concern here. The source of the AppConfig information is the Container, so there isn't any registry information that the Container doesn't already have.

However, I am very concerned about what I see as misguided security concerns.

The container would have access to any property on any app

The Container owner controls the page, so there really isn't any way to protect the App from the Container. If the Container wants to mess with the App, it can load its own version of F2. The only way that an App can guard against the Container is to refuse to load at all.

only F2 controls the app instances as a firewall between containers and apps.

Placing trust in F2 is misguided. F2 is a public object in global space with public methods. Any of these methods can be easily hijacked. If an App from evil.com wants to know the registry information, it can just hijack F2.registerApps().

As for being a firewall, there are three avenues to guard:

1. Protect App from Container
2. Protect App from other Apps
3. Protect Container from Apps

It seems to me like F2 thinks a lot about the first avenue and neglects the other two, especially the last. I'm not sure what the reason for this emphasis is, but it's contrary to my sense of security. I would recommend the approach that F2 give up on protecting the App from the Container using F2, and instead enlist the Container's help in hardening defenses against 2 & 3.

[brianbaker]

Is there a need for the container to get at the actual instance of an App? Or does the container just need access to the configs?

Today _apps contains objects of the form { app: AppClass, config: AppConfig }. If this proposal is just to get at the config part, then the app part can be stored separately alleviating the concern of @montlebalm.

[brianbaker]

@ilinkuo Would you see this request and #46 as accomplishing some of the same goals? To borrow from your jsfiddle, when manifest requests are sent they are sent the filteredCopy from a call to F2.registry.findByAppId("cheating_app") rather than a call to registry.findByAppId("cheating_app") as it is today.

[ilinkuo]

Is there a need for the container to get at the actual instance of an App? Or does the container just need access to the configs?

In our implementation there is, but that's because we're filling a hole in the F2 spec. The spec does not specify an F2 App lifecycle. The F2.removeApp() is inadequate, as all it does is remove the App from the DOM, leaving orphaned handlers and other resources. We've asked Ivan to implement two lifecycle methods destroy() and unload().

The second reason we're doing this is because the App is asking for a list of all instanceIds of a certain type via findByAppId (You can ask Ivan about the use case for this), and this is a nice way to provide it

I would agree that ideally, the framework is the only one that should be modifying the AppConfig list or its contents. But I disagree with @montlebalm about the reason. I think the correct reason is a clear division of responsibility -- the framework is the one that owns the list and all modification requests should be made through the framework. However, since the framework is currently providing no access at all to the AppConfig list, I'm forced to hacks like these.

Nonetheless, this fiddle illustrates the one of the major philosophical differences I've had with the F2 team. I think the Container should have more privileged access to F2 internals than the F2 Apps, whereas most F2 developers seem to think the Container should have less access, because of what I consider misguided security priorities. This fiddle illustrates a technique for how my security vision can happen that I hope F2 adopts.

Would you see this request and #46 as accomplishing some of the same goals?

This uses the same filterClone function toward security goals, but I think the final goal is quite different.

[montlebalm]

I think the Container should have more privileged access to F2 internals than the F2 Apps, whereas most F2 developers seem to think the Container should have less access

I don't think I've heard anyone argue that Apps should have more access than the Container. Rather, the arguments I've seen have been advocating equal privacy for both.

I think there's little chance the library would open up App instances (or InstanceIds) to the container. Clever use of targeted events could solve those issues without compromising private data. A registry of AppConfigs sounds reasonable since the container most likely has access to them already.

We've implemented a dispose method in v2, which is triggered after the app is removed. Also, all App events are removed automatically, so the need for more lifecycle methods may be diminished.

[ilinkuo]

I should qualify the above comment by saying that we are still on 1.1.2. In the newest versions of F2, there are the AppHandler hooks which does attempt to invoke destroy() on the App instance. This will replace the hook we currently have put in. However, it still doesn't remove the need for an unload() and we still need a way for the App to find other instances of the same type of App.

Also, the app.destroy() isn't part of the documentation. I only found out about it by looking at the code.

[markhealey]

@ilinkuo destroy is detailed in the SDK docs and briefly in the spec.

I didn't see a reply to @brianbaker's question above.

Regarding the firewall comments and, specifically, these 3 avenues:

Protect App from Container
Protect App from other Apps
Protect Container from Apps

No one has mentioned secure apps. The current version of F2 has adequate support for App/Container protections via secure apps. Very few implementers use them though because iframes lead to poor user experiences. If implementers aren't using secure apps, they're inherently trusting the other parties they're sharing a page with and throwing a bit of caution to the wind.

Let's wrap this one up. I am in favor of adding a registry for Containers because every single Container I've seen has a duplicate copy of apps which if nothing else is a waste of time. I don't see an issue with providing the contents of _apps (including both AppClass and AppConfig) to the Container.

[ilinkuo]

@markhealey The documentation is insufficient. It isn't mentioned in the docs that the destroy() method must be implemented by the App itself. I only found out that is the case by looking at this line of the code from https://github.com/OpenF2/F2/blob/master/sdk/src/app_handlers.js

    // call the apps destroy method, if it has one
            if(appInstance && appInstance.app && appInstance.app.destroy && typeof(appInstance.app.destroy) == 'function')

The documentation does mention events of appDestroyBefore, appDestroyAfter, but it also mentions appCreateRoot, and appRenderAfter and none of these need app.createRoot or app.render methods. I already have this kind of appInstance && appInstance.destroy hook logic in my adulterated 1.1.2 F2. Before reading the code, I had assumed I would have to write a handler to an AppDestroyXXX event in order to invoke app.destroy because nowhere in the documentation does it say that destroy() is called on the app instance.

I didn't see a reply to @brianbaker's question above.

Do you mean this question?

Would you see this request and #46 as accomplishing some of the same goals?

This was my answer. Did you need more detail?

This uses the same filterClone function toward security goals, but I think the final goal is quite different.

Finally,...

No one has mentioned secure apps. The current version of F2 has adequate support for App/Container protections via secure apps. Very few implementers use them though because iframes lead to poor user experiences. If implementers aren't using secure apps, they're inherently trusting the other parties they're sharing a page with and throwing a bit of caution to the wind.

That's also why we're not using secure iframed apps. Hence it's not part of our discussion. While I agree that by deciding not to use iframes we've also decided to abandon the best security option that browsers currently provide, I feel that there needs to be a security discussion for non-iframed F2 Apps, if only for the sake of satisfying corporate security watchdogs. There is a big difference between throwing a BIT of caution to the wind and throwing ALL caution to the wind.

I am in favor of adding a registry for Containers

Yay!

[montlebalm]

I don't see an issue with providing the contents of _apps (including both AppClass and AppConfig) to the Container.

@markhealey Just so we're clear on terms, I'm interpreting AppClass to mean the instantiated instance of the app. That would include all instance functions and properties.

If that's the case, then I strongly advise against adding AppClass to the container registry. App developers deserve the same level of privacy as containers. I can't think of any use case that couldn't just as easily be solved through targeted messaging, which allows the app to specify what it wants to share.

[markhealey]

The documentation is insufficient.

@ilinkuo have you considered submitting a pull request? All of the spec documentation is open source in addition to the JS.

Just so we're clear on terms

@montlebalm I'm thinking the AppClass is the same thing as what's assigned to F2.Apps["my_app"] today. That's the instance but not the instance funcs or props. Unless I'm missing something?

[montlebalm]

@markhealey So would that be the function that returns the app class function? For example:

F2.Apps['...'] = function() { // <-- Option 1
  var App = function() {}; // <-- Option 2
  return App;
}

It sounds like you're thinking of AppClass as "Option 1" in the above example. Is that correct?

What would the container do with that? I can't come up with a good use case. Of course, I have the perspective that nothing should be exposed except what's needed. Exclusionist, deletionist, whatever you want to call it.

[markhealey]

Yeah, a container wouldn't do anything with that—it's just what's in _apps today so I was going with @brianbaker's comment.

[ilinkuo]

@ilinkuo have you considered submitting a pull request? All of the spec documentation is open source in addition to the JS.

I'm currently handicapped in that respect. Pull requests would only be relevant with the current release, but we are working with a version too far back. However, an F2 upgrade is tentatively scheduled for the release after this, so I may be able to more actively contribute then.

If that's the case, then I strongly advise against adding AppClass to the container registry.

@montlebalm I would recommend using the filterClone method to filter out anything you don't want the Container to know about.