[exim-cvs] Auths: fix cyrus-sasl driver for gssapi use. Bug…

Gitweb: https://git.exim.org/exim.git/commitdiff/5c329a4388e7113925109e093e8cbb12ddf6fa8b

Commit:     5c329a4388e7113925109e093e8cbb12ddf6fa8b
Parent:     d89f32c2711de4e1272b8132a13bf98b9a17580d
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Thu Feb 13 13:43:45 2020 +0000
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Thu Feb 13 13:47:46 2020 +0000

    Auths: fix cyrus-sasl driver for gssapi use.  Bug 2524

    Broken-by: c0fb53b74e
---
 doc/doc-txt/ChangeLog          | 6 ++++++
 src/src/auths/cram_md5.c       | 4 ++--
 src/src/auths/cyrus_sasl.c     | 8 ++++----
 src/src/auths/get_data.c       | 8 ++++----
 src/src/auths/heimdal_gssapi.c | 3 +--
 5 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 3b160cb..a5367f9 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -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
-----------------
diff --git a/src/src/auths/cram_md5.c b/src/src/auths/cram_md5.c
index 4b4602f..59fbeef 100644
--- a/src/src/auths/cram_md5.c
+++ b/src/src/auths/cram_md5.c
@@ -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;

diff --git a/src/src/auths/cyrus_sasl.c b/src/src/auths/cyrus_sasl.c
index d6ddc01..28592a1 100644
--- a/src/src/auths/cyrus_sasl.c
+++ b/src/src/auths/cyrus_sasl.c
@@ -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",
diff --git a/src/src/auths/get_data.c b/src/src/auths/get_data.c
index 8a05a82..7475588 100644
--- a/src/src/auths/get_data.c
+++ b/src/src/auths/get_data.c
@@ -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
diff --git a/src/src/auths/heimdal_gssapi.c b/src/src/auths/heimdal_gssapi.c
index f6d09d5..886f3f2 100644
--- a/src/src/auths/heimdal_gssapi.c
+++ b/src/src/auths/heimdal_gssapi.c
@@ -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;