Add opt-in support for hosts and origins other then local loopback interfaces
Categories
(Testing :: geckodriver, defect, P3)
Tracking
(firefox-esr91 unaffected, firefox93 wontfix, firefox94 wontfix, firefox95 wontfix, firefox96 wontfix, firefox97 wontfix, firefox98 fixed)
People
(Reporter: whimboo, Assigned: jgraham)
References
(Regression, )
Details
(Keywords: regression)
Attachments
(4 files)
As reported this is a regression in geckodriver 0.30.0 as caused by bug 1652612. Since it has been landed we only allow connections from a local loopback device. But there are scenarios where folks setup different docker images, and as such need a way to whitelist specific network addresses.
More details can be found at https://github.com/mozilla/geckodriver/issues/1935.
James, can you please take this up?
Reporter | ||
Updated•3 years ago
|
Reporter | ||
Comment 1•3 years ago
|
||
There has also https://github.com/mozilla/geckodriver/issues/1936 been filed, which covers the Ipv6 part. ::1
should actually be allowed.
Comment 2•3 years ago
|
||
Thanks for creating this issue ticket Henrik.
Could we consider providing a configuration that allows connections from all network hosts (with all Host headers) while still rejecting requests with Origin headers other than localhost or those explicitly allowed?
From my understanding, this would protect from CSRF attacks (since these include an Origin header other than localhost), while making the typical remote client setup feasible (where clients send requests from a non-web context, which does not include an Origin header).
Reporter | ||
Updated•3 years ago
|
Comment 3•3 years ago
|
||
For the Host header, could we:
- allow all literal IP addresses and "localhost" by default;
- add an CLI option to enable extra values of the Host header.
Reporter | ||
Comment 4•3 years ago
|
||
While work is happening to get this bug fixed I'm going to add an entry to the release notes to mention this particular regression. As James suggested we will phrase it like: geckodriver restricts connections to local ip addresses. This can interfere with deployments in which geckodriver is running on a different network node to the tests e.g. some container or virtual-machine based setups.
Reporter | ||
Comment 5•3 years ago
|
||
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d8c148e11dce [geckodriver] Update release notes for host check regression. r=webdriver-reviewers,jgraham
Reporter | ||
Comment 7•3 years ago
|
||
I've also updated the upstream release notes and change.md file:
https://github.com/mozilla/geckodriver/releases/tag/v0.30.0
Adding the leave-open keyword given that we are still waiting for the fix.
Comment 8•3 years ago
|
||
bugherder |
Assignee | ||
Comment 9•3 years ago
|
||
The policy is now as follows:
-
The
--host
command line flag can now be an actual hostname as well
as in IP address. -
By default only requests with a
Host
header that is an IP address,
or matching the value of the--host
argument are accepted. -
If
--host
is a local IP address, we by default acceptlocalhost
in theHost
header. -
When
--allow-hosts
is provided, only requests with a hostname
matching the listed values, or an IP address, are accepted. -
By default any request with an
Origin
header is rejected. -
When
--allow-origins
is provided, only requests with noOrigin
header, or an origin matching the given values, are accepted.
Assignee | ||
Comment 10•3 years ago
|
||
I've put up the patch as a kind of proposal; this isn't a definite thing. Hopefully the commit message in the comment above explains the proposed policy well. Feedback welcome on a) whether this enforces the necessary properties for safety and b) whether this addresses the use cases.
Comment 11•3 years ago
|
||
The severity field is not set for this bug.
:whimboo, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 12•3 years ago
|
||
Thanks for the proposed solution James.
I think it covers a wide variety of use cases, but not the one I outlined in my comment above:
Could we consider providing a configuration that allows connections from all network hosts (with all Host headers) while still rejecting requests with Origin headers other than localhost or those explicitly allowed?
What I'm asking for is basically allowing the previously possible usage of --host 0.0.0.0
(as well as the IPv6 equivalent), while still rejecting requests with Origin headers (unless explicitly allowed).
A sample use case is the provision of this Docker image:
https://github.com/blueimp/geckodriver/blob/165c41f8640540729b377dd7c5052de05bd67e4f/Dockerfile#L41
By setting --host 0.0.0.0
in the image definition, users of the image can provision it without having to provide command-line arguments (ease of setup).
I think it would be beneficial to have a discussion and mutual understanding of potential attack vectors if containers with geckodriver are provisioned that way, see also my comment above:
From my understanding, this would protect from CSRF attacks (since these include an Origin header other than localhost), while making the typical remote client setup feasible (where clients send requests from a non-web context, which does not include an Origin header).
I would really like to understand what security aspect we're accomplishing by limiting the host header, beyond what restricting Origin headers already provides.
Assignee | ||
Comment 13•3 years ago
•
|
||
What I'm asking for is basically allowing the previously possible usage of --host 0.0.0.0 (as well as the IPv6 equivalent), while still rejecting requests with Origin headers (unless explicitly allowed).
In the current design that's possible if either a) requests happen by ip address (although that might be a bad idea, see below), or b) you add the hostname that will be used to connect to geckodriver to the --allow-hosts
list. I'd assume that it must be possible to know the hostname of the container running geckodriver in a docker-compose (or similar) setup, so although it's some effort to pass in the hostname explictly it shouldn't actually be impossible.
I've heard two concerns with relying on the Origin header exclusively:
- A DNS rebinding attack might allow a website to perform a "same origin" request that actually ends up on a local ip. That could at least allow malicious sites to make GET requests without an Origin header.
- Old clients that might not correctly set the Origin header and so leave users vunerable (this is the one that still seems problematic if we allow any ip address).
Freddy might have more insight into how concerned we should be about these kind of attack, but in general I'm wary of a mode to disable all Host validation becoming an attractive nusicance in the sense that it makes life easier for people like yourself who are building software that uses geckodriver as a dependency, and so is widely used, but with the actual end users being unaware that they're running in a mode which turns off some of the security checks.
Comment 14•3 years ago
|
||
Thanks for your detailed reply James.
The DNS rebinding attack is a valid concern that I didn't consider before.
I am not sure it is a strong concern, since aside from the GET /status
endpoint, all other GET
endpoints (https://www.w3.org/TR/webdriver/#endpoints) require the session ID, which I think an attacker won't have access to.
(And POST
endpoints would still send the Origin
header for same-host requests.)
I'm also not sure if old clients are a strong concern, since I think it's unlikely an old browser client is used in a context where we're running a geckodriver version that requires a modern version of Firefox.
But yeah, the host check does add another layer of security with very little in terms of configuration overhead.
And it's definitely possible to support the Docker container use case with e.g. --allow-host=geckodriver
or --host=geckodriver
in a setup where geckodriver
is the Docker hostname of the container running it.
Reporter | ||
Comment 15•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] (away 10/23 - 10/31) from comment #1)
There has also https://github.com/mozilla/geckodriver/issues/1936 been filed, which covers the Ipv6 part.
::1
should actually be allowed.
James, before I'm back and being able to review maybe you can make sure that this scenario works as well as the following issue gets fixed:
https://github.com/mozilla/geckodriver/issues/1947
Thanks
Updated•3 years ago
|
Assignee | ||
Comment 16•3 years ago
|
||
I don't think it makes sense to add additional information to the error response with details of the server configuration; that could leak some information to an attacker. But I've added warn!
messages to the geckodriver logging with more detail about the rejected requests.
Reporter | ||
Updated•3 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 17•2 years ago
|
||
This allows us to write some gecko-specific tests for command line argument
handling.
In addition use a file and single environment variable for all the data
we're passing into pytest. Having two separate mechanisms doesn't make
much sense.
Assignee | ||
Comment 18•2 years ago
|
||
Using cargo clippy --fix
Updated•2 years ago
|
Comment 19•2 years ago
|
||
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/autoland/rev/46f1fb7f6fc0 Enable passing webdriver binary and args to pytest, r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/577a5b37f781 Enable allowing given hosts and origins for geckodriver, r=webdriver-reviewers,freddyb,whimboo https://hg.mozilla.org/integration/autoland/rev/ab9d1e14386a Fix clippy errors, r=webdriver-reviewers,whimboo
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/32412 for changes under testing/web-platform/tests
Comment 21•2 years ago
|
||
Backed out 3 changesets (Bug 1732622) for causing build bustages on server.rs.
Backout link
Push with failures
Failure Log
Upstream PR was closed without merging
Comment 23•2 years ago
|
||
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/autoland/rev/a3f88a74d97c Enable passing webdriver binary and args to pytest, r=webdriver-reviewers,whimboo https://hg.mozilla.org/integration/autoland/rev/854151593a85 Enable allowing given hosts and origins for geckodriver, r=webdriver-reviewers,freddyb,whimboo https://hg.mozilla.org/integration/autoland/rev/8b355e29a5ef Fix clippy errors, r=webdriver-reviewers,whimboo
Comment 24•2 years ago
|
||
bugherder |
Reporter | ||
Updated•2 years ago
|
Reporter | ||
Updated•2 years ago
|
Upstream PR merged by moz-wptsync-bot
Description
•