From 10bee81ced3a595e556bd7ebc218c95e22d262c6 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 24 Nov 2018 16:01:42 +0000 Subject: [PATCH] Fix cyrus-sasl authenticator for $authenticated_fail_id. Bug 2338 Cherry-picked from: 49ff55b6a9 --- doc/doc-txt/ChangeLog | 4 + src/src/auths/cyrus_sasl.c | 155 +++++++++--------- test/confs/{9300 => 3800} | 0 test/log/{9300 => 3800} | 4 +- test/rejectlog/{9300 => 3800} | 4 +- .../9300 => 3800-Cyrus-SASL/3800} | 1 + .../REQUIRES | 0 test/stdout/{9300 => 3800} | 4 +- 8 files changed, 90 insertions(+), 82 deletions(-) rename test/confs/{9300 => 3800} (100%) rename test/log/{9300 => 3800} (67%) rename test/rejectlog/{9300 => 3800} (66%) rename test/scripts/{9300-Cyrus-SASL/9300 => 3800-Cyrus-SASL/3800} (96%) rename test/scripts/{9300-Cyrus-SASL => 3800-Cyrus-SASL}/REQUIRES (100%) rename test/stdout/{9300 => 3800} (93%) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 77e56c16d..da4e1a8a7 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -67,6 +67,10 @@ JH/31 Fix a bad use of a copy function, which could be used to pointlessly copy a string over itself. The library routine is documented as not supporting overlapping copies, and on MacOS it actually raised a SIGABRT. +JH/33 Bug 2338: Fix the cyrus-sasl authenticator to fill in the + $authenticated_fail_id variable on authentication failure. Previously + it was unset. + Exim version 4.91 ----------------- diff --git a/src/src/auths/cyrus_sasl.c b/src/src/auths/cyrus_sasl.c index 7bb86b815..e8c28ec17 100644 --- a/src/src/auths/cyrus_sasl.c +++ b/src/src/auths/cyrus_sasl.c @@ -397,108 +397,105 @@ while(rc==SASL_CONTINUE) sasl_done(); return UNEXPECTED; } - else if( rc==SASL_FAIL || rc==SASL_BUFOVER - || rc==SASL_BADMAC || rc==SASL_BADAUTH - || rc==SASL_NOAUTHZ || rc==SASL_ENCRYPT - || rc==SASL_EXPIRED || rc==SASL_DISABLED - || rc==SASL_NOUSER ) + if (rc == SASL_CONTINUE) + continue; + + /* Get the username and copy it into $auth1 and $1. The former is now the + preferred variable; the latter is the original variable. */ + + if ((sasl_getprop(conn, SASL_USERNAME, (const void **)(&out2))) != SASL_OK) { - /* these are considered permanent failure codes */ HDEBUG(D_auth) - debug_printf("Cyrus SASL permanent failure %d (%s)\n", rc, sasl_errstring(rc, NULL, NULL)); + debug_printf("Cyrus SASL library will not tell us the username: %s\n", + sasl_errstring(rc, NULL, NULL)); log_write(0, LOG_REJECT, "%s authenticator (%s):\n " - "Cyrus SASL permanent failure: %s", ablock->name, ob->server_mech, + "Cyrus SASL username fetch problem: %s", ablock->name, ob->server_mech, sasl_errstring(rc, NULL, NULL)); sasl_dispose(&conn); sasl_done(); return FAIL; } - else if(rc==SASL_NOMECH) - { - /* this is a temporary failure, because the mechanism is not - * available for this user. If it wasn't available at all, we - * shouldn't have got here in the first place... - */ - HDEBUG(D_auth) - debug_printf("Cyrus SASL temporary failure %d (%s)\n", rc, sasl_errstring(rc, NULL, NULL)); - auth_defer_msg = - string_sprintf("Cyrus SASL: mechanism %s not available", ob->server_mech); - sasl_dispose(&conn); - sasl_done(); - return DEFER; - } - else if(!(rc==SASL_OK || rc==SASL_CONTINUE)) - { - /* Anything else is a temporary failure, and we'll let SASL print out - * the error string for us - */ - HDEBUG(D_auth) - debug_printf("Cyrus SASL temporary failure %d (%s)\n", rc, sasl_errstring(rc, NULL, NULL)); - auth_defer_msg = - string_sprintf("Cyrus SASL: %s", sasl_errstring(rc, NULL, NULL)); - sasl_dispose(&conn); - sasl_done(); - return DEFER; - } - else if(rc==SASL_OK) + auth_vars[0] = expand_nstring[1] = string_copy(out2); + expand_nlength[1] = Ustrlen(out2); + expand_nmax = 1; + + switch (rc) { - /* Get the username and copy it into $auth1 and $1. The former is now the - preferred variable; the latter is the original variable. */ - rc = sasl_getprop(conn, SASL_USERNAME, (const void **)(&out2)); - if (rc != SASL_OK) - { + case SASL_FAIL: case SASL_BUFOVER: case SASL_BADMAC: case SASL_BADAUTH: + case SASL_NOAUTHZ: case SASL_ENCRYPT: case SASL_EXPIRED: + case SASL_DISABLED: case SASL_NOUSER: + /* these are considered permanent failure codes */ HDEBUG(D_auth) - debug_printf("Cyrus SASL library will not tell us the username: %s\n", - sasl_errstring(rc, NULL, NULL)); + debug_printf("Cyrus SASL permanent failure %d (%s)\n", rc, sasl_errstring(rc, NULL, NULL)); log_write(0, LOG_REJECT, "%s authenticator (%s):\n " - "Cyrus SASL username fetch problem: %s", ablock->name, ob->server_mech, - sasl_errstring(rc, NULL, NULL)); + "Cyrus SASL permanent failure: %s", ablock->name, ob->server_mech, + sasl_errstring(rc, NULL, NULL)); sasl_dispose(&conn); sasl_done(); return FAIL; - } - - auth_vars[0] = expand_nstring[1] = string_copy(out2); - expand_nlength[1] = Ustrlen(expand_nstring[1]); - expand_nmax = 1; - HDEBUG(D_auth) - debug_printf("Cyrus SASL %s authentication succeeded for %s\n", - ob->server_mech, auth_vars[0]); - - rc = sasl_getprop(conn, SASL_SSF, (const void **)(&negotiated_ssf_ptr)); - if (rc != SASL_OK) - { + case SASL_NOMECH: + /* this is a temporary failure, because the mechanism is not + * available for this user. If it wasn't available at all, we + * shouldn't have got here in the first place... + */ HDEBUG(D_auth) - debug_printf("Cyrus SASL library will not tell us the SSF: %s\n", - sasl_errstring(rc, NULL, NULL)); - log_write(0, LOG_REJECT, "%s authenticator (%s):\n " - "Cyrus SASL SSF value not available: %s", ablock->name, ob->server_mech, - sasl_errstring(rc, NULL, NULL)); + debug_printf("Cyrus SASL temporary failure %d (%s)\n", rc, sasl_errstring(rc, NULL, NULL)); + auth_defer_msg = + string_sprintf("Cyrus SASL: mechanism %s not available", ob->server_mech); sasl_dispose(&conn); sasl_done(); - return FAIL; - } - negotiated_ssf = *negotiated_ssf_ptr; - HDEBUG(D_auth) - debug_printf("Cyrus SASL %s negotiated SSF: %d\n", ob->server_mech, negotiated_ssf); - if (negotiated_ssf > 0) - { + return DEFER; + + case SASL_OK: HDEBUG(D_auth) - debug_printf("Exim does not implement SASL wrapping (needed for SSF %d).\n", negotiated_ssf); - log_write(0, LOG_REJECT, "%s authenticator (%s):\n " - "Cyrus SASL SSF %d not supported by Exim", ablock->name, ob->server_mech, negotiated_ssf); + debug_printf("Cyrus SASL %s authentication succeeded for %s\n", + ob->server_mech, auth_vars[0]); + + if ((rc = sasl_getprop(conn, SASL_SSF, (const void **)(&negotiated_ssf_ptr)))!= SASL_OK) + { + HDEBUG(D_auth) + debug_printf("Cyrus SASL library will not tell us the SSF: %s\n", + sasl_errstring(rc, NULL, NULL)); + log_write(0, LOG_REJECT, "%s authenticator (%s):\n " + "Cyrus SASL SSF value not available: %s", ablock->name, ob->server_mech, + sasl_errstring(rc, NULL, NULL)); + sasl_dispose(&conn); + sasl_done(); + return FAIL; + } + negotiated_ssf = *negotiated_ssf_ptr; + HDEBUG(D_auth) + debug_printf("Cyrus SASL %s negotiated SSF: %d\n", ob->server_mech, negotiated_ssf); + if (negotiated_ssf > 0) + { + HDEBUG(D_auth) + debug_printf("Exim does not implement SASL wrapping (needed for SSF %d).\n", negotiated_ssf); + log_write(0, LOG_REJECT, "%s authenticator (%s):\n " + "Cyrus SASL SSF %d not supported by Exim", ablock->name, ob->server_mech, negotiated_ssf); + sasl_dispose(&conn); + sasl_done(); + return FAIL; + } + + /* close down the connection, freeing up library's memory */ sasl_dispose(&conn); sasl_done(); - return FAIL; - } - /* close down the connection, freeing up library's memory */ - sasl_dispose(&conn); - sasl_done(); + /* Expand server_condition as an authorization check */ + return auth_check_serv_cond(ablock); - /* Expand server_condition as an authorization check */ - return auth_check_serv_cond(ablock); + default: + /* Anything else is a temporary failure, and we'll let SASL print out + * the error string for us + */ + HDEBUG(D_auth) + debug_printf("Cyrus SASL temporary failure %d (%s)\n", rc, sasl_errstring(rc, NULL, NULL)); + auth_defer_msg = + string_sprintf("Cyrus SASL: %s", sasl_errstring(rc, NULL, NULL)); + sasl_dispose(&conn); + sasl_done(); + return DEFER; } } /* NOTREACHED */ diff --git a/test/confs/9300 b/test/confs/3800 similarity index 100% rename from test/confs/9300 rename to test/confs/3800 diff --git a/test/log/9300 b/test/log/3800 similarity index 67% rename from test/log/9300 rename to test/log/3800 index eb08483b8..b62ed3986 100644 --- a/test/log/9300 +++ b/test/log/3800 @@ -1,2 +1,4 @@ + +******** SERVER ******** 1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 -1999-03-02 09:44:33 sasl2 authenticator failed for (xxxx) [127.0.0.1]: 535 Incorrect authentication data +1999-03-02 09:44:33 sasl2 authenticator failed for (xxxx) [127.0.0.1]: 535 Incorrect authentication data (set_id=ph10) diff --git a/test/rejectlog/9300 b/test/rejectlog/3800 similarity index 66% rename from test/rejectlog/9300 rename to test/rejectlog/3800 index c802a9484..47511b748 100644 --- a/test/rejectlog/9300 +++ b/test/rejectlog/3800 @@ -1,3 +1,5 @@ + +******** SERVER ******** 1999-03-02 09:44:33 sasl2 authenticator (PLAIN): Cyrus SASL permanent failure: user not found -1999-03-02 09:44:33 sasl2 authenticator failed for (xxxx) [127.0.0.1]: 535 Incorrect authentication data +1999-03-02 09:44:33 sasl2 authenticator failed for (xxxx) [127.0.0.1]: 535 Incorrect authentication data (set_id=ph10) diff --git a/test/scripts/9300-Cyrus-SASL/9300 b/test/scripts/3800-Cyrus-SASL/3800 similarity index 96% rename from test/scripts/9300-Cyrus-SASL/9300 rename to test/scripts/3800-Cyrus-SASL/3800 index 4be6c17c0..a033ea0c6 100644 --- a/test/scripts/9300-Cyrus-SASL/9300 +++ b/test/scripts/3800-Cyrus-SASL/3800 @@ -8,6 +8,7 @@ EHLO xxxx ??? 250- ??? 250- ??? 250- +??? 250- ??? 250 AUTH PLAIN AHBoMTAAc2VjcmV0 ??? 535 diff --git a/test/scripts/9300-Cyrus-SASL/REQUIRES b/test/scripts/3800-Cyrus-SASL/REQUIRES similarity index 100% rename from test/scripts/9300-Cyrus-SASL/REQUIRES rename to test/scripts/3800-Cyrus-SASL/REQUIRES diff --git a/test/stdout/9300 b/test/stdout/3800 similarity index 93% rename from test/stdout/9300 rename to test/stdout/3800 index b9e6b6d54..998df3a19 100644 --- a/test/stdout/9300 +++ b/test/stdout/3800 @@ -7,6 +7,8 @@ Connecting to 127.0.0.1 port 1225 ... connected ??? 250- <<< 250-SIZE 52428800 ??? 250- +<<< 250-8BITMIME +??? 250- <<< 250-PIPELINING ??? 250- <<< 250-AUTH ANONYMOUS PLAIN @@ -18,7 +20,7 @@ Connecting to 127.0.0.1 port 1225 ... connected >>> AUTH ANONYMOUS ??? 334 <<< 334 ->>> CALLER +>>> ph10 ??? 235 <<< 235 Authentication succeeded >>> quit -- 2.30.2