From: Jeremy Harris Date: Wed, 21 Nov 2018 00:50:38 +0000 (+0000) Subject: Fix cyrus-sasl authenticator for $authenticated_fail_id. Bug 2238 X-Git-Tag: exim-4.92-RC1~22 X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=c0fb53b74e;p=exim.git Fix cyrus-sasl authenticator for $authenticated_fail_id. Bug 2238 --- diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 240781223..f575a10e1 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -151,6 +151,10 @@ JH/32 For main options check_spool_space and check_inode_space, where the (i.e. more than 2 TB). Accept E, P and T multipliers in addition to the previous G, M, k. +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 2cfed7a4b..546c20beb 100644 --- a/src/src/auths/cyrus_sasl.c +++ b/src/src/auths/cyrus_sasl.c @@ -118,13 +118,13 @@ uschar *rs_point, *expanded_hostname; char *realm_expanded; sasl_conn_t *conn; -sasl_callback_t cbs[]={ +sasl_callback_t cbs[] = { {SASL_CB_GETOPT, NULL, NULL }, {SASL_CB_LIST_END, NULL, NULL}}; /* default the mechanism to our "public name" */ -if(ob->server_mech == NULL) - ob->server_mech=string_copy(ablock->public_name); +if (ob->server_mech == NULL) + ob->server_mech = string_copy(ablock->public_name); expanded_hostname = expand_string(ob->server_hostname); if (expanded_hostname == NULL) @@ -132,7 +132,7 @@ if (expanded_hostname == NULL) "couldn't expand server_hostname [%s]: %s", ablock->name, ob->server_hostname, expand_string_message); -realm_expanded=NULL; +realm_expanded = NULL; if (ob->server_realm != NULL) { realm_expanded = CS expand_string(ob->server_realm); if (realm_expanded == NULL) @@ -148,45 +148,42 @@ if (ob->server_realm != NULL) { cbs[0].proc = (int(*)(void)) &mysasl_config; cbs[0].context = ob->server_mech; -rc=sasl_server_init(cbs, "exim"); - -if( rc != SASL_OK ) +if ((rc = sasl_server_init(cbs, "exim")) != SASL_OK ) log_write(0, LOG_PANIC_DIE|LOG_CONFIG_FOR, "%s authenticator: " "couldn't initialise Cyrus SASL library.", ablock->name); -rc=sasl_server_new(CS ob->server_service, CS expanded_hostname, - realm_expanded, NULL, NULL, NULL, 0, &conn); -if( rc != SASL_OK ) +if ((rc = sasl_server_new(CS ob->server_service, CS expanded_hostname, + realm_expanded, NULL, NULL, NULL, 0, &conn)) != SASL_OK ) log_write(0, LOG_PANIC_DIE|LOG_CONFIG_FOR, "%s authenticator: " "couldn't initialise Cyrus SASL server connection.", ablock->name); -rc=sasl_listmech(conn, NULL, "", ":", "", (const char **)&list, &len, &i); -if( rc != SASL_OK ) +if ((rc = sasl_listmech(conn, NULL, "", ":", "", (const char **)&list, &len, &i)) != SASL_OK ) log_write(0, LOG_PANIC_DIE|LOG_CONFIG_FOR, "%s authenticator: " "couldn't get Cyrus SASL mechanism list.", ablock->name); -i=':'; -listptr=list; +i = ':'; +listptr = list; -HDEBUG(D_auth) { +HDEBUG(D_auth) + { debug_printf("Initialised Cyrus SASL service=\"%s\" fqdn=\"%s\" realm=\"%s\"\n", ob->server_service, expanded_hostname, realm_expanded); debug_printf("Cyrus SASL knows mechanisms: %s\n", list); -} + } /* the store_get / store_reset mechanism is hierarchical * the hierarchy is stored for us behind our back. This point * creates a hierarchy point for this function. */ -rs_point=store_get(0); +rs_point = store_get(0); /* loop until either we get to the end of the list, or we match the * public name of this authenticator */ -while( ( buffer = string_nextinlist(&listptr, &i, NULL, 0) ) && +while ( ( buffer = string_nextinlist(&listptr, &i, NULL, 0) ) && strcmpic(buffer,ob->server_mech) ); -if(!buffer) +if (!buffer) log_write(0, LOG_PANIC_DIE|LOG_CONFIG_FOR, "%s authenticator: " "Cyrus SASL doesn't know about mechanism %s.", ablock->name, ob->server_mech); @@ -218,54 +215,48 @@ auth_cyrus_sasl_options_block *ob = (auth_cyrus_sasl_options_block *)(ablock->options_block); uschar *output, *out2, *input, *clear, *hname; uschar *debug = NULL; /* Stops compiler complaining */ -sasl_callback_t cbs[]={{SASL_CB_LIST_END, NULL, NULL}}; +sasl_callback_t cbs[] = {{SASL_CB_LIST_END, NULL, NULL}}; sasl_conn_t *conn; -char *realm_expanded; -int rc, i, firsttime=1, clen, *negotiated_ssf_ptr=NULL, negotiated_ssf; +char * realm_expanded = NULL; +int rc, i, firsttime = 1, clen, *negotiated_ssf_ptr = NULL, negotiated_ssf; unsigned int inlen, outlen; -input=data; -inlen=Ustrlen(data); +input = data; +inlen = Ustrlen(data); -HDEBUG(D_auth) debug=string_copy(data); +HDEBUG(D_auth) debug = string_copy(data); -hname=expand_string(ob->server_hostname); -realm_expanded=NULL; +hname = expand_string(ob->server_hostname); if (hname && ob->server_realm) - realm_expanded= CS expand_string(ob->server_realm); -if((hname == NULL) || - ((realm_expanded == NULL) && (ob->server_realm != NULL))) + realm_expanded = CS expand_string(ob->server_realm); +if (!hname || !realm_expanded && ob->server_realm) { auth_defer_msg = expand_string_message; return DEFER; } -if(inlen) +if (inlen) { - clen = b64decode(input, &clear); - if(clen < 0) - { + if ((clen = b64decode(input, &clear)) < 0) return BAD64; - } - input=clear; - inlen=clen; + input = clear; + inlen = clen; } -rc=sasl_server_init(cbs, "exim"); -if (rc != SASL_OK) +if ((rc = sasl_server_init(cbs, "exim")) != SASL_OK) { auth_defer_msg = US"couldn't initialise Cyrus SASL library"; return DEFER; } -rc=sasl_server_new(CS ob->server_service, CS hname, realm_expanded, NULL, +rc = sasl_server_new(CS ob->server_service, CS hname, realm_expanded, NULL, NULL, NULL, 0, &conn); HDEBUG(D_auth) debug_printf("Initialised Cyrus SASL server connection; service=\"%s\" fqdn=\"%s\" realm=\"%s\"\n", ob->server_service, hname, realm_expanded); -if( rc != SASL_OK ) +if (rc != SASL_OK ) { auth_defer_msg = US"couldn't initialise Cyrus SASL connection"; sasl_done(); @@ -274,8 +265,7 @@ if( rc != SASL_OK ) if (tls_in.cipher) { - rc = sasl_setprop(conn, SASL_SSF_EXTERNAL, (sasl_ssf_t *) &tls_in.bits); - if (rc != SASL_OK) + if ((rc = sasl_setprop(conn, SASL_SSF_EXTERNAL, (sasl_ssf_t *) &tls_in.bits)) != SASL_OK) { HDEBUG(D_auth) debug_printf("Cyrus SASL EXTERNAL SSF set %d failed: %s\n", tls_in.bits, sasl_errstring(rc, NULL, NULL)); @@ -298,7 +288,7 @@ inet_ntop which we wrap in our host_ntoa() function. So the docs are too strict and we shouldn't worry about :: contractions. */ /* Set properties for remote and local host-ip;port */ -for (i=0; i < 2; ++i) +for (i = 0; i < 2; ++i) { struct sockaddr_storage ss; int (*query)(int, struct sockaddr *, socklen_t *); @@ -322,8 +312,7 @@ for (i=0; i < 2; ++i) } sslen = sizeof(ss); - rc = query(fileno(smtp_in), (struct sockaddr *) &ss, &sslen); - if (rc < 0) + if ((rc = query(fileno(smtp_in), (struct sockaddr *) &ss, &sslen)) < 0) { HDEBUG(D_auth) debug_printf("Failed to get %s address information: %s\n", @@ -334,8 +323,7 @@ for (i=0; i < 2; ++i) address = host_ntoa(-1, &ss, NULL, &port); address_port = string_sprintf("%s;%d", address, port); - rc = sasl_setprop(conn, propnum, address_port); - if (rc != SASL_OK) + if ((rc = sasl_setprop(conn, propnum, address_port)) != SASL_OK) { s_err = sasl_errdetail(conn); HDEBUG(D_auth) @@ -347,24 +335,21 @@ for (i=0; i < 2; ++i) label, address_port); } -rc=SASL_CONTINUE; - -while(rc==SASL_CONTINUE) +for (rc = SASL_CONTINUE; rc == SASL_CONTINUE; ) { - if(firsttime) + if (firsttime) { - firsttime=0; + firsttime = 0; HDEBUG(D_auth) debug_printf("Calling sasl_server_start(%s,\"%s\")\n", ob->server_mech, debug); - rc=sasl_server_start(conn, CS ob->server_mech, inlen?CS input:NULL, inlen, + rc = sasl_server_start(conn, CS ob->server_mech, inlen?CS input:NULL, inlen, (const char **)(&output), &outlen); } else { /* make sure that we have a null-terminated string */ - out2=store_get(outlen+1); - memcpy(out2,output,outlen); - out2[outlen]='\0'; - if((rc=auth_get_data(&input, out2, outlen))!=OK) + out2 = string_copyn(output, outlen); + + if ((rc = auth_get_data(&input, out2, outlen)) != OK) { /* we couldn't get the data, so free up the library before * returning whatever error we get */ @@ -372,133 +357,130 @@ while(rc==SASL_CONTINUE) sasl_done(); return rc; } - inlen=Ustrlen(input); + inlen = Ustrlen(input); - HDEBUG(D_auth) debug=string_copy(input); - if(inlen) + HDEBUG(D_auth) debug = string_copy(input); + if (inlen) { - clen = b64decode(input, &clear); - if(clen < 0) + if ((clen = b64decode(input, &clear)) < 0) { - sasl_dispose(&conn); - sasl_done(); + sasl_dispose(&conn); + sasl_done(); return BAD64; } - input=clear; - inlen=clen; + input = clear; + inlen = clen; } HDEBUG(D_auth) debug_printf("Calling sasl_server_step(\"%s\")\n", debug); - rc=sasl_server_step(conn, CS input, inlen, (const char **)(&output), &outlen); + rc = sasl_server_step(conn, CS input, inlen, (const char **)(&output), &outlen); } - if(rc==SASL_BADPROT) + + if (rc == SASL_BADPROT) { sasl_dispose(&conn); 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 */ @@ -512,12 +494,12 @@ return 0; /* Stop compiler complaints */ void auth_cyrus_sasl_version_report(FILE *f) { - const char *implementation, *version; - sasl_version_info(&implementation, &version, NULL, NULL, NULL, NULL); - fprintf(f, "Library version: Cyrus SASL: Compile: %d.%d.%d\n" - " Runtime: %s [%s]\n", - SASL_VERSION_MAJOR, SASL_VERSION_MINOR, SASL_VERSION_STEP, - version, implementation); +const char *implementation, *version; +sasl_version_info(&implementation, &version, NULL, NULL, NULL, NULL); +fprintf(f, "Library version: Cyrus SASL: Compile: %d.%d.%d\n" + " Runtime: %s [%s]\n", + SASL_VERSION_MAJOR, SASL_VERSION_MINOR, SASL_VERSION_STEP, + version, implementation); } /************************************************* 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