Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: permissions #297

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

RFC: permissions #297

wants to merge 2 commits into from

Conversation

bmeck
Copy link

@bmeck bmeck commented Dec 16, 2020

This RFC proposes adding the ability for package consumers to grant specific capabilities to packages and package authors to list the capabilities that they require. I am fairly open to ongoing discussion on what such a field in package.json should look wise as a schema and this might require upstream changes if they are discovered here.

References

@arcanis
Copy link

arcanis commented Dec 19, 2020

Let's say I want to generate a policy file that allows DirectDep to access the filesystem, but not TransitiveDep. What would I write? Would the following work? Or would I have to generate a resources entry for each and every file in the project?

{
  "resources": {
    "./node_modules/direct-dep": {
      "dependencies": {
        "fs": true,
      }
    },
    "./node_modules/transitive-dep": {
      "dependencies": {
      }
    }
  }
}

If folder paths are accepted, how would that work with regard to the hoisting (or, rather, non-hoisting)? Ie, would TransitiveDep have access to the network if I write the following? In other words, are policies extended or overriden?

{
  "resources": {
    "./node_modules/direct-dep": {
      "dependencies": {
        "fs": true,
      }
    },
    "./node_modules/direct-dep/node_modules/transitive-dep": {
      "dependencies": {
      }
    }
  }
}

The Node documentation mentions that:

Redirection does not prevent access to APIs through means such as direct access to require.cache or through module.constructor which allow access to loading modules. Policy redirection only affects specifiers to require() and import. Other means, such as to prevent undesired access to APIs through variables, are necessary to lock down that path of loading modules.

Does it mean that there will be a different mechanism down the road that will add support for preventing undesired access to APIs? Will it use the same information from the policy file? If not, does it matter if we add support for generating the dependencies field in the policy file, since it will be escapable and may bring a false sense of security?

@bmeck
Copy link
Author

bmeck commented Dec 19, 2020

@arcanis

If folder paths are accepted, how would that work with regard to the hoisting (or, rather, non-hoisting)? Ie, would TransitiveDep have access to the network if I write the following? In other words, are policies extended or overriden?

They are using "scopes", use "cascade" to defer to an outer scope for any field not set. So, they aren't really overriden or extended implicitly, but rather they are explicitly composed with the default behavior being "overriden" and cascade causing it to "extend". This is done so that nested dependencies can have different privileges that their containing dependents (both more or less).

Does it mean that there will be a different mechanism down the road that will add support for preventing undesired access to APIs? Will it use the same information from the policy file? If not, does it matter if we add support for generating the dependencies field in the policy file, since it will be escapable and may bring a false sense of security?

What you are describing is one kind of security model based upon call site location. When people talk about security/integrity within a JS program evaluation they usually are generally talking about the Object-capability model. This is the model used by SES, Trusted Types, and a Node.js Security Model. So, what policies are doing is approaching using that model for various reasons.

There will always be more security toggles for various mitigations, but for this RFC we are talking about enforcing an object capability around module linkage within a single global space. We will continue to have more toggles around global constraints from things like nodejs/node#33504 , and can start to migrate to having multiple globals as Realms and Compartments evolve through TC39.

Importantly JS is mutable by default and has a shared set of globals. We have --frozen-intrinsics but that won't make Node.js' core immutable and won't prevent user code from leaking the references to these things or even proxying and invoking them in a confused deputy manner. An easy example of a proxying problem is using graceful-fs wraps the native implementation and would then need the same permissions as any module using graceful-fs. If we want to provide a unique graceful-fs per dependency using it, limited to a specific constraint / folder it needs to be done by the reference obtained when importing it.

@arcanis
Copy link

arcanis commented Dec 19, 2020

Yep for sure, JS isn't ready for the kind of protected environment we need, and I'm really glad to see both TC39 and Node tackling this issue! 🌟

My point is more: if we, package managers, add support for a permissions field, then our users will expect it to be a security mechanism. While we both know that it's more complex than that and that Realms will need to move forward before true isolation can become a thing, many developers aren't as well versed, and may hold wrong assumptions about their system's safety. Which then begs the question: is it better to start by restricting the import by generating policy files now, even if it won't protect against malicious packages? Or to wait until proper encapsulation is available?

For instance, let's say a package lists this:

{
  "permissions": {}
}

That would imply that accessing the network isn't needed, so a user wouldn't expect it to be available in any way - which isn't a guarantee we can make at the moment, escapes being possible. Given that, is it still useful? The main pro I see would be to start preparing the ecosystem for when the time is right and all the required pieces are there, but on the other hand we would have to settle on a syntax before having all the information in our hands.

@bmeck
Copy link
Author

bmeck commented Dec 19, 2020

@arcanis

That would imply that accessing the network isn't needed, so a user wouldn't expect it to be available in any way - which isn't a guarantee we can make at the moment, escapes being possible. Given that, is it still useful? The main pro I see would be to start preparing the ecosystem for when the time is right and all the required pieces are there, but on the other hand we would have to settle on a syntax before having all the information in our hands.

The only ability to access the needed APIs would be through a global, the module loader, or a parameter. Package managers likely need to approach all 3 separately as they have different implications. I would note that through any future mechanism, such as Realms/Compartments, tackling the global to be extremely breaking and need something for preventing leaking the second global with different permissions such as a membrane system if they can communicate directly (hence why most things are doing serialized based communication these days since it avoids that need for now).

You are correct that people need to understand what is being said. In this instance for people reading permissions they can understand that the module loader will refuse to load things not listed. Overall, you do get a valuable property with limiting the integrity and module linkage which is that when auditing you don't have to audit things like dynamic import/require values, you don't need to audit for mutation of the files on disk, etc. I'd agree this is incremental when you talk about preparation, but I expect this to always be occurring and never really be "done". So, perhaps the usage guide in the RFC needs to cover specifics of what isn't prevented. We could just voice something is "secure" but like you saw in that policy documentation, honesty seems to be the better approach to avoid confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants