Mercurial > public > mercurial-scm > hg
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') |