You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "Gordon Sim (Created) (JIRA)" <ji...@apache.org> on 2011/10/05 15:16:34 UTC
[jira] [Created] (QPID-3522) SASL EXTERNAL mechanism no longer
works
SASL EXTERNAL mechanism no longer works
---------------------------------------
Key: QPID-3522
URL: https://issues.apache.org/jira/browse/QPID-3522
Project: Qpid
Issue Type: Bug
Components: C++ Broker
Affects Versions: 0.13
Reporter: Gordon Sim
Priority: Blocker
Fix For: 0.13
Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124174#comment-13124174 ]
jiraposter@reviews.apache.org commented on QPID-3522:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2271/#review2483
-----------------------------------------------------------
Ship it!
- mick
On 2011-10-10 06:32:28, Gordon Sim wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2271/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-10 06:32:28)
bq.
bq.
bq. Review request for Alan Conway, michael goulish and Chug Rolke.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. The stubs and skeletons used to invoke and handle AMQP 0-10 commands/controls at present does not distinguish between null and an empty string. It turns out this distinction is critical for Cyrus SASL integration.
bq.
bq. Rather than altering the general approach - which I fear would mean a large patch and the potential for lots of irritating bugs to creep in - I've restricted the change to the specific control of relevance here. By operating on the command body directly rather than using the parameter list to- and from- which it is converted, the appropriate check can be made.
bq.
bq. I have also had to alter the SASL interfaces to make the same distinction with regard to the initial response.
bq.
bq.
bq. This addresses bug QPID-3522.
bq. https://issues.apache.org/jira/browse/QPID-3522
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/cpp/src/qpid/Sasl.h 1179157
bq. /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/tests/ssl_test 1179157
bq.
bq. Diff: https://reviews.apache.org/r/2271/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested both CRAM-MD5 and EXTERNAL work. Added automated test for EXTERNAL which was previously missing. I haven't tested at all on windows however.
bq.
bq.
bq. Thanks,
bq.
bq. Gordon
bq.
bq.
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13123140#comment-13123140 ]
jiraposter@reviews.apache.org commented on QPID-3522:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2271/#review2439
-----------------------------------------------------------
Getting through the compile phase:
/trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp
<https://reviews.apache.org/r/2271/#comment5544>
This doesn't compile in windows. Change '*response[0]' to '(*response).c_str()[0]'
/trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp
<https://reviews.apache.org/r/2271/#comment5543>
This doesn't compile in windows. The 'response' and 'externalSettings' args are out of order.
- Chug
On 2011-10-07 15:42:32, Gordon Sim wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2271/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-07 15:42:32)
bq.
bq.
bq. Review request for Alan Conway, michael goulish and Chug Rolke.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. The stubs and skeletons used to invoke and handle AMQP 0-10 commands/controls at present does not distinguish between null and an empty string. It turns out this distinction is critical for Cyrus SASL integration.
bq.
bq. Rather than altering the general approach - which I fear would mean a large patch and the potential for lots of irritating bugs to creep in - I've restricted the change to the specific control of relevance here. By operating on the command body directly rather than using the parameter list to- and from- which it is converted, the appropriate check can be made.
bq.
bq. I have also had to alter the SASL interfaces to make the same distinction with regard to the initial response.
bq.
bq.
bq. This addresses bug QPID-3522.
bq. https://issues.apache.org/jira/browse/QPID-3522
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/cpp/src/qpid/Sasl.h 1179157
bq. /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/tests/ssl_test 1179157
bq.
bq. Diff: https://reviews.apache.org/r/2271/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested both CRAM-MD5 and EXTERNAL work. Added automated test for EXTERNAL which was previously missing. I haven't tested at all on windows however.
bq.
bq.
bq. Thanks,
bq.
bq. Gordon
bq.
bq.
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Assigned] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "Gordon Sim (Assigned) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gordon Sim reassigned QPID-3522:
--------------------------------
Assignee: Gordon Sim
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "Gordon Sim (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13121317#comment-13121317 ]
Gordon Sim commented on QPID-3522:
----------------------------------
The issue is indeed related to a distinction between null and a zero-length string. In the cyrus sasl lib, when calling sasl_client_start, the response returned is sometimes null (e.g. for CRAM-MD5) and sometimes a zero-length string (e.g. for EXTERNAL). The corresponding call to sasl_server_start makes a similar distinction. While AMQP 0-10 allows this distinction to be made on the wire, the stubs and skeletons for the c++ client and broker don't distinguish between the two cases.
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124153#comment-13124153 ]
jiraposter@reviews.apache.org commented on QPID-3522:
-----------------------------------------------------
bq. On 2011-10-10 14:56:29, mick goulish wrote:
bq. > /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp, line 193
bq. > <https://reviews.apache.org/r/2271/diff/2/?file=49037#file49037line193>
bq. >
bq. > What do you think about the question in this comment? Should we indeed throw an error here if this is not a valid PLAIN response?
Could do. I would do it as a separate change from this. It's not a particularly critical point as this is only for the case where authentication has been turned off, the client has chosen PLAIN, but doesn't then format the response correctly.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2271/#review2480
-----------------------------------------------------------
On 2011-10-10 06:32:28, Gordon Sim wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2271/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-10 06:32:28)
bq.
bq.
bq. Review request for Alan Conway, michael goulish and Chug Rolke.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. The stubs and skeletons used to invoke and handle AMQP 0-10 commands/controls at present does not distinguish between null and an empty string. It turns out this distinction is critical for Cyrus SASL integration.
bq.
bq. Rather than altering the general approach - which I fear would mean a large patch and the potential for lots of irritating bugs to creep in - I've restricted the change to the specific control of relevance here. By operating on the command body directly rather than using the parameter list to- and from- which it is converted, the appropriate check can be made.
bq.
bq. I have also had to alter the SASL interfaces to make the same distinction with regard to the initial response.
bq.
bq.
bq. This addresses bug QPID-3522.
bq. https://issues.apache.org/jira/browse/QPID-3522
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/cpp/src/qpid/Sasl.h 1179157
bq. /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/tests/ssl_test 1179157
bq.
bq. Diff: https://reviews.apache.org/r/2271/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested both CRAM-MD5 and EXTERNAL work. Added automated test for EXTERNAL which was previously missing. I haven't tested at all on windows however.
bq.
bq.
bq. Thanks,
bq.
bq. Gordon
bq.
bq.
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13123913#comment-13123913 ]
jiraposter@reviews.apache.org commented on QPID-3522:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2271/
-----------------------------------------------------------
(Updated 2011-10-10 06:32:28.713798)
Review request for Alan Conway, michael goulish and Chug Rolke.
Changes
-------
Made changes pointed out by Chuck. Thanks, Chuck!
Summary
-------
The stubs and skeletons used to invoke and handle AMQP 0-10 commands/controls at present does not distinguish between null and an empty string. It turns out this distinction is critical for Cyrus SASL integration.
Rather than altering the general approach - which I fear would mean a large patch and the potential for lots of irritating bugs to creep in - I've restricted the change to the specific control of relevance here. By operating on the command body directly rather than using the parameter list to- and from- which it is converted, the appropriate check can be made.
I have also had to alter the SASL interfaces to make the same distinction with regard to the initial response.
This addresses bug QPID-3522.
https://issues.apache.org/jira/browse/QPID-3522
Diffs (updated)
-----
/trunk/qpid/cpp/src/qpid/Sasl.h 1179157
/trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1179157
/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h 1179157
/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1179157
/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h 1179157
/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1179157
/trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp 1179157
/trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1179157
/trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp 1179157
/trunk/qpid/cpp/src/tests/ssl_test 1179157
Diff: https://reviews.apache.org/r/2271/diff
Testing
-------
Tested both CRAM-MD5 and EXTERNAL work. Added automated test for EXTERNAL which was previously missing. I haven't tested at all on windows however.
Thanks,
Gordon
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Resolved] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "Gordon Sim (Resolved) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gordon Sim resolved QPID-3522.
------------------------------
Resolution: Fixed
Thanks to all reviewers, especially Chuck for testing compilation on windows!
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124151#comment-13124151 ]
jiraposter@reviews.apache.org commented on QPID-3522:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2271/#review2480
-----------------------------------------------------------
I think this all makes sense -- I just have one question inspired by a comment in SaslAuthenticator.cpp
/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp
<https://reviews.apache.org/r/2271/#comment5616>
What do you think about the question in this comment? Should we indeed throw an error here if this is not a valid PLAIN response?
- mick
On 2011-10-10 06:32:28, Gordon Sim wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2271/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-10 06:32:28)
bq.
bq.
bq. Review request for Alan Conway, michael goulish and Chug Rolke.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. The stubs and skeletons used to invoke and handle AMQP 0-10 commands/controls at present does not distinguish between null and an empty string. It turns out this distinction is critical for Cyrus SASL integration.
bq.
bq. Rather than altering the general approach - which I fear would mean a large patch and the potential for lots of irritating bugs to creep in - I've restricted the change to the specific control of relevance here. By operating on the command body directly rather than using the parameter list to- and from- which it is converted, the appropriate check can be made.
bq.
bq. I have also had to alter the SASL interfaces to make the same distinction with regard to the initial response.
bq.
bq.
bq. This addresses bug QPID-3522.
bq. https://issues.apache.org/jira/browse/QPID-3522
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/cpp/src/qpid/Sasl.h 1179157
bq. /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/tests/ssl_test 1179157
bq.
bq. Diff: https://reviews.apache.org/r/2271/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested both CRAM-MD5 and EXTERNAL work. Added automated test for EXTERNAL which was previously missing. I haven't tested at all on windows however.
bq.
bq.
bq. Thanks,
bq.
bq. Gordon
bq.
bq.
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13123218#comment-13123218 ]
jiraposter@reviews.apache.org commented on QPID-3522:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2271/#review2443
-----------------------------------------------------------
Ship it!
- Alan
On 2011-10-07 15:42:32, Gordon Sim wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2271/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-07 15:42:32)
bq.
bq.
bq. Review request for Alan Conway, michael goulish and Chug Rolke.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. The stubs and skeletons used to invoke and handle AMQP 0-10 commands/controls at present does not distinguish between null and an empty string. It turns out this distinction is critical for Cyrus SASL integration.
bq.
bq. Rather than altering the general approach - which I fear would mean a large patch and the potential for lots of irritating bugs to creep in - I've restricted the change to the specific control of relevance here. By operating on the command body directly rather than using the parameter list to- and from- which it is converted, the appropriate check can be made.
bq.
bq. I have also had to alter the SASL interfaces to make the same distinction with regard to the initial response.
bq.
bq.
bq. This addresses bug QPID-3522.
bq. https://issues.apache.org/jira/browse/QPID-3522
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/cpp/src/qpid/Sasl.h 1179157
bq. /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/tests/ssl_test 1179157
bq.
bq. Diff: https://reviews.apache.org/r/2271/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested both CRAM-MD5 and EXTERNAL work. Added automated test for EXTERNAL which was previously missing. I haven't tested at all on windows however.
bq.
bq.
bq. Thanks,
bq.
bq. Gordon
bq.
bq.
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125339#comment-13125339 ]
jiraposter@reviews.apache.org commented on QPID-3522:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2271/#review2519
-----------------------------------------------------------
Ship it!
bulds on windows
- Chug
On 2011-10-10 06:32:28, Gordon Sim wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2271/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-10 06:32:28)
bq.
bq.
bq. Review request for Alan Conway, michael goulish and Chug Rolke.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. The stubs and skeletons used to invoke and handle AMQP 0-10 commands/controls at present does not distinguish between null and an empty string. It turns out this distinction is critical for Cyrus SASL integration.
bq.
bq. Rather than altering the general approach - which I fear would mean a large patch and the potential for lots of irritating bugs to creep in - I've restricted the change to the specific control of relevance here. By operating on the command body directly rather than using the parameter list to- and from- which it is converted, the appropriate check can be made.
bq.
bq. I have also had to alter the SASL interfaces to make the same distinction with regard to the initial response.
bq.
bq.
bq. This addresses bug QPID-3522.
bq. https://issues.apache.org/jira/browse/QPID-3522
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /trunk/qpid/cpp/src/qpid/Sasl.h 1179157
bq. /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1179157
bq. /trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp 1179157
bq. /trunk/qpid/cpp/src/tests/ssl_test 1179157
bq.
bq. Diff: https://reviews.apache.org/r/2271/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested both CRAM-MD5 and EXTERNAL work. Added automated test for EXTERNAL which was previously missing. I haven't tested at all on windows however.
bq.
bq.
bq. Thanks,
bq.
bq. Gordon
bq.
bq.
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org
[jira] [Commented] (QPID-3522) SASL EXTERNAL mechanism no longer
works
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/QPID-3522?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13122888#comment-13122888 ]
jiraposter@reviews.apache.org commented on QPID-3522:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2271/
-----------------------------------------------------------
Review request for Alan Conway, michael goulish and Chug Rolke.
Summary
-------
The stubs and skeletons used to invoke and handle AMQP 0-10 commands/controls at present does not distinguish between null and an empty string. It turns out this distinction is critical for Cyrus SASL integration.
Rather than altering the general approach - which I fear would mean a large patch and the potential for lots of irritating bugs to creep in - I've restricted the change to the specific control of relevance here. By operating on the command body directly rather than using the parameter list to- and from- which it is converted, the appropriate check can be made.
I have also had to alter the SASL interfaces to make the same distinction with regard to the initial response.
This addresses bug QPID-3522.
https://issues.apache.org/jira/browse/QPID-3522
Diffs
-----
/trunk/qpid/cpp/src/qpid/Sasl.h 1179157
/trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1179157
/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.h 1179157
/trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1179157
/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.h 1179157
/trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1179157
/trunk/qpid/cpp/src/qpid/broker/windows/SaslAuthenticator.cpp 1179157
/trunk/qpid/cpp/src/qpid/client/ConnectionHandler.cpp 1179157
/trunk/qpid/cpp/src/qpid/client/windows/SaslFactory.cpp 1179157
/trunk/qpid/cpp/src/tests/ssl_test 1179157
Diff: https://reviews.apache.org/r/2271/diff
Testing
-------
Tested both CRAM-MD5 and EXTERNAL work. Added automated test for EXTERNAL which was previously missing. I haven't tested at all on windows however.
Thanks,
Gordon
> SASL EXTERNAL mechanism no longer works
> ---------------------------------------
>
> Key: QPID-3522
> URL: https://issues.apache.org/jira/browse/QPID-3522
> Project: Qpid
> Issue Type: Bug
> Components: C++ Broker
> Affects Versions: 0.13
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Blocker
> Fix For: 0.13
>
>
> Seems to be as a result of QPID-3393 (i.e. regression since 0.12) which was a fix for CRAM-MD5. From a simplistic point of view it seems like the CRAM-MD5 mechanism requires an empty response string to be treated as null, whereas for the EXTERNAL mechanism an empty response should be treated as a zero length string. It may be though that there is more to this than that.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
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:dev-subscribe@qpid.apache.org