Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.22
    • Fix Version/s: 0.23
    • Component/s: C++ Broker
    • Labels:
      None

      Description

      Though e.g. creation of queues and exchanges is governed by ACL rules, creation of connections is not and this should be rectified.

        Activity

        Gordon Sim created issue -
        Hide
        Gordon Sim added a comment -

        Initial patch for connection approval in 1.0 available for review: https://reviews.apache.org/r/12029/

        Show
        Gordon Sim added a comment - Initial patch for connection approval in 1.0 available for review: https://reviews.apache.org/r/12029/
        Hide
        ASF subversion and git services added a comment -

        Commit 1496466 from Gordon Sim
        [ https://svn.apache.org/r1496466 ]

        QPID-4712: authorisation for AMQP 1.0 connections

        Show
        ASF subversion and git services added a comment - Commit 1496466 from Gordon Sim [ https://svn.apache.org/r1496466 ] QPID-4712 : authorisation for AMQP 1.0 connections
        Chuck Rolke made changes -
        Field Original Value New Value
        Comment [ The existing code has a per-IP-address ACL connection quota specified by a single value in the command line or by specification of individual users in the ACL file. The proposed new feature would add a new Action/Object pair to the ACL rule file:
        {noformat}
        acl allow create connection address=<address spec> [user=<user spec>]
        {noformat}
        Impact assessment:

        ||Design consideration||Proposed feature||
        |Threading model|multithread - ACL structures need locks|
        |Memory management|Per-IPaddress counters kept in new instance of existing structure|
        |Automated testing approach|Existing ACL test scheme could prove this feature|
        |Impact on public API|Changes ACL file syntax|
        |- Interoperability with implementations in other languages|n/a|
        |- Backwards compatibility|not backward compatible|
        |Performance implications|Insignificant|
        |Security implications|This feature is a security enhancement|
        |Platform support|n/a|
        |Logging|Logs in 'usual' ACL log format|
        |Monitoring|Count of denied connections already exists|
        |Management|no changes|

        However, how, practically, would one *specify host addresses?* If you want to allow connections from the 10.1.0.0/16 subnet the code today would force you to specify 65k lines of ACL rules and that's not very useful. It would be _easier_ coding for ACL to allow the address to be specified as:
        {noformat}
        10.1.*
        {noformat}
        and _harder_ coding to allow the address to be specified as
        {noformat}
        10.1.0.0/16
        {noformat}
        The same wildcard rule would apply to both IPv4 and IPv6 addresses.

        If a simple wildcard is OK then this feature would be relatively easy to add. ]
        Hide
        Gordon Sim added a comment -

        The goal is to ensure that enabling AMQP 1.0 doesn't allow any existing authorisation policy to be circumvented. The existing rules are heavily influenced (distorted?) by AMQP 0-10 without directly following the that version in full. A redesign of the permissioning model to be more generic would make things more intuitive for AMQP 1.0 users, but is considered out of scope for the time being.

        Link attachment (i.e. creating a sender or receiver) will always require access permission to the node in question (i.e. the queue or exchange). Generally there will be other permissions required as well.

        To attach as a sender to a queue, publish permission is required for the default exchange (amq.default keyword in ACL file) with the queue name as the routing key. For a sending link to an exchange the publish permission cannot be checked on attach since the routing key is unknown at that point. This is similar to 0-10 and the publish permission will be checked (if and only if there are any publish rules) for each message (again, exactly like 0-10).

        To attach as a receiver from a queue, consume permission is also required for the queue in question. To attach as a receiver from an exchange, create and consume permission will be required for the subscription queue. The name of this is chosen by the broker, but it uses the container id (which can be set as a connection option in qpid::messaging) and the link name (which can be set as the name proerty in link properties of address in qpid::messaging). In addition bind permission is required for the exchange in question. Note however that unbind permission is not required in order to detach, as that permission is implied (it will happen anyway).

        In the 1.0 codepath, the properties used in checking access etc are those of the node (queue or exchange). In AMQP 0-10 by contrast the fields of the command being checked are used instead (which leads to some anomalies in my view since e.g. queue-query doesn't involve any test for autodelete, exclusive etc as would a passive declare).

        Show
        Gordon Sim added a comment - The goal is to ensure that enabling AMQP 1.0 doesn't allow any existing authorisation policy to be circumvented. The existing rules are heavily influenced (distorted?) by AMQP 0-10 without directly following the that version in full. A redesign of the permissioning model to be more generic would make things more intuitive for AMQP 1.0 users, but is considered out of scope for the time being. Link attachment (i.e. creating a sender or receiver) will always require access permission to the node in question (i.e. the queue or exchange). Generally there will be other permissions required as well. To attach as a sender to a queue, publish permission is required for the default exchange (amq.default keyword in ACL file) with the queue name as the routing key. For a sending link to an exchange the publish permission cannot be checked on attach since the routing key is unknown at that point. This is similar to 0-10 and the publish permission will be checked (if and only if there are any publish rules) for each message (again, exactly like 0-10). To attach as a receiver from a queue, consume permission is also required for the queue in question. To attach as a receiver from an exchange, create and consume permission will be required for the subscription queue. The name of this is chosen by the broker, but it uses the container id (which can be set as a connection option in qpid::messaging) and the link name (which can be set as the name proerty in link properties of address in qpid::messaging). In addition bind permission is required for the exchange in question. Note however that unbind permission is not required in order to detach, as that permission is implied (it will happen anyway). In the 1.0 codepath, the properties used in checking access etc are those of the node (queue or exchange). In AMQP 0-10 by contrast the fields of the command being checked are used instead (which leads to some anomalies in my view since e.g. queue-query doesn't involve any test for autodelete, exclusive etc as would a passive declare).
        Gordon Sim made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        ASF subversion and git services added a comment -

        Commit 1496545 from Gordon Sim
        [ https://svn.apache.org/r1496545 ]

        QPID-4712: fixes for windows, rhel5

        Show
        ASF subversion and git services added a comment - Commit 1496545 from Gordon Sim [ https://svn.apache.org/r1496545 ] QPID-4712 : fixes for windows, rhel5
        Hide
        Justin Ross added a comment -
        Show
        Justin Ross added a comment - Released in Qpid 0.24, http://qpid.apache.org/releases/qpid-0.24/index.html
        Justin Ross made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        81d 4h 10m 1 Gordon Sim 25/Jun/13 14:19
        Resolved Resolved Closed Closed
        74d 23h 17m 1 Justin Ross 08/Sep/13 13:37

          People

          • Assignee:
            Gordon Sim
            Reporter:
            Gordon Sim
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development