Remove support for 'system' SSL on Mac (SecureTransport) and Win (SChannel)

Remove support for 'system' SSL on Mac (SecureTransport) and Win (SChannel)

Remove support for the command-line fallback --use-system-ssl, as it
adds to the maintenance burden and presents a security risk due to
inconsistent patching by the OS vendors. On all platforms but Android,
NSS is used for SSL in all situations.

This does not change how certificate verification is handled, just
the underlying SSL implementation.

BUG=170214
TBR=sky

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177068

[Ryan Sleevi, 2013-01-15 20:43:23]

wtc: Since I'm not sure the policy on mentioning UMA numbers on bugs, looking at
the UMAs for Chrome.CommandLineUseSystemSSL on both Windows and Mac show this is
not really being used. Further, given the correlation with malware on Windows,
I'm convinced the effective percentage of users actually relying on this are
much lower on Windows.

Let's just eliminate the code duplication here.

[wtc, 2013-01-15 23:26:35]

Patch set 2 LGTM.

Please file a bug report so that this change can also
be found in the issue tracker with a Mstone label. Thanks.

https://codereview.chromium.org/11876044/diff/2001/content/browser/browser_ma...
File content/browser/browser_main_loop.cc (right):

https://codereview.chromium.org/11876044/diff/2001/content/browser/browser_ma...
content/browser/browser_main_loop.cc:282: // Use NSS for SSL by default.

Remove "by default", which means "if --use-system-ssl is not
specified" rather than "if USE_OPENSSL is not defined".

https://codereview.chromium.org/11876044/diff/2001/net/socket/client_socket_f...
File net/socket/client_socket_factory.cc (left):

https://codereview.chromium.org/11876044/diff/2001/net/socket/client_socket_f...
net/socket/client_socket_factory.cc:180:
SSLConfigService::SetDefaultVersionMax(SSL_PROTOCOL_VERSION_TLS1);

Please find out if the SSLConfigService::SetDefaultVersionMax()
method can also be removed. I remember I added the method
for the system SSL client sockets.

I am not sure if we also use the method for the
--ssl-version-max command-line option.

https://codereview.chromium.org/11876044/diff/2001/net/socket/client_socket_f...
File net/socket/client_socket_factory.cc (right):

https://codereview.chromium.org/11876044/diff/2001/net/socket/client_socket_f...
net/socket/client_socket_factory.cc:27: bool g_use_system_ssl = false;

Remove g_use_system_ssl.

[commit-bot: I haz the power, 2013-01-15 23:46:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/11876044/11001

[commit-bot: I haz the power, 2013-01-15 23:46:52]

Presubmit check for 11876044-11001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Messages **
You should prefer to refer to HTTPS URLs, rather than HTTP URLs.
Do not bypass this warning if there is an equivalent HTTPS endpoint
that you could refer to instead. (Contact: palmer@chromium.org)
  content/browser/browser_main_loop.cc:169
    << "http://crosbug.com/15496";

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/browser
    content/public

Presubmit checks took 2.4s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[wtc, 2013-01-15 23:50:21]

https://chromiumcodereview.appspot.com/11876044/diff/11001/content/browser/br...
File content/browser/browser_main_loop.cc (right):

https://chromiumcodereview.appspot.com/11876044/diff/11001/content/browser/br...
content/browser/browser_main_loop.cc:282: // We want to be sure to init NSPR on
the main thread.

It is useful to keep the "Use NSS for SSL" part of the
original comment, so that we know why we need to initialize
NSPR.

[Ryan Sleevi, 2013-01-15 23:57:45]

https://chromiumcodereview.appspot.com/11876044/diff/11001/content/browser/br...
File content/browser/browser_main_loop.cc (right):

https://chromiumcodereview.appspot.com/11876044/diff/11001/content/browser/br...
content/browser/browser_main_loop.cc:282: // We want to be sure to init NSPR on
the main thread.
On 2013/01/15 23:50:22, wtc wrote:
> 
> It is useful to keep the "Use NSS for SSL" part of the
> original comment, so that we know why we need to initialize
> NSPR.

We still need to initialize NSPR for Crypto - independent of the SSL bit. That's
why I removed it.

[Ryan Sleevi, 2013-01-15 23:58:42]

joi: content/ stamp for removing code?

[Jói, 2013-01-16 00:29:59]

LGTM

[wtc, 2013-01-16 00:36:03]

https://chromiumcodereview.appspot.com/11876044/diff/11001/content/browser/br...
File content/browser/browser_main_loop.cc (right):

https://chromiumcodereview.appspot.com/11876044/diff/11001/content/browser/br...
content/browser/browser_main_loop.cc:282: // We want to be sure to init NSPR on
the main thread.

On 2013/01/15 23:57:45, Ryan Sleevi wrote:
>
> We still need to initialize NSPR for Crypto - independent of the SSL bit.
That's
> why I removed it.

Ah, you're right.

[commit-bot: I haz the power, 2013-01-16 00:39:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/11876044/11001

[commit-bot: I haz the power, 2013-01-16 00:39:21]

Presubmit check for 11876044-11001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Messages **
You should prefer to refer to HTTPS URLs, rather than HTTP URLs.
Do not bypass this warning if there is an equivalent HTTPS endpoint
that you could refer to instead. (Contact: palmer@chromium.org)
  content/browser/browser_main_loop.cc:169
    << "http://crosbug.com/15496";

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/browser

Presubmit checks took 1.2s to calculate.

[Ryan Sleevi, 2013-01-16 00:40:39]

sky: TBR

[commit-bot: I haz the power, 2013-01-16 00:40:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/11876044/11001

[commit-bot: I haz the power, 2013-01-16 03:08:58]

Change committed as 177068

[jam, 2013-01-16 18:23:43]

all the win7_aura trybots are failing with linking errors like below. do
you think this could be the cause?

FAILED: E:\b\depot_tools\python_bin\python.exe gyp-win-tool
link-wrapper environment.x86 link.exe /nologo /IMPLIB:content.dll.lib
/DLL /OUT:content.dll /PDB:content.dll.pdb @content.dll.rsp &&
E:\b\depot_tools\python_bin\python.exe gyp-win-tool manifest-wrapper
environment.x86 mt.exe -nologo -manifest
obj\content\content.content.dll.intermediate.manifest
-out:content.dll.manifest
libssl.cmpcert.obj :error LNK2019: unresolved external symbol
_CERT_FindCertByName referenced in function _NSS_CmpCertChainWCANames

libssl.cmpcert.obj :error LNK2019: unresolved external symbol
_DER_Lengths_Util referenced in function _NSS_CmpCertChainWCANames

libssl.authcert.obj :error LNK2019: unresolved external symbol
_CERT_FreeNicknames referenced in function _NSS_GetClientAuthData

libssl.authcert.obj :error LNK2019: unresolved external symbol
_CERT_CheckCertValidTimes referenced in function
_NSS_GetClientAuthData

libssl.authcert.obj :error LNK2019: unresolved external symbol
_CERT_GetCertNicknames referenced in function _NSS_GetClientAuthData

libssl.authcert.obj :error LNK2019: unresolved external symbol
_PK11_FindKeyByAnyCert referenced in function _NSS_GetClientAuthData

libssl.authcert.obj :error LNK2019: unresolved external symbol
_CERT_FindUserCertByUsage referenced in function
_NSS_GetClientAuthData

libjingle.nssidentity.obj :error LNK2019: unresolved external symbol
_SECKEY_CopyPublicKey referenced in function "public: class
talk_base::NSSKeyPair * __thiscall
talk_base::NSSKeyPair::GetReference(void)"
(?GetReference@NSSKeyPair@talk_base@@QAEPAV12@XZ)

content.dll : fatalerror LNK1120: 8 unresolved externals



On Tue, Jan 15, 2013 at 7:08 PM, <commit-bot@chromium.org> wrote:

> Change committed as 177068
>
>
https://chromiumcodereview.**appspot.com/11876044/<https://chromiumcodereview...
>

[Avi (use Gerrit), 2013-01-16 18:26:24]

Farewell SSLClientSocketMac.

[wtc, 2013-01-16 19:46:36]

On 2013/01/16 18:23:43, John Abd-El-Malek wrote:
> all the win7_aura trybots are failing with linking errors like below. do
> you think this could be the cause?

Seems unlikely. The unresolved symbols are exactly the functions
to be exported by ronghuawu's CL:
https://codereview.chromium.org/11884022/

So it is best to ping ronghuawu about this linking error.

[Ryan Sleevi, 2013-01-16 19:47:37]

On Wed, Jan 16, 2013 at 11:46 AM,  <wtc@chromium.org> wrote:
> On 2013/01/16 18:23:43, John Abd-El-Malek wrote:
>>
>> all the win7_aura trybots are failing with linking errors like below. do
>> you think this could be the cause?
>
>
> Seems unlikely. The unresolved symbols are exactly the functions
> to be exported by ronghuawu's CL:
> https://codereview.chromium.org/11884022/
>
> So it is best to ping ronghuawu about this linking error.
>
> https://chromiumcodereview.appspot.com/11876044/

Yes, troopers are on it and clobbering bots. libcrnss was not rebuilt
when exports_win.def changed (likely a GYP issue, since it's in the
'sources' portion), and this caused the link failures.