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

Disallow HTTP by default #7004

Closed
agamm opened this issue Aug 10, 2020 · 23 comments
Closed

Disallow HTTP by default #7004

agamm opened this issue Aug 10, 2020 · 23 comments
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)

Comments

@agamm
Copy link

agamm commented Aug 10, 2020

Considering the main title of deno: "A secure runtime for JavaScript and TypeScript." it seems wise to disallow HTTP by default, thus requiring https for all external interactions.

I know this might hinder local development speed, but it shouldn't be any different than accessing the fs or the network (in the same way it is done at the moment).
A reasonable approach would be to add a --allow-http or --insecure-http when needing HTTP (not secure).
If there is a strong opinion against it at least a warning should be helpful.

Version:

deno 1.2.3
v8 8.6.334
typescript 3.9.2

To reproduce

async function d() {
   let r = await fetch('http://example.com')
   let t = await r.text()
   console.log(t)
  }
d()
// returns the example.com website contents without even a warning.

Note: I tried searching for a corresponding issue but couldn't find one, please close if this is indeed a duplicate 🙏

@bartlomieju
Copy link
Member

Already discussed in #1063. Deno allows HTTP imports just like browsers: #1063 (comment)

@agamm
Copy link
Author

agamm commented Aug 12, 2020

Browsers allow networking by default but deno doesn't. So why disallowing a widely known security issue is any different?
Having a bowser 1:1 argument is fine if it is consistent, but it isn't. The idea works great with the current arguments deno has with permissions, no?
Shouldn't this be revisited?

@nayeemrmn
Copy link
Collaborator

We have #5680 but it doesn't apply to runtime fetch(). I think it does in browsers.

We shouldn't disallow http, nor should it require a flag as long as https -> http downgrades are disallowed (if you don't import http in the code you control then nothing else can) but maybe it should also apply to runtime fetch() though I don't know how we could do that.

@agamm
Copy link
Author

agamm commented Aug 12, 2020

We shouldn't disallow http, nor should it require a flag as long as https -> http downgrades are disallowed (if you don't import http in the code you control then nothing else can

Not knowing how to do something shouldn't be a blocker, but good points.

@nayeemrmn

This comment has been minimized.

@itayronen
Copy link

@agamm Is talking about HTTP communication in general not just imports.
Which will make deno much more secured by default.

@nayeemrmn
Copy link
Collaborator

Oops, I forgot what I wrote above :P

@Spoonbender
Copy link
Contributor

On one of the other issues I suggested enabling scheme-based fine-grained allows, so you can do --allow-net=https://* to only allow https (or =amqps://* to only allow AMQP over TLS, etc.)

@agamm
Copy link
Author

agamm commented Aug 16, 2020

On one of the other issues I suggested enabling scheme-based fine-grained allows, so you can do --allow-net=https://* to only allow https (or =amqps://* to only allow AMQP over TLS, etc.)

That sounds great, can you link it?

@Spoonbender
Copy link
Contributor

On one of the other issues I suggested enabling scheme-based fine-grained allows, so you can do --allow-net=https://* to only allow https (or =amqps://* to only allow AMQP over TLS, etc.)

That sounds great, can you link it?

See discussion here: #6532

@agamm
Copy link
Author

agamm commented Aug 16, 2020

On one of the other issues I suggested enabling scheme-based fine-grained allows, so you can do --allow-net=https://* to only allow https (or =amqps://* to only allow AMQP over TLS, etc.)

That sounds great, can you link it?

See discussion here: #6532

Just to clarify, it doesn't related to the "By default" part we are talking here, only an implementation, right?

@Spoonbender
Copy link
Contributor

On one of the other issues I suggested enabling scheme-based fine-grained allows, so you can do --allow-net=https://* to only allow https (or =amqps://* to only allow AMQP over TLS, etc.)

That sounds great, can you link it?

See discussion here: #6532

Just to clarify, it doesn't related to the "By default" part we are talking here, only an implementation, right?

Right, this isn't about "by default", just about providing the users with the abilities to impose limitations such as "disallow HTTP". Currently there is no way to disallow HTTP but allow HTTPS.

My opinion is that we shouldn't make HTTP an exception (no need for a new command-line switch).
I like the following model:

  • By default: no network allowed whatsoever
  • Running with --allow-net switch, without specifying a value - everything on the net is available (including HTTP), same as today
  • Running with--allow-net=https://* - only HTTPS endpoints are available

@agamm
Copy link
Author

agamm commented Aug 16, 2020

On one of the other issues I suggested enabling scheme-based fine-grained allows, so you can do --allow-net=https://* to only allow https (or =amqps://* to only allow AMQP over TLS, etc.)

That sounds great, can you link it?

See discussion here: #6532

Just to clarify, it doesn't related to the "By default" part we are talking here, only an implementation, right?

Right, this isn't about "by default", just about providing the users with the abilities to impose limitations such as "disallow HTTP". Currently there is no way to disallow HTTP but allow HTTPS.

My opinion is that we shouldn't make HTTP an exception (no need for a new command-line switch).
I like the following model:

  • By default: no network allowed whatsoever
  • Running with --allow-net switch, without specifying a value - everything on the net is available (including HTTP), same as today
  • Running with--allow-net=https://* - only HTTPS endpoints are available

Imposing default secure configurations isn't something you think is necessary? and allowing to add exceptions afterwards?

@Spoonbender
Copy link
Contributor

On one of the other issues I suggested enabling scheme-based fine-grained allows, so you can do --allow-net=https://* to only allow https (or =amqps://* to only allow AMQP over TLS, etc.)

That sounds great, can you link it?

See discussion here: #6532

Just to clarify, it doesn't related to the "By default" part we are talking here, only an implementation, right?

Right, this isn't about "by default", just about providing the users with the abilities to impose limitations such as "disallow HTTP". Currently there is no way to disallow HTTP but allow HTTPS.
My opinion is that we shouldn't make HTTP an exception (no need for a new command-line switch).
I like the following model:

  • By default: no network allowed whatsoever
  • Running with --allow-net switch, without specifying a value - everything on the net is available (including HTTP), same as today
  • Running with--allow-net=https://* - only HTTPS endpoints are available

Imposing default secure configurations isn't something you think is necessary? and allowing to add exceptions afterwards?

The default is secure: no network is allowed.
I think the --allow-net switch, when no value is provided, should remain as-is: "allow everything on the net" and not "allow secure net endpoints".
I think if we follow the idea of fine-grained secure defaults, then why stop at HTTPS?
Lets only allow TLS 1.3, and only support strong ciphersuites in the TLS handshake, and only bearer tokens in auth headers, etc.
But hey, that is just my opinion :-)

@ghost
Copy link

ghost commented Aug 16, 2020

Already discussed in #1063. Deno allows HTTP imports just like browsers: #1063 (comment)

When in a secure context (HTTPS) in a browser, importing from HTTP is disallowed.

@lucacasonato lucacasonato added cli related to cli/ dir suggestion suggestions for new features (yet to be agreed) labels Aug 17, 2020
@ellismarkf
Copy link

ellismarkf commented Nov 6, 2020

I can't find anything in the documentation about this specifically, but in this example from the manual :

const url = Deno.args[0];
const res = await fetch(url);

const body = new Uint8Array(await res.arrayBuffer());
await Deno.stdout.write(body);

They demonstrate how the script will fail without the --allow-net flag, and then suggest the following:

deno run --allow-net=example.com https://deno.land/[email protected]/examples/curl.ts https://example.com

Just for fun, I tried hitting Google, changing the domain to google.com and the URL argument to https://www.google.com:

λ deno run --allow-net=google.com https://deno.land/[email protected]/examples/curl.ts https://www.google.com
error: Uncaught (in promise) PermissionDenied: network access to "https://www.google.com/", run again with the --allow-net flag
    at processResponse (core.js:226:13)
    at Object.jsonOpAsync (core.js:244:12)
    at async fetch (deno:op_crates/fetch/26_fetch.js:1273:29)
    at async https://deno.land/[email protected]/examples/curl.ts:3:13

You'll see that the error suggests running the command again with the --allow-net flag, even though that flag has been specified, with the domain included. Running the command with only --allow-net set, ie not passing a domain to the flag, does work.

This thread seemed like a good place to ask about it — anyone know why this might be happening? For reference, I get the same error when the domain passed to --allow-net does not match the domain of the URL passed to the script.

At the very least it seems like the error message could be improved?

@lucacasonato
Copy link
Member

@ellismarkf www.google.com is not the same as google.com. That is why this does not work. I agree the error message could be improved to say:

PermissionDenied: Network access to `https://www.google.com/`, add `www.google.com` to the `--allow-net` allowlist.

@hagenek
Copy link

hagenek commented Nov 29, 2021

Yeah I am getting this error.. the tutorial doesnt really work, which is not good.

❯ deno run server.ts --allow-net=www.google.com www.google.com
error: Uncaught TypeError: Invalid URLoogle.com www.google.com                                                                                                                                                    ─╯
const res = await fetch(url);

Here is my server.ts code:

const url = Deno.args[0];
const res = await fetch(url);

const body = new Uint8Array(await res.arrayBuffer());
await Deno.stdout.write(body);

@nayeemrmn
Copy link
Collaborator

@hagenek Your command should be deno run server.ts --allow-net=www.google.com https://www.google.com.

@crowlKats
Copy link
Member

@hagenek Your command should be deno run server.ts --allow-net=www.google.com https://www.google.com.

You mean deno run --allow-net=www.google.com server.ts https://www.google.com, right?

@hagenek
Copy link

hagenek commented Nov 29, 2021

@crowlKats that works, thanks! Feel like a newbie..

@crowlKats
Copy link
Member

@hagenek no worries. FYI: all flags related to the deno cli have to be put before the path of the file you want to run. everything after the path gets passed to the script and is accessible via Deno.args

@lucacasonato
Copy link
Member

lucacasonato commented Sep 16, 2024

We're not going to change this to be disallowed by default. However, a PR that enables --deny-net=*:80 to disallow any connections to port 80 would be interesting. Closing in favor of #6532

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

9 participants