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

--allow-net=0.0.0.0:port allows both fetch and Deno.listen #16532

Open
mcodik opened this issue Nov 3, 2022 · 6 comments
Open

--allow-net=0.0.0.0:port allows both fetch and Deno.listen #16532

mcodik opened this issue Nov 3, 2022 · 6 comments

Comments

@mcodik
Copy link

mcodik commented Nov 3, 2022

deno 1.24.2 (release, aarch64-apple-darwin)
v8 10.4.132.20
typescript 4.7.4

If your code uses Deno.listen, it requires you to give permission via --allow-net=0.0.0.0:port

In my testing on Mac 12.6.1, calling fetch("http://0.0.0.0:8080") ends up hitting localhost on port 8080. Apparently this is true for Macs generally as curl http://0.0.0.0:8080 works too. Some discussion here: whatwg/fetch#1117

A consequence of this is that --allow-net cant distinguish the two flows: if you allow listening to http requests, you also allow code to hit localhost on those ports. The same permission allows both.

This isn't a problem for us, but since the behavior was surprising to me I thought I'd post an issue to share. Example code snippet below.

const startServer = async function (port: number) {
  const server = Deno.listen({ port });
  for await (const conn of server) {
    serveHttp(conn);
  }

  async function serveHttp(conn: Deno.Conn) {
    const httpConn = Deno.serveHttp(conn);
    for await (const requestEvent of httpConn) {
      const url = new URL(requestEvent.request.url);
      console.log("GET " + url.search);
      if (url.search == "?a=1") {
        await fetch("http://0.0.0.0:8080/foo?a=2");
      }

      await requestEvent.respondWith(
        new Response("OK", {
          status: 200,
        }),
      ).catch((e) => console.log(`Uncaught exception: ${e}`));
    }
  }
};

if (import.meta.main) {
  const port = 8080;
  await startServer(port);
}

Run this with deno run --no-prompt --allow-net=0.0.0.0:8080 ./localhost.ts in one terminal, then if you curl "localhost:8080/foo?a=1 in another terminal, you can see see both ?a=1 (from curl) and ?a=2 (from fetch(0.0.0.0)) logged

@teleclimber
Copy link

Yes, it's a problem that --allow-net does not differentiate between listen and dial. If you want togive liberal permissions to a script to make requests to a any host, you also give the script the ability to listen on any port. This can have huge consequences on a server environment. I wrote an issue long ago but it got stale-botted into oblivion:

#2705

@bartlomieju
Copy link
Member

CC @crowlKats I believe this is something you wanted to look into for Deno 2.0. The question is what the flag names would be

@jtoppine
Copy link

It seems a sensible option would be to change the meaning of --allow-net for Deno 2.0 to only allow making requests / dial. This would break the least amount of existing code, as most often the intention is to allow some code to act as client to whatever service.

Then introduce --allow-listen or such for Deno 2.0. Which allows listening for ports / creating servers. Server apps are not as numerous, and are more security sensitive. So it would make sense to err on the side of caution, let existing code fail by permission denied, and expect server apps/bootstrap code to adapt to the changed semantics of --allow-net vs --allow-listen.

@Aloso
Copy link

Aloso commented Apr 30, 2023

How about adding two new flags: allow-request and --allow-listen. Then --allow-net would not change, and just be an abbreviation for --allow-listen --allow-request.

@carragom
Copy link

carragom commented Nov 25, 2023

Just stumbled upon this and IMHO I agree that leaving --allow-net as it's today would be great since it does not break anything, maybe deprecate it and remove it later. As for the other two flags: --allow-listen and --allow-connect would be great names.

Finally the new parameters could use a URL like syntax. A few examples come to mind.

General purpose using transport protocol as scheme, should fit all needs:

  • --allow-{connect,listen}=udp://8.8.8.8:53 => {protocol: "udp", hostname: "8.8.8.8", port: "53"}
  • --allow-{connect,listen}=tcp://gitlab.com:22 => {protocol: "tcp", hostname: "gitlab.com", port: "22"}
  • --allow-{connect,listen}=sctp://9.9.9.9 => {protocol: "sctp", hostname: "9.9.9.9", port: ""}

Some shortcuts could exist for well known application protocols using application protocol as scheme:

  • --allow-{connect,listen}=http://example.com => {protocol: "tcp", hostname: "example.com", port: "80"}
  • --allow-{connect,listen}=https://example.com => {protocol: "tcp", hostname: "example.com", port: "443"}
  • --allow-{connect,listen}=ssh://github.com => {protocol: "tcp", hostname: "github.com", port: "22"}

@jtoppine
Copy link

jtoppine commented Jan 30, 2024

Since we are getting closer to Deno 2.0 and some mildly breaking changes can be expected, should we reopen this discussion?

Now would be great opportunity to introduce separate permission for listening on a port versus dialing to somewhere else.

From a user's point of view, as in what permission a user might want to give to a script.. this distinction of creating a server (listen) vs allowing external fetch/connect (dial) is quite clear typically. It is quite confusing and unexpected that these are lumped together in to a singular permission flag.

The worst thing I can imagine a malicious module could attempt is to open up port to for a longlived server listening for whatever actions an attacker might want to execute on the machine. To me an explicit permission flag for binding to a port would make so much more sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants