Support subdomain matching in trial tokens

Support subdomain matching in trial tokens

This CL adds support for subdomain trial tokens, such that a properly
issued token will enable a trial for multiple subdomains, rather than a
single specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a
wildcard token (e.g. *.example.com). In this implementation, an explicit
flag was added to the token format. The explicit flag keeping the token
parsing simple, by keeping the url::Origin as the representation of the
origin, and avoiding custom string parsing.

BUG=653349
Committed: https://crrev.com/4f0cb8e9b16a270d0a7fd313f161b4c2c9a0df29
Cr-Commit-Position: refs/heads/master@{#425168}

[chasej, 2016-10-11 17:29:26]

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

[commit-bot: I haz the power, 2016-10-11 17:29:46]

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-10-11 18:14:44]

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

[commit-bot: I haz the power, 2016-10-11 18:14:45]

Dry run: This issue passed the CQ dry run.

[chasej, 2016-10-11 19:08:10]

Description was changed from

==========
Support wildcard subdomains in trial tokens

TODO: Add more details

BUG=653349
==========

to

==========
Support wildcard subdomains in trial tokens

This CL adds support for wildcard trial tokens, such that a properly issued
token will enable a trial for multiple subdomains, rather than a single
specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a wildcard token
(e.g. *.example.com). In this implementation, an explicit flag was added to the
token format. The explicit flag keeping the token parsing simple, by keeping the
url::Origin as the representation of the origin, and avoiding custom string
parsing.

BUG=653349
==========

[chasej, 2016-10-11 19:08:11]

chasej@chromium.org changed reviewers:
+ iclelland@chromium.org

[iclelland, 2016-10-12 04:18:16]

lgtm

https://codereview.chromium.org/2411803002/diff/1/content/common/origin_trial...
File content/common/origin_trials/trial_token.h (right):

https://codereview.chromium.org/2411803002/diff/1/content/common/origin_trial...
content/common/origin_trials/trial_token.h:96: bool is_wildcard_origin_;
Bikeshed: Would this be better named something like match_subdomains_ ? (Just
thinking about the fact that we're not using a wildcard character anymore)

Fine either way, really, though.

https://codereview.chromium.org/2411803002/diff/1/content/common/origin_trial...
File content/common/origin_trials/trial_token_unittest.cc (right):

https://codereview.chromium.org/2411803002/diff/1/content/common/origin_trial...
content/common/origin_trials/trial_token_unittest.cc:79: const char*
kIncorrectDomainOrigin = "https://valid.example2.com";
Can you add one more bad domain for wildcard testing? I'd like to see the
wildcard token tested against "https://com", just to ensure that
Origin::DomainIs is filtering the domains in the right direction.

https://codereview.chromium.org/2411803002/diff/1/content/common/origin_trial...
content/common/origin_trials/trial_token_unittest.cc:142: "{\"origin\":
\"https://a.a\", \"feature\": \"a\"}",
Good catch

[chasej, 2016-10-12 18:05:59]

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

[commit-bot: I haz the power, 2016-10-12 18:06:29]

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-10-12 19:53:30]

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

[commit-bot: I haz the power, 2016-10-12 19:53:31]

Dry run: This issue passed the CQ dry run.

[chasej, 2016-10-13 06:37:09]

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

[commit-bot: I haz the power, 2016-10-13 06:37:25]

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-10-13 07:21:19]

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

[commit-bot: I haz the power, 2016-10-13 07:21:19]

Dry run: This issue passed the CQ dry run.

[chasej, 2016-10-13 12:00:56]

Description was changed from

==========
Support wildcard subdomains in trial tokens

This CL adds support for wildcard trial tokens, such that a properly issued
token will enable a trial for multiple subdomains, rather than a single
specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a wildcard token
(e.g. *.example.com). In this implementation, an explicit flag was added to the
token format. The explicit flag keeping the token parsing simple, by keeping the
url::Origin as the representation of the origin, and avoiding custom string
parsing.

BUG=653349
==========

to

==========
Support subdomain matching in trial tokens

This CL adds support for subdomain trial tokens, such that a properly
issued token will enable a trial for multiple subdomains, rather than a single
specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a wildcard token
(e.g. *.example.com). In this implementation, an explicit flag was added to the
token format. The explicit flag keeping the token parsing simple, by keeping the
url::Origin as the representation of the origin, and avoiding custom string
parsing.

BUG=653349
==========

[chasej, 2016-10-13 12:01:43]

Description was changed from

==========
Support subdomain matching in trial tokens

This CL adds support for subdomain trial tokens, such that a properly
issued token will enable a trial for multiple subdomains, rather than a single
specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a wildcard token
(e.g. *.example.com). In this implementation, an explicit flag was added to the
token format. The explicit flag keeping the token parsing simple, by keeping the
url::Origin as the representation of the origin, and avoiding custom string
parsing.

BUG=653349
==========

to

==========
Support subdomain matching in trial tokens

This CL adds support for subdomain trial tokens, such that a properly
issued token will enable a trial for multiple subdomains, rather than a
single specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a
wildcard token (e.g. *.example.com). In this implementation, an explicit
flag was added to the token format. The explicit flag keeping the token
parsing simple, by keeping the url::Origin as the representation of the
origin, and avoiding custom string parsing.

BUG=653349
==========

[chasej, 2016-10-13 15:36:19]

iclelland, please take another look.

I've changed to use the "subdomain" naming, instead of "wildcard". Also added
fuzzer data.

[iclelland, 2016-10-13 15:51:24]

Thanks - still lgtm!

One question about whether we ever need to mint a token with isSubdomain=false,
but this looks good either way.

https://codereview.chromium.org/2411803002/diff/100001/tools/origin_trials/ge...
File tools/origin_trials/generate_token.py (right):

https://codereview.chromium.org/2411803002/diff/100001/tools/origin_trials/ge...
tools/origin_trials/generate_token.py:93: if is_subdomain is not None:
Could this be just
    if is_subdomain:
?
(Is there any need to store false values in the token?)

[chasej, 2016-10-13 16:05:43]

https://codereview.chromium.org/2411803002/diff/100001/tools/origin_trials/ge...
File tools/origin_trials/generate_token.py (right):

https://codereview.chromium.org/2411803002/diff/100001/tools/origin_trials/ge...
tools/origin_trials/generate_token.py:93: if is_subdomain is not None:
On 2016/10/13 15:51:23, iclelland wrote:
> Could this be just
>     if is_subdomain:
> ?
> (Is there any need to store false values in the token?)

There isn't an obvious need - I debated back and forth. For testing purposes, I
wanted the ability to explicitly set it to false. Same reason for having the
--no-subdomain flag - unlikely it will be used for production tokens.

[iclelland, 2016-10-13 16:06:47]

https://codereview.chromium.org/2411803002/diff/100001/tools/origin_trials/ge...
File tools/origin_trials/generate_token.py (right):

https://codereview.chromium.org/2411803002/diff/100001/tools/origin_trials/ge...
tools/origin_trials/generate_token.py:93: if is_subdomain is not None:
On 2016/10/13 16:05:42, chasej wrote:
> On 2016/10/13 15:51:23, iclelland wrote:
> > Could this be just
> >     if is_subdomain:
> > ?
> > (Is there any need to store false values in the token?)
> 
> There isn't an obvious need - I debated back and forth. For testing purposes,
I
> wanted the ability to explicitly set it to false. Same reason for having the
> --no-subdomain flag - unlikely it will be used for production tokens.

sgtm

[chasej, 2016-10-13 20:35:25]

The CQ bit was checked by chasej@chromium.org

[commit-bot: I haz the power, 2016-10-13 20:35:49]

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-10-13 21:32:58]

Description was changed from

==========
Support subdomain matching in trial tokens

This CL adds support for subdomain trial tokens, such that a properly
issued token will enable a trial for multiple subdomains, rather than a
single specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a
wildcard token (e.g. *.example.com). In this implementation, an explicit
flag was added to the token format. The explicit flag keeping the token
parsing simple, by keeping the url::Origin as the representation of the
origin, and avoiding custom string parsing.

BUG=653349
==========

to

==========
Support subdomain matching in trial tokens

This CL adds support for subdomain trial tokens, such that a properly
issued token will enable a trial for multiple subdomains, rather than a
single specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a
wildcard token (e.g. *.example.com). In this implementation, an explicit
flag was added to the token format. The explicit flag keeping the token
parsing simple, by keeping the url::Origin as the representation of the
origin, and avoiding custom string parsing.

BUG=653349
==========

[commit-bot: I haz the power, 2016-10-13 21:33:00]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-13 21:35:19]

Description was changed from

==========
Support subdomain matching in trial tokens

This CL adds support for subdomain trial tokens, such that a properly
issued token will enable a trial for multiple subdomains, rather than a
single specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a
wildcard token (e.g. *.example.com). In this implementation, an explicit
flag was added to the token format. The explicit flag keeping the token
parsing simple, by keeping the url::Origin as the representation of the
origin, and avoiding custom string parsing.

BUG=653349
==========

to

==========
Support subdomain matching in trial tokens

This CL adds support for subdomain trial tokens, such that a properly
issued token will enable a trial for multiple subdomains, rather than a
single specified origin.

In bug 653349, it was proposed to use a leading "*" to indicate a
wildcard token (e.g. *.example.com). In this implementation, an explicit
flag was added to the token format. The explicit flag keeping the token
parsing simple, by keeping the url::Origin as the representation of the
origin, and avoiding custom string parsing.

BUG=653349

Committed: https://crrev.com/4f0cb8e9b16a270d0a7fd313f161b4c2c9a0df29
Cr-Commit-Position: refs/heads/master@{#425168}
==========

[commit-bot: I haz the power, 2016-10-13 21:35:20]

Patchset 6 (id:??) landed as
https://crrev.com/4f0cb8e9b16a270d0a7fd313f161b4c2c9a0df29
Cr-Commit-Position: refs/heads/master@{#425168}

[chasej, 2016-10-17 20:36:44]

chasej@chromium.org changed reviewers:
+ estark@chromium.org

[chasej, 2016-10-17 20:36:45]

estark, please take a look.

The linked bug provides context, but this an extension to the token format. We
want to merge to 55, but want a sanity check from security. No merge has been
requested yet.

[estark, 2016-10-18 19:48:54]

lgtm with a question

https://codereview.chromium.org/2411803002/diff/100001/content/common/origin_...
File content/common/origin_trials/trial_token.cc (right):

https://codereview.chromium.org/2411803002/diff/100001/content/common/origin_...
content/common/origin_trials/trial_token.cc:185: origin.DomainIs(origin_.host())
&&
Is token issuance restricted by the Public Suffix List? That is, is it possible
to get a token for, say, github.io or appspot.com?

[chasej, 2016-10-18 20:10:49]

On 2016/10/18 19:48:54, estark wrote:
> lgtm with a question
> 
>
https://codereview.chromium.org/2411803002/diff/100001/content/common/origin_...
> File content/common/origin_trials/trial_token.cc (right):
> 
>
https://codereview.chromium.org/2411803002/diff/100001/content/common/origin_...
> content/common/origin_trials/trial_token.cc:185:
origin.DomainIs(origin_.host())
> &&
> Is token issuance restricted by the Public Suffix List? That is, is it
possible
> to get a token for, say, github.io or appspot.com?

Yes, token issuance will be restricted. The registration process is being
updated so it will not allow tokens for domains on the Public Suffix List.