comparison mercurial/sslutil.py @ 33381:3bdbbadddecc

sslutil: check for missing certificate and key files (issue5598) Currently, sslutil._hostsettings() performs validation that web.cacerts exists. However, client certificates are passed in to the function and not all callers may validate them. This includes httpconnection.readauthforuri(), which loads the [auth] section. If a missing file is specified, the ssl module will raise a generic IOException. And, it doesn't even give us the courtesy of telling us which file is missing! Mercurial then prints a generic "abort: No such file or directory" (or similar) error, leaving users to scratch their head as to what file is missing. This commit introduces explicit validation of all paths passed as arguments to wrapsocket() and wrapserversocket(). Any missing file is alerted about explicitly. We should probably catch missing files earlier - as part of loading the [auth] section. However, I think the sslutil functions should check for file presence regardless of what callers do because that's the only way to be sure that missing files are always detected.
author Gregory Szorc <gregory.szorc@gmail.com>
date Mon, 10 Jul 2017 21:09:46 -0700
parents bd872f64a8ba
children 30f2715be123
comparison
equal deleted inserted replaced
33380:892d255ec2a1 33381:3bdbbadddecc
340 server (and client) support SNI, this tells the server which certificate 340 server (and client) support SNI, this tells the server which certificate
341 to use. 341 to use.
342 """ 342 """
343 if not serverhostname: 343 if not serverhostname:
344 raise error.Abort(_('serverhostname argument is required')) 344 raise error.Abort(_('serverhostname argument is required'))
345
346 for f in (keyfile, certfile):
347 if f and not os.path.exists(f):
348 raise error.Abort(_('certificate file (%s) does not exist; '
349 'cannot connect to %s') % (f, serverhostname),
350 hint=_('restore missing file or fix references '
351 'in Mercurial config'))
345 352
346 settings = _hostsettings(ui, serverhostname) 353 settings = _hostsettings(ui, serverhostname)
347 354
348 # We can't use ssl.create_default_context() because it calls 355 # We can't use ssl.create_default_context() because it calls
349 # load_default_certs() unless CA arguments are passed to it. We want to 356 # load_default_certs() unless CA arguments are passed to it. We want to
497 504
498 ``requireclientcert`` specifies whether to require client certificates. 505 ``requireclientcert`` specifies whether to require client certificates.
499 506
500 Typically ``cafile`` is only defined if ``requireclientcert`` is true. 507 Typically ``cafile`` is only defined if ``requireclientcert`` is true.
501 """ 508 """
509 # This function is not used much by core Mercurial, so the error messaging
510 # doesn't have to be as detailed as for wrapsocket().
511 for f in (certfile, keyfile, cafile):
512 if f and not os.path.exists(f):
513 raise error.Abort(_('referenced certificate file (%s) does not '
514 'exist') % f)
515
502 protocol, options, _protocolui = protocolsettings('tls1.0') 516 protocol, options, _protocolui = protocolsettings('tls1.0')
503 517
504 # This config option is intended for use in tests only. It is a giant 518 # This config option is intended for use in tests only. It is a giant
505 # footgun to kill security. Don't define it. 519 # footgun to kill security. Don't define it.
506 exactprotocol = ui.config('devel', 'serverexactprotocol') 520 exactprotocol = ui.config('devel', 'serverexactprotocol')