Re: [DISCUSS] ARTEMIS-1884 Plugin API to allow message level authentication

[Ryan Yeats]

I have gotten a lot of good review comments and I think the PR is in a good 
place if anyone wants to take another look. I have time to do some work to get 
this in the main branch if there are more changes needed. I did some 
performance testing on my machine to compare performance of message level 
authentication with various amounts or roles and consumers 
https://github.com/apache/activemq-artemis/pull/3281#issuecomment-740243430.

There is a performance hit when more consumers are subscribed to topics or 
non-authorized consumers on queues. In my view however it's acceptable as its 
the cost of doing message level authentication. The only thing that could 
really be improved is the role caching, since java stores role(Principals) 
internally as a linked list. I am open to ideas, but I think this PR has made 
the API changes and a plugin that should cover most normal cases. If end-users 
need a ton of roles or are doing authorization slightly different, they now 
have the API's to optimize for their own solution. 

Thanks for your time,

Ryan Yeats 

On 10/5/20, 8:53 AM, "Ryan Yeats"  wrote:

Please be cautious
This email was sent from outside of your organization


I have a PR up for adding a plugin API and a plugin to support message 
level authentication but there are some assumptions and API changes that would 
be better explained and discussed here.

The premise for this change is that address level permissions while 
suitable for a majority of use cases are not suitable for cases where message 
permissions are more dynamic and or coordination with consumers to subscribe on 
different topics can't happen before hand.

First issue that prevents this feature is that it needs access to a cached 
instance of authenticated users and roles in order to make message policy 
decisions. Instead of re-authenticate it made sense to reuse already 
authenticated users from the SecurityStore but it does not expose an API to 
retrieve the Subject or RolePrinciples. Therefore, I propose adding Subject 
SecurityStore#getSessionSubject(SecurityAuth session). In leu of changing the 
SecurityStore API it is entirely possible for plugins to use session 
information to reauthenticate using the ActiveMQSecurityManager5 API and then 
cache the Roles themselves, but this seems too burdensome.

The second issue I found is that none of the existing APIs can affect 
delivery of a message. ActiveMQServerMessagePlugin#beforeDelivery is close 
since it has all the needed information, but it happens after the handle 
decision and can't affect delivery without the negative side effect of making 
it not deliverable to other consumers. The ActiveMQSecurityManager API would 
also make sense but it doesn't have access to the SecurityStore and the 
ServerConsumer is already calling out to ActiveMQServerPlugin API so extending 
that makes more sense. Therefore, I propose modifying the ActiveMQServerPlugin 
specifically extending ActiveMQServerMessagePlugin to add a boolean 
canAccept(ServerConsumer consumer, MessageReference reference).

However, the drawback to this is that Artemis allows multiple plugins all 
of which could implement canAccept. This means the results of multiple 
plugin#canAccept need to be taken into account by either requiring ALL(logical 
AND) or ANY(logical OR) canAccept to be true before delivering a message to a 
consumer. Both are acceptable but ANY makes it hard to have a default 
implantation of canAccept because a true would override any other false and 
false would filter all messages if a plugin doesn't care about implementing 
canAccept. So, I am proposing that the results of ALL plugin#canAccepts be true 
before the ServerConsumer delivers a message to a consumer.

Hopefully this gives more context to my PR and can help reviewers.

Ryan Yeats

[Octo | Emerging Technology. Human 
Impact.]

Re: [VOTE] Apache ActiveMQ 5.15.14 release (take #2)

[Christopher Shannon]

+1

On Mon, Dec 7, 2020 at 5:21 PM Francois Papon 
wrote:

> +1 (non-binding)
>
> regards,
>
> François
> fpa...@apache.org
>
> Le 07/12/2020 à 20:29, Matt Pavlovich a écrit :
> > +1
> >
> > [PASSED] Ran full build with unit tests against the tag/5.15.14
> > [PASSED] Ran internal test suite validating various configurations
> >
> > Thanks JB!
> >
> >> On Dec 4, 2020, at 6:41 AM, Jean-Baptiste Onofre 
> wrote:
> >>
> >> +1 (binding)
> >>
> >> Regards
> >> JB
> >>
> >>> Le 3 déc. 2020 à 06:03, Jean-Baptiste Onofre  a
> écrit :
> >>>
> >>> Hi everyone,
> >>>
> >>> I submit Apache ActiveMQ 5.15.14 to your vote. This second take
> includes fixes on jetty-realm file protocol and new updates.
> >>> This release includes CVE related updates and bug fixes.
> >>>
> >>> Please take a look on the Release Notes for details:
> >>>
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210=12348294
> <
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210=12348294
> >
> >>>
> >>> The Maven staging repository is:
> >>>
> https://repository.apache.org/content/repositories/orgapacheactivemq-1221/
> <
> https://repository.apache.org/content/repositories/orgapacheactivemq-1221/
> >
> >>>
> >>> The dist staging repository is:
> >>> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.14/ <
> https://dist.apache.org/repos/dist/dev/activemq/activemq/5.15.14/>
> >>>
> >>> Git tag:
> >>> activemq-5.15.14
> >>>
> >>> Please vote to approve this release:
> >>>
> >>> [ ] +1 Approve the release
> >>> [ ] -1 Don't approve the release (please provide specific comments)
> >>>
> >>> This vote will be open for at least 72 hours.
> >>>
> >>> Thanks !
> >>> Regards
> >>> JB
>