Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2144693004:
Add the ability to send Expect-Staple reports. (Closed)
DescriptionAdd the ability to send Expect-Staple reports.
This adds a |ProcessExpectStaple| method to TransportSecurityState that
implements reporting for Expect-Staple, using contents of SSLInfo and
OCSPVerifyResult. No report is sent if a valid staple is provided, or if
the remote host is not on the Expect-Staple preload list. This method is
not currently called by any socket implementation.
BUG=598021
Committed: https://crrev.com/c87b456d823a4bd61599c5ce4512848a6f2be7b6
Cr-Commit-Position: refs/heads/master@{#406742}
Patch Set 1 #Patch Set 2 : Compile after rebase #
Total comments: 16
Patch Set 3 : Clean tests, add documentation. #
Total comments: 19
Patch Set 4 : More test cleanups. #
Total comments: 1
Patch Set 5 : Remove stragglers. #Patch Set 6 : Make switch statements Android/ChromeOS friendly #
Messages
Total messages: 28 (11 generated)
dadrian@google.com changed reviewers: + estark@chromium.org, rsleevi@chromium.org, svaldez@chromium.org
More Expect-Staple!
https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state.cc (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.cc:640: std::string ResponseStatusToString(OCSPVerifyResult::ResponseStatus status) { Documentation needed :) For example, are these strings part of an API contract? For debugging purposes? Something else? https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.cc:659: return ""; return std::string() https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.cc:670: } Why do you return "" on 659, but not return "" (or equivalent) here? You're either expecting that the compiler can't handle that you've enumerated all the values (which MSVC 2013 wasn't smart enough to realize), which explains 659, or you're expecting that it is (here)... would be good to be consistent https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state.h (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.h:454: const std::string& ocsp_response); Documentation is needed here. Just from the header, it's unclear why ocsp_response is needed. From a design standpoint, the naming here leaves much to be desired - I thought at first this processed ExpectStaple headers, but no, it performs the logic of ExpectStaple processing. From a documentation standpoint, it's unclear how one would use this. For example, do you call this for all hosts when you get a stapled response? Do you only call it if the host is in the expect-staple list? Do you only call it when the stapled response is bad? I would try, as much as possible, to examine the other methods of TSS and try to make it consistent with them. https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state_unittest.cc (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:243: bool ResponseStatusFromString(std::string in, From a testing design standpoint, it doesn't seem you need this. If you're wanting to test the serialized reports, shouldn't you do the string comparison? You only call this function once, does it really need to be a helper? https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:267: bool RevocationStatusFromString(std::string in, OCSPRevocationStatus* status) { Same remarks
https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state.cc (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.cc:670: } On 2016/07/19 00:02:49, Ryan Sleevi (extremely slow) wrote: > Why do you return "" on 659, but not return "" (or equivalent) here? > > You're either expecting that the compiler can't handle that you've enumerated > all the values (which MSVC 2013 wasn't smart enough to realize), which explains > 659, or you're expecting that it is (here)... would be good to be consistent I meant to do it this way---does MSVC 2015 catch this? https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state.h (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.h:454: const std::string& ocsp_response); On 2016/07/19 00:02:49, Ryan Sleevi (extremely slow) wrote: > Documentation is needed here. Just from the header, it's unclear why > ocsp_response is needed. > > From a design standpoint, the naming here leaves much to be desired - I thought > at first this processed ExpectStaple headers, but no, it performs the logic of > ExpectStaple processing. > > From a documentation standpoint, it's unclear how one would use this. For > example, do you call this for all hosts when you get a stapled response? Do you > only call it if the host is in the expect-staple list? Do you only call it when > the stapled response is bad? > > I would try, as much as possible, to examine the other methods of TSS and try to > make it consistent with them. I modeled this after ProcessExpectCTHeader. It follows almost identical structure, modulo one less reporting abstract class. It doesn't take a header because ExpectStaple is preload-only. The plan is to call this from ssl_client_socket_impl.cc in DoVerifyCertComplete(). That being said, I totally forgot to put any documentation on this. :) https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state_unittest.cc (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:243: bool ResponseStatusFromString(std::string in, On 2016/07/19 00:02:49, Ryan Sleevi (extremely slow) wrote: > From a testing design standpoint, it doesn't seem you need this. If you're > wanting to test the serialized reports, shouldn't you do the string comparison? > > You only call this function once, does it really need to be a helper? I did it this way since the reverse function is in a private namespace in a .cc file. That, and testing it against itself doesn't actually test we set the right value---this way I'd have to make the same naming error twice to miss a name mismatch. I could move it back into CheckExpectStaple(), but this seemed a little more clear the first time around.
https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state.cc (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.cc:670: } On 2016/07/19 00:18:54, dadrian wrote: > I meant to do it this way---does MSVC 2015 catch this? Right, I *think* all our compilers now handle knowing that this is unreachable (unless a new value is added, in which case, we *want* it to be a compile failure) https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state_unittest.cc (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:243: bool ResponseStatusFromString(std::string in, On 2016/07/19 00:18:54, dadrian wrote: > On 2016/07/19 00:02:49, Ryan Sleevi (extremely slow) wrote: > > From a testing design standpoint, it doesn't seem you need this. If you're > > wanting to test the serialized reports, shouldn't you do the string > comparison? > > > > You only call this function once, does it really need to be a helper? > > I did it this way since the reverse function is in a private namespace in a .cc > file. That, and testing it against itself doesn't actually test we set the right > value---this way I'd have to make the same naming error twice to miss a name > mismatch. > > I could move it back into CheckExpectStaple(), but this seemed a little more > clear the first time around. Unfortunately, I'm having a lot of trouble understanding your reply, so I'll try to phrase it differently to see if perhaps I didn't communicate it well. I was specifically trying to suggest that, rather than lines 302 - 305, which parse the string value into a numeric value and compare the numeric value, what you should be testing is that the string value is what is expected. And that means supplying the string value you expect. That is, instead of 302-305, you have EXPECT_EQ(some_variable_you_pass_as_a_string, report_response_status) This function seems superfluous - you're trying to test that the report is properly generated, but what you're testing here is that the report was properly generated and your utility function doesn't have issues. And that seems to be unnecessary to test. Naturally, this means threading the string parameter through both CheckExpectStapleReport and CheckExpectStaple, which then *also* means that lines 2063-2070 need to supply the numeric value to use to construct the report, and the expected string value of the constructed report. The reason I mention all of this is that it seems you should be more explicit that you're expecting a specific string value in the test; this mapping feels like it duplicates code under test (as you note, it's a counterpart to a .cc), and isn't "really" what you're wanting to test (you're wanting to test the string is a specific value, but you're hiding it via an enum for convenience)
https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state.cc (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.cc:640: std::string ResponseStatusToString(OCSPVerifyResult::ResponseStatus status) { On 2016/07/19 00:02:49, Ryan Sleevi (extremely slow) wrote: > Documentation needed :) > > For example, are these strings part of an API contract? For debugging purposes? > Something else? Done. https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.cc:659: return ""; On 2016/07/19 00:02:49, Ryan Sleevi (extremely slow) wrote: > return std::string() Done. https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state.cc:670: } On 2016/07/19 01:17:22, Ryan Sleevi (extremely slow) wrote: > On 2016/07/19 00:18:54, dadrian wrote: > > I meant to do it this way---does MSVC 2015 catch this? > > Right, I *think* all our compilers now handle knowing that this is unreachable > (unless a new value is added, in which case, we *want* it to be a compile > failure) Done. https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... File net/http/transport_security_state_unittest.cc (right): https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:243: bool ResponseStatusFromString(std::string in, On 2016/07/19 01:17:22, Ryan Sleevi (extremely slow) wrote: > On 2016/07/19 00:18:54, dadrian wrote: > > On 2016/07/19 00:02:49, Ryan Sleevi (extremely slow) wrote: > > > From a testing design standpoint, it doesn't seem you need this. If you're > > > wanting to test the serialized reports, shouldn't you do the string > > comparison? > > > > > > You only call this function once, does it really need to be a helper? > > > > I did it this way since the reverse function is in a private namespace in a > .cc > > file. That, and testing it against itself doesn't actually test we set the > right > > value---this way I'd have to make the same naming error twice to miss a name > > mismatch. > > > > I could move it back into CheckExpectStaple(), but this seemed a little more > > clear the first time around. > > Unfortunately, I'm having a lot of trouble understanding your reply, so I'll try > to phrase it differently to see if perhaps I didn't communicate it well. > > I was specifically trying to suggest that, rather than lines 302 - 305, which > parse the string value into a numeric value and compare the numeric value, what > you should be testing is that the string value is what is expected. And that > means supplying the string value you expect. > > That is, instead of 302-305, you have > EXPECT_EQ(some_variable_you_pass_as_a_string, report_response_status) > > This function seems superfluous - you're trying to test that the report is > properly generated, but what you're testing here is that the report was properly > generated and your utility function doesn't have issues. And that seems to be > unnecessary to test. > > Naturally, this means threading the string parameter through both > CheckExpectStapleReport and CheckExpectStaple, which then *also* means that > lines 2063-2070 need to supply the numeric value to use to construct the report, > and the expected string value of the constructed report. > > The reason I mention all of this is that it seems you should be more explicit > that you're expecting a specific string value in the test; this mapping feels > like it duplicates code under test (as you note, it's a counterpart to a .cc), > and isn't "really" what you're wanting to test (you're wanting to test the > string is a specific value, but you're hiding it via an enum for convenience) I reworked the tests to be table-driven with TEST_P. I was trying to avoid that the first time around, primarily because I hate creating TEST_P scaffolding, but I think it works well now. https://codereview.chromium.org/2144693004/diff/20001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:267: bool RevocationStatusFromString(std::string in, OCSPRevocationStatus* status) { On 2016/07/19 00:02:49, Ryan Sleevi (extremely slow) wrote: > Same remarks Done.
https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... File net/http/transport_security_state.cc (right): https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state.cc:1299: if (!GetStaticExpectStapleState(host_port_pair.host(), &expect_staple_state)) In terms of matching conditions to documentation, it's not immediately obvious that this performs the build timeliness check and returns false. I had to check the code to see if it was true, so it might be worth documenting. // Determine if the host is on the Expect-Staple preload list. // Note: If the build is not timely/the preload list is not fresh, this will fail and return false. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... File net/http/transport_security_state.h (right): https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state.h:455: // report-uri, and the build is timely (i.e. preload list is fresh). This is really two conditions (same with #3) :) 1. Sending expect staple reports is enabled (via |enable_static_expect_staple_|) 2. A report sender is provided via SetReportSender() 3. The build is timely (i.e. the preload list is fresh). 4. The given host is present on the Expect-Staple preload list. 5. |ssl_info| indicates the connection did not provide an OCSP response indicating a revocation status of GOOD. "with a valid report-uri" seems redundant/unnecessary, since it's a condition of being on the Expect-Staple preload list that there be a valid report-uri. (Note that I reordered this so that the comment reads similar to the code, to make it easily to naturally parse) https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state.h:460: void ProcessExpectStaple(const HostPortPair& host_port_pair, Naming wise, I think this should be CheckExpectStaple (since it's preloaded), or possibly MaybeSendExpectStapleReport, and moved to either 316 or 305 - it doesn't really fit with the header processing logic, and makes it clear it's an active action. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... File net/http/transport_security_state_unittest.cc (right): https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:321: HostPortPair host_port(kExpectStapleStaticHostname, 443); Where does this come from and why? The test lacks documentation, so it was unclear. It sounds like this is baked directly into the test data? https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2010: TEST_P(ExpectStapleErrorResponseTest, CheckResponseStatusSerialization) { Improve high-level docs for the test https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2040: test.response_status_string, std::string())); These tests (2081 - 2089) could benefit from improved documentation - it's unclear why you're testing both or what you're testing for or why. Are you testing an implementation detail? Is it guaranteed by API contract that we send regardless? Why :) (Same applies to other tests) https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2059: TEST_P(ExpectStapleErrorCertStatusTest, CheckCertStatusSerialization) { Improve high-level docs for the test https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2131: HostPortPair host_port("not-preloaded.badssl.com", 443); Similar to my remarks above (about it being preloaded), this is unclear why it's guaranteed it will *not* be preloaded. It would make more sense to use an IANA-reserved hostname, rather than assuming badssl won't ever set expect-staple with include-subdomains That is, not-preloaded.host.example (for example) https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2141: ssl_info.is_issued_by_known_root = true; By contrast with your other tests (and part of why I recommended more documentation), it's unclear why it's OK not to fiddle with is_issued_by_known_root in this test (and in ExpectStapleDoesNotReportValidStaple) If we accept the possible argument that you fiddle with report sending for the first two since "reports might be disabled based on the value of is_issued_by_known_root" (I suspect the probable answer), then wouldn't the response here be to test both values in both subsequent tests as well?
https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... File net/http/transport_security_state.cc (right): https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state.cc:1299: if (!GetStaticExpectStapleState(host_port_pair.host(), &expect_staple_state)) On 2016/07/19 19:11:04, Ryan Sleevi (extremely slow) wrote: > In terms of matching conditions to documentation, it's not immediately obvious > that this performs the build timeliness check and returns false. I had to check > the code to see if it was true, so it might be worth documenting. > > // Determine if the host is on the Expect-Staple preload list. > // Note: If the build is not timely/the preload list is not fresh, this will > fail and return false. Done. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... File net/http/transport_security_state.h (right): https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state.h:455: // report-uri, and the build is timely (i.e. preload list is fresh). On 2016/07/19 19:11:04, Ryan Sleevi (extremely slow) wrote: > This is really two conditions (same with #3) :) > > 1. Sending expect staple reports is enabled (via |enable_static_expect_staple_|) > 2. A report sender is provided via SetReportSender() > 3. The build is timely (i.e. the preload list is fresh). > 4. The given host is present on the Expect-Staple preload list. > 5. |ssl_info| indicates the connection did not provide an OCSP response > indicating a revocation status of GOOD. > > "with a valid report-uri" seems redundant/unnecessary, since it's a condition of > being on the Expect-Staple preload list that there be a valid report-uri. > > (Note that I reordered this so that the comment reads similar to the code, to > make it easily to naturally parse) Done. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state.h:455: // report-uri, and the build is timely (i.e. preload list is fresh). On 2016/07/19 19:11:04, Ryan Sleevi (extremely slow) wrote: > This is really two conditions (same with #3) :) > > 1. Sending expect staple reports is enabled (via |enable_static_expect_staple_|) > 2. A report sender is provided via SetReportSender() > 3. The build is timely (i.e. the preload list is fresh). > 4. The given host is present on the Expect-Staple preload list. > 5. |ssl_info| indicates the connection did not provide an OCSP response > indicating a revocation status of GOOD. > > "with a valid report-uri" seems redundant/unnecessary, since it's a condition of > being on the Expect-Staple preload list that there be a valid report-uri. > > (Note that I reordered this so that the comment reads similar to the code, to > make it easily to naturally parse) Done. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state.h:460: void ProcessExpectStaple(const HostPortPair& host_port_pair, On 2016/07/19 19:11:04, Ryan Sleevi (extremely slow) wrote: > Naming wise, I think this should be CheckExpectStaple (since it's preloaded), or > possibly MaybeSendExpectStapleReport, and moved to either 316 or 305 - it > doesn't really fit with the header processing logic, and makes it clear it's an > active action. Done. This is just a ploy to cause name collisions in my test file, isn't it :P https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... File net/http/transport_security_state_unittest.cc (right): https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:321: HostPortPair host_port(kExpectStapleStaticHostname, 443); On 2016/07/19 19:11:05, Ryan Sleevi (extremely slow) wrote: > Where does this come from and why? The test lacks documentation, so it was > unclear. It sounds like this is baked directly into the test data? Yup, it's baked into the preload list itself as a test domain. I added a comment explaining the choice of domains. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2010: TEST_P(ExpectStapleErrorResponseTest, CheckResponseStatusSerialization) { On 2016/07/19 19:11:05, Ryan Sleevi (extremely slow) wrote: > Improve high-level docs for the test Done. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2040: test.response_status_string, std::string())); On 2016/07/19 19:11:05, Ryan Sleevi (extremely slow) wrote: > These tests (2081 - 2089) could benefit from improved documentation - it's > unclear why you're testing both or what you're testing for or why. > > Are you testing an implementation detail? Is it guaranteed by API contract that > we send regardless? Why :) > > (Same applies to other tests) Done. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2059: TEST_P(ExpectStapleErrorCertStatusTest, CheckCertStatusSerialization) { On 2016/07/19 19:11:04, Ryan Sleevi (extremely slow) wrote: > Improve high-level docs for the test Done. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2131: HostPortPair host_port("not-preloaded.badssl.com", 443); On 2016/07/19 19:11:05, Ryan Sleevi (extremely slow) wrote: > Similar to my remarks above (about it being preloaded), this is unclear why it's > guaranteed it will *not* be preloaded. > > It would make more sense to use an IANA-reserved hostname, rather than assuming > badssl won't ever set expect-staple with include-subdomains > > That is, not-preloaded.host.example (for example) Done. https://codereview.chromium.org/2144693004/diff/40001/net/http/transport_secu... net/http/transport_security_state_unittest.cc:2141: ssl_info.is_issued_by_known_root = true; On 2016/07/19 19:11:05, Ryan Sleevi (extremely slow) wrote: > By contrast with your other tests (and part of why I recommended more > documentation), it's unclear why it's OK not to fiddle with > is_issued_by_known_root in this test (and in > ExpectStapleDoesNotReportValidStaple) > > If we accept the possible argument that you fiddle with report sending for the > first two since "reports might be disabled based on the value of > is_issued_by_known_root" (I suspect the probable answer), then wouldn't the > response here be to test both values in both subsequent tests as well? Done.
lgtm https://codereview.chromium.org/2144693004/diff/60001/net/http/transport_secu... File net/http/transport_security_state.h (right): https://codereview.chromium.org/2144693004/diff/60001/net/http/transport_secu... net/http/transport_security_state.h:474: void CheckExpectStapleAndMaybeSendReport(const HostPortPair& host_port_pair, Leftover? :)
The CQ bit was checked by dadrian@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from rsleevi@chromium.org Link to the patchset: https://codereview.chromium.org/2144693004/#ps80001 (title: "Remove stragglers.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
The CQ bit was checked by dadrian@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from rsleevi@chromium.org Link to the patchset: https://codereview.chromium.org/2144693004/#ps100001 (title: "Make switch statements Android/ChromeOS friendly")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dadrian@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dadrian@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Add the ability to send Expect-Staple reports. This adds a |ProcessExpectStaple| method to TransportSecurityState that implements reporting for Expect-Staple, using contents of SSLInfo and OCSPVerifyResult. No report is sent if a valid staple is provided, or if the remote host is not on the Expect-Staple preload list. This method is not currently called by any socket implementation. BUG=598021 ========== to ========== Add the ability to send Expect-Staple reports. This adds a |ProcessExpectStaple| method to TransportSecurityState that implements reporting for Expect-Staple, using contents of SSLInfo and OCSPVerifyResult. No report is sent if a valid staple is provided, or if the remote host is not on the Expect-Staple preload list. This method is not currently called by any socket implementation. BUG=598021 Committed: https://crrev.com/c87b456d823a4bd61599c5ce4512848a6f2be7b6 Cr-Commit-Position: refs/heads/master@{#406742} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/c87b456d823a4bd61599c5ce4512848a6f2be7b6 Cr-Commit-Position: refs/heads/master@{#406742} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
