foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-26 00:59:24 -05:00
Code Issues Packages 1 Wiki Activity

BREAKING(unstable): Use hosts for net allowlists (#8845)

Browse source 
Allowlist checking already uses hosts but for some reason 
requests, revokes and the runtime permissions API use URLs.

- BREAKING(lib.deno.unstable.d.ts): Change 
NetPermissionDescriptor::url to NetPermissionDescriptor::host

- fix(runtime/permissions): Don't add whole URLs to the 
allowlist on request

- fix(runtime/permissions): Harden strength semantics:
({ name: "net", host: "127.0.0.1" } is stronger than 
{ name: "net", host: "127.0.0.1:8000" }) for blocklisting

- refactor(runtime/permissions): Use tuples for hosts, make 
the host optional in Permissions::{query_net, request_net, revoke_net}()
This commit is contained in:
Nayeem Rahman 2020-12-30 22:35:28 +00:00 committed by GitHub
parent bcdc2da4c7
commit 22e0ee92a6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 161 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
cli/dts/lib.deno.unstable.d.ts vendored
View file

@ -1085,13 +1085,12 @@ declare namespace Deno {
export interface NetPermissionDescriptor {
name: "net";
/** Optional url associated with this descriptor.
/** Optional host string of the form `"<hostname>[:<port>]"`. Examples:
*
* If specified: must be a valid url. Expected format: <scheme>://<host_or_ip>[:port][/path]
* If the scheme is unknown, callers should specify some scheme, such as x:// na:// unknown://
*
* See: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml */
url?: string;
* "github.com"
* "deno.land:8080"
*/
host?: string;
}
export interface EnvPermissionDescriptor {

2
cli/tests/error_015_dynamic_import_permissions.out
View file

@ -1 +1 @@
error: Uncaught (in promise) TypeError: network access to "http://localhost:4545/cli/tests/subdir/mod4.js", run again with the --allow-net flag
error: Uncaught (in promise) TypeError: network access to "localhost:4545", run again with the --allow-net flag

2
cli/tests/error_worker_permissions_remote.ts.out
View file

@ -1,3 +1,3 @@
[WILDCARD]
error: Uncaught (in worker "") network access to "http://localhost:4545/cli/tests/subdir/worker_types.ts", run again with the --allow-net flag
error: Uncaught (in worker "") network access to "localhost:4545", run again with the --allow-net flag
[WILDCARD]

4
cli/tests/unit/permissions_test.ts
View file

@ -13,9 +13,9 @@ unitTest(async function permissionInvalidName(): Promise<void> {
}, Error);
});
unitTest(async function permissionNetInvalidUrl(): Promise<void> {
unitTest(async function permissionNetInvalidHost(): Promise<void> {
await assertThrowsAsync(async () => {
await Deno.permissions.query({ name: "net", url: ":" });
await Deno.permissions.query({ name: "net", host: ":" });
}, URIError);
});

6
docs/runtime/permission_apis.md
View file

@ -32,7 +32,7 @@ const desc2 = { name: "write", path: "foo/bar" } as const;
const desc3 = { name: "net" } as const;
// Net permission to 127.0.0.1:8000.
const desc4 = { name: "net", url: "127.0.0.1:8000" } as const;
const desc4 = { name: "net", host: "127.0.0.1:8000" } as const;
// High-resolution time permission.
const desc5 = { name: "hrtime" } as const;
@ -86,9 +86,9 @@ const desc1 = { name: "write" } as const;
// is stronger than
const desc2 = { name: "write", path: "/foo" } as const;
const desc3 = { name: "net" } as const;
const desc3 = { name: "net", host: "127.0.0.1" } as const;
// is stronger than
const desc4 = { name: "net", url: "127.0.0.1:8000" } as const;
const desc4 = { name: "net", host: "127.0.0.1:8000" } as const;
```
### Request permissions

6
runtime/ops/net.rs
View file

@ -200,7 +200,7 @@ async fn op_datagram_send(
{
let s = state.borrow();
s.borrow::<Permissions>()
.check_net(&args.hostname, args.port)?;
.check_net(&(&args.hostname, Some(args.port)))?;
}
let addr = resolve_addr(&args.hostname, args.port)
.await?
@ -268,7 +268,7 @@ async fn op_connect(
let state_ = state.borrow();
state_
.borrow::<Permissions>()
.check_net(&args.hostname, args.port)?;
.check_net(&(&args.hostname, Some(args.port)))?;
}
let addr = resolve_addr(&args.hostname, args.port)
.await?
@ -473,7 +473,7 @@ fn op_listen(
if transport == "udp" {
super::check_unstable(state, "Deno.listenDatagram");
}
permissions.check_net(&args.hostname, args.port)?;
permissions.check_net(&(&args.hostname, Some(args.port)))?;
}
let addr = resolve_addr_sync(&args.hostname, args.port)?
.next()

38
runtime/ops/permissions.rs
View file

@ -2,10 +2,12 @@
use crate::permissions::Permissions;
use deno_core::error::custom_error;
use deno_core::error::uri_error;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_core::serde_json::json;
use deno_core::serde_json::Value;
use deno_core::url;
use deno_core::OpState;
use deno_core::ZeroCopyBuf;
use serde::Deserialize;
@ -20,8 +22,8 @@ pub fn init(rt: &mut deno_core::JsRuntime) {
#[derive(Deserialize)]
struct PermissionArgs {
name: String,
url: Option<String>,
path: Option<String>,
host: Option<String>,
}
pub fn op_query_permission(
@ -35,7 +37,13 @@ pub fn op_query_permission(
let perm = match args.name.as_ref() {
"read" => permissions.query_read(&path.as_deref().map(Path::new)),
"write" => permissions.query_write(&path.as_deref().map(Path::new)),
"net" => permissions.query_net_url(&args.url.as_deref())?,
"net" => permissions.query_net(
&match args.host.as_deref() {
None => None,
Some(h) => Some(parse_host(h)?),
}
.as_ref(),
),
"env" => permissions.query_env(),
"run" => permissions.query_run(),
"plugin" => permissions.query_plugin(),
@ -61,7 +69,13 @@ pub fn op_revoke_permission(
let perm = match args.name.as_ref() {
"read" => permissions.revoke_read(&path.as_deref().map(Path::new)),
"write" => permissions.revoke_write(&path.as_deref().map(Path::new)),
"net" => permissions.revoke_net(&args.url.as_deref())?,
"net" => permissions.revoke_net(
&match args.host.as_deref() {
None => None,
Some(h) => Some(parse_host(h)?),
}
.as_ref(),
),
"env" => permissions.revoke_env(),
"run" => permissions.revoke_run(),
"plugin" => permissions.revoke_plugin(),
@ -87,7 +101,13 @@ pub fn op_request_permission(
let perm = match args.name.as_ref() {
"read" => permissions.request_read(&path.as_deref().map(Path::new)),
"write" => permissions.request_write(&path.as_deref().map(Path::new)),
"net" => permissions.request_net(&args.url.as_deref())?,
"net" => permissions.request_net(
&match args.host.as_deref() {
None => None,
Some(h) => Some(parse_host(h)?),
}
.as_ref(),
),
"env" => permissions.request_env(),
"run" => permissions.request_run(),
"plugin" => permissions.request_plugin(),
@ -101,3 +121,13 @@ pub fn op_request_permission(
};
Ok(json!({ "state": perm.to_string() }))
}
fn parse_host(host_str: &str) -> Result<(String, Option<u16>), AnyError> {
let url = url::Url::parse(&format!("http://{}/", host_str))
.map_err(|_| uri_error("Invalid host"))?;
if url.path() != "/" {
return Err(uri_error("Invalid host"));
}
let hostname = url.host_str().unwrap();
Ok((hostname.to_string(), url.port()))
}

6
runtime/ops/tls.rs
View file

@ -83,7 +83,7 @@ async fn op_start_tls(
super::check_unstable2(&state, "Deno.startTls");
let s = state.borrow();
let permissions = s.borrow::<Permissions>();
permissions.check_net(&domain, 0)?;
permissions.check_net(&(&domain, Some(0)))?;
if let Some(path) = cert_file.clone() {
permissions.check_read(Path::new(&path))?;
}
@ -147,7 +147,7 @@ async fn op_connect_tls(
{
let s = state.borrow();
let permissions = s.borrow::<Permissions>();
permissions.check_net(&args.hostname, args.port)?;
permissions.check_net(&(&args.hostname, Some(args.port)))?;
if let Some(path) = cert_file.clone() {
permissions.check_read(Path::new(&path))?;
}
@ -290,7 +290,7 @@ fn op_listen_tls(
let key_file = args.key_file;
{
let permissions = state.borrow::<Permissions>();
permissions.check_net(&args.hostname, args.port)?;
permissions.check_net(&(&args.hostname, Some(args.port)))?;
permissions.check_read(Path::new(&cert_file))?;
permissions.check_read(Path::new(&key_file))?;
}

177
runtime/permissions.rs
View file

@ -223,44 +223,29 @@ impl Permissions {
PermissionState::Prompt
}
pub fn query_net(&self, host: &str, port: Option<u16>) -> PermissionState {
pub fn query_net<T: AsRef<str>>(
&self,
host: &Option<&(T, Option<u16>)>,
) -> PermissionState {
if self.net.global_state == PermissionState::Denied
|| check_host_and_port_list(host, port, &self.net.denied_list)
&& match host.as_ref() {
None => true,
Some(host) => check_host_blocklist(host, &self.net.denied_list),
}
{
return PermissionState::Denied;
}
if self.net.global_state == PermissionState::Granted
|| check_host_and_port_list(host, port, &self.net.granted_list)
|| match host.as_ref() {
None => false,
Some(host) => check_host_allowlist(host, &self.net.granted_list),
}
{
return PermissionState::Granted;
}
PermissionState::Prompt
}
pub fn query_net_url(
&self,
url: &Option<&str>,
) -> Result<PermissionState, AnyError> {
if url.is_none() {
return Ok(self.net.global_state);
}
let url: &str = url.unwrap();
// If url is invalid, then throw a TypeError.
let parsed = url::Url::parse(url)?;
// The url may be parsed correctly but still lack a host, i.e. "localhost:235" or "mailto:someone@somewhere.com" or "file:/1.txt"
// Note that host:port combos are parsed as scheme:path
if parsed.host().is_none() {
return Err(custom_error(
"URIError",
"invalid url format: <scheme>://<host>[:port][/subpath]",
));
}
Ok(self.query_net(
&parsed.host().unwrap().to_string(),
parsed.port_or_known_default(),
))
}
pub fn query_env(&self) -> PermissionState {
self.env
}
@ -361,39 +346,49 @@ impl Permissions {
}
}
pub fn request_net(
pub fn request_net<T: AsRef<str>>(
&mut self,
url: &Option<&str>,
) -> Result<PermissionState, AnyError> {
if let Some(url) = url {
let state = self.query_net_url(&Some(url))?;
host: &Option<&(T, Option<u16>)>,
) -> PermissionState {
if let Some(host) = host {
let state = self.query_net(&Some(host));
if state == PermissionState::Prompt {
let host_string = format_host(host);
if permission_prompt(&format!(
"Deno requests network access to \"{}\"",
url
host_string,
)) {
self.net.granted_list.insert(url.to_string());
return Ok(PermissionState::Granted);
if host.1.is_none() {
self
.net
.granted_list
.retain(|h| !h.starts_with(&format!("{}:", host.0.as_ref())));
}
self.net.granted_list.insert(host_string);
return PermissionState::Granted;
} else {
self.net.denied_list.insert(url.to_string());
if host.1.is_some() {
self.net.denied_list.remove(host.0.as_ref());
}
self.net.denied_list.insert(host_string);
self.net.global_state = PermissionState::Denied;
return Ok(PermissionState::Denied);
return PermissionState::Denied;
}
}
Ok(state)
state
} else {
let state = self.query_net_url(&None)?;
let state = self.query_net::<&str>(&None);
if state == PermissionState::Prompt {
if permission_prompt("Deno requests network access") {
self.net.granted_list.clear();
self.net.global_state = PermissionState::Granted;
return Ok(PermissionState::Granted);
return PermissionState::Granted;
} else {
self.net.global_state = PermissionState::Denied;
return Ok(PermissionState::Denied);
return PermissionState::Denied;
}
}
Ok(state)
state
}
}
@ -473,19 +468,25 @@ impl Permissions {
self.query_write(path)
}
pub fn revoke_net(
pub fn revoke_net<T: AsRef<str>>(
&mut self,
url: &Option<&str>,
) -> Result<PermissionState, AnyError> {
if let Some(url) = url {
self.net.granted_list.remove(*url);
host: &Option<&(T, Option<u16>)>,
) -> PermissionState {
if let Some(host) = host {
self.net.granted_list.remove(&format_host(host));
if host.1.is_none() {
self
.net
.granted_list
.retain(|h| !h.starts_with(&format!("{}:", host.0.as_ref())));
}
} else {
self.net.granted_list.clear();
if self.net.global_state == PermissionState::Granted {
self.net.global_state = PermissionState::Prompt;
}
}
self.query_net_url(url)
self.query_net(host)
}
pub fn revoke_env(&mut self) -> PermissionState {
@ -545,18 +546,31 @@ impl Permissions {
)
}
pub fn check_net(&self, hostname: &str, port: u16) -> Result<(), AnyError> {
self.query_net(hostname, Some(port)).check(
&format!("network access to \"{}:{}\"", hostname, port),
pub fn check_net<T: AsRef<str>>(
&self,
host: &(T, Option<u16>),
) -> Result<(), AnyError> {
self.query_net(&Some(host)).check(
&format!("network access to \"{}\"", format_host(host)),
"--allow-net",
)
}
pub fn check_net_url(&self, url: &url::Url) -> Result<(), AnyError> {
let host = url.host_str().ok_or_else(|| uri_error("missing host"))?;
let hostname = url
.host_str()
.ok_or_else(|| uri_error("Missing host"))?
.to_string();
let display_host = match url.port() {
None => hostname.clone(),
Some(port) => format!("{}:{}", hostname, port),
};
self
.query_net(host, url.port_or_known_default())
.check(&format!("network access to \"{}\"", url), "--allow-net")
.query_net(&Some(&(hostname, url.port_or_known_default())))
.check(
&format!("network access to \"{}\"", display_host),
"--allow-net",
)
}
/// A helper function that determines if the module specifier is a local or
@ -689,14 +703,35 @@ fn check_path_blocklist(path: &Path, blocklist: &HashSet<PathBuf>) -> bool {
false
}
fn check_host_and_port_list(
host: &str,
port: Option<u16>,
fn check_host_allowlist<T: AsRef<str>>(
host: &(T, Option<u16>),
allowlist: &HashSet<String>,
) -> bool {
allowlist.contains(host)
|| (port.is_some()
&& allowlist.contains(&format!("{}:{}", host, port.unwrap())))
let (hostname, port) = host;
allowlist.contains(hostname.as_ref())
|| (port.is_some() && allowlist.contains(&format_host(host)))
}
fn check_host_blocklist<T: AsRef<str>>(
host: &(T, Option<u16>),
blocklist: &HashSet<String>,
) -> bool {
let (hostname, port) = host;
match port {
None => blocklist.iter().any(|host| {
host == hostname.as_ref()
|| host.starts_with(&format!("{}:", hostname.as_ref()))
}),
Some(_) => blocklist.contains(&format_host(host)),
}
}
fn format_host<T: AsRef<str>>(host: &(T, Option<u16>)) -> String {
let (hostname, port) = host;
match port {
None => hostname.as_ref().to_string(),
Some(port) => format!("{}:{}", hostname.as_ref(), port),
}
}
#[cfg(test)]
@ -851,8 +886,8 @@ mod tests {
assert_eq!(*is_ok, perms.check_net_url(&u).is_ok());
}
for (host, port, is_ok) in domain_tests.iter() {
assert_eq!(*is_ok, perms.check_net(host, *port).is_ok());
for (hostname, port, is_ok) in domain_tests.iter() {
assert_eq!(*is_ok, perms.check_net(&(hostname, Some(*port))).is_ok());
}
}
@ -1008,10 +1043,10 @@ mod tests {
assert_eq!(perms2.query_write(&None), PermissionState::Prompt);
assert_eq!(perms2.query_write(&Some(&Path::new("/foo"))), PermissionState::Granted);
assert_eq!(perms2.query_write(&Some(&Path::new("/foo/bar"))), PermissionState::Granted);
assert_eq!(perms1.query_net_url(&None).unwrap(), PermissionState::Granted);
assert_eq!(perms1.query_net_url(&Some("http://127.0.0.1:8000")).unwrap(), PermissionState::Granted);
assert_eq!(perms2.query_net_url(&None).unwrap(), PermissionState::Prompt);
assert_eq!(perms2.query_net_url(&Some("http://127.0.0.1:8000")).unwrap(), PermissionState::Granted);
assert_eq!(perms1.query_net::<&str>(&None), PermissionState::Granted);
assert_eq!(perms1.query_net(&Some(&("127.0.0.1", None))), PermissionState::Granted);
assert_eq!(perms2.query_net::<&str>(&None), PermissionState::Prompt);
assert_eq!(perms2.query_net(&Some(&("127.0.0.1", Some(8000)))), PermissionState::Granted);
assert_eq!(perms1.query_env(), PermissionState::Granted);
assert_eq!(perms2.query_env(), PermissionState::Prompt);
assert_eq!(perms1.query_run(), PermissionState::Granted);
@ -1057,9 +1092,9 @@ mod tests {
set_prompt_result(true);
assert_eq!(perms.request_write(&None), PermissionState::Denied);
set_prompt_result(true);
assert_eq!(perms.request_net(&None).unwrap(), PermissionState::Granted);
assert_eq!(perms.request_net(&Some(&("127.0.0.1", None))), PermissionState::Granted);
set_prompt_result(false);
assert_eq!(perms.request_net(&Some("http://127.0.0.1:8000")).unwrap(), PermissionState::Granted);
assert_eq!(perms.request_net(&Some(&("127.0.0.1", Some(8000)))), PermissionState::Granted);
set_prompt_result(true);
assert_eq!(perms.request_env(), PermissionState::Granted);
set_prompt_result(false);
@ -1093,7 +1128,8 @@ mod tests {
..Default::default()
},
net: UnaryPermission {
global_state: PermissionState::Denied,
global_state: PermissionState::Prompt,
granted_list: svec!["127.0.0.1"].iter().cloned().collect(),
..Default::default()
},
env: PermissionState::Granted,
@ -1109,7 +1145,8 @@ mod tests {
assert_eq!(perms.revoke_write(&Some(&Path::new("/foo/bar"))), PermissionState::Granted);
assert_eq!(perms.revoke_write(&None), PermissionState::Prompt);
assert_eq!(perms.query_write(&Some(&Path::new("/foo/bar"))), PermissionState::Prompt);
assert_eq!(perms.revoke_net(&None).unwrap(), PermissionState::Denied);
assert_eq!(perms.revoke_net(&Some(&("127.0.0.1", Some(8000)))), PermissionState::Granted);
assert_eq!(perms.revoke_net(&Some(&("127.0.0.1", None))), PermissionState::Prompt);
assert_eq!(perms.revoke_env(), PermissionState::Prompt);
assert_eq!(perms.revoke_run(), PermissionState::Prompt);
assert_eq!(perms.revoke_plugin(), PermissionState::Prompt);

4
std/permissions/mod.ts
View file

@ -14,8 +14,8 @@ function getPermissionString(descriptors: Deno.PermissionDescriptor[]): string {
? `--allow-${pd.name}=${pd.path}`
: `--allow-${pd.name}`;
case "net":
return pd.url
? `--allow-${pd.name}=${pd.url}`
return pd.host
? `--allow-${pd.name}=${pd.host}`
: `--allow-${pd.name}`;
default:
return `--allow-${pd.name}`;