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