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.patch
        9 kB
        Rajith Attapattu
      2. QPID-2488.test.patch
        3 kB
        Rajith Attapattu

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        6d 2h 50m 1 Rajith Attapattu 14/Apr/10 20:19
        Resolved Resolved Closed Closed
        1201d 23h 34m 1 Justin Ross 29/Jul/13 19:54
        Justin Ross made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Rajith Attapattu made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        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
        Rajith Attapattu made changes -
        Attachment QPID-2488.patch [ 12441202 ]
        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.
        Rajith Attapattu made changes -
        Attachment QPID-2488.patch [ 12441170 ]
        Rajith Attapattu made changes -
        Attachment QPID-2488.test.patch [ 12441171 ]
        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.
        Rajith Attapattu made changes -
        Field Original Value New Value
        Attachment QPID-2488.patch [ 12441170 ]
        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
        Rajith Attapattu created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development