This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2717

wireprotoserver: check permissions in main dispatch function
ClosedPublic

Authored by indygreg on Mar 7 2018, 7:24 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHGc638a13093cf: wireprotoserver: check permissions in main dispatch function
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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Mar 7 2018, 7:24 PM
durin42 accepted this revision.Mar 9 2018, 1:51 PM
This revision is now accepted and ready to land.Mar 9 2018, 1:51 PM
Closed by commit rHGc638a13093cf: wireprotoserver: check permissions in main dispatch function (authored by indygreg). · Explain WhyMar 9 2018, 2:29 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 6771

mercurial/hgweb/hgweb_mod.py

query = req.env[r'QUERY_STRING'].partition(r'&')[0] query = req.env[r'QUERY_STRING'].partition(r'&')[0]
query = query.partition(r';')[0] query = query.partition(r';')[0]
# Route it to a wire protocol handler if it looks like a wire protocol # Route it to a wire protocol handler if it looks like a wire protocol
# request. # request.
protohandler = wireprotoserver.parsehttprequest(rctx.repo, req, query) protohandler = wireprotoserver.parsehttprequest(rctx.repo, req, query)
if protohandler: if protohandler:
cmd = protohandler['cmd']
try: try:
if query: if query:
raise ErrorResponse(HTTP_NOT_FOUND) raise ErrorResponse(HTTP_NOT_FOUND)
# TODO fold this into parsehttprequest # TODO fold this into parsehttprequest
req.checkperm = lambda op: self.check_perm(rctx, req, op) checkperm = lambda op: self.check_perm(rctx, req, op)
protohandler['proto'].checkperm = req.checkperm protohandler['proto'].checkperm = checkperm
# Assume commands with no defined permissions are writes / return protohandler['dispatch'](checkperm)
# 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']()
except ErrorResponse as inst: except ErrorResponse as inst:
return protohandler['handleerror'](inst) return protohandler['handleerror'](inst)
# translate user-visible url structure to internal structure # translate user-visible url structure to internal structure
args = query.split('/', 2) args = query.split('/', 2)
if 'cmd' not in req.form and args and args[0]: if 'cmd' not in req.form and args and args[0]:
cmd = args.pop(0) cmd = args.pop(0)

mercurial/wireprotoserver.py

if not iscmd(cmd): if not iscmd(cmd):
return None return None
proto = httpv1protocolhandler(req, repo.ui) proto = httpv1protocolhandler(req, repo.ui)
return { return {
'cmd': cmd, 'cmd': cmd,
'proto': proto, '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), 'handleerror': lambda ex: _handlehttperror(ex, req, cmd),
} }
def _httpresponsetype(ui, req, prefer_uncompressed): def _httpresponsetype(ui, req, prefer_uncompressed):
"""Determine the appropriate response type and compression settings. """Determine the appropriate response type and compression settings.
Returns a tuple of (mediatype, compengine, engineopts). Returns a tuple of (mediatype, compengine, engineopts).
""" """
# legacy protocol. # legacy protocol.
# Don't allow untrusted settings because disabling compression or # Don't allow untrusted settings because disabling compression or
# setting a very high compression level could lead to flooding # setting a very high compression level could lead to flooding
# the server's network or CPU. # the server's network or CPU.
opts = {'level': ui.configint('server', 'zliblevel')} opts = {'level': ui.configint('server', 'zliblevel')}
return HGTYPE, util.compengines['zlib'], opts return HGTYPE, util.compengines['zlib'], opts
def _callhttp(repo, req, proto, cmd): def _callhttp(repo, req, proto, cmd, checkperm):
def genversion2(gen, engine, engineopts): def genversion2(gen, engine, engineopts):
# application/mercurial-0.2 always sends a payload header # application/mercurial-0.2 always sends a payload header
# identifying the compression engine. # identifying the compression engine.
name = engine.wireprotosupport().name name = engine.wireprotosupport().name
assert 0 < len(name) < 256 assert 0 < len(name) < 256
yield struct.pack('B', len(name)) yield struct.pack('B', len(name))
yield name yield name
for chunk in gen: for chunk in gen:
yield chunk yield chunk
if not wireproto.commands.commandavailable(cmd, proto): if not wireproto.commands.commandavailable(cmd, proto):
req.respond(HTTP_OK, HGERRTYPE, req.respond(HTTP_OK, HGERRTYPE,
body=_('requested wire protocol command is not available ' body=_('requested wire protocol command is not available '
'over HTTP')) 'over HTTP'))
return [] 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) rsp = wireproto.dispatch(repo, proto, cmd)
if isinstance(rsp, bytes): if isinstance(rsp, bytes):
req.respond(HTTP_OK, HGTYPE, body=rsp) req.respond(HTTP_OK, HGTYPE, body=rsp)
return [] return []
elif isinstance(rsp, wireprototypes.bytesresponse): elif isinstance(rsp, wireprototypes.bytesresponse):
req.respond(HTTP_OK, HGTYPE, body=rsp.data) req.respond(HTTP_OK, HGTYPE, body=rsp.data)
return [] return []