Skip to content

src,process: initial permission model implementation

Rodrigo Muino Tomonari requested to merge github/fork/RafaelGSS/feat/permission-system into main

Reference issue: https://github.com/nodejs/security-wg/issues/791

The tagged issue contains the initial proposal for this MVP. This Pull Request includes the foundation of the Permission Model.

Constraints

This Permission Model is not bulletproof, which means, there are constraints we agree on before landing this system:

  • It’s not a sandbox, we assume the user trusts in the running code.
  • No break-changes are ideal.
  • It must add a low/no overhead when disabled and low overhead when enabled.

API Design

The Node.js Permission Model is a mechanism to restrict access to specific resources during the program execution. The API exists behind a flag --experimental-permission which when enabled, will restrict access to all available permissions.

Currently, the available permissions are:

  • File System - manageable through --allow-fs-read and --allow-fs-write flags
  • Child Process - manageable through --allow-child-process flag
  • Worker Threads - manageable through --allow-worker flag

Therefore, when starting a Node.js process with --experimental-permission, the ability to access the filesystem, spawn process and, create worker_threads will be restricted.

The CLI Arguments

To allow access to the filesystem, use the --allow-fs-read and --allow-fs-write flags:

The valid arguments for both flags are:

  • * - To allow all operations to given scope (read/write).
  • Paths delimited by comma (,) to manage reading/writing operations.

Example:

  • --allow-fs-read=/tmp/ - It will allow FileSystemRead access to the /tmp/ folder
  • --allow-fs-read=/tmp/,/home/.gitignore - It allows FileSystemRead access to the /tmp/ folder and the /home/.gitignore file — Relative paths are NOT supported.

Due to the PrefixRadixTree (fs_permission) lookup, relative paths are not supported. For this reason, the possiblyTransformPath was needed. I do have plans to create a pretty similar path.resolve on the C++ side so the possiblyTransformPath won't be needed, but I'll do it in a second iteration.

You can also mix both arguments:

  • --allow-fs-write=* --allow-fs-read=/tmp/ - It will allow FileSystemRead access to the /tmp/ folder and allow all the FileSystemWrite operations. Note: It accepts wildcard parameters as well. For instance: --allow-fs-write=/home/test* will allow everything that matches the wildcard. e.g: /home/test/file1 / /home/test2

Note: I rather prefer reading those arguments from a file (policy-deny.json), instead passing them in the command line. However, to reduce the PR scope, I've decided to do it in a separate PR.

The API Arguments

A new property permission was added to the process module. The property contains two functions:

  • deny(scope [,parameters])

API Call to deny permissions at runtime. e.g (REMOVED)

process.permission.deny('fs') // deny permissions to ALL fs operations

process.permission.deny('fs.out') // deny permissions to ALL FileSystemWrite operations
process.permission.deny('fs.out', '/home/rafaelgss/protected-folder') // deny FileSystemWrite permissions to the protected-folder
process.permission.deny('fs.in') // deny permissions to ALL FileSystemRead operations
process.permission.deny('fs.in', '/home/rafaelgss/protected-folder') // deny FileSystemRead permissions to the protected-folder
  • has(scope [,parameters])

API Call to check permissions at runtime. e.g:

process.permission.has('fs.out') // true
process.permission.has('fs.out', '/home/rafaelgss/protected-folder') // true

process.permission.deny('fs.out', '/home/rafaelgss/protected-folder')

process.permission.has('fs.out') // true
process.permission.has('fs.out', '/home/rafaelgss/protected-folder') // false

Future implementations

The implementation of the next features like “net” or “env” will be easily possible just by creating a new net_permission.h and implementing the PermissionBase methods.

image

FAQ

The user should be able to grant permissions in runtime?

No. Much like with other well-known and well-used permissions systems, code ought to be able to decide it can drop privileges, but never be able to grant itself any expanded privileges.

Can I deny permissions to just a specific module?

No. The permission system is process-scoped. You can use the [policy](https://nodejs.org/api/policy.html) to restrict module access.

What if I spawn a process, it will inherit the root permissions?

A process that has --experimental-permission will not be able to spawn a child process by default. If the user explicitly allows it to spawn a child process, then it will be the user's responsibility to pass along the correct arguments.

Benchmarks

This feature adds a very low overhead (if any), either enabled or disabled). Please, note I'm measuring only the feature usage without restricted files/resources (a better benchmark suite will be created in subsequent PRs). The principal behavior is that it doesn't add overhead to the main fs usage. For example, using the benchmark/fs/readfile.js

comparisson between main and this PR (permission model disabled)

➜ node-benchmark-compare compare_fs_permission.csv
                                                                      confidence improvement accuracy (*)   (**)  (***)
fs/readfile.js concurrent=1 len=1024 encoding='' duration=5                          -0.58 %       ±0.78% ±1.05% ±1.38%
fs/readfile.js concurrent=1 len=1024 encoding='utf-8' duration=5                     -0.24 %       ±0.44% ±0.58% ±0.76%
fs/readfile.js concurrent=1 len=16777216 encoding='' duration=5                       0.15 %       ±0.96% ±1.28% ±1.67%
fs/readfile.js concurrent=1 len=16777216 encoding='utf-8' duration=5                  0.76 %       ±1.16% ±1.56% ±2.04%
fs/readfile.js concurrent=10 len=1024 encoding='' duration=5                          0.32 %       ±0.39% ±0.52% ±0.68%
fs/readfile.js concurrent=10 len=1024 encoding='utf-8' duration=5              *     -0.73 %       ±0.57% ±0.76% ±0.99%
fs/readfile.js concurrent=10 len=16777216 encoding='' duration=5                     -0.71 %       ±1.54% ±2.06% ±2.69%
fs/readfile.js concurrent=10 len=16777216 encoding='utf-8' duration=5                 0.09 %       ±0.69% ±0.91% ±1.19%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 8 comparisons, you can thus expect the following amount of false-positive results:
  0.40 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.08 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

comparisson enabling permission model (this branch)

node on  feat/permission-system [⇕$] took 40.4s
➜ ./node benchmark/fs/readfile.js
fs/readfile.js concurrent=1 len=1024 encoding="" duration=5: 24,192.15827315814
fs/readfile.js concurrent=10 len=1024 encoding="" duration=5: 95,917.90322639987
fs/readfile.js concurrent=1 len=16777216 encoding="" duration=5: 612.9672164291768
fs/readfile.js concurrent=10 len=16777216 encoding="" duration=5: 1,107.9227500862503
fs/readfile.js concurrent=1 len=1024 encoding="utf-8" duration=5: 24,314.915724686496
fs/readfile.js concurrent=10 len=1024 encoding="utf-8" duration=5: 92,569.37571752333
fs/readfile.js concurrent=1 len=16777216 encoding="utf-8" duration=5: 278.3922768971328
fs/readfile.js concurrent=10 len=16777216 encoding="utf-8" duration=5: 338.3870755499357

node on  feat/permission-system [⇕$!] took 40.4s
➜ ./node benchmark/fs/readfile-permission-enabled.js
(node:62815) ExperimentalWarning: Permission is an experimental feature
(Use `node --trace-warnings ...` to show where the warning was created)
^C

node on  feat/permission-system [⇕$!]
➜ ./node --no-warnings benchmark/fs/readfile-permission-enabled.js
fs/readfile-permission-enabled.js concurrent=1 len=1024 encoding="" duration=5: 24,185.367751785307
fs/readfile-permission-enabled.js concurrent=10 len=1024 encoding="" duration=5: 96,253.07607327335
fs/readfile-permission-enabled.js concurrent=1 len=16777216 encoding="" duration=5: 612.8715221066299
fs/readfile-permission-enabled.js concurrent=10 len=16777216 encoding="" duration=5: 1,106.1275272124165
fs/readfile-permission-enabled.js concurrent=1 len=1024 encoding="utf-8" duration=5: 24,375.648655236808
fs/readfile-permission-enabled.js concurrent=10 len=1024 encoding="utf-8" duration=5: 91,882.5214931248
fs/readfile-permission-enabled.js concurrent=1 len=16777216 encoding="utf-8" duration=5: 277.1205079822853
fs/readfile-permission-enabled.js concurrent=10 len=16777216 encoding="utf-8" duration=5: 337.30003270280775

Additional Considerations

cc: @nodejs/security-wg

Merge request reports