"The publish action should be performed at send()." - I'm not sure I agree. If you aren't allowed to publish to a queue, why is it wrong to deny the establishment of a sending link to it?

The ACL permissions are obviously heavily influenced by 0-10 where the concept of a sender link doesn't exist. Certainly in moving to use 1.0 the aim is that the same permissions are needed, but in this case, whether you fail when creating the sender or on first trying to send with it, seems to me to have little real impact.

I have to agree that ACLs and also my point of view is influenced by 0-10.

I see your point, and It's good. I agree that it makes a little sense to create sender link without sender, but for example, adding a binding without touching other object was possible with spout without sending any data, it could be helpful when you have plenty of clients already working on exchanges, queues, and you don't want to disturb them, only just add one additional bind.

Or another IMO good security example. Let's have AMQP1.0 client, constantly sending data over one existing uninterrupted connection/session/sender, and with current state "publish" is authorized at start only once, then is allowed forever. And let say you want to cutoff this sender, you add rule to ACL, reload, and nothing, sender happily continue to push data.

On the other hand, checking publish rule for every single one publish (as in AMQP 0-10) could impact performance.

(In reply to Zdenek Kraus from comment #2)
> I see your point, and It's good. I agree that it makes a little sense to
> create sender link without sender, but for example, adding a binding without
> touching other object was possible with spout without sending any data, it
> could be helpful when you have plenty of clients already working on
> exchanges, queues, and you don't want to disturb them, only just add one
> additional bind.

An existing application that was creating a sender only for the side-effect of the bindings it would produce  would not work over AMQP 1.0 anyway since x-bindings is not supported in that context. There would also arguably be little point in changing the protocol in that case.

> Or another IMO good security example. Let's have AMQP1.0 client, constantly
> sending data over one existing uninterrupted connection/session/sender, and
> with current state "publish" is authorized at start only once, then is
> allowed forever. And let say you want to cutoff this sender, you add rule to
> ACL, reload, and nothing, sender happily continue to push data.

That is true, certainly. However if you deny access to a user that is already subscribed, then any active receivers will continue to be able to consume messages on either 0-10 or 1.0.

If you want to bar a user completely, that won't happen automatically for any pre-existing connections.  These would need to be closed through management. The same could be done in the case of more limited restrictions, e.g. causing the sender to be re-established at which point access and publish permission could be reverified.
 
> On the other hand, checking publish rule for every single one publish (as in
> AMQP 0-10) could impact performance.

(In reply to Gordon Sim from comment #3)
> (In reply to Zdenek Kraus from comment #2)
> > I see your point, and It's good. I agree that it makes a little sense to
> > create sender link without sender, but for example, adding a binding without
> > touching other object was possible with spout without sending any data, it
> > could be helpful when you have plenty of clients already working on
> > exchanges, queues, and you don't want to disturb them, only just add one
> > additional bind.
> 
> An existing application that was creating a sender only for the side-effect
> of the bindings it would produce  would not work over AMQP 1.0 anyway since
> x-bindings is not supported in that context. There would also arguably be
> little point in changing the protocol in that case.
> 

Ah, I always forgot about that. You're right.


> > Or another IMO good security example. Let's have AMQP1.0 client, constantly
> > sending data over one existing uninterrupted connection/session/sender, and
> > with current state "publish" is authorized at start only once, then is
> > allowed forever. And let say you want to cutoff this sender, you add rule to
> > ACL, reload, and nothing, sender happily continue to push data.
> 
> That is true, certainly. However if you deny access to a user that is
> already subscribed, then any active receivers will continue to be able to
> consume messages on either 0-10 or 1.0.
> 
> If you want to bar a user completely, that won't happen automatically for
> any pre-existing connections.  These would need to be closed through
> management. The same could be done in the case of more limited restrictions,
> e.g. causing the sender to be re-established at which point access and
> publish permission could be reverified.
>  
> > On the other hand, checking publish rule for every single one publish (as in
> > AMQP 0-10) could impact performance.

But I cannot agree on this one. Because in AMQP 0-10 is publish checked on every send, when you reload an acl, current sending client receives a Unathorized Access.

/var/dtests/node_data/clients/qc2_spout -b UserA/UserA@localhost:5672 -c 10 --duration 30 "q;{node:{type:queue},create:always}" --connection-options "{protocol:'amqp0-10'}" --log-msgs dict                                      
{'redelivered': False, 'reply_to': None, 'subject': None, 'content_type': None, 'id': None, 'user_id': None, 'correlation_id': None, 'priority': 0, 'durable': False, 'ttl': 0.000000e+00, 'size': None, 'properties': {'spout-id': '2d44ecdb-dfb8-4515-854e-a486ae5d48da:0'}, 'content': None}
{'redelivered': False, 'reply_to': None, 'subject': None, 'content_type': None, 'id': None, 'user_id': None, 'correlation_id': None, 'priority': 0, 'durable': False, 'ttl': 0.000000e+00, 'size': None, 'properties': {'spout-id': '2d44ecdb-dfb8-4515-854e-a486ae5d48da:1'}, 'content': None}
{'redelivered': False, 'reply_to': None, 'subject': None, 'content_type': None, 'id': None, 'user_id': None, 'correlation_id': None, 'priority': 0, 'durable': False, 'ttl': 0.000000e+00, 'size': None, 'properties': {'spout-id': '2d44ecdb-dfb8-4515-854e-a486ae5d48da:2'}, 'content': None}
2014-07-29 08:15:40 [Client] warning Exception received from broker: unauthorized-access: UserA@QPID cannot publish to  with routing-key q (/builddir/build/BUILD/qpid-0.22/cpp/src/qpid/broker/SemanticState.cpp:499) [caused by 5 \x00:\x00]
unauthorized-access: UserA@QPID cannot publish to  with routing-key q (/builddir/build/BUILD/qpid-0.22/cpp/src/qpid/broker/SemanticState.cpp:499)



acl deny-log UserA@QPID publish all # added by reload
acl allow-log all all


2014-07-29 08:14:57 [Security] notice ACL: Read file "/etc/qpid/qpidd.acl"
2014-07-29 08:14:57 [Security] info ACL Plugin loaded
2014-07-29 08:15:34 [Security] info ACL Allow id:UserA@QPID action:create ObjectType:queue Name:q
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:access ObjectType:exchange Name:qmf.default.topic
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:access ObjectType:queue Name:qmf.default.topic
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:create ObjectType:queue Name:66d1735f-4296-44ba-8f92-ad74eb7e66c4:0.0
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:bind ObjectType:exchange Name:qmf.default.topic
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:consume ObjectType:queue Name:66d1735f-4296-44ba-8f92-ad74eb7e66c4:0.0
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:access ObjectType:exchange Name:qmf.default.direct
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:access ObjectType:queue Name:qmf.default.direct
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:access ObjectType:method Name:reloadACLFile
2014-07-29 08:15:39 [Security] notice ACL: Read file "/etc/qpid/qpidd.acl"
2014-07-29 08:15:39 [Security] info ACL Allow id:root@QPID action:delete ObjectType:queue Name:66d1735f-4296-44ba-8f92-ad74eb7e66c4:0.0
2014-07-29 08:15:40 [Security] info ACL Deny id:UserA@QPID action:publish ObjectType:exchange Name:

(In reply to Zdenek Kraus from comment #4)
> (In reply to Gordon Sim from comment #3)
> > (In reply to Zdenek Kraus from comment #2)
> > > Or another IMO good security example. Let's have AMQP1.0 client, constantly
> > > sending data over one existing uninterrupted connection/session/sender, and
> > > with current state "publish" is authorized at start only once, then is
> > > allowed forever. And let say you want to cutoff this sender, you add rule to
> > > ACL, reload, and nothing, sender happily continue to push data.
> > 
> > That is true, certainly. However if you deny access to a user that is
> > already subscribed, then any active receivers will continue to be able to
> > consume messages on either 0-10 or 1.0.
> 
> But I cannot agree on this one. Because in AMQP 0-10 is publish checked on
> every send, when you reload an acl, current sending client receives a
> Unathorized Access.

Yes, I agree that in that one instance there is a difference between 1.0 and 0-10 and 0-10 responds immediately to the change.

My point was that the effects of an acl reload don't affect existing connections in general, publish is a special case[*]. E.g. if instead of publishing we have a consumer on a queue, and we modify the acl to deny that user the permission to consume, then reload, the consumer can continue to consume messages regardless of the protocol. If I add a rule to deny bind permission for a user, any existing bindings made by that user remain unaffected.

So while 1.0 may in practice differ from 0-10 for the one specific case, the general rule is that changes to acl permissions won't impact existing connections that have already been authorised. Without such a general rule, the behaviour of publish with 0-10 is in my view of limited value and the fact that 1.0 behaviour is different in that case is not significant.

If a general rule is to be enforced, that would of course be different,


[*] I don't believe it is deliberate either, the permission is checked per-message because in 0-10 there is no other way to do it, not tin order to react to dynamic changes to the rules.

OK, I understand. I agree that publish should be there in AMQP1.0, but on the other hand.

When creating sender on the exchange object, the publish rule is not requested.

/var/dtests/node_data/clients/qc2_connector --obj-ctrl CES -b UserA/UserA@localhost:5672 -a "amq.topic;{node:{type:topic}}" --connection-options "{protocol:'amqp1.0'}"
2014-07-29 14:22:05 [Security] info ACL Allow id:UserA@QPID action:access ObjectType:exchange Name:amq.topic
2014-07-29 14:22:05 [Security] info ACL Allow id:UserA@QPID action:access ObjectType:exchange Name:amq.topic

also works for custom exchange. Thus only when you supply queue in address, then client tries to establish publish link to a default exchange.




when spout client tries to actually publish a message it's correct:

/var/dtests/node_data/clients/qc2_spout -b UserA/UserA@localhost:5672 -c 1 "amq.topic;{node:{type:topic}}" --connection-options "{protocol:'amqp1.0'}" --log-msgs dict
{'redelivered': False, 'reply_to': None, 'subject': None, 'content_type': None, 'id': None, 'user_id': None, 'correlation_id': None, 'priority': 0, 'durable': False, 'ttl': 0.000000e+00, 'size': None, 'properties': {'spout-id': '5b040b14-147e-44f1-a3ec-d38bffb56dba:0'}, 'content': None}
Link detached by peer with amqp:unauthorized-access: UserA@QPID cannot publish to amq.topic with routing-key  (/builddir/build/BUILD/qpid-0.22/cpp/src/qpid/broker/amqp/Authorise.cpp:118)

2014-07-29 14:24:35 [Security] info ACL Allow id:UserA@QPID action:access ObjectType:exchange Name:amq.topic
2014-07-29 14:24:35 [Security] info ACL Allow id:UserA@QPID action:access ObjectType:exchange Name:amq.topic
2014-07-29 14:24:35 [Security] info ACL Deny id:UserA@QPID action:publish ObjectType:exchange Name:amq.topic

(In reply to Zdenek Kraus from comment #6)
> OK, I understand. I agree that publish should be there in AMQP1.0, but on
> the other hand.
> 
> When creating sender on the exchange object, the publish rule is not
> requested.

Right. Permission to publish is based on both the exchange and (optionally) the routing key. When creating a sender to e.g. my-exchange/abc over 1.0, the sending link is attached to the node, i.e. to my-exchange. At that point it has no information about the routing key, which is sent as the subject of each published message, and so authorisation needs to be per-message here.

[...]
> also works for custom exchange. Thus only when you supply queue in address,
> then client tries to establish publish link to a default exchange.

Right, the permission to publish to a queue in 0-10 is the permission to publish to the default exchange with a routing key equal to the queue name. Over 1.0, a sender results in the attaching of a link to the queue. The queue name is known, the default exchange is implied, so at this point we have all the information needed to authorise.

(In reply to Gordon Sim from comment #7)
> (In reply to Zdenek Kraus from comment #6)
> > OK, I understand. I agree that publish should be there in AMQP1.0, but on
> > the other hand.
> > 
> > When creating sender on the exchange object, the publish rule is not
> > requested.
> 
> Right. Permission to publish is based on both the exchange and (optionally)
> the routing key. When creating a sender to e.g. my-exchange/abc over 1.0,
> the sending link is attached to the node, i.e. to my-exchange. At that point
> it has no information about the routing key, which is sent as the subject of
> each published message, and so authorisation needs to be per-message here.

Is it currently per-message?  Given what you said above, it's not clear that any per-message authorization should take place.  And here you're saying it needs to be per-message.  Should this be understood as an exception?

(In reply to Justin Ross from comment #8)
> Is it currently per-message?  Given what you said above, it's not clear that
> any per-message authorization should take place.  And here you're saying it
> needs to be per-message.  Should this be understood as an exception?

In 0-10, the message-transfer has an exchange name and a routing key. The values of those fields may differ on every message-transfer, even on the same session. Therefore each one is authorised individually. This is true whether or not the exchange name is the empty string - i.e. the default exchange.

In 1.0 there is a concept of sender link, which is missing in 0-10. The target of that link is specified on attach. Every message sent over that link is then assumed to be destined for that target.

Where the target is a queue, authorisation involves checking for publish permission to the default exchange with the queue's name as the routing key (i.e. the permission that would be required to send to that queue directly over 0-10). We can do this on attach, since we have all the information.

Where the target is an exchange however, the routing key for the authorisation check comes from the message itself (the subject of the message to be specific). Therefore authorisation in that case is done per-message (as it must be).

Does that make sense?

(In reply to Gordon Sim from comment #9)
> (In reply to Justin Ross from comment #8)
> > Is it currently per-message?  Given what you said above, it's not clear that
> > any per-message authorization should take place.  And here you're saying it
> > needs to be per-message.  Should this be understood as an exception?
> 
> In 0-10, the message-transfer has an exchange name and a routing key. The
> values of those fields may differ on every message-transfer, even on the
> same session. Therefore each one is authorised individually. This is true
> whether or not the exchange name is the empty string - i.e. the default
> exchange.
> 
> In 1.0 there is a concept of sender link, which is missing in 0-10. The
> target of that link is specified on attach. Every message sent over that
> link is then assumed to be destined for that target.
> 
> Where the target is a queue, authorisation involves checking for publish
> permission to the default exchange with the queue's name as the routing key
> (i.e. the permission that would be required to send to that queue directly
> over 0-10). We can do this on attach, since we have all the information.
> 
> Where the target is an exchange however, the routing key for the
> authorisation check comes from the message itself (the subject of the
> message to be specific). Therefore authorisation in that case is done
> per-message (as it must be).
> 
> Does that make sense?

Yes, thanks.

Thanks again for clarifying. Based on what I have learned, We could probably CLOSE NOTABUG.