[users at bb.net] invalid origin bugfix

Pierre Tardy tardyp at gmail.com
Fri Sep 2 21:10:10 UTC 2016


Indeed, this should work like this with *.
Let us know if it doesn't.

You open up yourself to XSS attacks though. This is why it's not the
default setup.

Le ven. 2 sept. 2016 23:05, Dave Vitek <dvitek at grammatech.com> a écrit :

> Pierre,
>
> Is that to say that even with:
> allowed_origins=['*'],
>
> buildbot can only be used via a single hostname?  This is somewhat less
> than ideal for people accessing buildbot through ssh tunnels and the like.
>
>
> - Dave
>
>
> On 9/2/2016 2:50 PM, Pierre Tardy wrote:
>
> Hi Dave,
>
> Thanks for the report. Please note that we only accept patch through
> github on the master branch.
> You would have realized that this bug has been fixed in master branch and
> has been released last week in version 0.9.0rc2.
>
> The problem you have is a misconfiguration of buildbotURL.
> If you go on the home page of the UI, you will see a message that tells
> you what the buildbotURL should look like.
>
> You are not the first one to have this issue. I think I will make a patch
> which will make the UI completly refuse to work if the URL is not setup
> correctly.
>
> Regards,
> Pierre
>
> Le ven. 2 sept. 2016 à 18:25, Dave Vitek <dvitek at grammatech.com> a écrit :
>
>> Hi all,
>>
>> There is (or at least was in 0.9rc1) a type error in the error handling
>> code for the "invalid origin" error.  If you tried to use a "force build"
>> button from an invalid origin, it would cause this exception:
>>
>>         Traceback (most recent call last):
>>           File
>> "/usr/local/lib/python2.7/dist-packages/buildbot-0.9.0rc1-py2.7.egg/buildbot/www/rest.py",
>> line 431, in render
>>             return self.asyncRenderHelper(request, self.asyncRender,
>> writeError)
>>           File
>> "/usr/local/lib/python2.7/dist-packages/buildbot-0.9.0rc1-py2.7.egg/buildbot/www/resource.py",
>> line 83, in asyncRenderHelper
>>             @d.addErrback
>>           File
>> "/usr/local/lib/python2.7/dist-packages/Twisted-16.2.0-py2.7-linux-x86_64.egg/twisted/internet/defer.py",
>> line 328, in addErrback
>>             errbackKeywords=kw)
>>           File
>> "/usr/local/lib/python2.7/dist-packages/Twisted-16.2.0-py2.7-linux-x86_64.egg/twisted/internet/defer.py",
>> line 306, in addCallbacks
>>             self._runCallbacks()
>>         --- <exception caught here> ---
>>           File
>> "/usr/local/lib/python2.7/dist-packages/Twisted-16.2.0-py2.7-linux-x86_64.egg/twisted/internet/defer.py",
>> line 588, in _runCallbacks
>>             current.result = callback(current.result, *args, **kw)
>>           File
>> "/usr/local/lib/python2.7/dist-packages/buildbot-0.9.0rc1-py2.7.egg/buildbot/www/resource.py",
>> line 87, in failHttpError
>>             writeError(e.message, errcode=e.status)
>>           File
>> "/usr/local/lib/python2.7/dist-packages/buildbot-0.9.0rc1-py2.7.egg/buildbot/www/rest.py",
>> line 427, in writeError
>>             request.setResponseCode(errcode)
>>           File
>> "/usr/local/lib/python2.7/dist-packages/Twisted-16.2.0-py2.7-linux-x86_64.egg/twisted/web/http.py",
>> line 1059, in setResponseCode
>>             raise TypeError("HTTP response code must be int or long")
>>         exceptions.TypeError: HTTP response code must be int or long
>>
>> Below is a patch that fixes the bug.  Basically, the problem was that
>> errcode was the string '400' instead of the number 400.  It would likely be
>> better to prevent errcode from being a string in the first place, if
>> someone more familiar with the code knows how to do this.  However, it
>> looks like the code raising the error does so with a number:
>> from twisted.web.error import Error
>> ...
>> err = "invalid origin"
>> ...
>> raise Error(400, err)
>>
>> So maybe the unexpected int -> str conversion is somewhere inside twistd.
>>
>> After having fixed that bug, I discovered that the UI, at least when
>> using Chrome, completely failed to show any kind of feedback when an
>> "invalid origin" error was correctly sent back to the browser.  The force
>> build dialog just stayed open and did nothing when the button was clicked.
>> In light of this, I added the "if True" below so we would at least be able
>> to look up errors in the log, even if the UI wouldn't show them.  This is
>> more of a workaround for a UI bug.
>>
>> Index: buildbot/buildbot/www/rest.py
>> ===================================================================
>> --- buildbot/buildbot/www/rest.py	(revision 124703)
>> +++ buildbot/buildbot/www/rest.py	(revision 128749)
>> @@ -405,42 +405,53 @@
>>                  request.setHeader("content-length", len(data))
>>              else:
>>                  request.write(data)
>>
>>      def reconfigResource(self, new_config):
>>          # buildbotURL may contain reverse proxy path, Origin header is just
>>          # scheme + host + port
>>          buildbotURL = urlparse(new_config.buildbotURL)
>>          origin_self = buildbotURL.scheme + "://" + buildbotURL.netloc
>>          # pre-translate the origin entries in the config
>>          self.origins = [re.compile(fnmatch.translate(o.lower()))
>>                          for o in new_config.www.get('allowed_origins',
>>                                                      [origin_self])]
>>
>>          # and copy some other flags
>>          self.debug = new_config.www.get('debug')
>>          self.cache_seconds = new_config.www.get('json_cache_seconds', 0)
>>
>>      def render(self, request):
>>          def writeError(msg, errcode=400):
>> -            if self.debug:
>> -                log.msg("HTTP error: %s" % (msg,))
>> +            # dvitek: Made this unconditional because buildbot's UI
>> +            # often does not show the error it gets back in a failed
>> +            # response!  For example, if you click force build and
>> +            # trigger an 'invalid origin' error.
>> +            if True or self.debug:
>> +                log.msg("HTTP error: %s: %s" % (repr(errcode), msg,))
>> +            # dvitek: Work around bug where errcode is a string but
>> +            # twistd wants a number!  The 'invalid origin' case seems
>> +            # to trigger this.
>> +            try:
>> +                errcode = int(errcode)
>> +            except ValueError:
>> +                errcode = 500
>>              request.setResponseCode(errcode)
>>              request.setHeader('content-type', 'text/plain; charset=utf-8')
>>              request.write(json.dumps(dict(error=msg)))
>>              request.finish()
>>          return self.asyncRenderHelper(request, self.asyncRender, writeError)
>>
>>      @defer.inlineCallbacks
>>      def asyncRender(self, request):
>>
>>          # Handle CORS, if necessary.
>>          origins = self.origins
>>          if origins is not None:
>>              isPreflight = False
>>              reqOrigin = request.getHeader('origin')
>>              if reqOrigin:
>>                  err = None
>>                  reqOrigin = reqOrigin.lower()
>>                  if not any(o.match(reqOrigin) for o in self.origins):
>>                      err = "invalid origin"
>>                  elif request.method == 'OPTIONS':
>>
>>
>>
>> _______________________________________________
>> users mailing list
>> users at buildbot.net
>> https://lists.buildbot.net/mailman/listinfo/users
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildbot.net/pipermail/users/attachments/20160902/7b670a9a/attachment.html>


More information about the users mailing list