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

Extract 'SecureContext' status in json files #1142

Closed
OnkarRuikar opened this issue Feb 2, 2024 · 9 comments
Closed

Extract 'SecureContext' status in json files #1142

OnkarRuikar opened this issue Feb 2, 2024 · 9 comments

Comments

@OnkarRuikar
Copy link

Consider subtle property of Crypto interface: https://developer.mozilla.org/en-US/docs/Web/API/Crypto/subtle

It requires secure context. WebCryptoAPI.idl:

[Exposed=(Window,Worker)]
interface Crypto {
  [SecureContext] readonly attribute SubtleCrypto subtle;
  ArrayBufferView getRandomValues(ArrayBufferView array);
  [SecureContext] DOMString randomUUID();
};

It would be really great if it is extracted in json files as well.
/ed/dfns/WebCryptoAPI.json:

{
      "id": "dfn-Crypto-attribute-subtle",
      "href": "https://w3c.github.io/webcrypto/#dfn-Crypto-attribute-subtle",
++    "requiresSecureContext": true,
      "linkingText": [
        "subtle"
      ],
      "localLinkingText": [
        "Crypto.subtle"
      ],
      "type": "attribute",
      "for": [
        "Crypto"
      ],
      "access": "public",
      "informative": false,
      "heading": {
        "id": "Crypto-attribute-subtle",
        "href": "https://w3c.github.io/webcrypto/#Crypto-attribute-subtle",
        "title": "The subtle attribute",
        "number": "10.2.1"
      },
      "definedIn": "prose"
    },

We want to automate adding these secure context headers to mdn/content repo. It will be really easy to extract the info from .json files rather than processing .idl files.

@tidoust
Copy link
Member

tidoust commented Feb 2, 2024

Depending on what your exact needs are, I'd say that this information already exists in JSON files in Webref... or indeed that it does not ;)

Parsed versions of (curated) IDL extracts are available as JSON files in the curated branch (roughly described in the README):
https://github.com/w3c/webref/tree/curated/ed/idlparsed

The idlparsed.json JSON schema in Reffy describes the outline of these extracts. The items of interest are going to be under idlNames and idlExtendedNames. The values there are the result of running the webidl2.js parser on the underlying IDL fragments.

Taking the example of the Crypto interface, the file lists the extended attributes of the subtle attribute, which include SecureContext.

Now, while these JSON extracts are more directly processable than the IDL files, they are not enough to answer the question "Is this particular attribute or method only available in secure contexts?" because answering that question in the generic case requires resolving mixins and partials, as done by @wbamberg in the SecureContext checker. Is that the question that you'd like to answer?

If so, then there's indeed no direct way to gather that information from Webref. The idlparsed extracts could perhaps be extended to add some sort of requiresSecureContext property to note the practical outcome. But then whether a mixin member is SecureContext or not can change depending on the interfaces that include the mixin, as discussed with @wbamberg in the #writing-docs channel of the Open Web Docs Slack a few weeks ago, and illustrated by the following example:

interface mixin M {
  undefined mixinMember();
};

/* Interface I is only exposed to secure contexts. */
/* So mixinMember() can only exist in secure contexts for I. */
[Exposed=*, SecureContext]
interface I {};
I includes M;

/* For mixins, evaluation depends on the underlying interface. */
/* Here, the same mixinMember() member is defined in non secure contexts for K. */
interface K {};
K includes M;

... and the idlparsed extracts (same for dfns extracts) only contain one entry per mixin.

The idlnamesparsed extracts, which collect all IDL fragments per interface, might be a better place to store the info, especially since I think they would provide a more natural hook to find the right information in an MDN context (where I suppose the interface name is a good starting point), as I don't know how you'd find the right idlparsed to look at otherwise. The idlnamesparsed extracts don't detail anything on top of the IDL fragment and don't directly map to idlparsed extracts either. But we can probably improve that!

Side note: We would prefer dfns extraction to remain fully orthogonal to any kind of IDL curation and processing, so storing the information in dfns extracts does not seem like the right approach for this (and wouldn't be a good match with the mixin issue mentioned above).

@OnkarRuikar
Copy link
Author

@tidoust Thanks for the detailed info!

Side note: We would prefer dfns extraction to remain fully orthogonal to any kind of IDL curation and processing, so storing the information in dfns extracts does not seem like the right approach for this

I see and agree. I've found some cases where spec docs simply put blank statement saying all the feature here require secure context but they don't use [SecureContext] in syntax sections. Consider following cases:

  1. Web Periodic Background Synchronization doc mentions "As this API relies on service workers, functionality provided by this API is only available in a secure context." But it doesn't use the [SecureContext] tag in any syntax.
  2. ExtendableCookieChangeEvent: the interface in the section before mentions secure context but the event doesn't.

I mean, if we fix the source (spec docs) itself then we may not have to worry about so many inconsistencies. If you wish, I can send PR(s) to fix the docs.

@tidoust
Copy link
Member

tidoust commented Feb 2, 2024

I mean, if we fix the source (spec docs) itself then we may not have to worry about so many inconsistencies. If you wish, I can send PR(s) to fix the docs.

Yes, that's the process we try to follow in Webref: never create a patch unless there's a related issue/PR raised against the spec, and always link back to that issue/PR from the patch.

Now, the IDL patches that we have in Webref are not even meant to fix the problem that you raise here: from a Webref perspective, we only want to make sure that the IDL can be parsed and is consistent with the IDL defined in other specs. The goal is to stay as close as possible to what the spec defines because the spec should be the single source of truth, and not to improve the IDL fragments ourselves. In other words, by all means, go ahead and report these issues in the repos of the specs!

@OnkarRuikar
Copy link
Author

OnkarRuikar commented Feb 5, 2024

The goal is to stay as close as possible to what the spec defines because the spec should be the single source of truth, and not to improve the IDL fragments ourselves. In other words, by all means, go ahead and report these issues in the repos of the specs!

@tidoust I have submitted some pull requests to the repos. But they do not want redundancy in the spec docs.

Inviting @annevk to the conversation.

@annevk
Copy link
Member

annevk commented Feb 5, 2024

  1. Something only exposed to service workers is by definition limited to secure contexts because service workers are limited to secure contexts. An IDL tool can presumably know this, though it's somewhat of an advanced question I suppose. We should not add [SecureContext] redundantly to such interfaces.
  2. There are some interfaces that are exposed everywhere, but their functionality is guarded by a secure context check (or a permission check that is guarded by a secure context check). Example: Notifications API. This will require some kind of custom handling.

I very much doubt any of the PRs opened above is correct as the features are likely either 1 or 2.

@tidoust
Copy link
Member

tidoust commented Feb 6, 2024

  1. Something only exposed to service workers is by definition limited to secure contexts because service workers are limited to secure contexts. An IDL tool can presumably know this, though it's somewhat of an advanced question I suppose. We should not add [SecureContext] redundantly to such interfaces.

I see. The interface is defined with an [Exposed=ServiceWorker] attribute, Service workers are secure context only, so the interface de facto has [SecureContext] as well, no need to repeat that. That seems fine and means that the globals need to be taken into account when computing whether an interface is exposed to secure contexts only.

Now I'm wondering whether there's a way to infer that "Service Workers are secure context only" through IDL only. Typically, the [Exposed=ServiceWorker] targets the global name ServiceWorker, which is attached to the interface ServiceWorkerGlobalScope. That interface is not defined as [SecureContext] (although most other Service Workers interfaces are).

Should ServiceWorkerGlobalScope have a [SecureContext] extended attribute? Or do tools need to maintain a list of globals that are secure context only? Or am I misreading the IDL?

  1. There are some interfaces that are exposed everywhere, but their functionality is guarded by a secure context check (or a permission check that is guarded by a secure context check). Example: Notifications API. This will require some kind of custom handling.

OK, if they are exposed everywhere, it seems wrong to flag them as secure context only in any case. @OnkarRuikar I'll let you figure out a way to document these cases in BCD/MDN, I don't think there's much we can do at the Webref level.

@annevk
Copy link
Member

annevk commented Feb 6, 2024

It's a good question if ServiceWorkerGlobalScope should have [SecureContext]. It ends up being redundant with other requirements, so probably not, but I suppose a case could be made.

@tidoust
Copy link
Member

tidoust commented Feb 6, 2024

Looking into globals known to Webref, the only globals that are secure context but that are not flagged as such in the IDL seem to be ServiceWorkerGlobalScope and the 3 globals defined in Turtledove:

Other globals are either not secure context only, I believe:

... or they inherit from WorkletGlobalScope, which is explicitly defined with a [SecureContext] attribute:

I don't know whether that makes a case for adding [SecureContext] to ServiceWorkerGlobalScope. It's at least interesting to note that WorkletGlobalScope has the attribute ;)

tidoust added a commit to tidoust/ServiceWorker that referenced this issue Feb 7, 2024
Context for this is Open Web Docs people looking into automating the handling
of "this feature is available only in secure contexts" banners in MDN pages.
This led to the discussion in:
w3c/webref#1142 (comment)

When it is set, the `[SecureContext]` IDL extended attribute explicitly gives
the information. That said, to avoid redundancies, that attribute is not set on
interfaces that are exposed (through `[Exposed=xxx]`) on globals that are
already restricted to secure contexts/

The Service workers spec is clear that service workers must execute in secure
contexts:
https://w3c.github.io/ServiceWorker/#secure-context

However, it does not fully say so in the IDL itself. More specifically, when
an interface defined in another spec has `[Exposed=ServiceWorker]`, that's a
reference to the `ServiceWorkerGlobalScope` interface, and that interface does
not have a `[SecureContext]` attribute.

This commit adds the `[SecureContext]` attribute to `ServiceWorkerGlobalScope`.

This approach is consistent with the way `WorkletGlobalScope`, from which a
number of other globals inherit, is defined:
https://html.spec.whatwg.org/multipage/worklets.html#worklets-global
@tidoust
Copy link
Member

tidoust commented Feb 21, 2024

Also of interest: @wbamberg's blog post on Using Web IDL for better web docs.

One concrete outcome of this discussion is that [SecureContext] was added to ServiceWorkerGlobalScope, which seems good and improves the ability to derive the secure context only status automatically.

Looking at it from a Webref perspective, @dontcallmedom and I would prefer to continue to restrict Webref extracts to the raw IDL (and raw result of parsing that IDL) and leave computations and analyses on the IDL to Webref consumers for now, especially since we don't see a fantastic place where we could store the result of these computations (idlparsed extracts do not seem like a good place given the need to consider mixins, idlnamesparsed extracts do not contain the right level of details, and we're not clear that many people rely on these extracts in any case). I'm closing this issue accordingly but we're happy to revisit if the need proves to be a common one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants