Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(125)

Issue 1781003003: Implement referred Token Bindings (Closed)

Created:
4 years, 9 months ago by nharper
Modified:
4 years, 8 months ago
Reviewers:
dcheng, davidben
CC:
cbentzel+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement referred Token Bindings As described in https://tools.ietf.org/html/draft-ietf-tokbind-https-02: when a server sends the 'Include-Referer-Token-Binding-ID' header on a redirect response, the client will include the TokenBindingID from the referrer in its request to the new location in the redirect. If either connection does not have Token Binding enabled, then this has no effect. BUG=467312 Committed: https://crrev.com/d6e658265a6a3baa7cd7befecb24b12acb181047 Cr-Commit-Position: refs/heads/master@{#384134}

Patch Set 1 #

Total comments: 24

Patch Set 2 : fix nits; add unittest #

Total comments: 6

Patch Set 3 : rebase #

Patch Set 4 : Clean up states in HttpNetworkTransaction; add another test; fix nit #

Patch Set 5 : s/StringPice/StringPiece/ #

Total comments: 4

Patch Set 6 : Fix dcheng's nits, fix broken unittest #

Patch Set 7 : Add #ifdef for OS_ANDROID in TokenBindingURLRequestTest #

Unified diffs Side-by-side diffs Delta from patch set Stats (+408 lines, -101 lines) Patch
M content/common/resource_messages.h View 1 chunk +1 line, -0 lines 0 comments Download
M net/http/http_network_transaction.h View 1 2 3 3 chunks +13 lines, -7 lines 0 comments Download
M net/http/http_network_transaction.cc View 1 2 3 4 5 5 chunks +68 lines, -19 lines 0 comments Download
M net/http/http_network_transaction_ssl_unittest.cc View 1 2 chunks +42 lines, -9 lines 0 comments Download
M net/http/http_request_info.h View 1 chunk +4 lines, -0 lines 0 comments Download
M net/ssl/token_binding.h View 1 2 3 4 5 3 chunks +25 lines, -13 lines 0 comments Download
M net/ssl/token_binding_nss.cc View 1 2 3 4 1 chunk +7 lines, -5 lines 0 comments Download
M net/ssl/token_binding_openssl.cc View 1 2 3 4 5 3 chunks +45 lines, -44 lines 0 comments Download
M net/tools/testserver/testserver.py View 1 2 chunks +21 lines, -0 lines 0 comments Download
M net/url_request/redirect_info.h View 1 chunk +5 lines, -0 lines 0 comments Download
M net/url_request/url_request.h View 2 chunks +8 lines, -0 lines 0 comments Download
M net/url_request/url_request.cc View 1 chunk +1 line, -0 lines 0 comments Download
M net/url_request/url_request_http_job.cc View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M net/url_request/url_request_job.cc View 1 chunk +8 lines, -0 lines 0 comments Download
M net/url_request/url_request_unittest.cc View 1 2 3 4 5 6 1 chunk +141 lines, -4 lines 0 comments Download
M third_party/tlslite/README.chromium View 1 chunk +2 lines, -0 lines 0 comments Download
A third_party/tlslite/patches/save_randoms.patch View 1 chunk +13 lines, -0 lines 0 comments Download
M third_party/tlslite/tlslite/tlsconnection.py View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 29 (9 generated)
nharper
4 years, 9 months ago (2016-03-11 01:38:07 UTC) #2
davidben
This looks pretty good, modulo my being skeptical about the protocol itself as we talked ...
4 years, 9 months ago (2016-03-15 22:49:57 UTC) #3
nharper
https://codereview.chromium.org/1781003003/diff/1/net/http/http_network_transaction.cc File net/http/http_network_transaction.cc (right): https://codereview.chromium.org/1781003003/diff/1/net/http/http_network_transaction.cc#newcode999 net/http/http_network_transaction.cc:999: next_state_ = STATE_GET_TOKEN_BINDING_KEY; On 2016/03/15 22:49:56, davidben wrote: > ...
4 years, 9 months ago (2016-03-16 17:49:22 UTC) #4
davidben
Sorry, this slipped through. https://codereview.chromium.org/1781003003/diff/1/net/http/http_network_transaction.cc File net/http/http_network_transaction.cc (right): https://codereview.chromium.org/1781003003/diff/1/net/http/http_network_transaction.cc#newcode999 net/http/http_network_transaction.cc:999: next_state_ = STATE_GET_TOKEN_BINDING_KEY; On 2016/03/16 ...
4 years, 9 months ago (2016-03-24 20:53:52 UTC) #5
nharper
https://codereview.chromium.org/1781003003/diff/1/net/http/http_network_transaction.cc File net/http/http_network_transaction.cc (right): https://codereview.chromium.org/1781003003/diff/1/net/http/http_network_transaction.cc#newcode999 net/http/http_network_transaction.cc:999: next_state_ = STATE_GET_TOKEN_BINDING_KEY; On 2016/03/24 20:53:51, davidben wrote: > ...
4 years, 9 months ago (2016-03-25 01:34:30 UTC) #6
davidben
lgtm
4 years, 9 months ago (2016-03-26 01:11:36 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1781003003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1781003003/60001
4 years, 9 months ago (2016-03-26 01:33:05 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator_gn/builds/9628) ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED, ...
4 years, 9 months ago (2016-03-26 01:37:58 UTC) #11
nharper
R+dcheng for content/common/resource_messages.h
4 years, 9 months ago (2016-03-26 01:42:52 UTC) #13
dcheng
lgtm https://codereview.chromium.org/1781003003/diff/80001/net/ssl/token_binding.h File net/ssl/token_binding.h (right): https://codereview.chromium.org/1781003003/diff/80001/net/ssl/token_binding.h#newcode18 net/ssl/token_binding.h:18: enum TokenBindingType { Nit: enum class and just ...
4 years, 9 months ago (2016-03-26 08:54:58 UTC) #14
nharper
PTAL. I also found that the unittest I added in ps5 was broken, so I ...
4 years, 8 months ago (2016-03-28 22:02:06 UTC) #15
davidben
lgtm
4 years, 8 months ago (2016-03-28 22:05:49 UTC) #16
dcheng
lgtm
4 years, 8 months ago (2016-03-28 22:06:49 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1781003003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1781003003/100001
4 years, 8 months ago (2016-03-28 22:10:17 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/44248)
4 years, 8 months ago (2016-03-29 00:52:21 UTC) #21
nharper
Two tests that I added are failing on android, because the test server spawner will ...
4 years, 8 months ago (2016-03-29 22:09:06 UTC) #22
davidben
On 2016/03/29 22:09:06, nharper wrote: > Two tests that I added are failing on android, ...
4 years, 8 months ago (2016-03-30 15:18:43 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1781003003/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1781003003/120001
4 years, 8 months ago (2016-03-30 20:47:35 UTC) #26
commit-bot: I haz the power
Committed patchset #7 (id:120001)
4 years, 8 months ago (2016-03-30 23:05:57 UTC) #27
commit-bot: I haz the power
4 years, 8 months ago (2016-03-30 23:07:34 UTC) #29
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/d6e658265a6a3baa7cd7befecb24b12acb181047
Cr-Commit-Position: refs/heads/master@{#384134}

Powered by Google App Engine
This is Rietveld 408576698