Security update 10.1.30-0ubuntu0.17.10.1 regresses smoke test, mariadb not started upon install

[Dimitri John Ledkov]

[Impact]

- In debian/rules, -pie introduced, yet previously the build was fully hardened. (I believe this is cosmetic, as supposedly the build system defaults to pie anyway)

- There used to be a default client socket set, but not anymore.

- Most importantly, the dh_systemd_start workarounds were dropped; yet are still needed on artful, as otherwise after server installation the unit is not started / not running

- Due to last one, autopkgtests are failing - see http://autopkgtest.ubuntu.com/packages/mariadb-10.1 and thus blocking releasing other SRUs, e.g. systemd one.

[Solution]

- perform partial revert of packaging changes, back to what it was before (all the hacks & workarounds included)

[Testcase]

- autopkgtests should pass for mariadb-10.1 package, specifically the smoke test case

[Regression Potential]
Reverting packaging changes introduced in the last update; back to what they were before and what they are in the release pocket. This revert may have been done incorrectly, thus yielding more maintainer script bugs, but interactive testing of this revert appears to be functioning correctly.

[Original Bug report]

10.1.30-0ubuntu0.17.10.1 upload of mariadb-10.1 regresses autopkgtest smoke test like so:

Setting up mariadb-server-core-10.1 (10.1.30-0ubuntu0.17.10.1) ...
Setting up mariadb-client-core-10.1 (10.1.30-0ubuntu0.17.10.1) ...
Setting up mariadb-client-10.1 (10.1.30-0ubuntu0.17.10.1) ...
Setting up mariadb-server-10.1 (10.1.30-0ubuntu0.17.10.1) ...
Created symlink /etc/systemd/system/mysql.service → /lib/systemd/system/mariadb.service.
Created symlink /etc/systemd/system/mysqld.service → /lib/systemd/system/mariadb.service.
Created symlink /etc/systemd/system/multi-user.target.wants/mariadb.service → /lib/systemd/system/mariadb.service.
Setting up autopkgtest-satdep (0) ...
Processing triggers for libc-bin (2.26-0ubuntu2.1) ...
Processing triggers for systemd (234-2ubuntu12.3) ...
(Reading database ... 69452 files and directories currently installed.)
Removing autopkgtest-satdep (0) ...
autopkgtest [00:52:38]: test smoke: [-----------------------
+ mysql
ERROR 2002 (HY000): Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2 "No such file or directory")
autopkgtest [00:52:39]: test smoke: -----------------------]
autopkgtest [00:52:39]: test smoke: - - - - - - - - - - results - - - - - - - - - -
smoke FAIL non-zero exit status 1

Looks like the server is not started ?

It appears so, at least reproducible in lxd container. There are maintainer scripts to start init.d script, however that is not done for systemd. I do not se for example deb-systemd-helper start mariadb.service at all. Possibly regression due to using a new/different debhelper during package build.

[Otto Kekäläinen (otto)]

It is easiest if we list the commits that caused the regressions, and then selectively revert them.

Here are some I found quickly:
- -pie https://salsa.debian.org/mariadb-team/mariadb-10.1/commit/b4fe91faee29bc1597e2876e1c2cc6bdd162fb66
- dh_systemd_start removal https://salsa.debian.org/mariadb-team/mariadb-10.1/commit/9f49e4b494f3dad8c403972996f7a1ebceb4b34f

At least the commits above are well documented and will help to understand if reverting them is the truly correct thing to do.

[Dimitri John Ledkov (xnox)]

@otto

I was working from the reverse. Specifically, performing debdiff between artful-release & artful-security, and noticing un-necessary changes to packaging. Note that -security/-updates uploads should not, in general, introduce any packaging changes or improvements. Yes, there are commits changing the packaging to match unstable toolchain/debhelper/etc, however debhelper will remain stable in artful and thus continues to need all the pre-existing hacks and workarounds as shipped in artful.

Thus this is not about "reverts" this is about the fact that one simply cannot use packaging which targets unstable, as the basis for security uploads, unless packaging is carefully maintained to be backwards compatible. One way to do this is to maintain separate per-release branches (as i do for systemd), or have an extensive amount of conditionals to keep packages building for older releases (like e.g. gcc packaging does, by comparing target release series, and trying to do the right thing for each one).

[Dimitri John Ledkov (xnox)]

The reason behind this debdiff, is that autopkgtests started to fail, and the database is not running after the install, which is grave.

Binaries are missing PIE, yet they were built with PIE in the release pocket.

See e.g.: http://autopkgtest.ubuntu.com/packages/mariadb-10.1/artful/amd64

[Otto Kekäläinen (otto)]

All of the changes in https://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.1.git/diff/debian/?id=ubuntu/10.1.30-0ubuntu0.17.10.1&id2=debian/10.1.25-1 are packaging wise necessary minimum changes for the security update to work (e.g. refreshing patches, applying a FTBFS patch, adopt to upstream build artifact changes due to use of --fail-missing).

The debdiff you attached shows other changes. I am trying to find out how that happened.

[Otto Kekäläinen (otto)]

I am unable to figure out what has happened here.

For example the change you did now:

diff -Nru mariadb-10.1-10.1.30/debian/rules mariadb-10.1-10.1.30/debian/rules
--- mariadb-10.1-10.1.30/debian/rules 2018-01-02 11:51:23.000000000 +0000
+++ mariadb-10.1-10.1.30/debian/rules 2018-03-20 21:36:52.000000000 +0000
@@ -4,7 +4,7 @@

 # enable Debian Hardening
 # see: https://wiki.debian.org/Hardening
-export DEB_BUILD_MAINT_OPTIONS = hardening=+all,-pie
+export DEB_BUILD_MAINT_OPTIONS = hardening=+all

..is identical to what was done in July 2017 in Debian in https://salsa.debian.org/mariadb-team/mariadb-10.1/commit/b4fe91faee29bc1597e2876e1c2cc6bdd162fb66. This was done in Debian however after 10.1.25-1 was released, thus it didn't make into Artful. So it is correct that you do that change now, but it is not because you would be reverting something done in an Artful security update.

[Otto Kekäläinen (otto)]

Just to be clear, in https://salsa.debian.org/mariadb-team/mariadb-10.1/branches there is a branch for each Ubuntu release, and they have been branches of the release tag of the last Debian release they synced from and shipped with.

All security updates are done on their own branch and there is no "use packaging which targets unstable, as the basis for security uploads" going on to my knowledge.

[Otto Kekäläinen (otto)]

@xnox Waiting for your comments. I would like to know where you get the sources you are comparing and what unfit-for-security-upload changes did you see in them exactly?

[Simon Quigley (tsimonq2)]

Unsubscribing the Sponsors Team for now as there's nothing to sponsor quite yet.

Please resubscribe us when there is.

Thank you.

[Dimitri John Ledkov (xnox)]

There are many packaging changes that were introduced in the security update, that regress security.

For example:

$ pull-lp-source -d mariadb-10.1 artful-release
$ pull-lp-source -d mariadb-10.1 artful-security
$ debdiff mariadb-10.1_10.1.25-1.dsc mariadb-10.1_10.1.30-0ubuntu0.17.10.1.dsc | filterdiff -i '*debian/rules' | head
gpgv: Signature made Tue 16 Jan 2018 05:38:44 PM GMT
gpgv: using RSA key 45BCE75B840B1F69
gpgv: Can't check signature: No public key
dpkg-source: warning: failed to verify signature on /tmp/mariadb-10.1_10.1.30-0ubuntu0.17.10.1.dsc
diff -Nru mariadb-10.1-10.1.25/debian/rules mariadb-10.1-10.1.30/debian/rules
--- mariadb-10.1-10.1.25/debian/rules 2017-07-30 13:15:48.000000000 +0100
+++ mariadb-10.1-10.1.30/debian/rules 2018-01-02 11:51:23.000000000 +0000
@@ -4,7 +4,7 @@

 # enable Debian Hardening
 # see: https://wiki.debian.org/Hardening
-export DEB_BUILD_MAINT_OPTIONS = hardening=+all
+export DEB_BUILD_MAINT_OPTIONS = hardening=+all,-pie
 DPKG_EXPORT_BUILDFLAGS = 1

See how the -security update, changes DEB_BUILD_MAIN_OPTIONS that remove pie. That makes all binaries compiled without pie, which is not the default/previous behaviour on Ubuntu.

Similarly systemd units changes were dropped in the security update - but these are still required on artful. As seen in autopkgtest failures / regression in artful-

My debdiff mentioned in https://launchpadlibrarian.net/361413400/lp1757107.diff reintroduced back the changes to the artful-security update, that are present in artful-release.

I'm not sure how I can make this any clearer. @otto Does above make sense or no?

[Otto Kekäläinen (otto)]

Hello!

As you can see in the git blame of debian/rules, I did not do any changes to the line with -pie:
https://salsa.debian.org/mariadb-team/mariadb-10.1/blame/ubuntu-17.10/debian/rules

I think I found out what has happened. If I look at the changelog found in via 'pull-lp-source -d mariadb-10.1 artful-release' it says:

****
mariadb-10.1 (10.1.25-1) unstable; urgency=medium

  * New upstream version 10.1.25
  * Update quilt patches on top of mariadb-10.1.25 release
  * Explicitly add dh_systemd_start snippets to mariadb-server-10.1
    because it's all messed up with different name for sysvinit ('mysql')
    and systemd ('mariadb') (Closes: #865870)
  * Don't disable PIE, it's enabled by upstream anyway (Closes: #865737)
  * Add default socket location for client (Closes: #864662)

 -- Ondřej Surý <email address hidden> Sun, 30 Jul 2017 14:15:48 +0200
****

While in the Debian git repository what was committed and tagged as the 10.1.25-1 upload is:
****
mariadb-10.1 (10.1.25-1) unstable; urgency=medium

  * New upstream version 10.1.25
  * Update quilt patches on top of mariadb-10.1.25 release

 -- Ondřej Surý <email address hidden> Wed, 12 Jul 2017 09:39:19 +0200
****

(source https://salsa.debian.org/mariadb-team/mariadb-10.1/commit/d6c1b1f9cac58d988882c3a0494c501e40d905b5)

After that the next commit was importing 10.1.26 so I had no reason to suspect this would be a wrong commit.

So the mistake here is that I have been working off a false base while preparing the debdiff. I could have spotted this if I would have double checked the contents of git by downloading the package and extracting it over the repository and running git diff.

The person who handled the Launchpad issue could also have spotted that what they were about to upload did not match the output of the debdiffs I attached and linked to.

So I have not intentionally done anything to pie or any of the other things you said. My commits have none of those changes. They have been introduced when the person doing the Ubuntu upload has recreated the debdiffs using a different base version.

I guess we need to start safeguarding the git contents better and run regular syncing/checking scripts that ensure that whatever is uploaded to Debian/Ubuntu is tracked in git and not done outside of it, eroding the whole maintenance process.

[Marc Deslauriers (mdeslaur)]

> They have been introduced when the person doing the Ubuntu upload has recreated the debdiffs using a different base version.

$ git clone https://salsa.debian.org/mariadb-team/mariadb-10.1
$ cd mariadb-10.1/
$ git checkout ubuntu-17.10
$ grep pie debian/rules
export DEB_BUILD_MAINT_OPTIONS = hardening=+all,-pie

We don't recreate the debdiffs. We just grabbed the repo you've told us to grab: "Please use git-buildpackage to fetch and build from the ubuntu-17.10 branch at http://anonscm.debian.org/cgit/pkg-mysql/mariadb-10.1.git/log/?h=ubuntu-17.10" in bug 1740768.

Perhaps we should stop using git to do these security updates.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Otto Kekäläinen (otto)]

Using git is good. The root of the problem here was that another DD had tagged commit https://salsa.debian.org/mariadb-team/mariadb-10.1/commit/d6c1b1f9cac58d988882c3a0494c501e40d905b5 as https://salsa.debian.org/mariadb-team/mariadb-10.1/commits/debian/10.1.25-1 falsely.

In reality, the tag debian/10.1.25-1 should have been on commit https://salsa.debian.org/mariadb-team/mariadb-10.1/commit/8927cd141a3c7bbfb918d3c8f8e0e6ee6acef178

This lead me to work on the wrong revision, as I trusted the tag to not be false (never before has it been wrong). Thus the resulting thing lost the commits d6c1b1f..8927cd. The long term solution would be to code into tracker.debian.org or some similar a tool that automaticaly checks the git tags and alerts if the source uploaded to Debian does not match and the tag is wrongly placed.

[Otto Kekäläinen (otto)]

I have now adopted a workflow where I use dgit to verify that what is in the packaging git matches what is uploaded.

Example:
 git clone salsa..../mariadb-10.1.git salsa
 dgit -d ubuntu clone mariadb-10.1 bionic,-security distro
 cp -ra distro/* salsa
 cd salsa
 git diff

This will make me able to catch is another maintainer has tagged a release that was not actually that release.

@xnox: the autopkgtests have been fixed via commits
https://salsa.debian.org/mariadb-team/mariadb-10.1/commit/96d3f8abcbe51894d0a5f7c7cadd5219e0dc2823
https://salsa.debian.org/mariadb-team/mariadb-10.1/commit/596c2581176102b29751786e5d8fac05dde3a3e4

[Mathew Hodson (mhodson)]

Fixed with 1:10.1.38-0ubuntu0.18.04.2 on bionic