-
Notifications
You must be signed in to change notification settings - Fork 67
pydevd: access tokens #1710
Comments
@int19h I was thinking a bit about this... instead of having a |
The problem with shared token is that the client has to send the token first (in "initialize" request) before it can validate the one that it gets from the server (in "initialize" response). If the server is not real pydevd, but a malicious process, it would simply return the same token in response that it received in the request, making client access unsecured. And we do want to secure it, because of the "runInTerminal" reverse request. |
Ok |
I'm currently implementing this (I hope to finish it today) -- now, I've actually named it just i.e.: I think that by using just |
|
We've had some lack of clarity on how different endpoints are referred to for a while. Officially it's "DAP client" and "DAP server", but like you say, that can be confusing from user perspective, because sometimes it's the client that is listening for connections, and the server that is connecting. So for ptvsd, we've been using "IDE" and "debug server" also, but not very consistently. Recently, I cleaned up the docstrings for our public API, and it's always "IDE" and "debug server" there now: Lines 36 to 134 in 5f2cb30
So when we wrap pydevd APIs, that will be the terminology. Of course, since we do wrap it, pydevd doesn't need to be consistent with it - but we can standardize on this across debuggers for even more clarity :) And I agree - for In the protocol, I'd still prefer it to have full names tho, mainly to make the message logs clearer - firstly to make it clear which token it is, when it's only specified for the server; and secondly, to make them easier to grep (if one is a substring of the other, that makes it harder). So maybe |
Ok, I've just created the pull request for this. I kept |
Done in the refactor branch. |
Starting pydevd in server mode has security implications - any process that can connect to the opened port can then execute arbitrary code inside the process that hosts pydevd. In scenarios like #1706, this becomes a potential local escalation of privilege exploit.
The same thing goes for the debug adapter operating in server mode (accepting incoming DAP connections from debug servers, as in #1706) - if the adapter crashes or otherwise exits before the server can connect to it, a lower-privileged process can then take over the port, and intercept the incoming DAP connection to execute arbitrary code.
To prevent it, we need to add some basic authorization via access tokens. Pydevd needs to provide a way to specify a server and a client token for socket connections, as keyword parameters to
pydevd.settrace()
, e.g.:When the server token is specified, any incoming connection must also communicate the matching token to pydevd in the "initialize" request:
If the token is not specified, or doesn't match the one that was passed to
settrace()
, pydevd should send a failure response with the appropriate message, and terminate the connection.When the client token is specified, pydevd must send this token in "initialize" response:
The client will immediately disconnect if the token it receives doesn't match the one it expects.
For the sake of simplicity and consistency, the checks should apply to all connections regardless of their direction (i.e. it should still check "serverAccessToken" even if the connection is server->client, and it should still check "clientAccessToken" even if the connection is client->server).
The text was updated successfully, but these errors were encountered: