Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Generated debug launch config sets listening port to 2345 #1906

Closed
antong opened this issue Sep 2, 2018 · 7 comments
Closed

Generated debug launch config sets listening port to 2345 #1906

antong opened this issue Sep 2, 2018 · 7 comments

Comments

@antong
Copy link

antong commented Sep 2, 2018

The debugging feature uses a launch.json file which sets parameters to use when running delve. The generated default includes "port": 2345, which instructs delve to expose the API TCP server on port 2345. The delve default is to use port 0, which will cause the OS to select a free ephemeral port. One implication of using a static port 2345 is that it will not work if another service is listening on port 2345. It also makes derekparker/delve#1332 trivial to exploit, i.e., a malicious user or program on the same host can hijack the debugger.

Recommendation: set "port": 0 in the generated launch.json or leave the setting out altogether. For the use of VS Code I don't see why the user would want to set a fixed port when debugging locally.

Steps to Reproduce:

  1. Start debugging (F5). This will have the delve listen port unset, so a free ephemeral port will be used. API server listening at: 127.0.0.1:16234. (16234 is picked by the OS).
  2. In the Delve view click the settings cogwheel. This generates and writes .vscode/launch.json which has "port": 2345.
  3. Stop and start debugging again. Now the launch config will be used and delve will listen on port 2345. API server listening at: 127.0.0.1:2345.

.vscode/launch.json with port set to 2345 will also be generated through many other paths, e.g., if there is an error running or attaching to delve, then the user will be prompted to open launch.json.

@ramya-rao-a
Copy link
Contributor

All three of remotePath, port and host properties in the debug configuration are not needed unless the mode is set to remote. I believe the original intention of adding them to the default config was to aid discoverability.

I agree that they can be removed from the code snippet.

PRs are welcome
Code pointers:

@antong
Copy link
Author

antong commented Sep 3, 2018

Actually, looking at the code, it seems "port": 0 does not imply an ephemeral port as it normally does and as it does for the dlv command. A zero port causes the extension to randomly pick a port between 2000 and 50000:
https://github.com/Microsoft/vscode-go/blob/79e4f3824b89ec3151650fa14f3c59f71e6b2dd8/src/debugAdapter/goDebug.ts#L567

@ramya-rao-a
Copy link
Contributor

Looks like we do need a host and port for the client to connect. See https://github.com/Microsoft/vscode-go/blob/79e4f3824b89ec3151650fa14f3c59f71e6b2dd8/src/debugAdapter/goDebug.ts#L427

So, even if you made the change so that delve figures out a port to use by itself, we then need to parse its output to find the port and then pass it to the connectClient above.

@antong
Copy link
Author

antong commented Sep 4, 2018

Yes, you are correct. I suggest we for now just set port to 0 in the autogenerated launch.json with the existing port || random(2000, 50000), which will make it work most of the time. Or is that sweeping the problem under the rug and would be better to just keep as is?

Anyway, let's see if delve can help (derekparker/delve#1332), because the current delve API makes it a bit difficult to use robustly without port clashes and securely.

@ramya-rao-a
Copy link
Contributor

I suggest we for now just set port to 0 in the autogenerated launch.json with the existing port || random(2000, 50000), which will make it work most of the time.

Agreed.

I've removed remotePath, port and host properties from the default configuration.

The port || random(2000, 50000) will ensure that a random port is selected.

@ramya-rao-a
Copy link
Contributor

@ramya-rao-a
Copy link
Contributor

The fix for this is now out in the latest update to this extension (0.6.90)

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

No branches or pull requests

2 participants