From 5c329a4388e7113925109e093e8cbb12ddf6fa8b Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 13 Feb 2020 13:43:45 +0000 Subject: [PATCH] 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 3b160cb86..a5367f960 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 4b4602fda..59fbeefcf 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 d6ddc0111..28592a1a0 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 8a05a82e4..7475588ba 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 f6d09d5ab..886f3f28f 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; -- 2.30.2