webdriver
webdriver copied to clipboard
Add some security checks when handling a HTTP connection.
Check the Host and Origin headers for the incoming connection to verify the connection is allowed.
The language is intended to allow the specific behaviour to be largely implementation defined, whilst recommending a default behaviour that prevents CSRF-type attacks (reject host headers that aren't an IP address or the server hostname, reject any requests with an origin header).
Hopefully adding this text will ensure that implementations consider the security issues accepting a connection, even though it's not possible to give precise requirements that apply to all implementations.
The BiDi version of this is https://github.com/w3c/webdriver-bidi/pull/155
CC @sadym-chromium, @bwalderman
@jgraham I assume the summary of this issue should say WebDriver and not WebSocket right?
Beside that please also have a look at issues #1578 and #1643. Both are related and I hope that we could fix them with your PR. Thanks.
The Browser Testing and Tools Working Group just discussed Host/Origin checks.
The full IRC log of that discussion
<jgraham> Topic: Host/Origin checks<jgraham> github: https://github.com/w3c/webdriver/pull/1634
<sadym> q+
<jgraham> jgraham: Allowing HTTP/WebSocket connections comes with some security concerns eg. if you don't check origin header then websites might be able to start a session. The spec didn't cover this before, but the PR adds some checks in when establishing a connection.
<jgraham> ack sadym
<jgraham> sadym: There is a use case with docker, because the host header for the container is different to the actual host. We need to account for this in the spec.
<jgraham> jgraham: Agreed, we should allow implementations to allow an implemtation-defined list of hosts to handle these cases.
<jgraham> jgraham: Also need to perform the checks on the WebSocker upgrade request for BiDi.
I've updated this a bit; I think it now covers all the points in the linked issues apart from:
- I didn't mention
AF_LOCALsockets. It definitely seems reasonable to support local-only communication as well asAF_INET*sockets, but I'm not sure in practice if they're usable in current clients ("local ends"). - I didn't mention HTTP Auth, although I did mention some kind of authorization being encouraged in cases where the "remote end" is really a publically accessible service. In practice I don't know what we could suggest which would add meaingful additional security and be compatible with common use cases like running tests in a CI system where the loal and remote ends are both on the same machine.
In any case I think if there's more to do for those points we should consider them in their own issues / PRs and limit this one to just the header validation requirements for accepting incomping requests.
@sadym-chromium in case you have some available time soon maybe you could have a look at this PR again and if your comments have been addressed or clarified? Thanks.