Merge ~sergiodj/ubuntu/+source/memcached:sasl-configuration-wrong-path-bug1878721-eoan into ubuntu/+source/memcached:ubuntu/eoan-devel

[Sergio Durigan Junior]

This is a simple regression that happens on memcached. The SASL configuration is being read from an erroneous file, '/etc/sasl2/memcached.conf/memcached.conf'. Upstream fixed this bug and made memcached load the configuration from the right file, '/etc/sasl2/memcached.conf', but kept the old behaviour (i.e., reading from the wrong location) just in case users have made a workaround.

The reporter was thorough and provided the debdiff along with an SRU template, so one could say I'm filing this MP for him. I kept his authorship intact.

There is a PPA with the proposed change here:

https://launchpad.net/~sergiodj/+archive/ubuntu/memcached-bug1878721

autopkgtest is still happy:

autopkgtest [09:22:43]: test client.pl: -----------------------]
autopkgtest [09:22:44]: test client.pl: - - - - - - - - - - results - - - - - - - - - -
client.pl PASS
autopkgtest [09:22:45]: @@@@@@@@@@@@@@@@@@@@ summary
daemon PASS
client.pl PASS

[Anders Kaseorg (andersk)]

review: Approve

[Bryce Harrington (bryce)]

* Changelog:
  - [√] old content and logical tag match as expected
  - [√] changelog entry correct version and targeted codename
  - [√] changelog entries correct
  - [√] update-maintainer has been run

* Actual changes:
  - [-] no upstream changes to consider
  - [-] no further upstream version to consider
  - [√] debian changes look safe

* Old Delta:
  - [-] dropped changes are ok to be dropped
  - [-] nothing else to drop
  - [√] changes forwarded upstream/debian (if appropriate)

* New Delta:
  - [-] no new patches added
  - [√] patches match what was proposed upstream
  - [√] patches correctly included in debian/patches/series
  - [√] patches have correct DEP3 metadata

* Build/Test:
  - [√] build is ok
  - [√] verified PPA package installs/uninstalls
  - [√] autopkgtest against the PPA package passes
  - [x] sanity checks test fine

See comments on the focal branch.

[Bryce Harrington (bryce)]

review: Needs Fixing

[Bryce Harrington (bryce)]

$ git ubuntu tag --upload
$ git push pkg upload/1.5.10-0ubuntu3.1
Enumerating objects: 16, done.
Counting objects: 100% (16/16), done.
Delta compression using up to 12 threads
Compressing objects: 100% (11/11), done.
Writing objects: 100% (11/11), 3.52 KiB | 1.17 MiB/s, done.
Total 11 (delta 7), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/memcached
 * [new tag] upload/1.5.10-0ubuntu3.1 -> upload/1.5.10-0ubuntu3.1
$ dput ubuntu ../memcached_1.5.10-0ubuntu3.1_source.changes
Checking signature on .changes
gpg: ../memcached_1.5.10-0ubuntu3.1_source.changes: Valid signature from E603B2578FB8F0FB
Checking signature on .dsc
gpg: ../memcached_1.5.10-0ubuntu3.1.dsc: Valid signature from E603B2578FB8F0FB
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading memcached_1.5.10-0ubuntu3.1.dsc: done.
  Uploading memcached_1.5.10-0ubuntu3.1.debian.tar.xz: done.
  Uploading memcached_1.5.10-0ubuntu3.1_source.buildinfo: done.
  Uploading memcached_1.5.10-0ubuntu3.1_source.changes: done.
Successfully uploaded packages.

review: Approve

diff --git a/debian/changelog b/debian/changelog
index 6df3b73..b0194fd 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,21 @@
+memcached (1.5.10-0ubuntu3.1) eoan; urgency=medium
+
+  * d/p/fix-bug-where-sasl-will-load-config-the-wrong-path.patch:
+    Fix the path from which SASL configuration is loaded.  (LP: #1878721)
+    The bug happened because sasl expects memcached to provide a
+    path (i.e., a directory, not a filename) where the sasl
+    configuration file(s) is (are).  However, memcached was passing
+    the filename (/etc/sasl2/memcached.conf) to sasl, which was
+    interpreting it as a directory, and looking for a configuration
+    file inside it (i.e., /etc/sasl2/memcached.conf/memcached.conf).
+    Users could workaround this bug by creating a directory named
+    /etc/sasl2/memcached.conf/, and putting the configuration file
+    inside it.  This patch not only fixes this bug (by passing the
+    right directory, /etc/sasl2/, to sasl) but also supports the
+    workaround described above.
+
+ -- Anders Kaseorg <[e-mail]>  Thu, 14 May 2020 17:13:17 -0700
+
 memcached (1.5.10-0ubuntu3) eoan; urgency=medium
 
   * SECURITY UPDATE: stack-based buffer over-read
diff --git a/debian/patches/fix-bug-where-sasl-will-load-config-the-wrong-path.patch b/debian/patches/fix-bug-where-sasl-will-load-config-the-wrong-path.patch
new file mode 100644
index 0000000..3901042
--- /dev/null
+++ b/debian/patches/fix-bug-where-sasl-will-load-config-the-wrong-path.patch
@@ -0,0 +1,103 @@
+From: Zheng Gu <[e-mail]>
+Date: Fri, 22 Nov 2019 22:34:16 +0800
+Subject: fix bug where sasl will load config the wrong path
+
+/etc/sasl2/memcached.conf/memcached.conf instead of
+/etc/sasl2/memcached.conf
+
+This bug was caused by two upstream commits:
+
+  https://github.com/memcached/memcached/commit/39151c870c5e598f039714bdb790bd46f614856e
+  https://github.com/memcached/memcached/commit/80dd99d831535ddeec73d55a0adcaeaac8cb7298
+
+Now, the reason for the bug is interesting.  Before the commits
+mentioned above, we had a big part of the sasl_defs.c file practically
+inactive due to the "#ifdef HAVE_SASL_CB_GETCONF" check (which fails
+when compiling with cyrrus-sasl), including the definition of the
+"sasl_getconf" function, which is the source of this bug, and which
+implements the "sasl_getconfpath_t" callback used by sasl to get the
+configuration path to be used.  According to sasl_getconfpath_t(3):
+
+  sasl_getconfpath_t is used if the application wishes to use a
+  different location for the SASL configuration files. If this callback
+  is not used SASL will either use the location in the environment
+  variable SASL_CONF_PATH (provided we are not SUID or SGID) or
+  /etc/sasl2 by default.
+
+Which means that, before both commits existed, memcached was always
+using /etc/sasl2 as the config path.
+
+When the commits were incorporated into the repo, memcached started
+using "sasl_getconf" for both the HAVE_SASL_CB_GETCONF and the
+HAVE_SASL_CB_GETCONFPATH cases, but the function only works is the sasl
+being used supports SASL_CB_GETCONF, because it passes the full path
+(including the filename) back to the sasl library for processing.  And
+this is the reason why the sasl library started getting
+"/etc/sasl2/memcached.conf/" as the config path.
+
+Author: Zheng Gu <[e-mail]>
+Origin: upstream, https://github.com/memcached/memcached/commit/6207330c2705fdb5f02de13b99a0d994f7c4f14a
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/memcached/+bug/1878721
+Last-Update: 2020-05-26
+Applied-Upstream: 1.6.0
+Reviewed-by: Sergio Durigan Junior <[e-mail]>
+---
+ sasl_defs.c | 29 +++++++++++++++++++++++++++++
+ 1 file changed, 29 insertions(+)
+
+diff --git a/sasl_defs.c b/sasl_defs.c
+index c60d1bf..370f947 100644
+--- a/sasl_defs.c
++++ b/sasl_defs.c
+@@ -16,6 +16,23 @@ const char * const locations[] = {
+     "/etc/sasl2/memcached.conf",
+     NULL
+ };
++
++/* If the element of locations is file, locations_dir_path stores the
++ * directory path of these elements */
++const char *const locations_dir_path[] = {
++    "/etc/sasl",
++    "/etc/sasl2",
++    NULL
++};
++
++/* If the element of locations is directory, locations_file_path stores
++ * the actual configue file which used by sasl, when GETCONFPATH is
++ * enabled */
++const char *const locations_file_path[] = {
++    "/etc/sasl/memcached.conf/memcached.conf",
++    "/etc/sasl2/memcached.conf/memcached.conf",
++    NULL
++};
+ #endif
+ 
+ #ifndef HAVE_SASL_CALLBACK_FT
+@@ -88,12 +105,24 @@ static int sasl_getconf(void *context, const char **path)
+     *path = getenv("SASL_CONF_PATH");
+ 
+     if (*path == NULL) {
++#if defined(HAVE_SASL_CB_GETCONF)
+         for (int i = 0; locations[i] != NULL; ++i) {
+             if (access(locations[i], F_OK) == 0) {
+                 *path = locations[i];
+                 break;
+             }
+         }
++#elif defined(HAVE_SASL_CB_GETCONFPATH)
++        for (int i = 0; locations[i] != NULL; ++i) {
++            if (access(locations_file_path[i], F_OK) == 0) {
++                *path = locations[i];
++                break;
++            } else if (access(locations[i], F_OK) == 0) {
++                *path = locations_dir_path[i];
++                break;
++            }
++        }
++#endif
+     }
+ 
+     if (settings.verbose) {
+-- 
+2.26.2
+
diff --git a/debian/patches/series b/debian/patches/series
index 71207c2..d6e04cf 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -4,3 +4,4 @@
 07_disable_tests.patch
 CVE-2019-11596.patch
 CVE-2019-15026.patch
+fix-bug-where-sasl-will-load-config-the-wrong-path.patch