Open Stack

Fix proposed to branch: master
Review: https://review.openstack.org/158480

** Changed in: glance
       Status: Triaged => In Progress

-- 
You received this bug notification because you are a member of OpenStack
Security Group, which is subscribed to OpenStack.
https://bugs.launchpad.net/bugs/1414532

Title:
  asserts used in cache.py

Status in OpenStack Image Registry and Delivery Service (Glance):
  In Progress
Status in OpenStack Security Advisories:
  Won't Fix

Bug description:
  The asserts in the snippet below check at #2 to see if the HTTP method
  match the HTTP methods actually specified in the patterns at #1.

  /opt/stack/glance/glance/api/middleware/cache.py

  PATTERNS = {   <--- #1
      ('v1', 'GET'): re.compile(r'^/v1/images/([^\/]+)$'),
      ('v1', 'DELETE'): re.compile(r'^/v1/images/([^\/]+)$'),
      ('v2', 'GET'): re.compile(r'^/v2/images/([^\/]+)/file$'),
      ('v2', 'DELETE'): re.compile(r'^/v2/images/([^\/]+)$')
  }

  ...

      @staticmethod
      def _match_request(request):
          """Determine the version of the url and extract the image id

          :returns tuple of version and image id if the url is a
          cacheable,
                   otherwise None
          """
          for ((version, method), pattern) in PATTERNS.items():
              match = pattern.match(request.path_info)
              try:
                  assert request.method == method  <--- #2
                  image_id = match.group(1)
                  # Ensure the image id we got looks like an image id to
                  filter
                  # out a URI like /images/detail. See LP Bug #879136
                  assert image_id != 'detail'
              except (AttributeError, AssertionError):
                  continue
              else:
                  return (version, method, image_id)

  As stated in the Python documentation assert statements will not be evaluated
  when the Python code is compiled with optimization flags. This means that these
  checks will not be properly executed and one can in that case call a specific
  method with a completely different HTTP verb. This can result in security
  issues.

  For example think of having some filtering in place in front of the glance API
  to maybe allow only certain API queries to come from certain IP addresses. For
  example: 'the HTTP verb DELETE may only be executed from this IP range'.  An
  attacker can now specify a completely different HTTP verb such as GET and make
  sure he still matches regular expressions at #1 and then bypass the firewall.

  It's a bit of a hypothetical scenario but in general one should never ever do
  error checking with assert statemetns. This should only be done for things
  which can never realistically fail and that is simply not an assumption one can
  hold when it comes to untrusted input from the network.

  For more information see
  https://docs.python.org/2/reference/simple_stmts.html#the-assert-statement and
  https://docs.python.org/2/using/cmdline.html#envvar-PYTHONOPTIMIZE

  
  This seems to be related to https://bugs.launchpad.net/cinder/+bug/1199354  but it's not fixed and maybe it should even be a security issue hence why I reported it again and tagged as a security vulnerability. I am not familiar enough with the code base to make that call.

To manage notifications about this bug go to:
https://bugs.launchpad.net/glance/+bug/1414532/+subscriptions