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.

Sign up for GitHub

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

Jump to bottom

doc: clarify policy expectations #48947

Merged
merged 3 commits into from
Aug 28, 2023
Merged

doc: clarify policy expectations #48947

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
058e7b5
doc: clarify policy expectations
RafaelGSS Jul 28, 2023
32b0a1a
fixup! mention new Module() supported and integrity approval
RafaelGSS Jul 28, 2023
8c0dfcf
fixup! fixup! mention new Module() supported and integrity approval
RafaelGSS Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 25 additions & 11 deletions doc/api/permissions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,18 @@ If you find a potential security vulnerability, please refer to our

Node.js contains experimental support for creating policies on loading code.

Policies are a security feature intended to allow guarantees
about what code Node.js is able to load. The use of policies assumes
safe practices for the policy files such as ensuring that policy
files cannot be overwritten by the Node.js application by using
file permissions.
Policies are a security feature intended to ensure the integrity
of the loaded code.

The use of policies assumes safe practices for the policy
files such as ensuring that policy files cannot be overwritten by the Node.js
application by using file permissions.
RafaelGSS marked this conversation as resolved.
Show resolved Hide resolved

While it does not function as a provenance mechanism to trace the origin of
code, it serves as a robust defense against the execution of malicious code.
Unlike runtime-based models that may restrict capabilities once the code is
loaded, Node.js policies focus on preventing malicious code from ever being
fully loaded into the application in the first place.

A best practice would be to ensure that the policy manifest is read-only for
the running Node.js application and that the file cannot be changed
Expand Down Expand Up @@ -202,12 +209,6 @@ the manifest and then immediately used without searching.
Any specifier string for which resolution is attempted and that is not listed in
the dependencies results in an error according to the policy.

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.

A boolean value of `true` for the dependencies map can be specified to allow a
module to load any specifier without redirection. This can be useful for local
development and may have some valid usage in production, but should be used
Expand All @@ -224,6 +225,9 @@ can be used to ensure some kinds of dynamic access are explicitly prevented.
Unknown values for the resolved module location cause failures but are
not guaranteed to be forward compatible.

All the guarantees for policy redirection are specified in the
[Guarantees](#guarantees) section.

##### Example: Patched dependency

Redirected dependencies can provide attenuated or modified functionality as fits
Expand Down Expand Up @@ -446,6 +450,16 @@ not adopt the origin of the `blob:` URL.
Additionally, import maps only work on `import` so it may be desirable to add a
`"import"` condition to all dependency mappings.

#### Guarantees

* The policies guarantee the file integrity when a module is loaded using
`require()` or `import()`.
* 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module.constructor` which allow access to
loading modules.

So this probably shouldn't be listed. The intent is to prevent loading new code without going through another module. Since Module is accessible through direct access in CJS and is not directly censorable without it likely should be considered at a similar tier to require or import since it is directly supplied and not leaked by a side channel like require.cache. I'd also be fine saying CJS isn't covering that specific object capability as designs for ESM avoided this kind of leakage (at least while I was more involved, unclear on these days).

Other means, such as to prevent undesired access to APIs through
variables, are necessary to lock down that path of loading modules.

Maybe we should state that it is expected that the approval of module integrity in policies' threat model implies they are allowed to muck with and even circumvent security features once loaded so environmental/runtime hardening is expected (this is true for other things like http as well anyway so not uncommon, but a callout might be good). Tools to do static analysis can aid here from linting to full blown security tools.

Copy link
Member Author

@RafaelGSS RafaelGSS Jul 28, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it now? 32b0a1a

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RafaelGSS it looks a lot better, though new Module() might need some workshopping though as that API doesn't actually guarantee context on its own so that goes against Policies in some way, the constructor itself doesn't actually load the code to eval. Module.prototype.load / Module._load are the paths to cause code to load, and using these without constraints is available using module.constructor._load leading to => extensions implementations that separately enforce policies such as ._compile for CJS. Lack of context means you can escape policy "dependencies" for a specific dependent if you get access to them. So it may be necessary to more clearly define how these APIs that lack a trusted context for enforcement need to be framed.

Copy link
Member Author

@RafaelGSS RafaelGSS Jul 29, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think we should list the API calls allowed when using Module and policies, therefore, any variant distinct from this list isn't supported:

  • module.require
  • require('module').require()

What do you think? Any other suggestions to make it clear?


## Process-based permissions

### Permission Model
Expand Down