Fix cyrus-sasl authenticator for $authenticated_fail_id. Bug 2338
authorJeremy Harris <jgh146exb@wizmail.org>
Sat, 24 Nov 2018 16:01:42 +0000 (16:01 +0000)
committerJeremy Harris <jgh146exb@wizmail.org>
Sat, 24 Nov 2018 16:05:12 +0000 (16:05 +0000)
Cherry-picked from: 49ff55b6a9

14 files changed:
doc/doc-txt/ChangeLog
src/src/auths/cyrus_sasl.c
test/confs/3800 [new file with mode: 0644]
test/confs/9300 [deleted file]
test/log/3800 [new file with mode: 0644]
test/log/9300 [deleted file]
test/rejectlog/3800 [new file with mode: 0644]
test/rejectlog/9300 [deleted file]
test/scripts/3800-Cyrus-SASL/3800 [new file with mode: 0644]
test/scripts/3800-Cyrus-SASL/REQUIRES [new file with mode: 0644]
test/scripts/9300-Cyrus-SASL/9300 [deleted file]
test/scripts/9300-Cyrus-SASL/REQUIRES [deleted file]
test/stdout/3800 [new file with mode: 0644]
test/stdout/9300 [deleted file]

index 77e56c16d5b3692be18d5a34510d28cc642dc820..da4e1a8a703c76e3f16c1f26ba7cd6f0f0f69fee 100644 (file)
@@ -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
 -----------------
index 7bb86b815f658ee81574f0548b2dd2bec0a2ec57..e8c28ec17dc3e05c792917e55c7c45068e39c895 100644 (file)
@@ -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/3800 b/test/confs/3800
new file mode 100644 (file)
index 0000000..086b506
--- /dev/null
@@ -0,0 +1,26 @@
+# Exim test configuration 9300
+
+SERVER=
+
+.include DIR/aux-var/std_conf_prefix
+
+primary_hostname = myhost.test.ex
+
+# ----- Main settings -----
+
+
+# ----- Authentication -----
+
+begin authenticators
+
+sasl1:
+  driver = cyrus_sasl
+  public_name = ANONYMOUS
+  server_set_id = $auth1
+
+sasl2:
+  driver = cyrus_sasl
+  public_name = PLAIN
+  server_set_id = $auth1
+
+# End
diff --git a/test/confs/9300 b/test/confs/9300
deleted file mode 100644 (file)
index 086b506..0000000
+++ /dev/null
@@ -1,26 +0,0 @@
-# Exim test configuration 9300
-
-SERVER=
-
-.include DIR/aux-var/std_conf_prefix
-
-primary_hostname = myhost.test.ex
-
-# ----- Main settings -----
-
-
-# ----- Authentication -----
-
-begin authenticators
-
-sasl1:
-  driver = cyrus_sasl
-  public_name = ANONYMOUS
-  server_set_id = $auth1
-
-sasl2:
-  driver = cyrus_sasl
-  public_name = PLAIN
-  server_set_id = $auth1
-
-# End
diff --git a/test/log/3800 b/test/log/3800
new file mode 100644 (file)
index 0000000..b62ed39
--- /dev/null
@@ -0,0 +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 (set_id=ph10)
diff --git a/test/log/9300 b/test/log/9300
deleted file mode 100644 (file)
index eb08483..0000000
+++ /dev/null
@@ -1,2 +0,0 @@
-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
diff --git a/test/rejectlog/3800 b/test/rejectlog/3800
new file mode 100644 (file)
index 0000000..47511b7
--- /dev/null
@@ -0,0 +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 (set_id=ph10)
diff --git a/test/rejectlog/9300 b/test/rejectlog/9300
deleted file mode 100644 (file)
index c802a94..0000000
+++ /dev/null
@@ -1,3 +0,0 @@
-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
diff --git a/test/scripts/3800-Cyrus-SASL/3800 b/test/scripts/3800-Cyrus-SASL/3800
new file mode 100644 (file)
index 0000000..a033ea0
--- /dev/null
@@ -0,0 +1,23 @@
+# Cyrus SASL authentication (server only)
+exim -DSERVER=server -bd -oX PORT_D
+****
+client 127.0.0.1 PORT_D
+??? 220
+EHLO xxxx
+??? 250-
+??? 250-
+??? 250-
+??? 250-
+??? 250-
+??? 250
+AUTH PLAIN AHBoMTAAc2VjcmV0
+??? 535
+AUTH ANONYMOUS
+??? 334
+ph10
+??? 235
+quit
+??? 221
+****
+killdaemon
+no_msglog_check
diff --git a/test/scripts/3800-Cyrus-SASL/REQUIRES b/test/scripts/3800-Cyrus-SASL/REQUIRES
new file mode 100644 (file)
index 0000000..f6c49fc
--- /dev/null
@@ -0,0 +1 @@
+authenticator cyrus_sasl
diff --git a/test/scripts/9300-Cyrus-SASL/9300 b/test/scripts/9300-Cyrus-SASL/9300
deleted file mode 100644 (file)
index 4be6c17..0000000
+++ /dev/null
@@ -1,22 +0,0 @@
-# Cyrus SASL authentication (server only)
-exim -DSERVER=server -bd -oX PORT_D
-****
-client 127.0.0.1 PORT_D
-??? 220
-EHLO xxxx
-??? 250-
-??? 250-
-??? 250-
-??? 250-
-??? 250
-AUTH PLAIN AHBoMTAAc2VjcmV0
-??? 535
-AUTH ANONYMOUS
-??? 334
-ph10
-??? 235
-quit
-??? 221
-****
-killdaemon
-no_msglog_check
diff --git a/test/scripts/9300-Cyrus-SASL/REQUIRES b/test/scripts/9300-Cyrus-SASL/REQUIRES
deleted file mode 100644 (file)
index f6c49fc..0000000
+++ /dev/null
@@ -1 +0,0 @@
-authenticator cyrus_sasl
diff --git a/test/stdout/3800 b/test/stdout/3800
new file mode 100644 (file)
index 0000000..998df3a
--- /dev/null
@@ -0,0 +1,29 @@
+Connecting to 127.0.0.1 port 1225 ... connected
+??? 220
+<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
+>>> EHLO xxxx
+??? 250-
+<<< 250-myhost.test.ex Hello xxxx [127.0.0.1]
+??? 250-
+<<< 250-SIZE 52428800
+??? 250-
+<<< 250-8BITMIME
+??? 250-
+<<< 250-PIPELINING
+??? 250-
+<<< 250-AUTH ANONYMOUS PLAIN
+??? 250
+<<< 250 HELP
+>>> AUTH PLAIN AHBoMTAAc2VjcmV0
+??? 535
+<<< 535 Incorrect authentication data
+>>> AUTH ANONYMOUS
+??? 334
+<<< 334 
+>>> ph10
+??? 235
+<<< 235 Authentication succeeded
+>>> quit
+??? 221
+<<< 221 myhost.test.ex closing connection
+End of script
diff --git a/test/stdout/9300 b/test/stdout/9300
deleted file mode 100644 (file)
index b9e6b6d..0000000
+++ /dev/null
@@ -1,27 +0,0 @@
-Connecting to 127.0.0.1 port 1225 ... connected
-??? 220
-<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
->>> EHLO xxxx
-??? 250-
-<<< 250-myhost.test.ex Hello xxxx [127.0.0.1]
-??? 250-
-<<< 250-SIZE 52428800
-??? 250-
-<<< 250-PIPELINING
-??? 250-
-<<< 250-AUTH ANONYMOUS PLAIN
-??? 250
-<<< 250 HELP
->>> AUTH PLAIN AHBoMTAAc2VjcmV0
-??? 535
-<<< 535 Incorrect authentication data
->>> AUTH ANONYMOUS
-??? 334
-<<< 334 
->>> CALLER
-??? 235
-<<< 235 Authentication succeeded
->>> quit
-??? 221
-<<< 221 myhost.test.ex closing connection
-End of script