Why is SASL authentication have to be so difficult? Round 2
Igor Brezac
igor at ipass.net
Tue Dec 7 10:08:13 EST 2004
On Tue, 7 Dec 2004, Derrick J Brashear wrote:
> On Mon, 6 Dec 2004, Robert Lubbers wrote:
>
>> * OK cyrus.domain.com Cyrus IMAP4 v2.2.9 server ready
>> . login cyrususer secret
>> . NO Login failed: can't request info until later in exchange
>> . logout
>> * BYE LOGOUT received
>> . OK Completed
>
> Ken pointed something out to me about this yesterday, notably, that imap has
> code e.g.
> r = sasl_getprop(imapd_saslconn, SASL_USERNAME,
> (const void **) &canon_user);
> while pop does not. And I don't know the history. This is one reason I'd
> rather have a real database of bugs, but making bugzilla manageable for this
> is somewhat hard.
>
> The commit log on 1.398.2.81 is "don't canonify a userid twice"
Derrick,
There was a long discussion about this on cyrus-sasl and cyrus-devel lists
awhile back: http://asg.web.cmu.edu/archive/message.php?mailbox=archive.cyrus-sasl&searchterm=saslpasswd2%20and%20virtdomains&msg=3683
-Igor
> So we switch from (effectively) calling auth_canonifyid from canonify_userid
> before doing sasl stuff, to this sasl_getprop after.
>
> Reversing that change would be as follows, you'll almost certainly need to
> apply it by hand.
> hand.
> Index: imapd.c
> ===================================================================
> RCS file: /afs/andrew.cmu.edu/system/cvs/src/cyrus/imap/imapd.c,v
> retrieving revision 1.398.2.81
> retrieving revision 1.398.2.80
> diff -u -r1.398.2.81 -r1.398.2.80
> --- imapd.c 29 May 2003 20:18:58 -0000 1.398.2.81
> +++ imapd.c 29 May 2003 14:50:45 -0000 1.398.2.80
> @@ -38,7 +38,7 @@
> * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> */
>
> -/* $Id: imapd.c,v 1.398.2.81 2003/05/29 20:18:58 rjs3 Exp $ */
> +/* $Id: imapd.c,v 1.398.2.80 2003/05/29 14:50:45 ken3 Exp $ */
>
> #include <config.h>
>
> @@ -1561,7 +1561,8 @@
> char c;
> struct buf passwdbuf;
> char *passwd;
> - const char *reply = NULL;
> + char *canon_user;
> + const char *reply = 0;
> int plaintextloginpause;
> int r;
>
> @@ -1571,10 +1572,20 @@
> return;
> }
>
> + canon_user = canonify_userid(user, NULL, NULL);
> +
> + if (!canon_user) {
> + syslog(LOG_NOTICE, "badlogin: %s plaintext %s invalid user",
> + imapd_clienthost, beautify_string(user));
> + prot_printf(imapd_out, "%s NO %s\r\n", tag, +
> error_message(IMAP_INVALID_USER));
> + return;
> + }
> +
> /* possibly disallow login */
> if ((imapd_starttls_done == 0) &&
> (config_getswitch(IMAPOPT_ALLOWPLAINTEXT) == 0) &&
> - !is_userid_anonymous(user)) {
> + strcmp(canon_user, "anonymous") != 0) {
> eatline(imapd_in, ' ');
> prot_printf(imapd_out, "%s NO Login only available under a
> layer\r\n",
> tag);
> @@ -1596,7 +1607,7 @@
>
> passwd = passwdbuf.s;
>
> - if (is_userid_anonymous(user)) {
> + if (!strcmp(canon_user, "anonymous")) {
> if (config_getswitch(IMAPOPT_ALLOWANONYMOUSLOGIN)) {
> passwd = beautify_string(passwd);
> if (strlen(passwd) > 500) passwd[500] = '\0';
> @@ -1615,21 +1626,22 @@
> }
> }
> else if ((r = sasl_checkpass(imapd_saslconn,
> - user,
> - strlen(user),
> + canon_user,
> + strlen(canon_user),
> passwd,
> strlen(passwd))) != SASL_OK) {
> syslog(LOG_NOTICE, "badlogin: %s plaintext %s %s",
> - imapd_clienthost, user, sasl_errdetail(imapd_saslconn));
> + imapd_clienthost, canon_user, sasl_errdetail(imapd_saslconn));
>
> sleep(3);
>
> - if ((reply = sasl_errstring(r, NULL, NULL)) != NULL) {
> + if (reply) {
> + prot_printf(imapd_out, "%s NO Login failed: %s\r\n", tag, reply);
> + } else if ((reply = sasl_errstring(r, NULL, NULL)) != NULL) {
> prot_printf(imapd_out, "%s NO Login failed: %s\r\n", tag, reply);
> } else {
> prot_printf(imapd_out, "%s NO Login failed: %d\r\n", tag, r);
> }
> -
> snmp_increment_args(AUTHENTICATION_NO, 1,
> VARIABLE_AUTH, 0 /* hash_simple("LOGIN") */,
> VARIABLE_LISTEND);
> @@ -1637,26 +1649,6 @@
> return;
> }
> else {
> - const char *canon_user;
> - - r = sasl_getprop(imapd_saslconn, SASL_USERNAME,
> - (const void **) &canon_user);
> -
> - if(r != SASL_OK) {
> - if ((reply = sasl_errstring(r, NULL, NULL)) != NULL) {
> - prot_printf(imapd_out, "%s NO Login failed: %s\r\n",
> - tag, reply);
> - } else {
> - prot_printf(imapd_out, "%s NO Login failed: %d\r\n", tag, r);
> - }
> -
> - snmp_increment_args(AUTHENTICATION_NO, 1,
> - VARIABLE_AUTH, 0 /* hash_simple("LOGIN") */,
> - VARIABLE_LISTEND);
> - freebuf(&passwdbuf);
> - return;
> - }
> -
> imapd_userid = xstrdup(canon_user);
> snmp_increment_args(AUTHENTICATION_YES, 1,
> VARIABLE_AUTH, 0 /*hash_simple("LOGIN") */, @@
> -1761,6 +1753,7 @@
> */
> sasl_result = sasl_getprop(imapd_saslconn, SASL_USERNAME,
> (const void **) &canon_user);
> + imapd_userid = xstrdup(canon_user);
> if (sasl_result != SASL_OK) {
> prot_printf(imapd_out, "%s NO weird SASL error %d SASL_USERNAME\r\n",
> tag, sasl_result);
> @@ -1769,7 +1762,6 @@
> reset_saslconn(&imapd_saslconn);
> return;
> }
> - imapd_userid = xstrdup(canon_user);
>
> proc_register("imapd", imapd_clienthost, imapd_userid, (char *)0);
>
> ---
> Cyrus Home Page: http://asg.web.cmu.edu/cyrus
> Cyrus Wiki/FAQ: http://cyruswiki.andrew.cmu.edu
> List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
>
--
Igor
---
Cyrus Home Page: http://asg.web.cmu.edu/cyrus
Cyrus Wiki/FAQ: http://cyruswiki.andrew.cmu.edu
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
More information about the Info-cyrus
mailing list