Issue 4091005: Remove SSL 2.0 support.

Issue 4091005: Remove SSL 2.0 support. (Closed)

Created:
10 years, 1 month ago by wtc

Modified:
9 years, 7 months ago

Reviewers:
agl

CC:
chromium-reviews, cbentzel+watch_chromium.org, ben+cc_chromium.org, Raghu Simha, idana, ncarter (slow), arv (Not doing code reviews), pam+watch_chromium.org, Paweł Hajdan Jr., darin-cc_chromium.org, tim (not reviewing)

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Remove SSL 2.0 support. R=agl BUG=53659 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67722

Patch Set 1 #

Patch Set 2 : gclient sync #

Patch Set 3 : Add ssl_config_service_manager_pref.cc #

Total comments: 9

Patch Set 4 : Sync with ToT #

Created: 10 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+51 lines, -197 lines)			Patch
M	chrome/app/generated_resources.grd	View	1 2 3	1 chunk	+0 lines, -3 lines	0 comments	Download
M	chrome/browser/dom_ui/options/advanced_options_handler.h	View	1 2 3	1 chunk	+0 lines, -4 lines	0 comments	Download
M	chrome/browser/dom_ui/options/advanced_options_handler.cc	View	1 2 3	6 chunks	+0 lines, -20 lines	0 comments	Download
M	chrome/browser/gtk/options/advanced_contents_gtk.cc	View	1 2 3	6 chunks	+0 lines, -30 lines	0 comments	Download
M	chrome/browser/net/ssl_config_service_manager_pref.cc	View	1 2 3	5 chunks	+1 line, -6 lines	0 comments	Download
M	chrome/browser/options_util.cc	View	1 2 3	1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/browser/resources/options/advanced_options.html	View	1 2 3	2 chunks	+0 lines, -9 lines	0 comments	Download
M	chrome/browser/resources/options/advanced_options.js	View	1 2 3	3 chunks	+4 lines, -14 lines	0 comments	Download
M	chrome/browser/ui/views/options/advanced_contents_view.cc	View	1 2 3	6 chunks	+1 line, -18 lines	0 comments	Download
M	chrome/common/pref_names.h	View	1 2 3	1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/common/pref_names.cc	View	1 2 3	1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/test/live_sync/two_client_live_preferences_sync_test.cc	View	1 2 3	4 chunks	+0 lines, -10 lines	0 comments	Download
M	net/base/ssl_config_service.h	View	1 2 3	4 chunks	+4 lines, -5 lines	0 comments	Download
M	net/base/ssl_config_service.cc	View	1 2 3	2 chunks	+1 line, -2 lines	0 comments	Download
M	net/base/ssl_config_service_mac.h	View	1 2 3	1 chunk	+0 lines, -1 line	0 comments	Download
M	net/base/ssl_config_service_mac.cc	View	1 2 3	5 chunks	+1 line, -13 lines	0 comments	Download
M	net/base/ssl_config_service_mac_unittest.cc	View	1 2 3	5 chunks	+6 lines, -20 lines	0 comments	Download
M	net/base/ssl_config_service_win.h	View	1 2 3	1 chunk	+0 lines, -1 line	0 comments	Download
M	net/base/ssl_config_service_win.cc	View	1 2 3	3 chunks	+0 lines, -7 lines	0 comments	Download
M	net/base/ssl_config_service_win_unittest.cc	View	1 2 3	4 chunks	+24 lines, -12 lines	0 comments	Download
M	net/socket/ssl_client_socket_mac.cc	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	net/socket/ssl_client_socket_nss.cc	View	1 2 3	1 chunk	+3 lines, -8 lines	0 comments	Download
M	net/socket/ssl_client_socket_openssl.cc	View	1 2 3	1 chunk	+1 line, -1 line	0 comments	Download
M	net/socket/ssl_client_socket_win.cc	View	1 2 3	3 chunks	+4 lines, -9 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

wtc

I will check this in after the M9 branch is created. How to review this ...

10 years ago (2010-11-29 21:06:48 UTC) #1

agl

LGTM http://codereview.chromium.org/4091005/diff/41001/chrome/browser/resources/options/advanced_options.js File chrome/browser/resources/options/advanced_options.js (right): http://codereview.chromium.org/4091005/diff/41001/chrome/browser/resources/options/advanced_options.js#newcode161 chrome/browser/resources/options/advanced_options.js:161: AdvancedOptions.SetMetricsReportingCheckboxState = function( This change appears to be ...

10 years ago (2010-11-30 01:07:27 UTC) #2

wtc

10 years ago (2010-11-30 02:14:30 UTC) #3

Thanks for the review.

http://codereview.chromium.org/4091005/diff/41001/chrome/browser/resources/op...
File chrome/browser/resources/options/advanced_options.js (right):

http://codereview.chromium.org/4091005/diff/41001/chrome/browser/resources/op...
chrome/browser/resources/options/advanced_options.js:161:
AdvancedOptions.SetMetricsReportingCheckboxState = function(
On 2010/11/30 01:07:27, agl wrote:
> This change appears to be unrelated. Just checking that you intended it.

You're right.  These changes are unrelated, coding style
fixes.  I included them in this CL for convenience.

http://codereview.chromium.org/4091005/diff/41001/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_openssl.cc (right):

http://codereview.chromium.org/4091005/diff/41001/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_openssl.cc:169:
ssl_ctx_.reset(SSL_CTX_new(SSLv23_client_method()));
On 2010/11/30 01:07:27, agl wrote:
> I think that we should add a TODO here as we might want to switch to
> SSLv3_client_method.

Thanks for the suggestion.

I looked into this.  According to the man page at
http://www.openssl.org/docs/ssl/SSL_CTX_new.html,
SSLv3_client_method supports SSLv3 only, and
SSLv23_client_method is the only client method that
supports SSLv3 and TLS1.

http://codereview.chromium.org/4091005/diff/41001/net/socket/ssl_client_socke...
File net/socket/ssl_client_socket_win.cc (right):

http://codereview.chromium.org/4091005/diff/41001/net/socket/ssl_client_socke...
net/socket/ssl_client_socket_win.cc:115: SSL3 = 1 << 0,
I will leave the code as is, because when we add TLS 1.1
and TLS 1.2 support, it'll be four bools instead of just
two bools, and it'll be more compact to encode them in a
bitmask.

Expand Messages | Collapse Messages