New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 634270 link

Starred by 6 users
Status: Fixed
Owner:
peria@chromium.org
Closed: Feb 2018
Cc:
yukishiino@chromium.org
█ mkwst@chromium.org
jsb...@chromium.org
haraken@chromium.org
cha...@chromium.org
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Feature

Blocked on:
issue 690714
issue 782121
issue 786153
Blocking:
issue 534788
issue 542499
issue 564245
issue 692289
issue 726510



Sign in to add a comment

Implement the '[SecureContext]' WebIDL attribute.

Project Member Reported by mkwst@chromium.org, Aug 4 2016 
The 'SecureContext' WebIDL attribute controls the exposure of a given interface, method, attribute, etc. If present, it ensures that the given item will be exposed only in a "Secure Context", as defined by https://w3c.github.io/webappsec-secure-contexts/.

Features wish to lock themselves to secure contexts; this IDL attribute enables them to do so in a standardized way, which should reduce the amount of custom code we stuff into various parts of the system.

The IDL attribute is in use in a number of specs today, and I expect that number to grow over time. Supporting it seems like a good idea.

Intent to ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/ILe65HVid88

Comment 1 by mkwst@chromium.org, Aug 4 2016

Components: Blink>Bindings
Labels: -Type-Bug -Pri-3 M-54 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows Pri-2 Type-Feature
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 5 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59dbf413248d36371afa36b4020c81dd42f72b69

commit 59dbf413248d36371afa36b4020c81dd42f72b69
Author: mkwst <mkwst@chromium.org>
Date: Fri Aug 05 09:17:06 2016

Implement '[SecureContext]' IDL attribute for methods and attributes.

This is step 1 of implementing '[SecureContext]'. It deals with interfaces,
attributes and methods, but does not yet correctly handle partial interfaces,
constants, dictionaries, etc.

BUG= 634270 

Review-Url: https://codereview.chromium.org/2207423002
Cr-Commit-Position: refs/heads/master@{#410019}

[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/IDLExtendedAttributes.md
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/IDLExtendedAttributes.txt
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/scripts/code_generator_v8.py
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/scripts/v8_methods.py
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/scripts/v8_utilities.py
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/templates/interface.cpp
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/templates/interface_base.cpp
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/templates/methods.cpp
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/tests/idls/core/TestInterface.idl
[add] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/tests/idls/core/TestInterfaceSecureContext.idl
[modify] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[add] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceSecureContext.cpp
[add] https://crrev.com/59dbf413248d36371afa36b4020c81dd42f72b69/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceSecureContext.h
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 5 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/795763cbbdf49f52dc0879f3405575bded157623

commit 795763cbbdf49f52dc0879f3405575bded157623
Author: mkwst <mkwst@chromium.org>
Date: Fri Aug 05 12:11:39 2016

Implement '[SecureContext]' IDL attribute for partial interfaces.

Step 2 of implementing '[SecureContext]'. Step 1 was
https://codereview.chromium.org/2207423002.

BUG= 634270 

Review-Url: https://codereview.chromium.org/2218863002
Cr-Commit-Position: refs/heads/master@{#410037}

[modify] https://crrev.com/795763cbbdf49f52dc0879f3405575bded157623/third_party/WebKit/Source/bindings/scripts/interface_dependency_resolver.py
[modify] https://crrev.com/795763cbbdf49f52dc0879f3405575bded157623/third_party/WebKit/Source/bindings/tests/idls/core/TestInterfacePartial2.idl
[add] https://crrev.com/795763cbbdf49f52dc0879f3405575bded157623/third_party/WebKit/Source/bindings/tests/idls/core/TestInterfacePartialSecureContext.idl
[modify] https://crrev.com/795763cbbdf49f52dc0879f3405575bded157623/third_party/WebKit/Source/bindings/tests/results/core/TestInterfaceOrLong.cpp
[modify] https://crrev.com/795763cbbdf49f52dc0879f3405575bded157623/third_party/WebKit/Source/bindings/tests/results/core/TestInterfaceOrTestInterfaceEmpty.cpp
[modify] https://crrev.com/795763cbbdf49f52dc0879f3405575bded157623/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[modify] https://crrev.com/795763cbbdf49f52dc0879f3405575bded157623/third_party/WebKit/Tools/Scripts/webkitpy/bindings/bindings_tests.py

Comment 4 by jsb...@chromium.org, Feb 9 2017 

I just tried switching navigator.storage over to [SecureContext] and ran into the limitation that an interface defined in core cannot implement any interface defined in modules.

The spec has:

[SecureContext,
 NoInterfaceObject,
 Exposed=(Window,Worker)]
interface NavigatorStorage {
  readonly attribute StorageManager storage;
};
Navigator implements NavigatorStorage;
WorkerNavigator implements NavigatorStorage;

Right now we use a partial and have individual methods on StorageManager do the security check.

Is addressing this on the roadmap for [SecureContext] (i.e. this bug) or do I need to pester more bindings folks for guidance here?

Comment 5 by jsb...@chromium.org, Feb 9 2017 

FYI - also ran into a bug trying to just apply [SecureContext] to navigator.storage within the partial: if an interface has two partials and they both define [SecureContext] on a method the generated code is invalid. 

I'll file a bug for that

Comment 6 by jsb...@chromium.org, Feb 10 2017

Blockedon: 690714

Comment 7 by xhw...@chromium.org, Feb 15 2017

Blocking: 692289
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 1 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1b9e6d3db16818dfd5b76199136f8745f005144

commit b1b9e6d3db16818dfd5b76199136f8745f005144
Author: jsbell <jsbell@chromium.org>
Date: Wed Mar 01 22:31:36 2017

Use [SecureContext] for navigator.storage

StorageManager (navigator.storage) is specified [1] as having
[SecureContext] on the interface. Replace per-method checks
with [SecureContext] on the navigator.storage attribute so
it simply doesn't exist in non-secure contexts as intended.

(The spec has [SecureContext] on a non-partial interface and
the interface is implemented by Navigator. We don't support
core interfaces implementing module interfaces, so we use a
partial interface instead. [SecureContext] does not yet
work when defined on the partial interface itself.)

[1] https://storage.spec.whatwg.org/#api

BUG= 690720 , 634270 

Review-Url: https://codereview.chromium.org/2692633002
Cr-Commit-Position: refs/heads/master@{#454073}

[delete] https://crrev.com/e8d21447d207616130a7d68e4b7b98c8d9fdca9c/third_party/WebKit/LayoutTests/external/wpt/storage/opaque-origin.https-expected.txt
[modify] https://crrev.com/b1b9e6d3db16818dfd5b76199136f8745f005144/third_party/WebKit/LayoutTests/http/tests/security/powerfulFeatureRestrictions/durable-storage-on-insecure-origin.html
[modify] https://crrev.com/b1b9e6d3db16818dfd5b76199136f8745f005144/third_party/WebKit/Source/modules/quota/NavigatorStorageQuota.idl
[modify] https://crrev.com/b1b9e6d3db16818dfd5b76199136f8745f005144/third_party/WebKit/Source/modules/quota/StorageManager.cpp
[modify] https://crrev.com/b1b9e6d3db16818dfd5b76199136f8745f005144/third_party/WebKit/Source/modules/quota/StorageManager.idl
[modify] https://crrev.com/b1b9e6d3db16818dfd5b76199136f8745f005144/third_party/WebKit/Source/modules/quota/WorkerNavigatorStorageQuota.idl

Comment 9 by xhw...@chromium.org, Mar 14 2017 

When an interface is marked by [SecureContext], the interface will be "undefined" JS on insecure contexts (e.g. http). Sometimes this caused confusions for devs when they try the API on http, only found it's not defined. Is it possible that in such case, we provide some warning message in the dev console, e.g. "navigator.foo is undefined because it requires secure context (e.g. https)"?

Comment 10 by jsb...@chromium.org, May 25 2017

Blocking: 726510

Comment 11 by jsb...@chromium.org, Jun 2 2017

Blocking: 564245
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba

commit 8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba
Author: Jason Chase <chasej@chromium.org>
Date: Thu Jul 27 23:48:56 2017

Fix exposure of interfaces with [SecureContext]

IDL interfaces with [SecureContext] applied are still visible in
insecure contexts. This was found when fixing [OriginTrialEnabled] and
[SecureContext] to work together correctly ( crbug.com/695123 ). Specifically,
with [SecureContext] in the IDL, the interface is still exposed as a
constructor attribute on Window.

When encountered on previous CLs, the solution was to run layout tests
in secure contexts:
https://codereview.chromium.org/2832813002
https://chromium-review.googlesource.com/c/506292/

This CL fixes the bindings logic so that [SecureContext] attribute is
flagged on the generated constructor attributes. The result is that the
existing bindings templates will generate the appropriate checks for
secure context.

Bug:  695123 ,  634270 
Change-Id: If6d37d4441ba06d4f2e04f4360ed28990a0e2547
Reviewed-on: https://chromium-review.googlesource.com/508328
Commit-Queue: Jason Chase <chasej@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490132}
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/LayoutTests/http/tests/credentialmanager/passwordcredential-fetch-registrabledomain.html
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/core/v8/WorkerOrWorkletScriptController.cpp
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/scripts/generate_global_constructors.py
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/scripts/v8_methods.py
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/templates/ConditionalFeaturesForCore.cpp.tmpl
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/templates/ConditionalFeaturesForCore.h.tmpl
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/templates/ConditionalFeaturesForModules.cpp.tmpl
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/templates/attributes.cpp.tmpl
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/templates/interface.h.tmpl
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/templates/partial_interface.h.tmpl
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/tests/results/core/ConditionalFeaturesForCore.cpp
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/tests/results/core/ConditionalFeaturesForCore.h
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceSecureContext.cpp
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/tests/results/modules/ConditionalFeaturesForModules.cpp
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/bindings/tests/results/modules/V8TestInterface5.cpp
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/platform/bindings/ConditionalFeatures.cpp
[modify] https://crrev.com/8e7aa98860b23dc0d395b8b8306b7cc382b2c0ba/third_party/WebKit/Source/platform/bindings/ConditionalFeatures.h

Comment 13 by jsb...@chromium.org, Jul 28 2017

Cc: cha...@chromium.org jsb...@chromium.org
FYI, an attribute marked [SecureContext] on a 'partial interface Window' definition ends up getting dropped entirely, rather than conditionally installed.

(Not new behavior, so far as I can tell)

Comment 14 by jsb...@chromium.org, Jul 28 2017 

Also, this is fun:

If you add [SecureContext] to an interface but there's a way of getting to an instance, you get an object with no methods on the prototype. So far as I can tell this should NOT be permitted with good Web IDL design once we treat [SecureContext] properly - it'd be a spec bug.

This showed up when I added [SecureContext] to ServiceWorkerContainer but navigator.serviceWorker did not have [SecureContext] (because of #13). Referencing navigator.serviceWorker.register would give you `undefined`; if you drop [SecureContext] on the interface then register is a real method, and we rely on the old behavior of it throwing (which is what happens in Firefox).

Again, so far as I can tell this should not happen with properly specified interfaces, but I wanted to note it here as a "WTF?!?! .... oh"

Comment 15 by raphael....@intel.com, Aug 9 2017

Labels: -M-54

Comment 16 by raphael....@intel.com, Aug 9 2017

Labels: M-54

Comment 17 by peria@chromium.org, Nov 8 2017

Cc: mkwst@chromium.org
Owner: peria@chromium.org
IIUC, the remaining task is to support it correctly on the IDL compiler, and no one is actively working on this.
I take this over.

Comment 18 by jsb...@chromium.org, Nov 16 2017

Blockedon: 782121

Comment 19 by jsb...@chromium.org, Nov 16 2017

Blocking: 542499

Comment 20 by jsb...@chromium.org, Nov 16 2017

Blockedon: 786153
Project Member

Comment 21 by bugdroid1@chromium.org, Nov 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a92cfc96142454dddc5805c1bf236454f0c5cf66

commit a92cfc96142454dddc5805c1bf236454f0c5cf66
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Mon Nov 27 02:54:56 2017

bindings: Remove InstallOriginTrialFeaturesOnGlobal

InstallOriginTrialFeatuersOnGlobal (IOTFOG for shot) calls
V8Foo(Partial)::InstallConditionalFeaturesOnGlobal internally,
where Foo is the global object which the context has.

This CL moves the contents of V8Foo::ICFOG to V8Foo::ICF.
It enables to call them through WrapperTypeInfo, and it means
we no longer need IOTFOG.


Bug: 787325,  786153 ,  634270 
Change-Id: I50dc653f0f19a77fd9aa3a7b71fb985b4f68f781
Reviewed-on: https://chromium-review.googlesource.com/778699
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519207}
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/core/v8/WorkerOrWorkletScriptController.cpp
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/scripts/v8_interface.py
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/templates/OriginTrialFeaturesForCore.cpp.tmpl
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/templates/OriginTrialFeaturesForModules.cpp.tmpl
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/templates/attributes.cpp.tmpl
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/templates/interface.h.tmpl
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/templates/partial_interface.h.tmpl
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/tests/results/core/OriginTrialFeaturesForCore.cpp
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/bindings/tests/results/modules/OriginTrialFeaturesForModules.cpp
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/platform/bindings/OriginTrialFeatures.cpp
[modify] https://crrev.com/a92cfc96142454dddc5805c1bf236454f0c5cf66/third_party/WebKit/Source/platform/bindings/OriginTrialFeatures.h

Comment 22 by rbyers@chromium.org, Feb 2 2018

Cc: haraken@chromium.org
Labels: Hotlist-Interop
Any update?  The more we ship things marked [SecureContext] that actually do get exposed via HTTP, the more we risk having web compat problems later when we try to fix this (and maybe even some accidental security issues?).

Comment 23 by falken@chromium.org, Feb 2 2018 

jsbell: Also https://chromium-review.googlesource.com/802676 seems to say [SecureContext] now works for interfaces? Is anything blocking us from adding to ServiceWorker like https://chromium-review.googlesource.com/c/chromium/src/+/592453 tried earlier?

Comment 24 by haraken@chromium.org, Feb 2 2018

Cc: yukishiino@chromium.org
peria, yukishiino: Is this probably fixed already?

Comment 25 by peria@chromium.org, Feb 2 2018

Status: Fixed (was: Started)
Yes, I believe so.

rbyers@, could you let us know the case if you have something wrong?

Comment 26 by jsb...@chromium.org, Feb 2 2018 

Yeah - we should be able to resurrect the parts of https://chromium-review.googlesource.com/c/chromium/src/+/592453 that haven't landed already.
►

Sign in to add a comment