Prohibit web payments on sites with bad SSL certificates

Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://w3c.github.io/webappsec-secure-contexts/#secure-context

Also note that credit card autofill additionally checks that the <form>
target URL scheme is not http://. Password autofill does not perform
this check. (Web payments API does not use <form> elements.)

BUG=678764

[please use gerrit instead, 2016-12-27 19:25:40]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 19:25:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-27 19:48:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-27 19:48:46]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[please use gerrit instead, 2016-12-27 20:29:50]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 20:30:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[please use gerrit instead, 2016-12-27 21:09:08]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-12-27 22:33:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-27 22:33:09]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[please use gerrit instead, 2017-01-05 21:39:32]

Description was changed from

==========
Prohibit web payments on sites with bad SSL certificates

BUG=FILEME
==========

to

==========
Prohibit web payments on sites with bad SSL certificates.

In this patch:
1) Reduce copy-pasta of SSL check by moving it to a common location.
2) Allow use of powerful features on localhost and in file:/// scheme.
3) Apply the same SSL check to autofill and web payments API.

BUG=678764
==========

[please use gerrit instead, 2017-01-05 21:39:57]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 21:40:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[please use gerrit instead, 2017-01-05 21:41:44]

Patchset #1 (id:20001) has been deleted

[please use gerrit instead, 2017-01-05 22:05:47]

Description was changed from

==========
Prohibit web payments on sites with bad SSL certificates.

In this patch:
1) Reduce copy-pasta of SSL check by moving it to a common location.
2) Allow use of powerful features on localhost and in file:/// scheme.
3) Apply the same SSL check to autofill and web payments API.

BUG=678764
==========

to

==========
Prohibit web payments on sites with bad SSL certificates.

In this patch:

- Reduce copy-pasta of SSL check by moving it to a common location.

- Allow use of powerful features on localhost and in file:/// scheme.

- Apply the same SSL check to credit card autofill, password autofill,
  and web payments API.

Note 1: Web payments API is available only in secure context.
        https://heycam.github.io/webidl/#SecureContext

Note 2: Credit card autofill also checks that the <form> target URL
        scheme is not http://. Password autofill does not perform this
        check. Web payments API does not use <form> elements.

BUG=678764
==========

[please use gerrit instead, 2017-01-05 22:11:50]

Description was changed from

==========
Prohibit web payments on sites with bad SSL certificates.

In this patch:

- Reduce copy-pasta of SSL check by moving it to a common location.

- Allow use of powerful features on localhost and in file:/// scheme.

- Apply the same SSL check to credit card autofill, password autofill,
  and web payments API.

Note 1: Web payments API is available only in secure context.
        https://heycam.github.io/webidl/#SecureContext

Note 2: Credit card autofill also checks that the <form> target URL
        scheme is not http://. Password autofill does not perform this
        check. Web payments API does not use <form> elements.

BUG=678764
==========

to

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://heycam.github.io/webidl/#SecureContext

Also not that credit card autofill also checks that the <form> target
URL scheme is not http://. Password autofill does not perform this
check. (Web payments API does not use <form> elements.)

BUG=678764
==========

[please use gerrit instead, 2017-01-05 22:12:26]

Description was changed from

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://heycam.github.io/webidl/#SecureContext

Also not that credit card autofill also checks that the <form> target
URL scheme is not http://. Password autofill does not perform this
check. (Web payments API does not use <form> elements.)

BUG=678764
==========

to

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://heycam.github.io/webidl/#SecureContext

Also not that credit card autofill also checks that the <form> target
URL scheme is not http://. Password autofill does not perform this
check. (Web payments API does not use <form> elements.)

BUG=678764
==========

[please use gerrit instead, 2017-01-05 22:13:07]

Description was changed from

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://heycam.github.io/webidl/#SecureContext

Also not that credit card autofill also checks that the <form> target
URL scheme is not http://. Password autofill does not perform this
check. (Web payments API does not use <form> elements.)

BUG=678764
==========

to

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://heycam.github.io/webidl/#SecureContext

Also note that credit card autofill also checks that the <form> target
URL scheme is not http://. Password autofill does not perform this
check. (Web payments API does not use <form> elements.)

BUG=678764
==========

[please use gerrit instead, 2017-01-05 22:16:01]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 22:16:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[please use gerrit instead, 2017-01-05 22:19:14]

Description was changed from

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://heycam.github.io/webidl/#SecureContext

Also note that credit card autofill also checks that the <form> target
URL scheme is not http://. Password autofill does not perform this
check. (Web payments API does not use <form> elements.)

BUG=678764
==========

to

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://heycam.github.io/webidl/#SecureContext

Also note that credit card autofill additionally checks that the <form>
target URL scheme is not http://. Password autofill does not perform
this check. (Web payments API does not use <form> elements.)

BUG=678764
==========

[please use gerrit instead, 2017-01-05 22:19:52]

Patchset #1 (id:40001) has been deleted

[please use gerrit instead, 2017-01-05 22:21:07]

Description was changed from

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://heycam.github.io/webidl/#SecureContext

Also note that credit card autofill additionally checks that the <form>
target URL scheme is not http://. Password autofill does not perform
this check. (Web payments API does not use <form> elements.)

BUG=678764
==========

to

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://w3c.github.io/webappsec-secure-contexts/#settings-object

Also note that credit card autofill additionally checks that the <form>
target URL scheme is not http://. Password autofill does not perform
this check. (Web payments API does not use <form> elements.)

BUG=678764
==========

[please use gerrit instead, 2017-01-05 22:22:00]

Description was changed from

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://w3c.github.io/webappsec-secure-contexts/#settings-object

Also note that credit card autofill additionally checks that the <form>
target URL scheme is not http://. Password autofill does not perform
this check. (Web payments API does not use <form> elements.)

BUG=678764
==========

to

==========
Prohibit web payments on sites with bad SSL certificates.

This patch moves the SSL certificate validity check to the common
location in WebContents and gates usage of web payments API on the
calling website having a valid SSL certificate. The same check is also
used by credit card and password autofill.

To better accommodate web developers, the SSL certificate check is
disabled for localhost and file:/// schemes.

Note that web payments API is available only in secure context.
https://w3c.github.io/webappsec-secure-contexts/#secure-context

Also note that credit card autofill additionally checks that the <form>
target URL scheme is not http://. Password autofill does not perform
this check. (Web payments API does not use <form> elements.)

BUG=678764
==========

[please use gerrit instead, 2017-01-05 22:25:22]

rouslan@chromium.org changed reviewers:
+ lgarron@chromium.org, palmer@chromium.org

[please use gerrit instead, 2017-01-05 22:25:23]

Lucas and Chris, ptal.

[palmer, 2017-01-05 22:55:06]

Are any tests necessary/possible?

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:831: return true;
Why return true if the scheme is *not* cryptographic?

[lgarron, 2017-01-05 23:00:20]

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:831: return true;
On 2017/01/05 at 22:55:06, palmer wrote:
> Why return true if the scheme is *not* cryptographic?

I was about to comment the same.

In general, is there a need to introduce a new IsContextSecure() function in so
many places?

palmer@ created IsOriginSecure() to unify such calculations, and if there is a
need for a new function we should think that through.

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:837:
!(ssl_status.content_status & SSLStatus::RAN_INSECURE_CONTENT);
This looks like an ad-hoc implementation of something between "is this
connection secure" and "do we display it as secure".

Do we prevent any other "secure context" features based on things like this? If
so, where are they calculated now. If not, are there strong reasons to add this
here for payments? Are there plans to unify calculations?

[commit-bot: I haz the power, 2017-01-05 23:40:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-05 23:40:25]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2017-01-06 18:07:15]

One of the goals of this patch is to move the existing SSL validity checks to a
common location. I propose to move it to WebContents::IsContextSecure(). That
method also calls into content::IsOriginSecure() to bypass the SSL requirement
for localhost and file:// schemes. Is this plan OK?

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:831: return true;
On 2017/01/05 22:55:06, palmer wrote:
> Why return true if the scheme is *not* cryptographic?

To prevent checking SSL status of localhost and file:// scheme.

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:831: return true;
On 2017/01/05 23:00:20, lgarron wrote:
> In general, is there a need to introduce a new IsContextSecure() function in
so
> many places?
>
> palmer@ created IsOriginSecure() to unify such calculations, and if there is a
> need for a new function we should think that through.

I would love to have a single IsOriginSecure() function that's approved by the
security team and used by credit card autofill, password autofill, and web
payments API. The file "content/public/common/origin_util.h" is supposed to
provide this functionality, but it does not have SSL validity checks. Is that
intentional?

The SSL validity check that you see below is moved from
chrome_autofill_client.cc. The same code was also present in
aw_autofill_client.cc, so I've unified them.

One caveat is that ChromeAutofillClient::IsContextSecure() returned false for
localhost and file://, which hampers web developers and our browser/integration
tests. That's why in this common version of IsContextSecure(), I added
IsOriginSecure() as well.

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:837:
!(ssl_status.content_status & SSLStatus::RAN_INSECURE_CONTENT);
On 2017/01/05 23:00:20, lgarron wrote:
> This looks like an ad-hoc implementation of something between "is this
> connection secure" and "do we display it as secure".

I also don't like this ad-hoc thing and would like to remove it to tighten the
validity check.

> Do we prevent any other "secure context" features based on things like this?
If
> so, where are they calculated now. If not, are there strong reasons to add
this
> here for payments? Are there plans to unify calculations?

Before this patch, only credit card autofill and password autofill checked the
validity of the SSL certificate:

https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofil...

https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofil...

https://cs.chromium.org/chromium/src/components/password_manager/core/browser...

https://cs.chromium.org/chromium/src/components/password_manager/core/browser...

Note that credit card autofill also checks the target URL of the <form>, but
password autofill does not. Is it an oversight?

[lgarron, 2017-01-06 23:21:25]

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:831: return true;
On 2017/01/06 at 18:07:15, rouslan wrote:
> On 2017/01/05 22:55:06, palmer wrote:
> > Why return true if the scheme is *not* cryptographic?
> 
> To prevent checking SSL status of localhost and file:// scheme.

IsOriginSecure() can't have SSL validity checks because you only pass it an
origin/URL.
In general, we don't try to prevent features in secure contexts when the
connection is invalid. If the page is non-secure but loads, the user often still
expects the behaviour of a secure page.

In any case, IsOriginSecure() returns true for localhost and file://.

https://codereview.chromium.org/2603823002/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:837:
!(ssl_status.content_status & SSLStatus::RAN_INSECURE_CONTENT);
On 2017/01/06 at 18:07:15, rouslan wrote:
> On 2017/01/05 23:00:20, lgarron wrote:
> > This looks like an ad-hoc implementation of something between "is this
> > connection secure" and "do we display it as secure".
> 
> I also don't like this ad-hoc thing and would like to remove it to tighten the
validity check.
> 
> > Do we prevent any other "secure context" features based on things like this?
If
> > so, where are they calculated now. If not, are there strong reasons to add
this
> > here for payments? Are there plans to unify calculations?
> 
> Before this patch, only credit card autofill and password autofill checked the
validity of the SSL certificate:
> 
>
https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofil...
> 
>
https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofil...
> 
>
https://cs.chromium.org/chromium/src/components/password_manager/core/browser...
> 
>
https://cs.chromium.org/chromium/src/components/password_manager/core/browser...
> 
> Note that credit card autofill also checks the target URL of the <form>, but
password autofill does not. Is it an oversight?

Interesting. Do you know how these decisions were made, and when?
I'll start an Enamel thread about this.

[please use gerrit instead, 2017-01-10 20:49:56]

Closing this patch in favor of the plan outlined in the e-mail discussion.