[
https://issues.apache.org/jira/browse/QPID-3415?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13105170#comment-13105170
]
[email protected] commented on QPID-3415:
-----------------------------------------------------
bq. On 2011-09-06 18:12:46, rajith attapattu wrote:
bq. >
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java,
line 38
bq. > <https://reviews.apache.org/r/1608/diff/1/?file=34085#file34085line38>
bq. >
bq. > I believe you forgot to add ClientConnectionDelegate ?
bq. > I can't seem to find this in the current source tree.
bq. >
bq. > Perhaps some of the missing code from ClientDelegate is moved to
this class?
bq.
bq. Robbie Gemmell wrote:
bq. Well damn, thatll teach me to say something looks fine without looking
at the diff to make sure its actually what I looked it ;(
bq.
bq. Pretty much all of your comments stem from this one issue as the
missing code will indeed be in the ClientConnectionDelegate, which is used in
place of ClientDelegate for actual client use now as can be seen further down
the class. ClientDelegate is really mainly used in ConnectionTest after this
(the same way ServerDelegate was really only used in ConnectionTest, but the
real broker uses the ServerConnectionDelegate subclass). The diff posted to
ReviewBoard is basically incomplete for some reason. I dont have a copy of the
original patch that I actually looked over a couple weeks ago and Keith is on
holiday just now, but I'm sure he will post the corrected version on his return
next week...until then, no need to test what wont evne compile... :S
You are both quite correct, I accidentally omitted the new class and the
removal of now redundant files from the udiff. Apologies. I have now updated
the udiff and believe this should answer Rajith's questions.
bq. On 2011-09-06 18:12:46, rajith attapattu wrote:
bq. >
/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java,
line 60
bq. > <https://reviews.apache.org/r/1608/diff/1/?file=34089#file34089line60>
bq. >
bq. > Why is this code (related to GSSAPI) is removed ?
bq. >
bq. > I don't see this code moved elsewhere either? This will break
existing functionality :(
Moved to ClientConnectionDelegate
bq. On 2011-09-06 18:12:46, rajith attapattu wrote:
bq. >
/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java,
line 127
bq. > <https://reviews.apache.org/r/1608/diff/1/?file=34089#file34089line127>
bq. >
bq. > Removing this without a suitable replacement will break existing
functionality.
bq. >
bq. > Is this check performed else where? Could you pls help me located it?
bq. >
bq. > This was in place to ensure the client to throw an exception if it's
not configured to support any of the mechanisms supported by the broker. It
used to be that we just ignored SASL all together if no matching mechs were
found.
bq. >
bq. > I'd argue that this is an important check. Could you please explain
your reasons behind the removal (if the same check is not performed elsewhere)?
Check now performed with ClientConnectionDelegate.
bq. On 2011-09-06 18:12:46, rajith attapattu wrote:
bq. >
/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java,
line 143
bq. > <https://reviews.apache.org/r/1608/diff/1/?file=34089#file34089line143>
bq. >
bq. > Lines 138 to 141 were required for SASL encryption support. Why is
this removed ?
bq. >
bq. > Is this taken care of elsewhere? If so my apologies (but could you
point me to the relevant code?).
Moved to ClientConnectionDelegate
bq. On 2011-09-06 18:12:46, rajith attapattu wrote:
bq. >
/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java,
line 215
bq. > <https://reviews.apache.org/r/1608/diff/1/?file=34089#file34089line215>
bq. >
bq. > The user identity when using GSSAPI and External are crucial for ACL
support.
bq. >
bq. > We have existing customers relying on this feature.
bq. >
bq. > Why was this removed? (Again is there a replacement for this some
where?)
Moved to ClientConnectionDelegate
bq. On 2011-09-06 18:12:46, rajith attapattu wrote:
bq. >
/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java,
line 304
bq. > <https://reviews.apache.org/r/1608/diff/1/?file=34089#file34089line304>
bq. >
bq. > Again this method was used to retrieved the kerberos identity of the
user.
Moved to ClientConnectionDelegate
- Keith
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1608/#review1767
-----------------------------------------------------------
On 2011-09-12 12:05:21, Keith Wall wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1608/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-09-12 12:05:21)
bq.
bq.
bq. Review request for qpid and rajith attapattu.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This patch changes the 0-10 code path to create the SASL callback handler
using the CallbackHandlerRegistry. This allows the 0-10 code path to support
SASL mechanisms requiring other callback handlers, such as CRAM-MD5-HASHED.
Support for the sasl_mechs client connection option has been retained and now
applies to the 0-8..0-9-1 code paths too.
bq.
bq. If the user *specifies* a sasl_mechs client connection option the
behaviour of the code is unchanged from the previous version: it restricts the
list of SASL mechanisms in use.
bq.
bq. If the user *does not specify* a sasl_mechs client connection option, the
old code used a hardcoded PLAIN default. This is no longer the case. Now the
client will use the first SASL mechanism from the list
CallbackHandlerRegistry.properties that is also available on the server.
bq.
bq. Removed dead code and strengthen unit tests.
bq.
bq.
bq.
bq. This addresses bug QPID-3415.
bq. https://issues.apache.org/jira/browse/QPID-3415
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
1169685
bq.
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java
1169685
bq.
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/CallbackHandlerRegistry.java
1169685
bq.
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/CallbackHandlerRegistry.properties
1169685
bq.
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/transport/ClientConnectionDelegate.java
PRE-CREATION
bq.
/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/security/CallbackHandlerRegistryTest.java
PRE-CREATION
bq.
/trunk/qpid/java/common/src/main/java/org/apache/qpid/security/AMQPCallbackHandler.java
1169685
bq.
/trunk/qpid/java/common/src/main/java/org/apache/qpid/security/UsernamePasswordCallbackHandler.java
1169685
bq.
/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java
1169685
bq.
/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java
1169685
bq.
/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionSettings.java
1169685
bq.
/trunk/qpid/java/common/src/test/java/org/apache/qpid/transport/ConnectionTest.java
1169685
bq.
/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java
1169685
bq.
/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/UTF8Test.java
1169685
bq.
bq. Diff: https://reviews.apache.org/r/1608/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Improved unit testing. Ran java, cpp and cpp.ssl profiles. I am not able
to test GSSAPI locally.
bq.
bq.
bq. Thanks,
bq.
bq. Keith
bq.
bq.
> CRAM-MD5-HASHED not supported by 0-10 protocol (+ no suppport for custom SASL
> mechanisms).
> ------------------------------------------------------------------------------------------
>
> Key: QPID-3415
> URL: https://issues.apache.org/jira/browse/QPID-3415
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.10
> Reporter: Keith Wall
> Assignee: Rajith Attapattu
> Fix For: 0.13
>
>
> If the Java broker is configured to use the Base64MD5Password password
> database the Java client is unable to connect even if they use the sasl_mechs
> broker option in the connection URL (sasl_mechs='CRAM-MD5-HASHED').
> Instead the user sees:
> {code}
> org.apache.qpid.AMQException: Cannot connect to broker: Callback handler with
> support for AuthorizeCallback required
> {code}
> The user can work around the problem by passing the -Dqpid.amqp.version
> system property to the client, and selecting a protocol < 0-10.
> The problem is happening because on the 0-10 code path on the client, the
> SASL CallbackHandler in use is hardcoded to UsernamePasswordCallbackhandler
> (ClientDelegate), rather than using the facilities of CallbackHandlerRegistry
> (as does the 0-8 and 0-9* code paths). CRAM-MD5-HASHED requires the use of a
> different Callbackhandler.
> This also inhibits the use of custom SASL methods by the client.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]