[PATCH stable RFC] url: add --insecure option to bypass verification of ssl certificates
Mads Kiilerich
mads at kiilerich.com
Wed Jan 26 13:14:06 CST 2011
On 01/26/2011 05:47 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara<yuya at tcha.org>
> # Date 1296060076 -32400
> # Branch stable
> # Node ID 2d52545f902469ba76524af647bec3ce2a09f5b1
> # Parent d0e0d3d43e1439d63564ab4dddfe0daa69ae2d86
> url: add --insecure option to bypass verification of ssl certificates
I assume this is a response to
http://www.selenic.com/pipermail/mercurial-devel/2011-January/027367.html
> If --insecure specified, it behaves in the same way as no web.cacerts
> configured, except for the error message.
>
> Also shows hint for --insecure option when verification failed.
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -3943,6 +3943,8 @@ remoteopts = [
> _('specify ssh command to use'), _('CMD')),
> ('', 'remotecmd', '',
> _('specify hg command to run on the remote side'), _('CMD')),
> + ('', 'insecure', None,
> + _('do not verify server certificate')),
should "ignoring web.cacerts configuration" be added?
> ]
>
> walkopts = [
> diff --git a/mercurial/hg.py b/mercurial/hg.py
> --- a/mercurial/hg.py
> +++ b/mercurial/hg.py
> @@ -545,6 +545,10 @@ def remoteui(src, opts):
> if r:
> dst.setconfig('bundle', 'mainreporoot', r)
>
> + # copy ssl-specific options
> + if opts.get('insecure'):
> + dst.setconfig('web', 'insecure', 'True')
Hmm. This seems like a hackish way to transfer command line options down
to url. It extends the existing ssh hack a bit. Wouldn't it be cleaner
to do it in dispatch._dispatch together with the real global options?
Note that _if_ we want to support it as a config setting then we would
have to copy the value from src too in order to support subrepos.
I think the config name should be internal and more obscure than
web.insecure. If not then it should be documented.
But: Wouldn't it be simpler to implement and explain the functionality
if it just cleared web.cacerts?
> # copy selected local settings to the remote ui
> for sect in ('auth', 'http_proxy'):
> for key, val in src.configitems(sect):
> diff --git a/mercurial/url.py b/mercurial/url.py
> --- a/mercurial/url.py
> +++ b/mercurial/url.py
> @@ -530,24 +530,31 @@ if has_https:
> cacerts = self.ui.config('web', 'cacerts')
> if cacerts:
> cacerts = util.expandpath(cacerts)
> + insecure = self.ui.configbool('web', 'insecure')
> else:
> cacerts = None
> + insecure = False
>
> - if cacerts:
> + if cacerts and not insecure:
> sock = _create_connection((self.host, self.port))
> self.sock = _ssl_wrap_socket(sock, self.key_file,
> self.cert_file, cert_reqs=CERT_REQUIRED,
> ca_certs=cacerts)
> msg = _verifycert(self.sock.getpeercert(), self.host)
> if msg:
> - raise util.Abort(_('%s certificate error: %s') %
> - (self.host, msg))
> + raise util.Abort(_('%s certificate error: %s '
> + '(use --insecure to connect '
> + 'insecurely)') % (self.host, msg))
> self.ui.debug('%s certificate successfully verified\n' %
> self.host)
> else:
> - self.ui.warn(_("warning: %s certificate not verified "
> - "(check web.cacerts config setting)\n") %
> - self.host)
> + if insecure:
> + self.ui.warn(_("warning: %s certificate not verified\n") %
> + self.host)
We could be a bit more specific and say something like
"certificate verification disabled" - and perhaps also "with
--insecure". (But I don't think that is important enough to justify
doing anything more complex than clearing cacerts.)
I guess some users would like to see an option for insecure without
warning. I think it is fine to say no and not have that option.
> + else:
> + self.ui.warn(_("warning: %s certificate not verified "
> + "(check web.cacerts config setting)\n") %
Should --insecure be mentioned here too? Or would that be too verbose?
> + self.host)
> httplib.HTTPSConnection.connect(self)
>
> class httpsconnection(BetterHTTPS):
> diff --git a/tests/test-debugcomplete.t b/tests/test-debugcomplete.t
> --- a/tests/test-debugcomplete.t
> +++ b/tests/test-debugcomplete.t
> @@ -179,16 +179,16 @@ Show all commands + options
> $ hg debugcommands
> add: include, exclude, subrepos, dry-run
> annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, include, exclude
> - clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh, remotecmd
> + clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh, remotecmd, insecure
> commit: addremove, close-branch, include, exclude, message, logfile, date, user
> diff: rev, change, text, git, nodates, show-function, reverse, ignore-all-space, ignore-space-change, ignore-blank-lines, unified, stat, include, exclude, subrepos
> export: output, switch-parent, rev, text, git, nodates
> forget: include, exclude
> - init: ssh, remotecmd
> + init: ssh, remotecmd, insecure
> log: follow, follow-first, date, copies, keyword, rev, removed, only-merges, user, only-branch, branch, prune, patch, git, limit, no-merges, stat, style, template, include, exclude
> merge: force, tool, rev, preview
> - pull: update, force, rev, branch, ssh, remotecmd
> - push: force, rev, branch, new-branch, ssh, remotecmd
> + pull: update, force, rev, branch, ssh, remotecmd, insecure
> + push: force, rev, branch, new-branch, ssh, remotecmd, insecure
> remove: after, force, include, exclude
> serve: accesslog, daemon, daemon-pipefds, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, templates, style, ipv6, certificate
> status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, copies, print0, rev, change, include, exclude, subrepos
> @@ -200,7 +200,7 @@ Show all commands + options
> bisect: reset, good, bad, skip, command, noupdate
> branch: force, clean
> branches: active, closed
> - bundle: force, rev, branch, base, all, type, ssh, remotecmd
> + bundle: force, rev, branch, base, all, type, ssh, remotecmd, insecure
> cat: output, rev, decode, include, exclude
> copy: after, force, include, exclude, dry-run
> debugancestor:
> @@ -228,10 +228,10 @@ Show all commands + options
> help:
> identify: rev, num, id, branch, tags
> import: strip, base, force, no-commit, exact, import-branch, message, logfile, date, user, similarity
> - incoming: force, newest-first, bundle, rev, branch, patch, git, limit, no-merges, stat, style, template, ssh, remotecmd, subrepos
> + incoming: force, newest-first, bundle, rev, branch, patch, git, limit, no-merges, stat, style, template, ssh, remotecmd, insecure, subrepos
> locate: rev, print0, fullpath, include, exclude
> manifest: rev
> - outgoing: force, rev, newest-first, branch, patch, git, limit, no-merges, stat, style, template, ssh, remotecmd, subrepos
> + outgoing: force, rev, newest-first, branch, patch, git, limit, no-merges, stat, style, template, ssh, remotecmd, insecure, subrepos
> parents: rev, style, template
> paths:
> recover:
> diff --git a/tests/test-https.t b/tests/test-https.t
> --- a/tests/test-https.t
> +++ b/tests/test-https.t
> @@ -167,7 +167,7 @@ variables in the filename
> cacert mismatch
>
> $ hg -R copy-pull pull --config web.cacerts=pub.pem https://127.0.0.1:$HGPORT/
> - abort: 127.0.0.1 certificate error: certificate is for localhost
> + abort: 127.0.0.1 certificate error: certificate is for localhost (use --insecure to connect insecurely)
> [255]
> $ hg -R copy-pull pull --config web.cacerts=pub-other.pem
> abort: error: *:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed (glob)
> @@ -188,3 +188,26 @@ Test server cert which no longer is vali
> $ hg -R copy-pull pull --config web.cacerts=pub-expired.pem https://localhost:$HGPORT2/
> abort: error: *:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed (glob)
If we want to give hints about --insecure then it would be very very
nice to get in this case too.
> [255]
> +
> +cacert match but pull insecurely
The logic in the tests could perhaps be easier to follow if these tests
came immediately after the corresponding secure test has failed.
> +
> + $ P=`pwd` hg -R copy-pull pull --insecure
What is this P?
> + warning: localhost certificate not verified
> + pulling from https://localhost:$HGPORT/
> + searching for changes
> + no changes found
> +
> +cacert mismatch but pull insecurely
> +
> + $ hg -R copy-pull pull --config web.cacerts=pub.pem --insecure \
> +> https://127.0.0.1:$HGPORT/
> + warning: 127.0.0.1 certificate not verified
> + pulling from https://127.0.0.1:$HGPORT/
> + searching for changes
> + no changes found
> + $ hg -R copy-pull pull --config web.cacerts=pub-other.pem --insecure
> + warning: localhost certificate not verified
> + pulling from https://localhost:$HGPORT/
> + searching for changes
> + no changes found
> +
Finally:
The --insecure option should perhaps also be mentioned in hgrc.5.txt
when web.cacerts is described.
/Mads
More information about the Mercurial-devel
mailing list