Skip to content

permission: add permission for udp #53398

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,58 @@ Error: Access to this API has been restricted
}
```

### `--allow-net-udp`

<!-- YAML
added: REPLACEME
-->

> Stability: 1.1 - Active development

When using the [Permission Model][], the process will not be able to create,
receive, and send UDP packages by default. Attempts to do so will throw an
`ERR_ACCESS_DENIED` unless the user explicitly passes the `--allow-net-udp` flag
when starting Node.js. Node.js will check the permission when create a UDP socket
from fd.

The argument format is `--allow-net-udp=domain_or_ip[/netmask][:port]`.
The valid arguments are:

* `*` - To allow all operations.
* `--allow-net-udp=nodejs.org`
* `--allow-net-udp=127.0.0.1`
* `--allow-net-udp=127.0.0.1:8888`
* `--allow-net-udp=127.0.0.1:*`
* `--allow-net-udp=*:9999`
* `--allow-net-udp=127.0.0.1/24:*`
* `--allow-net-udp=127.0.0.1/255.255.255.0:*`
* `--allow-net-udp=127.0.0.1:8080 --allow-net-udp=127.0.0.1:9090`
* `--allow-net-udp=127.0.0.1:8080,localhost:9090`

Example:

```js
const dgram = require('node:dgram');
dgram.createSocket('udp4').bind(9297, '127.0.0.1')
```

```console
$ node --experimental-permission --allow-fs-read=./index.js index.js
node:events:498
throw er; // Unhandled 'error' event
^

Error [ERR_ACCESS_DENIED]: Access to this API has been restricted. Permission: bind to 127.0.0.1/9297
at node:dgram:379:18
at process.processTicksAndRejections (node:internal/process/task_queues:77:11)
Emitted 'error' event on Socket instance at:
at afterDns (node:dgram:337:12)
at node:dgram:379:9
at process.processTicksAndRejections (node:internal/process/task_queues:77:11) {
code: 'ERR_ACCESS_DENIED'
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding "Permission: bind to 127.0.0.1/9297", we should add a resource field and a permission field to the error object. See how --allow-fs-read works

Copy link
Member

Choose a reason for hiding this comment

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

ping

}
```

### `--build-snapshot`

<!-- YAML
Expand Down Expand Up @@ -1014,6 +1066,7 @@ following permissions are restricted:
* Child Process - manageable through [`--allow-child-process`][] flag
* Worker Threads - manageable through [`--allow-worker`][] flag
* WASI - manageable through [`--allow-wasi`][] flag
* UDP - manageable through [`--allow-net-udp`][] flag

### `--experimental-require-module`

Expand Down Expand Up @@ -2835,6 +2888,7 @@ one is included in the list below.
* `--allow-child-process`
* `--allow-fs-read`
* `--allow-fs-write`
* `--allow-net-udp`
* `--allow-wasi`
* `--allow-worker`
* `--conditions`, `-C`
Expand Down Expand Up @@ -3390,6 +3444,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
[`--allow-child-process`]: #--allow-child-process
[`--allow-fs-read`]: #--allow-fs-read
[`--allow-fs-write`]: #--allow-fs-write
[`--allow-net-udp`]: #--allow-net-udp
[`--allow-wasi`]: #--allow-wasi
[`--allow-worker`]: #--allow-worker
[`--build-snapshot`]: #--build-snapshot
Expand Down
4 changes: 4 additions & 0 deletions doc/api/permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,8 @@ using the [`--allow-child-process`][] and [`--allow-worker`][] respectively.
To allow native addons when using permission model, use the [`--allow-addons`][]
flag. For WASI, use the [`--allow-wasi`][] flag.

For UDP, use [`--allow-net-udp`][] flag.

#### Runtime API

When enabling the Permission Model through the [`--experimental-permission`][]
Expand Down Expand Up @@ -583,6 +585,7 @@ There are constraints you need to know before using this system:
* Inspector protocol
* File system access
* WASI
* UDP
* The Permission Model is initialized after the Node.js environment is set up.
However, certain flags such as `--env-file` or `--openssl-config` are designed
to read files before environment initialization. As a result, such flags are
Expand All @@ -607,6 +610,7 @@ There are constraints you need to know before using this system:
[`--allow-child-process`]: cli.md#--allow-child-process
[`--allow-fs-read`]: cli.md#--allow-fs-read
[`--allow-fs-write`]: cli.md#--allow-fs-write
[`--allow-net-udp`]: cli.md#--allow-net-udp
[`--allow-wasi`]: cli.md#--allow-wasi
[`--allow-worker`]: cli.md#--allow-worker
[`--experimental-permission`]: cli.md#--experimental-permission
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ Allow execution of WASI when using the permission model.
.It Fl -allow-worker
Allow creating worker threads when using the permission model.
.
.It Fl -allow-net-udp
Allow create, receive, and, send UDP packages when using the permission model.
.
.It Fl -completion-bash
Print source-able bash completion script for Node.js.
.
Expand Down
81 changes: 72 additions & 9 deletions lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const {
ErrnoException,
ExceptionWithHostPort,
codes: {
ERR_ACCESS_DENIED,
ERR_BUFFER_OUT_OF_BOUNDS,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_FD_TYPE,
Expand Down Expand Up @@ -81,6 +82,7 @@ const {

const dc = require('diagnostics_channel');
const udpSocketChannel = dc.channel('udp.socket');
const permission = require('internal/process/permission');

const BIND_STATE_UNBOUND = 0;
const BIND_STATE_BINDING = 1;
Expand Down Expand Up @@ -327,12 +329,9 @@ Socket.prototype.bind = function(port_, address_ /* , callback */) {
else
address = '::';
}

// Resolve address first
state.handle.lookup(address, (err, ip) => {
const afterDns = (err, ip) => {
if (!state.handle)
return; // Handle has been closed in the mean time

if (err) {
state.bindState = BIND_STATE_UNBOUND;
this.emit('error', err);
Expand Down Expand Up @@ -361,7 +360,7 @@ Socket.prototype.bind = function(port_, address_ /* , callback */) {
this.emit('error', ex);
});
} else {
const err = state.handle.bind(ip, port || 0, flags);
const err = state.handle.bind(ip, port || 0, flags, false);
if (err) {
const ex = new ExceptionWithHostPort(err, 'bind', ip, port);
state.bindState = BIND_STATE_UNBOUND;
Expand All @@ -372,7 +371,22 @@ Socket.prototype.bind = function(port_, address_ /* , callback */) {

startListening(this);
}
});
};
if (permission.isEnabled()) {
const resource = `${address}/${port || '*'}`;
if (!permission.has('net.udp', resource)) {
process.nextTick(() => {
afterDns(new ERR_ACCESS_DENIED(
`bind to ${resource}`,
'NetUDP',
resource,
));
});
return this;
}
}
// Resolve address first
state.handle.lookup(address, afterDns);
Copy link
Member

Choose a reason for hiding this comment

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

Why don't we throw the error in .lookup()? This would reduce the duplicated code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the address which is allowed is a domain, we should allow bind to all IPs this domain ponit to, so I think we need check before DNS lookup.


return this;
};
Expand Down Expand Up @@ -413,13 +427,38 @@ function _connect(port, address, callback) {
this.once('connect', callback);

const afterDns = (ex, ip) => {
if (!ex && !address && permission.isEnabled()) {
const resource = `${ip}/${port}`;
if (!permission.has('net.udp', resource)) {
ex = new ERR_ACCESS_DENIED(
`connect to ${resource}`,
'NetUDP',
resource,
);
}
}
defaultTriggerAsyncIdScope(
this[async_id_symbol],
doConnect,
ex, this, ip, address, port, callback,
);
};

// If the address which is allowed is a domain,
// we should allow bind to all IPs this domain ponit to,
// so we need check here instead of in afterDns
if (address && permission.isEnabled()) {
const resource = `${address}/${port}`;
if (!permission.has('net.udp', resource)) {
process.nextTick(() => {
afterDns(new ERR_ACCESS_DENIED(
`connect to ${resource}`,
'NetUDP',
resource,
));
});
return;
}
}
state.handle.lookup(address, afterDns);
}

Expand All @@ -430,7 +469,7 @@ function doConnect(ex, self, ip, address, port, callback) {
return;

if (!ex) {
const err = state.handle.connect(ip, port);
const err = state.handle.connect(ip, port, false);
if (err) {
ex = new ExceptionWithHostPort(err, 'connect', address, port);
}
Expand Down Expand Up @@ -663,6 +702,17 @@ Socket.prototype.send = function(buffer,
}

const afterDns = (ex, ip) => {
// If we have not checked before dns, check it now
if (!ex && !connected && !address && permission.isEnabled()) {
const resource = `${ip}/${port}`;
if (!permission.has('net.udp', resource)) {
ex = new ERR_ACCESS_DENIED(
`send to ${resource}`,
'NetUDP',
resource,
);
}
}
defaultTriggerAsyncIdScope(
this[async_id_symbol],
doSend,
Expand All @@ -671,6 +721,19 @@ Socket.prototype.send = function(buffer,
};

if (!connected) {
if (address && permission.isEnabled()) {
const resource = `${address}/${port}`;
if (!permission.has('net.udp', resource)) {
process.nextTick(() => {
afterDns(new ERR_ACCESS_DENIED(
`send to ${resource}`,
'NetUDP',
resource,
));
});
return;
}
}
state.handle.lookup(address, afterDns);
} else {
afterDns(null, null);
Expand Down Expand Up @@ -703,7 +766,7 @@ function doSend(ex, self, ip, list, address, port, callback) {

let err;
if (port)
err = state.handle.send(req, list, list.length, port, ip, !!callback);
err = state.handle.send(req, list, list.length, port, ip, !!callback, false);
else
err = state.handle.send(req, list, list.length, !!callback);

Expand Down
2 changes: 1 addition & 1 deletion lib/internal/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function _createSocketHandle(address, port, addressType, fd, flags) {
err = handle.open(fd);
}
} else if (port || address) {
err = handle.bind(address, port || 0, flags);
err = handle.bind(address, port || 0, flags, true);
}

if (err) {
Expand Down
1 change: 1 addition & 0 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,7 @@ function initializePermission() {
'--allow-child-process',
'--allow-wasi',
'--allow-worker',
'--allow-net-udp',
];
ArrayPrototypeForEach(availablePermissionFlags, (flag) => {
const value = getOptionValue(flag);
Expand Down
2 changes: 2 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@
'src/permission/permission.cc',
'src/permission/wasi_permission.cc',
'src/permission/worker_permission.cc',
'src/permission/net_permission.cc',
'src/pipe_wrap.cc',
'src/process_wrap.cc',
'src/signal_wrap.cc',
Expand Down Expand Up @@ -283,6 +284,7 @@
'src/permission/permission.h',
'src/permission/wasi_permission.h',
'src/permission/worker_permission.h',
'src/permission/net_permission.h',
'src/pipe_wrap.h',
'src/req_wrap.h',
'src/req_wrap-inl.h',
Expand Down
4 changes: 4 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,10 @@ Environment::Environment(IsolateData* isolate_data,
options_->allow_fs_write,
permission::PermissionScope::kFileSystemWrite);
}
if (!options_->allow_net_udp.empty()) {
permission()->Apply(
this, options_->allow_net_udp, permission::PermissionScope::kNetUDP);
}
}
}

Expand Down
Loading
Loading