Bug 970718 - cyrus-sasl: A change in channel bindings on the client side introduced in cyrus-sasl 2.1.25 and above might lead to weaker mechanisms to be preferred (based on client configuration) before the stronger ones?
Summary: cyrus-sasl: A change in channel bindings on the client side introduced in cyr...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: cyrus-sasl
Version: 20
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Petr Lautrbach
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-06-04 17:02 UTC by Jan Lieskovsky
Modified: 2013-11-26 03:59 UTC (History)
4 users (show)

Fixed In Version: cyrus-sasl-2.1.26-14.fc20
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1030973 (view as bug list)
Environment:
Last Closed: 2013-11-26 03:59:46 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Debian BTS 709552 0 None None None Never

Description Jan Lieskovsky 2013-06-04 17:02:06 UTC
Description of problem:

Based on the following Debian bug:
  [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=709552

in cyrus-sasl of version 2.1.25 and later the following can happen (note quote from Debian bug report mentions 2.1.24 but IILC the change was introduced for the first time in v2.1.25):

<quote>

A bug in Cyrus-SASL 2.1.24 and up causes PLAIN to be chosen over SCRAM-SHA-1
when the server ordered PLAIN before SCRAM-SHA-1 (the order in which the
server specifies the mechanisms should be ignored by the client).

This can cause a client using Cyrus-SASL to disclose its password in plain,
while it doesn't need to. Here is an example of it happening in Pidgin,
compiled with Cyrus-support:

(01:39:37) jabber: Recv (ssl)(497): <?xml version='1.0'?><stream:stream xmlns:stream='http://etherx.jabber.org/streams' version='1.0' from='xnyhps.nl' id='9531b529-e769-4c00-a17f-fae4221436da' xml:lang='en' xmlns='jabber:client'><stream:features><register xmlns='http://jabber.org/features/iq-register'/><auth xmlns='http://jabber.org/features/iq-auth'/><mechanisms xmlns='urn:ietf:params:xml:ns:xmpp-sasl'><mechanism>PLAIN</mechanism><mechanism>SCRAM-SHA-1</mechanism><mechanism>DIGEST-MD5</mechanism></mechanisms></stream:features>
(01:39:37) sasl: Mechs found: PLAIN SCRAM-SHA-1 DIGEST-MD5
(01:39:37) sasl: No worthy mechs found
(01:39:41) sasl: Mechs found: PLAIN SCRAM-SHA-1 DIGEST-MD5
(01:39:41) jabber: Sending (ssl) (test): <auth xmlns='urn:ietf:params:xml:ns:xmpp-sasl' mechanism='PLAIN' xmlns:ga='http://www.google.com/talk/protocol/auth' ga:client-uses-full-bind-result='true'>password removed</auth>
(01:39:41) jabber: Recv (ssl)(60): <success xmlns='urn:ietf:params:xml:ns:xmpp-sasl'></success>

</quote>

Version-Release number of selected component (if applicable):
cyrus-sasl/2.1.26/7.fc19

How reproducible:
Always

Steps to Reproduce:
1. Specify PLAIN SCRAM-SHA-1 DIGEST-MD5 as the mechanisms order in cyrus-sasl client
2. Watch how PLAIN is preferred before the stronger ones

Actual results:
PLAIN method is preferred before the stronger ones

Expected results:
Stronger ones should be preferred regardless of the mechanisms order specified by client (behaviour prior to 2.1.25)

Additional info:
================
Seems to have been introduced by the following upstream commit:
  http://git.cyrusimap.org/cyrus-sasl/commit/?id=80d79113041b216e5b8a6687d9d3bc26f2f1553b

Related (mentioned) ChangeLog entry (to see) is as follows:
============================================================

2011-01-21  Alexey Melnikov <alexey.melnikov>
        * include/saslplug.h, lib/client.c: Fixed handling of channel bindings
          on the client side. The client side was failing to select a suitable
          SASL mechanism when the application specified channel bindings, but
          didn't make them mandatory to use. In such a configuration, if a
          non channel binding capable mechanism was selected through
          "client_mech_list" SASL option, sasl_client_start would fail.
          For example if the server supports both SCRAM-SHA-1[-PLUS] and
          PLAIN and "client_mech_list" was set to "PLAIN", authentication
          would never work.

Relevant source code part (lib/client.c sasl_client_start()) in 2.1.23:
=======================================================================

    520 #ifdef PREFER_MECH
    521             if (strcasecmp(m->m.plug->mech_name, PREFER_MECH) &&
    522                 bestm && m->m.plug->max_ssf <= bestssf) {
    523                 /* this mechanism isn't our favorite, and it's no better
    524                    than what we already have! */
    525                 break;
    526             }
    527 #else
    528             if (bestm && m->m.plug->max_ssf <= bestssf) {
    529                 /* this mechanism is no better than what we already have! */
    530                 break;
    531             }
    532 #endif
    533 
    534             /* compare security flags, only take new mechanism if it has
    535              * all the security flags of the previous one.
    536              *
    537              * From the mechanisms we ship with, this yields the order:
    538              *
    539              * SRP
    540              * GSSAPI + KERBEROS_V4
    541              * DIGEST + OTP
    542              * CRAM + EXTERNAL
    543              * PLAIN + LOGIN + ANONYMOUS
    544              *
    545              * This might be improved on by comparing the numeric value of
    546              * the bitwise-or'd security flags, which splits DIGEST/OTP,
    547              * CRAM/EXTERNAL, and PLAIN/LOGIN from ANONYMOUS, but then we
    548              * are depending on the numeric values of the flags (which may
    549              * change, and their ordering could be considered dumb luck.
    550              */

Relevant source code part / situation (lib/client.c sasl_client_start()) in 2.1.26:
=======================================================================================

    807             /* compare security flags, only take new mechanism if it has
    808              * all the security flags of the previous one.
    809              *
    810              * From the mechanisms we ship with, this yields the order:
    811              *
    812              * SRP
    813              * GSSAPI + KERBEROS_V4
    814              * DIGEST + OTP
    815              * CRAM + EXTERNAL
    816              * PLAIN + LOGIN + ANONYMOUS
    817              *
    818              * This might be improved on by comparing the numeric value of
    819              * the bitwise-or'd security flags, which splits DIGEST/OTP,
    820              * CRAM/EXTERNAL, and PLAIN/LOGIN from ANONYMOUS, but then we
    821              * are depending on the numeric values of the flags (which may
    822              * change, and their ordering could be considered dumb luck.
    823              */
    824 
    825             if (bestm &&
    826                 ((m->m.plug->security_flags ^ bestm->m.plug->security_flags) &
    827                  bestm->m.plug->security_flags)) {
    828                 break;
    829             }
    830 
    831             if (SASL_CB_PRESENT(c_conn->cparams) && plus) {
    832                 cur_cbindingdisp = SASL_CB_DISP_USED;
    833             } else {
    834                 cur_cbindingdisp = cbindingdisp;
    835             }
    836 
    837             if (bestm && (best_cbindingdisp > cur_cbindingdisp)) {
    838                 break;
    839             }
    840 
    841 #ifdef PREFER_MECH
    842             if (strcasecmp(m->m.plug->mech_name, PREFER_MECH) &&
    843                 bestm && m->m.plug->max_ssf <= bestssf) {
    844                 /* this mechanism isn't our favorite, and it's no better
    845                    than what we already have! */
    846                 break;
    847             }
    848 #else
    849             if (bestm && m->m.plug->max_ssf <= bestssf) {
    850                 /* this mechanism is no better than what we already have! */
    851                 break;
    852             }
    853 #endif

IOW PREFER_MECH check has been moved after the compare security flags check, it was / has been in earlier versions.

While the current patch form might work for use case described in the upstream ChangeLog case (SCRAM-SHA-1[-PLUS] and PLAIN mechanisms list), I am not sure
scenario / use case described in the Debian bug report [1] has been considered too (own purpose of this bug report).

Upstream cyrus-sasl bug is at:
  https://bugzilla.cyrusimap.org/show_bug.cgi?id=3793

Comment 1 Fedora End Of Life 2013-09-16 14:05:34 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 20 development cycle.
Changing version to '20'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora20

Comment 2 Fedora Update System 2013-11-15 14:49:42 UTC
cyrus-sasl-2.1.26-14.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/cyrus-sasl-2.1.26-14.fc20

Comment 3 Fedora Update System 2013-11-16 07:05:24 UTC
Package cyrus-sasl-2.1.26-14.fc20:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing cyrus-sasl-2.1.26-14.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-21470/cyrus-sasl-2.1.26-14.fc20
then log in and leave karma (feedback).

Comment 4 Fedora Update System 2013-11-26 03:59:46 UTC
cyrus-sasl-2.1.26-14.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.