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

Secure Contexts #602

Closed
ctavan opened this issue Dec 2, 2021 · 11 comments
Closed

Secure Contexts #602

ctavan opened this issue Dec 2, 2021 · 11 comments
Projects
Milestone

Comments

@ctavan
Copy link
Member

ctavan commented Dec 2, 2021

Now that #600 has been merged, I just realized that given WICG/uuid#23 this change may actually have introduced a subtle breaking change in that in modern browsers v4 UUID generation will now only work in secure contexts.

How are we going to deal with this? (cc @LinusU)

@ctavan ctavan added this to the 9.0.0 milestone Dec 2, 2021
@ctavan ctavan added this to To do in Version 9 Dec 2, 2021
@LinusU
Copy link
Member

LinusU commented Dec 2, 2021

I believe that crypto.randomUUID will simply be undefined in unsecure contexts, and then we'll fall back to the standard implementation:

image

image

@ctavan
Copy link
Member Author

ctavan commented Dec 2, 2021

OK, that's a good observation. I guess for the purpose of this library, which always tries to use the best-available resource for generating randomness, the observed behavior is fine.

I'm wondering what code paths our (browser) tests are testing though… I think we should investigate if they are loading the test page in a secure or insecure context (if we want to ensure that the native code path is covered).

@LinusU
Copy link
Member

LinusU commented Dec 2, 2021

I think we should investigate if they are loading the test page in a secure or insecure context (if we want to ensure that the native code path is covered).

Very good point 👍

@broofa
Copy link
Member

broofa commented Dec 2, 2021

FWIW, 'looks like BrowserStack tests use HTTP:
CleanShot 2021-12-02 at 08 44 42@2x

@ctavan
Copy link
Member Author

ctavan commented Dec 2, 2021

As far as I understand, APIs restricted to secure contexts are still available on localhost in insecure contexts… But still have to verify.

@ctavan
Copy link
Member Author

ctavan commented Dec 2, 2021

I tried Browserstack live local testing with Chrome 95 and it indeed appears like crypto.randomUUID is available on 127.0.0.1:9000:

image

@bcoe does this match your expectations as well?

@broofa
Copy link
Member

broofa commented Dec 6, 2021

From §3.1.5 of the Secure Context spec:

If the user agent conforms to the name resolution rules in [let-localhost-be-localhost] and one of the following is true:

  • origin’s host is "localhost" or "localhost."
  • origin’s host ends with ".localhost" or ".localhost."

then return "Potentially Trustworthy".

@broofa
Copy link
Member

broofa commented May 13, 2022

@ctavan Do you know if there's a way to configure our BrowserStack CI to use a non-localhost domain for testing? (like... add 127.0.0.1 test.com to "/etc/hosts", or use some pre-configured domain alias they might supply?)

As you've said, it'd be nice to actually test the non-secure case.

@ctavan
Copy link
Member Author

ctavan commented May 15, 2022

I haven’t investigated yet but I agree that we should try to establish test coverage for this case.

@broofa
Copy link
Member

broofa commented May 16, 2022

It looks like browserstack serves a DNS entry for bs-local.com that maps to 127.0.0.1. (source). In theory we could use that. However adding hostname: 'bs-local.com' to wdio.conf.js breaks the browserstack environment.

@broofa
Copy link
Member

broofa commented Jul 29, 2022

While I'd love to get insecure context testing in CI, I don't see a trivial path to that at the moment. Nor am I concerned enough to say this should hold up the v9 release. We can revisit if/when we miss a regression in a future release. I have every confidence our users will complain loudly when that happens. 😈

@ctavan If you feel this needs to be a v9 release blocker go ahead and reopen.

@broofa broofa closed this as completed Jul 29, 2022
@broofa broofa moved this from To do to Done in Version 9 Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants