Auths: fix cyrus-sasl driver for gssapi use. Bug 2524
authorJeremy Harris <jgh146exb@wizmail.org>
Thu, 13 Feb 2020 13:43:45 +0000 (13:43 +0000)
committerJeremy Harris <jgh146exb@wizmail.org>
Thu, 13 Feb 2020 13:47:46 +0000 (13:47 +0000)
Broken-by: c0fb53b74e
doc/doc-txt/ChangeLog
src/src/auths/cram_md5.c
src/src/auths/cyrus_sasl.c
src/src/auths/get_data.c
src/src/auths/heimdal_gssapi.c

index 3b160cb..a5367f9 100644 (file)
@@ -115,6 +115,12 @@ JH/23 Performance improvement in the initial phase of a two-pass queue run.  By
       queue_run_in_order means we cannot do this, as ordering becomes
       indeterminate.
 
+JH/24 Bug 2524: fix the cyrus_sasl auth driver gssapi usage.  A previous fix
+      had introduced a string-copy (for ensuring NUL-termination) which was not
+      appropriate for that case, which can include embedded NUL bytes in the
+      block of data.  Investigation showed the copy to actually be needless, the
+      data being length-specified.
+
 
 Exim version 4.93
 -----------------
index 4b4602f..59fbeef 100644 (file)
@@ -179,7 +179,7 @@ if (f.running_in_test_harness)
 
 /* No data should have been sent with the AUTH command */
 
-if (*data != 0) return UNEXPECTED;
+if (*data) return UNEXPECTED;
 
 /* Send the challenge, read the return */
 
@@ -192,7 +192,7 @@ The former is now the preferred variable; the latter is the original one. Then
 check that the remaining length is 32. */
 
 auth_vars[0] = expand_nstring[1] = clear;
-while (*clear != 0 && !isspace(*clear)) clear++;
+while (*clear && !isspace(*clear)) clear++;
 if (!isspace(*clear)) return FAIL;
 *clear++ = 0;
 
index d6ddc01..28592a1 100644 (file)
@@ -330,10 +330,10 @@ for (rc = SASL_CONTINUE; rc == SASL_CONTINUE; )
     }
   else
     {
-    /* make sure that we have a null-terminated string */
-    out2 = string_copyn(output, outlen);
+    /* auth_get_data() takes a length-specfied block of binary
+    which can include zeroes; no terminating NUL is needed */
 
-    if ((rc = auth_get_data(&input, out2, outlen)) != OK)
+    if ((rc = auth_get_data(&input, output, outlen)) != OK)
       {
       /* we couldn't get the data, so free up the library before
       returning whatever error we get */
@@ -372,7 +372,7 @@ for (rc = SASL_CONTINUE; rc == SASL_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)
+  if ((sasl_getprop(conn, SASL_USERNAME, (const void **)&out2)) != SASL_OK)
     {
     HDEBUG(D_auth)
       debug_printf("Cyrus SASL library will not tell us the username: %s\n",
index 8a05a82..7475588 100644 (file)
@@ -53,13 +53,13 @@ return OK;
 *      Issue a challenge and get a response      *
 *************************************************/
 
-/* This function is used by authentication drivers to output a challenge
-to the SMTP client and read the response line.
+/* This function is used by authentication drivers to b64-encode and
+output a challenge to the SMTP client, and read the response line.
 
 Arguments:
    aptr       set to point to the response (which is in big_buffer)
-   challenge  the challenge text (unencoded, may be binary)
-   challen    the length of the challenge text
+   challenge  the challenge data (unencoded, may be binary)
+   challen    the length of the challenge data, in bytes
 
 Returns:      OK on success
               BAD64 if response too large for buffer
index f6d09d5..886f3f2 100644 (file)
@@ -321,8 +321,7 @@ while (step < 4)
          }
 
        HDEBUG(D_auth) debug_printf("gssapi: missing initial response, nudging.\n");
-       error_out = auth_get_data(&from_client, US"", 0);
-       if (error_out != OK)
+       if ((error_out = auth_get_data(&from_client, US"", 0)) != OK)
          goto ERROR_OUT;
        handled_empty_ir = TRUE;
        continue;