Chromium Code Reviews
Help | Chromium Project | Sign in
(362)

Issue 131086: Implement the backend of SSL client authentication for... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 10 months ago by wtc
Modified:
2 years, 11 months ago
Reviewers:
eroman, rvargas
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Implement the backend of SSL client authentication for
Windows.

Create Schannel SSPI CredHandles with certificates for
SSL client authentication.

Remember the client certificates that the user selected
so that we don't ask the user again and again.

R=rvargas,eroman
BUG=http://crbug.com/318
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=18841

Patch Set 1 #

Patch Set 2 : Ready for review #

Total comments: 4

Patch Set 3 : Upload before checkin #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+220 lines, -34 lines) Lint Patch
M net/base/ssl_cert_request_info.h View 1 1 chunk +7 lines, -0 lines 0 comments 0 errors Download
M net/base/ssl_client_socket_win.cc View 1 2 7 chunks +164 lines, -30 lines 4 comments 0 errors Download
M net/http/http_network_transaction.h View 1 chunk +1 line, -1 line 0 comments 0 errors Download
M net/http/http_network_transaction.cc View 1 2 5 chunks +48 lines, -3 lines 1 comment 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 6
wtc
eroman,rvargas: please review the entire CL. rvargas: please focus on the ssl_client_socket_win.cc. eroman: please focus ...
4 years, 10 months ago #1
rvargas
LGTM http://codereview.chromium.org/131086/diff/1003/1004 File net/base/ssl_client_socket_win.cc (right): http://codereview.chromium.org/131086/diff/1003/1004#newcode124 Line 124: // CredHandleMapKey is a std::pair consistig of ...
4 years, 10 months ago #2
eroman
Sorry I didn't get to this earlier, was OOO this morning. http://codereview.chromium.org/131086/diff/2002/3001 File net/base/ssl_client_socket_win.cc (right): ...
4 years, 10 months ago #3
eroman
Also, are there any unit-tests for this?
4 years, 10 months ago #4
wtc
On 2009/06/19 22:00:35, eroman wrote: > Also, are there any unit-tests for this? I will ...
4 years, 10 months ago #5
wtc
4 years, 10 months ago #6
eroman: thanks for the review.  I will make the changes
you suggested except the one below.

http://codereview.chromium.org/131086/diff/2002/3001
File net/base/ssl_client_socket_win.cc (right):

http://codereview.chromium.org/131086/diff/2002/3001#newcode108
Line 108: DCHECK(0 < ssl_version_mask &&
On 2009/06/19 21:42:24, eroman wrote:
> nit: I would suggest writing this as two DCHECKs:
> 
> DCHECK_LT(0, ssl_version_mask);
> DCHECK_LT(ssl_version_mask, arraysize(anonymous_creds_));

I didn't make this change because the second DCHECK_LT
would require a static_cast to make both arguments the
same type (int or signed int).  I don't think the clutter
of the resulting code is worth the trouble.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6