D2717: wireprotoserver: check permissions in main dispatch function
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Thu Mar 8 00:24:54 UTC 2018
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
The permissions checking code merged from stable is out of place
in the refactored hgweb_mod module.
This commit moves the main call to wireprotoserver. We still have
some lingering code in hgweb_mod. This will get addressed later.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2717
AFFECTED FILES
mercurial/hgweb/hgweb_mod.py
mercurial/wireprotoserver.py
CHANGE DETAILS
diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -179,7 +179,8 @@
return {
'cmd': cmd,
'proto': proto,
- 'dispatch': lambda: _callhttp(repo, req, proto, cmd),
+ 'dispatch': lambda checkperm: _callhttp(repo, req, proto, cmd,
+ checkperm),
'handleerror': lambda ex: _handlehttperror(ex, req, cmd),
}
@@ -223,7 +224,7 @@
opts = {'level': ui.configint('server', 'zliblevel')}
return HGTYPE, util.compengines['zlib'], opts
-def _callhttp(repo, req, proto, cmd):
+def _callhttp(repo, req, proto, cmd, checkperm):
def genversion2(gen, engine, engineopts):
# application/mercurial-0.2 always sends a payload header
# identifying the compression engine.
@@ -241,6 +242,12 @@
'over HTTP'))
return []
+ # Assume commands with no defined permissions are writes /
+ # for pushes. This is the safest from a security perspective
+ # because it doesn't allow commands with undefined semantics
+ # from bypassing permissions checks.
+ checkperm(wireproto.permissions.get(cmd, 'push'))
+
rsp = wireproto.dispatch(repo, proto, cmd)
if isinstance(rsp, bytes):
diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
--- a/mercurial/hgweb/hgweb_mod.py
+++ b/mercurial/hgweb/hgweb_mod.py
@@ -357,22 +357,15 @@
protohandler = wireprotoserver.parsehttprequest(rctx.repo, req, query)
if protohandler:
- cmd = protohandler['cmd']
try:
if query:
raise ErrorResponse(HTTP_NOT_FOUND)
# TODO fold this into parsehttprequest
- req.checkperm = lambda op: self.check_perm(rctx, req, op)
- protohandler['proto'].checkperm = req.checkperm
+ checkperm = lambda op: self.check_perm(rctx, req, op)
+ protohandler['proto'].checkperm = checkperm
- # Assume commands with no defined permissions are writes /
- # for pushes. This is the safest from a security perspective
- # because it doesn't allow commands with undefined semantics
- # from bypassing permissions checks.
- req.checkperm(perms.get(cmd, 'push'))
-
- return protohandler['dispatch']()
+ return protohandler['dispatch'](checkperm)
except ErrorResponse as inst:
return protohandler['handleerror'](inst)
To: indygreg, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list