[GitHub] activemq-artemis pull request #2422: ARTEMIS-2168 Fix "populate-validated-us...

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2422: ARTEMIS-2168 Fix "populate-validated-us...

jbertram-2
GitHub user michaelandrepearce opened a pull request:

    https://github.com/apache/activemq-artemis/pull/2422

    ARTEMIS-2168 Fix "populate-validated-user" feature for AMQP messages

    Note, this fixes feature: https://issues.apache.org/jira/browse/ARTEMIS-584 for AMQP produced messages.
    Add additional test cases to cover AMQP for feature.
    Add fix to AMQP message
   
   
    This PR builds on top of other fixes currently in https://github.com/apache/activemq-artemis/pull/2418 as such either this PR can be merged which includes both fixes, or if the other PR is merged first, this PR will need a rebase on that.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/michaelandrepearce/activemq-artemis ARTEMIS-2168

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/2422.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 #2422
   
----
commit cce891325f72fc921ff964440861432b2f320fb3
Author: Michael André Pearce <michael.andre.pearce@...>
Date:   2018-11-06T21:30:52Z

    ARTEMIS-2142 Fix ServerJMSMessage
   
    Fix ServerJMSMessage so it correctly transposes JMSX headers.
    Push common JMS to Message Interface mappings into MessageUtil

commit 2682443247a30274c673f52f645de87667c759dd
Author: Michael André Pearce <michael.andre.pearce@...>
Date:   2018-11-08T22:18:29Z

    ARTEMIS-2168 Fix "populate-validated-user" feature for AMQP messages
   
    Note, this fixes feature: https://issues.apache.org/jira/browse/ARTEMIS-584 for AMQP produced messages.
    Add additional test cases to cover AMQP for feature.
    Add fix to AMQP message

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

jbertram-2
Github user tabish121 commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2422
 
    -1
   
    The properties section of an AMQP message is immutable and cannot be changed by the broker as per the AMQP 1.0 specification (Section 3.2)
   
    http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#section-message-format



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

jbertram-2
In reply to this post by jbertram-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2422
 
    @tabish121 this used to work. Also note this is only when the toggle populate-validated-user is enabled, which is off by default.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

jbertram-2
In reply to this post by jbertram-2
Github user tabish121 commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2422
 
    @michaelandrepearce I may have worked due to a bug that allowed the broker to violate the AMQP specification but was fixed later (https://issues.apache.org/jira/browse/ARTEMIS-1092) and should continue to not allow the broker to violate the AMQP 1.0 specification.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

jbertram-2
In reply to this post by jbertram-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2422
 
    @tabish121 ok, so the use case here, is per https://issues.apache.org/jira/browse/ARTEMIS-584 essentially you cannot/do not trust the user sent on the message, you only trust the user used during auth to the broker, this is quite important for an audit requirement.
   
    In this case as i noted as a user you are explicitly saying you wish to violate spec and modify the message. whilst i agree by default the broker should NOT break any specs, there does need ability to violate/or override for a feature, this would be on par with some of the FQQN stuff that allows users to violate some bits in JMS, (e.g. get a JMS Queue actually bound to a JMS Topic subscription), but you do this explicitly knowing this.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

jbertram-2
In reply to this post by jbertram-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2422
 
    @tabish121 im open to suggestion on how to implement this in a way thats more amenable to yourself.
   
    Other ideas i have:
   
    1) Introduce a broker flag that specially and clearly named something like "allow-amqp-modfication-spec-break" then allows AMQP spec break, so its so clear that we're enabling a spec break.
   
    2) If the populate-validated-user is set, then a or copy of the message is made.
   
    3) Another idea, is there a tamper flag on AMQP spec? If so could we can set that? basically allowing you to say the message was mutated via the broker (and even possible say what was mutated (in this case user)).



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

jbertram-2
In reply to this post by jbertram-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2422
 
    alternative option 1 - https://github.com/apache/activemq-artemis/pull/2423


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

jbertram-2
In reply to this post by jbertram-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2422
 
    closing this in liue of solution #2423


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2422: ARTEMIS-2168 Fix "populate-validated-us...

jbertram-2
In reply to this post by jbertram-2
Github user michaelandrepearce closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/2422


---