microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.35k stars 29.31k forks source link

Can't attach Node.js debugger to hostname any more #48392

Closed chinthakagodawita closed 6 years ago

chinthakagodawita commented 6 years ago

Issue Type: Bug

I used to be able to attach to a Docker-base Node.js debug instance with the following launch config:

        {
          "type": "node",
          "request": "attach",
          "name": "Attach to Docker API",
          "port": 9229,
          "address": "api.docker",
          "localRoot": "${workspaceFolder}",
          "remoteRoot": "/app",
          "protocol": "inspector",
          "sourceMaps": true,
          "restart": true,
          "skipFiles": [
            "main.js",
            "async_hooks.js",
            "*.js",
          ]
        },

This no longer works, the only way I could get it working was by specifying the container IP, e.g. "address": "172.17.0.8",. Disabling all extensions doesn't help either.

This is a little problematic since the container IP may not be preserved between re-launches.

VS Code version: Code 1.22.2 (3aeede733d9a3098f7b4bdc1f66b63b0f48c1ef9, 2018-04-12T16:32:53.389Z) OS version: Darwin x64 17.3.0

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz (8 x 2200)| |Load (avg)|3, 3, 3| |Memory (System)|16.00GB (0.14GB free)| |Process Argv|/Applications/Visual Studio Code.app/Contents/MacOS/Electron| |Screen Reader|no| |VM|0%|
Extensions (18) Extension|Author (truncated)|Version ---|---|--- swagger-viewer|Arj|1.6.0 EditorConfig|Edi|0.12.1 gc-excelviewer|Gra|2.0.20 beautify|Hoo|1.3.0 material-icon-theme|PKi|3.3.0 vscode-docker|Pet|0.0.26 fish|Ted|0.0.4 vscode-intelephense-client|bme|0.8.8 gitlens|eam|8.2.4 tslint|eg2|1.0.28 ansible|haa|0.2.8 terraform|mau|0.0.23 python|ms-|2018.3.1 sublime-keybindings|ms-|3.0.3 vscode-yaml|red|0.0.11 vscode-nginx|sha|0.5.0 ansible-autocomplete|tim|0.0.2 jinja|who|0.0.8
weinand commented 6 years ago

I'm pretty sure that we haven't changed the way we deal with network names on the VS Code side. Could you try whether this really works in VS Code 1.21.1?

chinthakagodawita commented 6 years ago

I did a bit more digging and looks like it's not the VS Code version that's the problem, sorry!

We recently upgraded from Node 9.8 to 9.11 and it looks like that's whats causing it. If I drop back down to 9.8, the debugger attaches fine.

I did a bit of testing and something has changed in 9.10:

For reference, my Dockerfile is:

FROM node:9.11-alpine

ENV NPM_CONFIG_LOGLEVEL info
ENV IS_DOCKER 1

RUN apk add --update --no-cache \
      bash \
      sed \
      curl \
      wget \
      git \
      python \
      g++ \
      make \
      fish \
    && rm -f /tmp/* /etc/apk/cache/*

RUN addgroup -S app && adduser -S -g app app
RUN mkdir -p /app
WORKDIR /app
ENV HOME /app
USER app

EXPOSE 4000 9229

And Node is launched with --inspect=0.0.0.0:9229.

Additionally, the Chrome Inspector is able to successfully attach via hostname.

roblourens commented 6 years ago
chinthakagodawita commented 6 years ago

The error is:

Cannot connect to runtime process, timeout after 10000 ms - (reason: Cannot connect to the target: WebSockets request was expected).

The response from http://api.docker:9229/json is the following in plaintext:

WebSockets request was expected
roblourens commented 6 years ago

Are you sure you added /json? That message usually appears when accessing the server root instead of /json.

How do you attach Chrome Devtools?

roblourens commented 6 years ago

Also could you set "trace": true in your launch config, try again, and upload the log here?

chinthakagodawita commented 6 years ago

Yep, definitely hit http://api.docker:9229/json when I got that response.

Chrome Devtools is attached by adding a new device in chrome://inspect with an address of api.docker:9229, like so: inspect_with_chrome_developer_tools

Trace logs are attached: debugadapter.txt

chinthakagodawita commented 6 years ago

Some more interesting findings - digging through the Node 9.10 changelog brought up this change:

Fix for inspector DNS rebinding vulnerability (CVE-2018-7160): A malicious website could use a DNS rebinding attack to trick a web browser to bypass same-origin-policy checks and allow HTTP connections to localhost or to hosts on the local network, potentially to an open inspector port as a debugger, therefore gaining full code execution access. The inspector now only allows connections that have a browser Host value of localhost or localhost6.

And the following curl calls correlate with this:

$ curl -H "Content-type: application/json"  api.docker:9229/json/list
WebSockets request was expected
$ curl -H 'Host: api.docker' -H "Content-type: application/json"  api.docker:9229/json/list
WebSockets request was expected
$ curl -H 'Host: localhost' -H "Content-type: application/json"  api.docker:9229/json/list
[ {
  "description": "node.js instance",
  "devtoolsFrontendUrl": "chrome-devtools://devtools/bundled/inspector.html?experiments=true&v8only=true&ws=localhost/fcd89c99-119e-4458-8b8c-66435300c793",
  "faviconUrl": "https://nodejs.org/static/favicon.ico",
  "id": "fcd89c99-119e-4458-8b8c-66435300c793",
  "title": "/app/build/main.js",
  "type": "node",
  "url": "file:///app/build/main.js",
  "webSocketDebuggerUrl": "ws://localhost/fcd89c99-119e-4458-8b8c-66435300c793"
} ]
roblourens commented 6 years ago

I can repro but it works if I use an IP address instead of a hostname, does that work for you?

roblourens commented 6 years ago

Even after getting the right websocket url, it still won't work with a hostname. I'm not sure what Chrome is doing - resolving the hostname to an IP address, and replacing 'localhost' in the webSocketDebuggerUrl with the IP address/port I guess.

roblourens commented 6 years ago

I also need to set the Host header in the WS upgrade request

roblourens commented 6 years ago

Thanks for the detailed report. Please try it out in tomorrow's Insiders build!

roblourens commented 6 years ago

Verifier:

chinthakagodawita commented 6 years ago

Good stuff, it's all fixed on the Insider's build - thanks for the speedy fix!

dbaeumer commented 6 years ago

Verified using a launch config of type docker. Hope that is correct.