Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot reliably test against different IP addresses #26166

Closed
letitz opened this issue Oct 19, 2020 · 7 comments
Closed

Cannot reliably test against different IP addresses #26166

letitz opened this issue Oct 19, 2020 · 7 comments

Comments

@letitz
Copy link
Contributor

letitz commented Oct 19, 2020

Hi there, I believe this issue deserves the type:untestable tag but I am not empowered to tag it myself.

CORS-RFC1918 is a spec aiming to prevent malicious public websites (e.g. http://evil.com) from attacking users' devices on the local network (e.g. smart fridge) or companies' intranet services.

It does so by partitioning IP addresses into 3 address spaces, ordered from most to least private:

  • local: contains only localhost
  • private: contains IPv4 addresses designated private by RFC1918 and non-publicly routable IPv6 addresses
  • public: all other addresses

See #26135 for an initial pass at introducing tests for this spec. As @mikewest pointed out in the review, it is currently impossible for tests to assert the address space from which they are loaded: on my machine they are served from localhost, but that is certainly not the case for https://wpt.live.

In full generality, testing this spec properly would require being able to load a web page from the local, private or public address spaces at will. This could be achieved by wptserve actually serving the data from localhost, a private or a public IP, or by fooling the test browser into believing that somehow.

@stephenmcgruer
Copy link
Contributor

cc @web-platform-tests/wpt-core-team

chromium-wpt-export-bot pushed a commit that referenced this issue Oct 19, 2020
These tests might not live very long, depending on the outcome of the related
issue: #26166.

In the meantime, they serve to document the existing spec intent, and hopefully
undergird discussions around address space inheritance.

Bug: chromium:1138905, chromium:1138906, chromium:1134601
Change-Id: I7d8d0e7cf8e70cfdf3bdc044e748ee384f3418dd
chromium-wpt-export-bot pushed a commit that referenced this issue Oct 19, 2020
These tests might not live very long, depending on the outcome of the related
issue: #26166.

In the meantime, they serve to document the existing spec intent, and hopefully
undergird discussions around address space inheritance.

Bug: chromium:1138905, chromium:1138906, chromium:1134601
Change-Id: I7d8d0e7cf8e70cfdf3bdc044e748ee384f3418dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475875
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818444}
chromium-wpt-export-bot pushed a commit that referenced this issue Oct 19, 2020
These tests might not live very long, depending on the outcome of the related
issue: #26166.

In the meantime, they serve to document the existing spec intent, and hopefully
undergird discussions around address space inheritance.

Bug: chromium:1138905, chromium:1138906, chromium:1134601
Change-Id: I7d8d0e7cf8e70cfdf3bdc044e748ee384f3418dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475875
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818444}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Oct 19, 2020
These tests might not live very long, depending on the outcome of the related
issue: web-platform-tests/wpt#26166.

In the meantime, they serve to document the existing spec intent, and hopefully
undergird discussions around address space inheritance.

Bug: chromium:1138905, chromium:1138906, chromium:1134601
Change-Id: I7d8d0e7cf8e70cfdf3bdc044e748ee384f3418dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475875
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818444}
@Hexcles
Copy link
Member

Hexcles commented Oct 20, 2020

With some rare exceptions (e.g. mobile testing), tests are almost always served from localhost in an automated system (wpt.live is for manual testing only).

To support private and public addresses from the infra side, we could bind additional IPs to a loopback device, which of course requires root permissions. I think we can do this on Taskcluster and Azure Pipelines, but not sure whether it's possible in each browser vendor's CI. There's also a slight complication here regarding the public IP -- we'd probably need to buy one to make sure it's never used for anything real.

To redirect IP addresses to localhost from the browser side, we'd need to change each browser.

In short, I think both approaches require an RFC to coordinate with browser vendors.

@stephenmcgruer
Copy link
Contributor

Forgive my networking ignorance, but are the domains we bind to in /etc/hosts treated as localhost by the browser (aka local scope) in these scenarios? See the output of ./wpt make-hosts-file:

127.0.0.1       op73.not-web-platform.test
127.0.0.1       xn--n8j6ds53lwwkrqhv28a.www.not-web-platform.test
127.0.0.1       op88.web-platform.test
127.0.0.1       xn--n8j6ds53lwwkrqhv28a.www1.not-web-platform.test
127.0.0.1       op35.not-web-platform.test
127.0.0.1       op29.web-platform.test
...

If someone were to set a URL as those (via server-side substitution, e.g. using {{hosts[alt][]}} I think?), would it count as public or local?

@mikewest
Copy link
Member

@stephenmcgruer: Yes. Anything bound to loopback (127.0.0.0/8) is considered to be a "local" endpoint, regardless of name. We look at the IP address associated with the socket, not at the domain.

@Hexcles: An alternative could be to teach the test driver some new tricks to classify some IP addresses as local and others as public. That would put the burden on the browsers to do the categorization in some reasonable way, but it wouldn't be out of the question.

@stephenmcgruer
Copy link
Contributor

I would personally lean towards a webdriver extension, since 'buying a public IP address' seems future-brittle, but in either case @Hexcles is correct and the next step is likely an RFC. Please feel free to pick your favorite solution to the problem and write up a short RFC with summary/details/risk sections, then browser vendors/etc can chime in :)

@letitz
Copy link
Contributor Author

letitz commented Oct 22, 2020

Alright, thanks! I'll read up on webdriver and how we could leverage that, and come back with an RFC in hand :)

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 27, 2020
…Platform Tests., a=testonly

Automatic update from web-platform-tests
[CORS-RFC1918] Initial addressSpace Web Platform Tests.

These tests might not live very long, depending on the outcome of the related
issue: web-platform-tests/wpt#26166.

In the meantime, they serve to document the existing spec intent, and hopefully
undergird discussions around address space inheritance.

Bug: chromium:1138905, chromium:1138906, chromium:1134601
Change-Id: I7d8d0e7cf8e70cfdf3bdc044e748ee384f3418dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475875
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818444}

--

wpt-commits: 6c6f4decce0e9dc5c526ac95d084380bb6f1133d
wpt-pr: 26135
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Oct 27, 2020
…Platform Tests., a=testonly

Automatic update from web-platform-tests
[CORS-RFC1918] Initial addressSpace Web Platform Tests.

These tests might not live very long, depending on the outcome of the related
issue: web-platform-tests/wpt#26166.

In the meantime, they serve to document the existing spec intent, and hopefully
undergird discussions around address space inheritance.

Bug: chromium:1138905, chromium:1138906, chromium:1134601
Change-Id: I7d8d0e7cf8e70cfdf3bdc044e748ee384f3418dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475875
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818444}

--

wpt-commits: 6c6f4decce0e9dc5c526ac95d084380bb6f1133d
wpt-pr: 26135
letitz added a commit to letitz/rfcs that referenced this issue Nov 17, 2020
letitz added a commit to letitz/rfcs that referenced this issue Nov 17, 2020
letitz added a commit to letitz/rfcs that referenced this issue Feb 22, 2021
foolip pushed a commit to web-platform-tests/rfcs that referenced this issue Mar 31, 2021
@foolip
Copy link
Member

foolip commented Mar 31, 2021

#72 was merged, considering this fixed.

@foolip foolip closed this as completed Mar 31, 2021
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
These tests might not live very long, depending on the outcome of the related
issue: web-platform-tests/wpt#26166.

In the meantime, they serve to document the existing spec intent, and hopefully
undergird discussions around address space inheritance.

Bug: chromium:1138905, chromium:1138906, chromium:1134601
Change-Id: I7d8d0e7cf8e70cfdf3bdc044e748ee384f3418dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475875
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818444}
GitOrigin-RevId: 92dddd32ed64130c7a5ddfa3bdf4aba28d06a034
ns-rsilva pushed a commit to ns-rsilva/chromium that referenced this issue Apr 25, 2024
These tests might not live very long, depending on the outcome of the related
issue: web-platform-tests/wpt#26166.

In the meantime, they serve to document the existing spec intent, and hopefully
undergird discussions around address space inheritance.

Bug: chromium:1138905, chromium:1138906, chromium:1134601
Change-Id: I7d8d0e7cf8e70cfdf3bdc044e748ee384f3418dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475875
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818444}

Former-commit-id: 92dddd32ed64130c7a5ddfa3bdf4aba28d06a034
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants