Qpid
  1. Qpid
  2. QPID-2488

ACL - error handling/bounds checking

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.5, 0.6
    • Fix Version/s: 0.7
    • Component/s: C++ Broker
    • Labels:
      None

      Description

      Qpid ACL properties maxqueuesize, maxqueuecount, policytype, ... currently accepts invalid values.
      Only valid ACL rules should be applied, at the moment broker throws an exception at the point when invalid ACL rule is triggered.

      How reproducible:
      Always

      Steps to Reproduce:
      #set ACL rules vith invalid values
      acl allow tester@QPID all queue maxqueuesize=18446744073709551617
      acl allow tester@QPID all queue maxqueuesize=-1
      acl allow tester@QPID all queue policytype=invalid_policy_type

      Actual results:
      ACL rules with invalid rules/values are processed without any error message.

      qpidd.log:
      2009-oct-23 07:11:56 debug ACL Processing 1 allow [tester@QPID] * queue
      maxqueuesize=18446744073709551617
      2009-oct-23 07:11:56 debug ACL: Adding actions

      {consume,publish,create,access,bind,unbind,delete,purge,update}

      to objects

      {queue}

      with props

      { maxqueuesize=18446744073709551617 }

      for users

      {tester@QPID}

      ...

      Expected results:
      ACL rules with invalid property values should not be processed

      1. QPID-2488.test.patch
        3 kB
        Rajith Attapattu
      2. QPID-2488.patch
        9 kB
        Rajith Attapattu

        Activity

        Hide
        Rajith Attapattu added a comment -

        Code is reviewed and test cases are added to acl.py

        Show
        Rajith Attapattu added a comment - Code is reviewed and test cases are added to acl.py
        Hide
        Rajith Attapattu added a comment -

        Attached a new patch with changes suggested by Alan.
        I modified the map<acl::Property, AclProperty* > to map<acl::Property, boost::shared_ptr<AclProperty> > and fixed an indentation issue.

        Show
        Rajith Attapattu added a comment - Attached a new patch with changes suggested by Alan. I modified the map<acl::Property, AclProperty* > to map<acl::Property, boost::shared_ptr<AclProperty> > and fixed an indentation issue.
        Hide
        Rajith Attapattu added a comment -

        Test cases to cover the new validation code.

        Show
        Rajith Attapattu added a comment - Test cases to cover the new validation code.
        Hide
        Rajith Attapattu added a comment -

        The attached patch adds a very basic validation model.
        It basically iterates through the ACL model and verify 3 properties (namely queue-policy, max-queue-count and size).
        However it's generic enough to add validation for any property.

        There is room for improvement. You could validate object and action combinations and also check if properties are valid for a given object.
        These are nice to have items, and will be tackled in the future.

        Show
        Rajith Attapattu added a comment - The attached patch adds a very basic validation model. It basically iterates through the ACL model and verify 3 properties (namely queue-policy, max-queue-count and size). However it's generic enough to add validation for any property. There is room for improvement. You could validate object and action combinations and also check if properties are valid for a given object. These are nice to have items, and will be tackled in the future.
        Hide
        Rajith Attapattu added a comment -

        In rev 911509 I added exception handling to ensure the broker doesn't blow off when an invalid rule is triggered during an acl lookip

        Show
        Rajith Attapattu added a comment - In rev 911509 I added exception handling to ensure the broker doesn't blow off when an invalid rule is triggered during an acl lookip

          People

          • Assignee:
            Rajith Attapattu
            Reporter:
            Rajith Attapattu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development