URLRequestContextBuilder should initialize a default CT state

URLRequestContextBuilder should initialize a default CT state

When using a URLRequestContextBuilder, the URLRequestContext
it built would not be initialized with any Certificate
Transparency logs. This is because CT logs are a policy of the
embedder. Without any CT logs, all certificates naturally fail
any CT policies the embedder has set.

With no way to override this on the URLRequestContextBuilder,
any CA which has CT compliance enforced on it will be rejected.
This is because per-CA policies are enforced at the //net layer
as part of a 'secure by default' policy, and thus fail closed.

The result is that embedders that used URLRequestContextBuilder,
such as Android WebView, rather than building URLRequestContexts
themselves, such as Chrome does, would reject certs from some
CAs. While this could be overridden by manipulating the
TransportSecurityState of the URLRequestContext that was built,
doing so would result in a less-secure solution.

This CL mitigates it two-fold, by allowing the embedder to specify
their own CTVerifier to handle evaluating Certificate Transparency
information, and if they fail to, by initializing with the set
of logs known to //net and trusted by Chrome according to the
Chrome CT policy. This gives embedders a sane default, while still
allowing them to override with their own application policies.

This doesn't fully bootstrap a CT enforcement for embedders, as
things like consistency proof fetching are concepts that extend
beyond the URLRequestContext (and handled by the IOThread in
Chrome), but offers a reasonable baseline. This also helps ensure
things like Expect-CT work consistently for embedders.

BUG=628421
Committed: https://crrev.com/0fe709bafb6c31586b2833d4974612d6eccc2e02
Cr-Commit-Position: refs/heads/master@{#406455}

[Ryan Sleevi, 2016-07-19 21:20:56]

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

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

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

[sgurun-gerrit only, 2016-07-19 21:25:49]

On 2016/07/19 21:21:54, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

compile errors:
/../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:481:29:
  required from here
../../base/memory/ref_counted.h:407:3: error: invalid use of incomplete type
'const class net::CTLogVerifier'
   ptr->Release();
   ^
In file included from ../../net/url_request/url_request_context_builder.cc:24:0:
../../net/cert/ct_known_logs.h:21:7: error: forward declaration of 'const class
net::CTLogVerifier'
 class CTLogVerifier;

[sgurun-gerrit only, 2016-07-19 21:28:52]

On 2016/07/19 21:25:49, sgurun wrote:
> On 2016/07/19 21:21:54, commit-bot: I haz the power wrote:
> > Dry run: CQ is trying da patch. Follow status at
> >  
> >
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> 
> compile errors:
>
/../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:481:29:
>   required from here
> ../../base/memory/ref_counted.h:407:3: error: invalid use of incomplete type
> 'const class net::CTLogVerifier'
>    ptr->Release();
>    ^
> In file included from
../../net/url_request/url_request_context_builder.cc:24:0:
> ../../net/cert/ct_known_logs.h:21:7: error: forward declaration of 'const
class
> net::CTLogVerifier'
>  class CTLogVerifier;

also

../../net/url_request/url_request_context_builder.h:302:18:   required from here
../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2429:33:
error: invalid application of 'sizeof' to incomplete type 'net::CTVerifier'
             static_assert(sizeof(_Tp) > 0, "default_delete can not delete
incomplete type");
                                 ^
[3/56] CXX obj/net/net/url_request_context_builder.o
FAILED: obj/net/net/url_request_context_builder.o

[sgurun-gerrit only, 2016-07-19 21:28:55]

On 2016/07/19 21:25:49, sgurun wrote:
> On 2016/07/19 21:21:54, commit-bot: I haz the power wrote:
> > Dry run: CQ is trying da patch. Follow status at
> >  
> >
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
> 
> compile errors:
>
/../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/vector:481:29:
>   required from here
> ../../base/memory/ref_counted.h:407:3: error: invalid use of incomplete type
> 'const class net::CTLogVerifier'
>    ptr->Release();
>    ^
> In file included from
../../net/url_request/url_request_context_builder.cc:24:0:
> ../../net/cert/ct_known_logs.h:21:7: error: forward declaration of 'const
class
> net::CTLogVerifier'
>  class CTLogVerifier;

also

../../net/url_request/url_request_context_builder.h:302:18:   required from here
../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2429:33:
error: invalid application of 'sizeof' to incomplete type 'net::CTVerifier'
             static_assert(sizeof(_Tp) > 0, "default_delete can not delete
incomplete type");
                                 ^
[3/56] CXX obj/net/net/url_request_context_builder.o
FAILED: obj/net/net/url_request_context_builder.o

[commit-bot: I haz the power, 2016-07-19 21:30:05]

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

[commit-bot: I haz the power, 2016-07-19 21:30:06]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Ryan Sleevi, 2016-07-19 21:40:20]

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

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

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

[Ryan Sleevi, 2016-07-19 21:41:21]

rsleevi@chromium.org changed reviewers:
+ eroman@chromium.org

[sgurun-gerrit only, 2016-07-19 22:29:48]

On 2016/07/19 21:41:22, Ryan Sleevi (extremely slow) wrote:

not an owner but lgtm

[eroman, 2016-07-19 22:55:41]

Is there a test that exercises and confirms this behavior?

Otherwise behavior and code LGTM

https://codereview.chromium.org/2161983003/diff/20001/net/url_request/url_req...
File net/url_request/url_request_context_builder.cc (right):

https://codereview.chromium.org/2161983003/diff/20001/net/url_request/url_req...
net/url_request/url_request_context_builder.cc:255: void
URLRequestContextBuilder::set_ct_verifier(
Why the naming difference from SetCertVerifier() / SetInterceptors() etc.

Arguably those other two are the aberrations in-so-far as chromium style is
concerned.

But it does mean this new function is not consistent within
url_request_context_builder.h

[Ryan Sleevi, 2016-07-19 23:04:53]

There's no unittest, because this is really something for the embedder level to
deal with.

For similar context, there's no request for URLRequestContextBuilder that
certificates verify or that EV is supported; this class seems to operate right
at the threshold of glue where any testing really becomes an integration level
test for the defaults, and it's unclear where those integration tests belong.

https://codereview.chromium.org/2161983003/diff/20001/net/url_request/url_req...
File net/url_request/url_request_context_builder.cc (right):

https://codereview.chromium.org/2161983003/diff/20001/net/url_request/url_req...
net/url_request/url_request_context_builder.cc:255: void
URLRequestContextBuilder::set_ct_verifier(
On 2016/07/19 22:55:40, eroman wrote:
> But it does mean this new function is not consistent within
> url_request_context_builder.h

It's consistent with the dominant style, and SetCertVerifier/SetInterceptors
seemed to be the inconsistent one.

The reason for not inlining in the header was because std::move would require
including the full header

[commit-bot: I haz the power, 2016-07-20 00:15:56]

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

[commit-bot: I haz the power, 2016-07-20 00:15:57]

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...)

[Ryan Sleevi, 2016-07-20 00:44:21]

The CQ bit was checked by rsleevi@chromium.org

[commit-bot: I haz the power, 2016-07-20 00:45:27]

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-07-20 02:04:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-20 02:05:34]

Description was changed from

==========
URLRequestContextBuilder should initialize a default CT state

When using a URLRequestContextBuilder, the URLRequestContext
it built would not be initialized with any Certificate
Transparency logs. This is because CT logs are a policy of the
embedder. Without any CT logs, all certificates naturally fail
any CT policies the embedder has set.

With no way to override this on the URLRequestContextBuilder,
any CA which has CT compliance enforced on it will be rejected.
This is because per-CA policies are enforced at the //net layer
as part of a 'secure by default' policy, and thus fail closed.

The result is that embedders that used URLRequestContextBuilder,
such as Android WebView, rather than building URLRequestContexts
themselves, such as Chrome does, would reject certs from some
CAs. While this could be overridden by manipulating the
TransportSecurityState of the URLRequestContext that was built,
doing so would result in a less-secure solution.

This CL mitigates it two-fold, by allowing the embedder to specify
their own CTVerifier to handle evaluating Certificate Transparency
information, and if they fail to, by initializing with the set
of logs known to //net and trusted by Chrome according to the
Chrome CT policy. This gives embedders a sane default, while still
allowing them to override with their own application policies.

This doesn't fully bootstrap a CT enforcement for embedders, as
things like consistency proof fetching are concepts that extend
beyond the URLRequestContext (and handled by the IOThread in
Chrome), but offers a reasonable baseline. This also helps ensure
things like Expect-CT work consistently for embedders.

BUG=628421
==========

to

==========
URLRequestContextBuilder should initialize a default CT state

When using a URLRequestContextBuilder, the URLRequestContext
it built would not be initialized with any Certificate
Transparency logs. This is because CT logs are a policy of the
embedder. Without any CT logs, all certificates naturally fail
any CT policies the embedder has set.

With no way to override this on the URLRequestContextBuilder,
any CA which has CT compliance enforced on it will be rejected.
This is because per-CA policies are enforced at the //net layer
as part of a 'secure by default' policy, and thus fail closed.

The result is that embedders that used URLRequestContextBuilder,
such as Android WebView, rather than building URLRequestContexts
themselves, such as Chrome does, would reject certs from some
CAs. While this could be overridden by manipulating the
TransportSecurityState of the URLRequestContext that was built,
doing so would result in a less-secure solution.

This CL mitigates it two-fold, by allowing the embedder to specify
their own CTVerifier to handle evaluating Certificate Transparency
information, and if they fail to, by initializing with the set
of logs known to //net and trusted by Chrome according to the
Chrome CT policy. This gives embedders a sane default, while still
allowing them to override with their own application policies.

This doesn't fully bootstrap a CT enforcement for embedders, as
things like consistency proof fetching are concepts that extend
beyond the URLRequestContext (and handled by the IOThread in
Chrome), but offers a reasonable baseline. This also helps ensure
things like Expect-CT work consistently for embedders.

BUG=628421

Committed: https://crrev.com/0fe709bafb6c31586b2833d4974612d6eccc2e02
Cr-Commit-Position: refs/heads/master@{#406455}
==========

[commit-bot: I haz the power, 2016-07-20 02:05:35]

Patchset 2 (id:??) landed as
https://crrev.com/0fe709bafb6c31586b2833d4974612d6eccc2e02
Cr-Commit-Position: refs/heads/master@{#406455}