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

General explainer review #42

Open
domenic opened this issue Feb 23, 2022 · 0 comments
Open

General explainer review #42

domenic opened this issue Feb 23, 2022 · 0 comments
Labels
documentation Improvements or additions to documentation

Comments

@domenic
Copy link

domenic commented Feb 23, 2022

I did a read-through of the explainer and in addition to some more specific issues I will file, here are some minor ones. Especially-important ones are in bold. The ordering is roughly top-of-document to bottom-of-document.


The frequent mention of XMLHttpRequest is a bit strange and distracting, since that API has been superceded by the fetch API. I'd suggest just replacing mentions of it with fetch.

The "Initial Focus" section could be clearer on which bullet points from "Use cases" are in scope vs. out of scope for the initial focus. It's clear DHTs are out of scope but I'm unclear if avoiding "IP multicast and UDP Broadcast" removes other possibilities. I'd suggest a summary paragraph like: "In terms of the [use cases] identified above, this means our initial focus will be on solving: X, Y, Z, and W. Whereas, A, B, and C will not be solvable with the current proposal."

The "Permissions Policy integration" section could benefit from some background explanation of why permissions policy should be integrated at all. Something like, "This means that the direct sockets APIs will not be available to cross-origin subframes, unless the outer page explicitly delegates that ability."

"Security Considerations" mentions "high-trust mode" but does not give any further detail. This should link somewhere, probably.

The mitigation for CORS bypasses should mention this is not a complete mitigation as it means you could attack servers using anything besides "the well known HTTPS port" (which I assume means 443). E.g. you could attack port 80 or port 8080 or port 3000.

"User agents should reject connection attempts when Content Security Policy allows the unsafe-eval source expression": is this a "should" or a "must"/"will be required to"? In general any use of "should" is a bit suspicious.

The explainer gestures at a user-facing security model several times but never explains it. E.g. it talks about users typing things, or about an option to permit future connections from an origin to specific hosts. I think you need an up-front section, probably right after "Initial Focus", that explains this. Even if the goal is not for the spec to constrain user agent UI, you should explain what Chromium is planning to implement, as an example of the kind of UI that this API and your security analysis is being designed around.

@GrapeGreen GrapeGreen added the documentation Improvements or additions to documentation label Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants