diff --git a/mercurial/help/internals/wireprotocol.txt b/mercurial/help/internals/wireprotocol.txt --- a/mercurial/help/internals/wireprotocol.txt +++ b/mercurial/help/internals/wireprotocol.txt @@ -203,6 +203,28 @@ Servers receiving requests with an invalid ``Content-Type`` header SHOULD respond with an HTTP 415. +The command to run is specified in the POST payload as defined by the +*Unified Frame-Based Protocol*. This is redundant with data already +encoded in the URL. This is by design, so server operators can have +better understanding about server activity from looking merely at +HTTP access logs. + +In most circumstances, the command specified in the URL MUST match +the command specified in the frame-based payload or the server will +respond with an error. The exception to this is the special +``multirequest`` URL. (See below.) In addition, HTTP requests +are limited to one command invocation. The exception is the special +``multirequest`` URL. + +The ``multirequest`` command endpoints (``ro/multirequest`` and +``rw/multirequest``) are special in that they allow the execution of +*any* command and allow the execution of multiple commands. If the +HTTP request issues multiple commands across multiple frames, all +issued commands will be processed by the server. Per the defined +behavior of the *Unified Frame-Based Protocol*, commands may be +issued interleaved and responses may come back in a different order +than they were issued. Clients MUST be able to deal with this. + SSH Protocol ============ diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py --- a/mercurial/wireprotoserver.py +++ b/mercurial/wireprotoserver.py @@ -327,7 +327,12 @@ _processhttpv2reflectrequest(rctx.repo.ui, rctx.repo, req, res) return - if command not in wireproto.commands: + # Extra commands that we handle that aren't really wire protocol + # commands. Think extra hard before making this hackery available to + # extension. + extracommands = {'multirequest'} + + if command not in wireproto.commands and command not in extracommands: res.status = b'404 Not Found' res.headers[b'Content-Type'] = b'text/plain' res.setbodybytes(_('unknown wire protocol command: %s\n') % command) @@ -338,7 +343,8 @@ proto = httpv2protocolhandler(req, ui) - if not wireproto.commands.commandavailable(command, proto): + if (not wireproto.commands.commandavailable(command, proto) + and command not in extracommands): res.status = b'404 Not Found' res.headers[b'Content-Type'] = b'text/plain' res.setbodybytes(_('invalid wire protocol command: %s') % command) @@ -434,18 +440,14 @@ # Need more data before we can do anything. continue elif action == 'runcommand': - # We currently only support running a single command per - # HTTP request. - if seencommand: - # TODO define proper error mechanism. - res.status = b'200 OK' - res.headers[b'Content-Type'] = b'text/plain' - res.setbodybytes(_('support for multiple commands per request ' - 'not yet implemented')) + sentoutput = _httpv2runcommand(ui, repo, req, res, authedperm, + reqcommand, reactor, meta, + issubsequent=seencommand) + + if sentoutput: return - _httpv2runcommand(ui, repo, req, res, authedperm, reqcommand, - reactor, meta) + seencommand = True elif action == 'error': # TODO define proper error mechanism. @@ -471,7 +473,7 @@ % action) def _httpv2runcommand(ui, repo, req, res, authedperm, reqcommand, reactor, - command): + command, issubsequent): """Dispatch a wire protocol command made from HTTPv2 requests. The authenticated permission (``authedperm``) along with the original @@ -484,34 +486,57 @@ # run doesn't have permissions requirements greater than what was granted # by ``authedperm``. # - # For now, this is no big deal, as we only allow a single command per - # request and that command must match the command in the URL. But when - # things change, we need to watch out... - if reqcommand != command['command']: - # TODO define proper error mechanism - res.status = b'200 OK' - res.headers[b'Content-Type'] = b'text/plain' - res.setbodybytes(_('command in frame must match command in URL')) - return - - # TODO once we get rid of the command==URL restriction, we'll need to - # revalidate command validity and auth here. checkperm, - # wireproto.commands.commandavailable(), etc. + # Our rule for this is we only allow one command per HTTP request and + # that command must match the command in the URL. However, we make + # an exception for the ``multirequest`` URL. This URL is allowed to + # execute multiple commands. We double check permissions of each command + # as it is invoked to ensure there is no privilege escalation. + # TODO consider allowing multiple commands to regular command URLs + # iff each command is the same. proto = httpv2protocolhandler(req, ui, args=command['args']) - assert wireproto.commands.commandavailable(command['command'], proto) - wirecommand = wireproto.commands[command['command']] - assert authedperm in (b'ro', b'rw') - assert wirecommand.permission in ('push', 'pull') + if reqcommand == b'multirequest': + if not wireproto.commands.commandavailable(command['command'], proto): + # TODO proper error mechanism + res.status = b'200 OK' + res.headers[b'Content-Type'] = b'text/plain' + res.setbodybytes(_('wire protocol command not available: %s') % + command['command']) + return True + + assert authedperm in (b'ro', b'rw') + wirecommand = wireproto.commands[command['command']] + assert wirecommand.permission in ('push', 'pull') - # We already checked this as part of the URL==command check, but - # permissions are important, so do it again. - if authedperm == b'ro': - assert wirecommand.permission == 'pull' - elif authedperm == b'rw': - # We are allowed to access read-only commands under the rw URL. - assert wirecommand.permission in ('push', 'pull') + if authedperm == b'ro' and wirecommand.permission != 'pull': + # TODO proper error mechanism + res.status = b'403 Forbidden' + res.headers[b'Content-Type'] = b'text/plain' + res.setbodybytes(_('insufficient permissions to execute ' + 'command: %s') % command['command']) + return True + + # TODO should we also call checkperm() here? Maybe not if we're going + # to overhaul that API. The granted scope from the URL check should + # be good enough. + + else: + # Don't allow multiple commands outside of ``multirequest`` URL. + if issubsequent: + # TODO proper error mechanism + res.status = b'200 OK' + res.headers[b'Content-Type'] = b'text/plain' + res.setbodybytes(_('multiple commands cannot be issued to this ' + 'URL')) + return True + + if reqcommand != command['command']: + # TODO define proper error mechanism + res.status = b'200 OK' + res.headers[b'Content-Type'] = b'text/plain' + res.setbodybytes(_('command in frame must match command in URL')) + return True rsp = wireproto.dispatch(repo, proto, command['command']) @@ -527,6 +552,7 @@ if action == 'sendframes': res.setbodygen(meta['framegen']) + return True elif action == 'noop': pass else: diff --git a/tests/test-http-api-httpv2.t b/tests/test-http-api-httpv2.t --- a/tests/test-http-api-httpv2.t +++ b/tests/test-http-api-httpv2.t @@ -412,4 +412,153 @@ s> received: \n s> {"action": "noop"} +Multiple requests to regular command URL are not allowed + + $ send << EOF + > httprequest POST api/$HTTPV2/ro/customreadonly + > accept: $MEDIATYPE + > content-type: $MEDIATYPE + > user-agent: test + > frame 1 command-name eos customreadonly + > frame 3 command-name eos customreadonly + > EOF + using raw connection to peer + s> POST /api/exp-http-v2-0001/ro/customreadonly HTTP/1.1\r\n + s> Accept-Encoding: identity\r\n + s> accept: application/mercurial-exp-framing-0002\r\n + s> content-type: application/mercurial-exp-framing-0002\r\n + s> user-agent: test\r\n + s> content-length: 40\r\n + s> host: $LOCALIP:$HGPORT\r\n (glob) + s> \r\n + s> \x0e\x00\x00\x01\x00\x11customreadonly\x0e\x00\x00\x03\x00\x11customreadonly + s> makefile('rb', None) + s> HTTP/1.1 200 OK\r\n + s> Server: testing stub value\r\n + s> Date: $HTTP_DATE$\r\n + s> Content-Type: text/plain\r\n + s> Content-Length: 46\r\n + s> \r\n + s> multiple commands cannot be issued to this URL + +Multiple requests to "multirequest" URL are allowed + + $ send << EOF + > httprequest POST api/$HTTPV2/ro/multirequest + > accept: $MEDIATYPE + > content-type: $MEDIATYPE + > user-agent: test + > frame 1 command-name eos customreadonly + > frame 3 command-name eos customreadonly + > EOF + using raw connection to peer + s> POST /api/exp-http-v2-0001/ro/multirequest HTTP/1.1\r\n + s> Accept-Encoding: identity\r\n + s> accept: application/mercurial-exp-framing-0002\r\n + s> content-type: application/mercurial-exp-framing-0002\r\n + s> user-agent: test\r\n + s> *\r\n (glob) + s> host: $LOCALIP:$HGPORT\r\n (glob) + s> \r\n + s> \x0e\x00\x00\x01\x00\x11customreadonly\x0e\x00\x00\x03\x00\x11customreadonly + s> makefile('rb', None) + s> HTTP/1.1 200 OK\r\n + s> Server: testing stub value\r\n + s> Date: $HTTP_DATE$\r\n + s> Content-Type: application/mercurial-exp-framing-0002\r\n + s> Transfer-Encoding: chunked\r\n + s> \r\n + s> *\r\n (glob) + s> \x1d\x00\x00\x01\x00Bcustomreadonly bytes response + s> \r\n + s> 23\r\n + s> \x1d\x00\x00\x03\x00Bcustomreadonly bytes response + s> \r\n + s> 0\r\n + s> \r\n + +Interleaved requests to "multirequest" are processed + + $ send << EOF + > httprequest POST api/$HTTPV2/ro/multirequest + > accept: $MEDIATYPE + > content-type: $MEDIATYPE + > user-agent: test + > frame 1 command-name have-args listkeys + > frame 3 command-name have-args listkeys + > frame 3 command-argument eoa \x09\x00\x09\x00namespacebookmarks + > frame 1 command-argument eoa \x09\x00\x0a\x00namespacenamespaces + > EOF + using raw connection to peer + s> POST /api/exp-http-v2-0001/ro/multirequest HTTP/1.1\r\n + s> Accept-Encoding: identity\r\n + s> accept: application/mercurial-exp-framing-0002\r\n + s> content-type: application/mercurial-exp-framing-0002\r\n + s> user-agent: test\r\n + s> content-length: 85\r\n + s> host: $LOCALIP:$HGPORT\r\n (glob) + s> \r\n + s> \x08\x00\x00\x01\x00\x12listkeys\x08\x00\x00\x03\x00\x12listkeys\x16\x00\x00\x03\x00" \x00 \x00namespacebookmarks\x17\x00\x00\x01\x00" \x00\n + s> \x00namespacenamespaces + s> makefile('rb', None) + s> HTTP/1.1 200 OK\r\n + s> Server: testing stub value\r\n + s> Date: $HTTP_DATE$\r\n + s> Content-Type: application/mercurial-exp-framing-0002\r\n + s> Transfer-Encoding: chunked\r\n + s> \r\n + s> 6\r\n + s> \x00\x00\x00\x03\x00B + s> \r\n + s> 24\r\n + s> \x1e\x00\x00\x01\x00Bbookmarks \n + s> namespaces \n + s> phases + s> \r\n + s> 0\r\n + s> \r\n + +Restart server to disable read-write access + + $ killdaemons.py + $ cat > server/.hg/hgrc << EOF + > [experimental] + > web.apiserver = true + > web.api.debugreflect = true + > web.api.http-v2 = true + > [web] + > push_ssl = false + > EOF + + $ hg -R server serve -p $HGPORT -d --pid-file hg.pid -E error.log + $ cat hg.pid > $DAEMON_PIDS + +Attempting to run a read-write command via multirequest on read-only URL is not allowed + + $ send << EOF + > httprequest POST api/$HTTPV2/ro/multirequest + > accept: $MEDIATYPE + > content-type: $MEDIATYPE + > user-agent: test + > frame 1 command-name eos unbundle + > EOF + using raw connection to peer + s> POST /api/exp-http-v2-0001/ro/multirequest HTTP/1.1\r\n + s> Accept-Encoding: identity\r\n + s> accept: application/mercurial-exp-framing-0002\r\n + s> content-type: application/mercurial-exp-framing-0002\r\n + s> user-agent: test\r\n + s> content-length: 14\r\n + s> host: $LOCALIP:$HGPORT\r\n (glob) + s> \r\n + s> \x08\x00\x00\x01\x00\x11unbundle + s> makefile('rb', None) + s> HTTP/1.1 403 Forbidden\r\n + s> Server: testing stub value\r\n + s> Date: $HTTP_DATE$\r\n + s> Content-Type: text/plain\r\n + s> Content-Length: 53\r\n + s> \r\n + s> insufficient permissions to execute command: unbundle + $ cat error.log