You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by jbertram <gi...@git.apache.org> on 2016/08/03 18:40:09 UTC
[GitHub] activemq-artemis pull request #701: ARTEMIS-529 finer-grained security for q...
GitHub user jbertram opened a pull request:
https://github.com/apache/activemq-artemis/pull/701
ARTEMIS-529 finer-grained security for queues
This was the simplest way I could think of to implement this feature. It allows a security-setting to match to a particular queue by using the syntax "<address>.<queue>" in the match. For example, if two queues A & B each mapped to the same address Z each queue could have independent security-settings by matching "Z.A" and "Z.B".
If people are in favor of this implementation then I will write up some documentation and squash that with this commit so don't merge this until there is a satisfactory consensus and docs are written (if necessary).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbertram/activemq-artemis ARTEMIS-592
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/701.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #701
----
commit 7f59eac04aad997cba1fc1cd6c9fd14527001fd9
Author: jbertram <jb...@apache.org>
Date: 2016-07-02T02:18:06Z
ARTEMIS-529 finer-grained security for queues
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
@jbertram got it.. thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
@jbertram any one...
is this about Temporary Shared Queues? or anything in particular.
if this is just about shared temp queues, then Martyn's suggestion could make sense.. if not.. it makes it even more confusing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
Look at ActiveMQServerImpl::createSharedQueue
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #701: ARTEMIS-592 finer-grained security for q...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/701#discussion_r73435791
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java ---
@@ -420,10 +420,20 @@ public ServerConsumer createConsumer(final long consumerID,
}
if (browseOnly) {
- securityCheck(binding.getAddress(), CheckType.BROWSE, this);
+ try {
+ securityCheck(binding.getAddress(), CheckType.BROWSE, this);
+ }
+ catch (Exception e) {
+ securityCheck(binding.getAddress().concat(".").concat(queueName), CheckType.BROWSE, this);
--- End diff --
I agree with concatenating the address with the queueName, as long as is documented and would make sense for users.
I'm a bit confused on why you would need to catch the exception and redo the verification. I would think you would need the opposite.. in case it passed? In case the address verification failed.
I guess I don't fully understand the use case. we should talk through IRC so we can interact a bit better.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
There's no way to eliminate the user's need to understand how non-core protocols map to the core implementation. We have addresses and queues and just about all the configuration and management details relate to those two resources.
Also, a "subscription" isn't really a core idea. In core, a "subscription" is just a queue. Core and JMS clients can already "share" queues. I understand the need for users to control access to individual queues which are mapped to the same address (which is what this PR is meant to address). I don't understand the whole "shared subscription" use-case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
>> Our Stomp implementation allows individual Stomp clients to connect to the same durable subscription by connecting with the same client-id <<
Wouldn't be easier if we just failed more than one connection with the same client-id? We do the same on JMS, through the setClientID, and metadata
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #701: ARTEMIS-592 finer-grained security for q...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/701
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
Also, I think this JIRA is only relevant when used in a shared subscription scenario. See Lionel's comment about CERN's use case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
This JIRA title is: Allow fine grain access control (durable subscriptions). There is some discussion on the JIRA that you might find useful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
@clebertsuconic, was your question for me or @mtaylor? If it's for me can you elaborate a bit more. I'm not clear on what you're asking.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
@jbertram @mtaylor I will merge this for now. But this wouldn't stop us from finding a better solution later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
There is one concept on core called createSharedQueue, which is a temporary queue that will exist as long as there is an user connect into it.
shared=true might confuse with that approach, unless this operation was specifically in regard to this type of shared queue. is that the case?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
@clebertsuconic I think probably the best solution is to fail multiple clients with the same ID and find an alternate way to handle shared subscriptions consistently across all protocols. For now though, this is the only way to share a durable STOMP subscription.
We can decide whether it needs improvement once we decide on said consistent approach. For now this works and does what CERN require.
@jbertram Nice work on this, I think the discussion on this thread was valuable and has helped us get a better understand requirements and current restrictions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
@clebertsuconic sure, I think that similar feature is virtual topics, which solve the same problem as shared subscriptions. The way it works in ActiveMQ 5.x is that the user specifies the virtual topic queue to allow the shared subscription. This JIRA is with respect to virtual topics, so the first thing we need to do is actually an equivilent (shared subscription) support. That's when this JIRA starts to become a problem. Once we allow users to share subscriptions, how can we control access to the shared subscription. Queues are just an implementation detail.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
@jbertram I'm not sure about using the queue name to control access. It requires users to understand how subscriptions work internally and how the queue name is constructed, which might be different across protocols.
Perhaps a neater approach could be to allow all users of a particular role to share subscriptions, controlled by an address setting. e.g.
User defines a role "production", all users within this role are able to share subscriptions between themselves. Many roles could be created that are able to share subscriptions, "production", "test", "application-x", only users within the role are able to share the subscription.
Production Role: user1, user2
Test Role: user3, user4
user1 and user2 can share the subscription, user3 and user4 can also share a subscription, user1 and user3 can not share a subscription.
We could also create a new security-setting to limit which roles are able to share subsctipions and for which addresses, something like "sharedSubscriptionGroup" (or better name), which specifies which roles are allowed to share subscriptions.
e.g.
```xml
<security-setting match="jms.topic.news.us.#">
<permission type="createDurableQueue" roles="user"/>
<permission type="send" roles="us-user"/>
<permission type="shareSubscription" roles="production,test"/>
</security-setting>
```
We already store which user created the queue "the queue owner", we could use this information to determine whether or not to allow another user to share this subscription. You can get the queue owner roles and the current user roles to make the comparison and do a check on the security setting.
Does this make sense? Any thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
I am not sure <permission type="shareSubscription" roles="production,test"/> would really be that user friendly as you defined. it could be confusing actually.
you already have permissions to createQueue, createDurableQueue and consume.
shareSubscriptions is exactly that... and it makes confusing to add an extra setting here.
Artemis is a multi-protocol broker.. we are making other protocols more mature and your definitions of shareSubscription there may not fit all the protocols.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
My understanding was. There was a similar feature in AMQ5 that needed to be similar here in which Lionel was using.if there is a way to not use the queue name it's better for sure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
To track our offline conversations.
For sake of conversation I'm describing the equivilent "virtual topics" feature as a "shared subscription".
We've established the use case for the linked JIRA: ARTEMIS-592 which is to do with securing "shared subscriptions" queues (Or what ever we want to call this functionality). Justin has added the ability to secure individual queues in this PR. This can be used to secure a shared subscription queue and also secure any other queue. At this moment in time, we can't explicitly define a shared subscription queue (as ActiveMQ do with virtual topics). But it may be possible to do something similar by connecting 2 clients with the same client ID to the broker (due to the way we internally create unique queueNames based on ClientID+SubscriptionName). I'm not even sure if this is possible it needs testing. I know with other protocols, for example MQTT, only a single client with the same ID can be connected at a time and allowing 2 clients to connect at the same time may not be a trivial change.
My comments above are an attempt to make this feature configurable in a more user friendly way. Right now, with this solution the user is required to understand and construct the queueName and explicitly define it for every shared subscription.
The above solution I outlined allows admins to group users together and allow them to share subscriptions between themselves, on any address that matches the security-setting filter. With out needing to deal with explicit queues, This is less explicit and more easily configurable imo. It does however, introduce the concept of a subscription to Artemis configuration which seems to be a sticking point for both @jbertram and @clebertsuconic as it's not part of the Artemis Core. (This could be configured in a non-core part of the broker config).
However, there is another use case for this patch, to do with securing queues on the core protocol. So perhaps we merge it for that reason, with the added benefit that it adds the ability to secure a subscription queue.
@clebertsuconic and @jbertram I think I'm out numbered here, so I'll let you guys go ahead and do what you feel is best.
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
The task is about using he queue name also on the control. So it has to do so by definition right? Unless you close the Jira as won't fix.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #701: ARTEMIS-592 finer-grained security for queues
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/701
The original use-case was related to Stomp durable subscriptions which in core terms is simply a durable, non-temporary queue. Our Stomp implementation allows individual Stomp clients to connect to the same durable subscription by connecting with the same client-id and durable-subscription-name values, hence "sharing" the subscription. The danger here is that a client which should not be allowed to connect to the durable subscription would in fact connect to it and essentially steal messages from the legitimate client. However, by enforcing security on the individual queue that situation can be avoided.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---