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

Issue 2865883002: Unit-test CAPI and CNG SSLPrivateKey implementations. (Closed)

Created:
3 years, 7 months ago by davidben
Modified:
3 years, 7 months ago
Reviewers:
Ryan Sleevi
CC:
chromium-reviews, cbentzel+watch_chromium.org, net-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Unit-test CAPI and CNG SSLPrivateKey implementations. This does not cover the CryptAcquireCertificatePrivateKey call (from my manual testing, it didn't appear that the imported keys showed up via CryptAcquireCertificatePrivateKey at all), but it tests that we are calling CAPI and CNG correctly for the algorithms we are trying to implement. This will be useful when we revise the interface for RSA-PSS and add support for it. Also, it means we actually have some test for this stuff! BUG=673058 Review-Url: https://codereview.chromium.org/2865883002 Cr-Commit-Position: refs/heads/master@{#470612} Committed: https://chromium.googlesource.com/chromium/src/+/d6d1e4d17563f475169ea8564be5e581759eb8fd

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : reorder headers #

Patch Set 4 : . #

Patch Set 5 : . #

Patch Set 6 : . #

Patch Set 7 : . #

Patch Set 8 : . #

Patch Set 9 : . #

Total comments: 9

Patch Set 10 : rsleevi comments #

Patch Set 11 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+387 lines, -17 lines) Patch
M net/BUILD.gn View 1 2 3 4 5 6 7 2 chunks +5 lines, -1 line 0 comments Download
A net/ssl/ssl_platform_key_win.h View 1 2 1 chunk +38 lines, -0 lines 0 comments Download
M net/ssl/ssl_platform_key_win.cc View 1 2 3 4 4 chunks +29 lines, -16 lines 0 comments Download
A net/ssl/ssl_platform_key_win_unittest.cc View 1 2 3 4 5 6 7 8 9 1 chunk +315 lines, -0 lines 0 comments Download

Messages

Total messages: 61 (45 generated)
davidben
Amazingly, this seems to actually work! Ryan, does this seem sound based on your understanding ...
3 years, 7 months ago (2017-05-06 22:25:58 UTC) #28
davidben
https://codereview.chromium.org/2865883002/diff/160001/net/ssl/ssl_platform_key_win_unittest.cc File net/ssl/ssl_platform_key_win_unittest.cc (right): https://codereview.chromium.org/2865883002/diff/160001/net/ssl/ssl_platform_key_win_unittest.cc#newcode183 net/ssl/ssl_platform_key_win_unittest.cc:183: header.dwMagic = BCRYPT_ECDSA_PRIVATE_P521_MAGIC; The MSDN docs call this field ...
3 years, 7 months ago (2017-05-06 22:27:34 UTC) #29
Ryan Sleevi
LGTM https://codereview.chromium.org/2865883002/diff/160001/net/BUILD.gn File net/BUILD.gn (right): https://codereview.chromium.org/2865883002/diff/160001/net/BUILD.gn#newcode5278 net/BUILD.gn:5278: "ncrypt.lib", Why this versus the #pragma? https://codereview.chromium.org/2865883002/diff/160001/net/ssl/ssl_platform_key_win_unittest.cc File ...
3 years, 7 months ago (2017-05-08 21:01:41 UTC) #30
davidben
https://codereview.chromium.org/2865883002/diff/160001/net/BUILD.gn File net/BUILD.gn (right): https://codereview.chromium.org/2865883002/diff/160001/net/BUILD.gn#newcode5278 net/BUILD.gn:5278: "ncrypt.lib", On 2017/05/08 21:01:41, Ryan Sleevi wrote: > Why ...
3 years, 7 months ago (2017-05-08 21:19:07 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2865883002/180001
3 years, 7 months ago (2017-05-08 21:20:28 UTC) #34
commit-bot: I haz the power
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_android_rel_ng/builds/288455)
3 years, 7 months ago (2017-05-08 23:29:03 UTC) #36
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2865883002/180001
3 years, 7 months ago (2017-05-09 16:45:25 UTC) #38
commit-bot: I haz the power
Failed to commit the patch.
3 years, 7 months ago (2017-05-09 17:58:57 UTC) #43
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2865883002/200001
3 years, 7 months ago (2017-05-09 18:48:28 UTC) #46
commit-bot: I haz the power
Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clang_dbg_recipe/builds/264948) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, ...
3 years, 7 months ago (2017-05-09 19:19:56 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2865883002/200001
3 years, 7 months ago (2017-05-09 20:54:23 UTC) #50
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/422172)
3 years, 7 months ago (2017-05-10 01:09:02 UTC) #52
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2865883002/200001
3 years, 7 months ago (2017-05-10 06:34:21 UTC) #54
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/449728)
3 years, 7 months ago (2017-05-10 09:27:32 UTC) #56
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2865883002/200001
3 years, 7 months ago (2017-05-10 14:59:40 UTC) #58
commit-bot: I haz the power
3 years, 7 months ago (2017-05-10 16:49:29 UTC) #61
Message was sent while issue was closed.
Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/d6d1e4d17563f475169ea8564be5...

Powered by Google App Engine
This is Rietveld 408576698