[SRU] postfix tls deploy-server-cert fails with "can't shift that many"

I agree that it seems to miss an argument in the call from deploy-server-cert.
I have compared the versions up to the much more recent 3.5.2-1 but the situation is the same.

Reproducing this doesn't need a lot of pre-setup:
$ apt install postfix ssl-cert
$ postfix tls deploy-server-cert /etc/ssl/certs/ssl-cert-snakeoil.pem /etc/ssl/private/ssl-cert-snakeoil.key

The man page doe snot hint to any wrong use in this case as far as I can see:
deploy-server-cert certfile keyfile
  This subcommand deploys the certificates in certfile and private key in keyfile (which are typically generated by the commands above, which will also log and display the
  full command needed to deploy the generated key and certificate). After the new certificate and key are deployed any obsolete keys and certificates may be removed by
  hand. The keyfile and certfile filenames may be relative to the Postfix configuration directory.

The fix could be as easy as
--- a/conf/postfix-tls-script
+++ b/conf/postfix-tls-script
@@ -1039,7 +1039,7 @@ deploy-server-cert)
         *) keyfile="${config_directory}/${2}" ;;
        esac

- deploy_server_cert "${certfile}" "${keyfile}" || exit 1
+ deploy_server_cert "${certfile}" "${keyfile}" "enable" || exit 1
        info_server_deployed "${certfile}" "${keyfile}" "deploy" | $INFO
        ;;

But I'm not expert enough to consider all the side effects. Maybe at this point one would better pass "" instead of "enable".

It seems it has been introduced in this form a long time ago at 3.1.0 and was broken since then.
I wonder why nobody ever hit that, maybe it isn't supposed to be used at all?

Since it seems to be broken in the last version as well I wanted to ask you if you'd mind to bug-report that upstream and we could take the fix from there and backport to Ubuntu releases.

But I must admit that I wasn't able to find a proper bug tracker for postfix?!
I guess that means you should report to one of the mailing lists http://www.postfix.org/lists.html and it would be great if you could then get back here and update the bug with the link to the mail archive entry.

That way we can recheck what the outcome there will be and consider backporting it.

Postfix doesn't have a bug tracker. Their policy is to fix bugs when they are identified. Emailing postfix-users is the correct path forward.

Ah. I'll report it to postfix-users as recommended.

Below is a reply from postfix-users. Works perfectly for me.

--

Below is a patch. I find that the handling of this differs a lot
among shell implementations, from terminating to ignoring.

    Wietse

diff -ur /var/tmp/postfix-3.6-20200523/conf/postfix-tls-script conf/postfix-tls-script
--- /var/tmp/postfix-3.6-20200523/conf/postfix-tls-script 2017-02-18 20:58:20.000000000 -0500
+++ conf/postfix-tls-script 2020-05-30 10:37:04.000000000 -0400
@@ -777,7 +777,7 @@
 deploy_server_cert() {
     certfile=$1; shift
     keyfile=$1; shift
- deploy=$1; shift
+ case $# in 0) deploy=;; *) deploy=$1; shift;; esac

     # Sets key_algo, key_param and cert_param
     check_key "$keyfile" || return 1

Link to postfix-users archive:

http://postfix.1071664.n5.nabble.com/postfix-tls-deploy-server-cert-fails-with-quot-can-t-shift-that-many-quot-td106183.html

After proposing the mentioned change to Debian [1] we decided to wait for the next snapshot release since upstream implementation might differ from what was presented in the mailing list. Moreover, this is not an urgent issue.

[1] https://salsa.debian.org/postfix-team/postfix-dev/-/merge_requests/7

https://github.com/vdukhovni/postfix/commit/
119d6abed969b1b30c62722ae31c854b5682beae#diff-1843f98f5710e97bd063d5807334442a

There will be a new 3.4 release in a few days, I'd just update your SRU to
encompass that when it comes out. It's all bug fixes, reported by users or
not.

This bug was fixed in the package postfix - 3.5.3-1

---------------
postfix (3.5.3-1) unstable; urgency=medium

  [Wietse Venema]

  * 3.5.3 LP: #1881196

  [Debian Janitor]

  * Trim trailing whitespace.
  * Fix day-of-week for changelog entries 0.0.20001030.SNAPSHOT-4,
    0.0.20001030.SNAPSHOT-3, 0.0.19991231pl02-1, 0.0.19990122-1.

 -- Scott Kitterman <email address hidden> Mon, 15 Jun 2020 16:23:34 -0400

Thanks for the pointer Scott, appreciated. And also for fixing it in Debian and consequently in Groovy which is a sync.

I see this fix in version 3.4.13. Hopefully this postfix SRU [1] will be accepted and we will have version 3.4.11 in Focal, so we have two patch level releases to consider if we want to update to 3.4.13. Checking the changelog between 3.4.11 and 3.4.13 there are 6 "Bugfix", 3 "Noise suppression", and 1 "Bitrot". After a discussion with my team they told me we would need to define self-contained test cases for each of those bug fixes and analyze the impact of the non "Bugfix" changes to convince the SRU team to accept this new micro release.

I did some digging in the postfix-users mailing list and most of those bugs are discussed there but some are hard to reproduce (build a test case), there is a case where the OP offered access to their testing server to one of the postfix maintainers to help debugging the issue because it was not easily reproducible. With that in mind I think I will not be able to define good test cases to justify all of them. If someone could help me defining those test cases I'd be glad to do the rest of the work.

For now, I am considering to just backport this patch to fix the bug reported here:

20200530

 Bugfix (introduced: Postfix 3.1): "postfix tls deploy-server-cert"
 did not handle a missing optional argument. File:
 conf/postfix-tls-script.

[1] https://bugs.launchpad.net/ubuntu/+source/postfix/+bug/1868955

On Wednesday, June 17, 2020 5:51:07 PM EDT Lucas Kanashiro wrote:
> I see this fix in version 3.4.13. Hopefully this postfix SRU [1] will be
> accepted and we will have version 3.4.11 in Focal, so we have two patch
> level releases to consider if we want to update to 3.4.13. Checking the
> changelog between 3.4.11 and 3.4.13 there are 6 "Bugfix", 3 "Noise
> suppression", and 1 "Bitrot". After a discussion with my team they told
> me we would need to define self-contained test cases for each of those
> bug fixes and analyze the impact of the non "Bugfix" changes to convince
> the SRU team to accept this new micro release.

This did not used to be the case for micro-releases where the TB approved the
micro-release exception (as is the case here). Back when I was doing this for
Ubuntu it was enough that the postfix passed it's test suite and it had been
smoke tested on a real system.

We do these updates in Debian routinely without anything more than that and
have never had a problem. I'd find it surprising that Ubuntu was being more
restrictive about post-release updates than Debian.

Scott K

Scott, I asked around about this subject and those past SRUs with micro-release updates were accepted probably based on this:

https://wiki.ubuntu.com/StableReleaseUpdates#New_upstream_microreleases

And since Debian is also including those micro-releases in stable point releases I also think we should that too. However, I did not find the upstream QA process documented and this is mandatory according to the SRU policy for a micro-release update:

"The upstream QA process must be documented/demonstrated and linked from the SRU tracking bug."

If it is documented somewhere could you please provide a link to it? In case it is not available, we could not take advantage of this micro-release SRU process and a Postfix exception would be needed, like we already have for other packages as you can see here:

https://wiki.ubuntu.com/StableReleaseUpdates#Documentation_for_Special_Cases

To do that a process needs to be defined for future Postfix SRUs containing micro-release updates. I see we already have some good DEP-8 tests, so we'd need to write a wiki page describing this process and ask the SRU team to acknowledge it. I am offering myself to drive this if that is the case.

On Thursday, June 18, 2020 9:46:34 AM EDT you wrote:
> To do that a process needs to be defined for future Postfix SRUs
> containing micro-release updates. I see we already have some good DEP-8
> tests, so we'd need to write a wiki page describing this process and ask
> the SRU team to acknowledge it. I am offering myself to drive this if
> that is the case.

The Ubuntu Technical Board already approved this. See:

https://lists.ubuntu.com/archives/ubuntu-devel-announce/2011-October/
000902.html
https://lists.ubuntu.com/archives/technical-board/2012-May/001266.html

Scott K

Awesome, thank you for that. Since it was already approved there is no reason to go through the whole process again, but to keep it documented I'll create a wiki page for this Postfix exception and link it here:

https://wiki.ubuntu.com/StableReleaseUpdates#Documentation_for_Special_Cases

After that I will prepare a SRU with version 3.4.13.

FYI the SRU exception for Postfix is documented here:

https://wiki.ubuntu.com/StableReleaseUpdates#Postfix

Hello robert, or anyone else affected,

Accepted postfix into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/postfix/3.4.13-0ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Fix verified for focal.

$ lsb_release -rd
Description: Ubuntu 20.04 LTS
Release: 20.04

Did a fresh, vanilla install of focal.
installed postfix, ssl-cert - accepted all defaults
  # sudo apt install postfix ssl-cert
Tested to ensure that fault existed
  # postfix tls deploy-server-cert /etc/ssl/certs/ssl-cert-snakeoil.pem /etc/ssl/private/ssl-cert-snakeoil.key

applied patch from http://launchpadlibrarian.net/485381331/postfix_3.4.10-1ubuntu1_3.4.13-0ubuntu1.diff.gz
  # patch -d/ --ignore-whitespace -p0 /usr/lib/postfix/sbin/postfix-tls-script <<'EOF'
diff -Nru postfix-3.4.10/conf/postfix-tls-script postfix-3.4.13/conf/postfix-tls-script
--- postfix-3.4.10/conf/postfix-tls-script 2017-02-19 01:58:20.000000000 +0000
+++ postfix-3.4.13/conf/postfix-tls-script 2020-05-30 14:37:04.000000000 +0000
@@ -777,7 +777,7 @@
 deploy_server_cert() {
     certfile=$1; shift
     keyfile=$1; shift
- deploy=$1; shift
+ case $# in 0) deploy=;; *) deploy=$1; shift;; esac

     # Sets key_algo, key_param and cert_param
     check_key "$keyfile" || return 1
EOF

Verified fix. - focal.

--

Did a fresh, vanilla install of focal.
installed ssl-cert
  $ sudo apt install ssl-cert
Installed postfix/focal-proposed from postfix_3.4.13-0ubuntu1_amd64.deb
  $ sudo dpkg -i postfix_3.4.13-0ubuntu1_amd64.deb
Tested script
  $ postfix tls deploy-server-cert /etc/ssl/certs/ssl-cert-snakeoil.pem /etc/ssl/private/ssl-cert-snakeoil.key

Verified fix - focal

--

Did a fresh , vanilla install of bionic
installed postfix, ssl-cert - accepted all defaults
  # sudo apt install postfix ssl-cert
Tested to ensure that fault existed
  # postfix tls deploy-server-cert /etc/ssl/certs/ssl-cert-snakeoil.pem /etc/ssl/private/ssl-cert-snakeoil.key

applied patch from http://launchpadlibrarian.net/485381331/postfix_3.4.10-1ubuntu1_3.4.13-0ubuntu1.diff.gz
  # patch -d/ --ignore-whitespace -p0 /usr/lib/postfix/sbin/postfix-tls-script <<'EOF'
diff -Nru postfix-3.4.10/conf/postfix-tls-script postfix-3.4.13/conf/postfix-tls-script
--- postfix-3.4.10/conf/postfix-tls-script 2017-02-19 01:58:20.000000000 +0000
+++ postfix-3.4.13/conf/postfix-tls-script 2020-05-30 14:37:04.000000000 +0000
@@ -777,7 +777,7 @@
 deploy_server_cert() {
     certfile=$1; shift
     keyfile=$1; shift
- deploy=$1; shift
+ case $# in 0) deploy=;; *) deploy=$1; shift;; esac

     # Sets key_algo, key_param and cert_param
     check_key "$keyfile" || return 1
EOF

Verified fix - Bionic

This bug was fixed in the package postfix - 3.4.13-0ubuntu1

---------------
postfix (3.4.13-0ubuntu1) focal; urgency=medium

  * New upstream release: 3.4.13
    - Workaround for broken DANE support after an incompatible change in
      GLIBC 2.31 (LP: #1868955)
    - Fix "postfix tls deploy-server-cert", now it handles a missing optional
      argument (LP: #1881196)
  * Drop patch 80_glibc2.30-ftbfs.diff. This patch is not needed anymore and
    it does not cleanly apply to this new upstream release.

 -- Lucas Kanashiro <email address hidden> Fri, 19 Jun 2020 14:11:03 -0300

The verification of the Stable Release Update for postfix has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Hello robert, or anyone else affected,

Accepted postfix into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/postfix/3.3.0-1ubuntu0.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Hello robert, or anyone else affected,

Accepted postfix into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/postfix/3.1.0-3ubuntu0.4 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

All autopkgtests for the newly accepted postfix (3.1.0-3ubuntu0.4) for xenial have finished running.
The following regressions have been reported in tests triggered by the package:

sbuild/0.67.0-2ubuntu7.1 (ppc64el, amd64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/xenial/update_excuses.html#postfix

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

All autopkgtests for the newly accepted postfix (3.3.0-1ubuntu0.3) for bionic have finished running.
The following regressions have been reported in tests triggered by the package:

sbuild/0.75.0-1ubuntu1 (arm64, ppc64el, s390x, amd64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/bionic/update_excuses.html#postfix

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

sbuild dep8 tests are failing in xenial and bionic in all architectures even before the postfix update, so this is not a regression introduced by postfix. I retried all the tests in the hope of this been a flaky test but they failed again. The architectures not reported by the SRU bot are hinted as badtest, we might consider doing the same for the other architectures.

Bionic verification:

$ lxc launch ubuntu-daily:bionic postfix-cert
$ lxc shell postfix-cert
# apt-get update && apt-get upgrade -y
# cat <<EOF >/etc/apt/sources.list.d/ubuntu-$(lsb_release -cs)-proposed.list
deb http://archive.ubuntu.com/ubuntu/ $(lsb_release -cs)-proposed restricted main multiverse universe
EOF
# apt-get update && apt-get install -y postfix/bionic-proposed
# postfix tls deploy-server-cert /etc/ssl/certs/ssl-cert-snakeoil.pem /etc/ssl/private/ssl-cert-snakeoil.key
postfix/postfix-tls-script: New TLS private key and certificate deployed.
postfix/postfix-tls-script: Run the command:
postfix/postfix-tls-script: # postfix reload
postfix/postfix-tls-script: if you want the new settings to take effect immediately.

Xenial verification:

$ lxc launch ubuntu-daily:xenial postfix-cert-xenial
$ lxc shell postfix-cert-xenial
# apt-get update && apt-get upgrade -y
# cat <<EOF >/etc/apt/sources.list.d/ubuntu-$(lsb_release -cs)-proposed.list
deb http://archive.ubuntu.com/ubuntu/ $(lsb_release -cs)-proposed restricted main multiverse universe
EOF
# apt-get update && apt-get install -y postfix/xenial-proposed
# postfix tls deploy-server-cert /etc/ssl/certs/ssl-cert-snakeoil.pem /etc/ssl/private/ssl-cert-snakeoil.key
postfix: Postfix is running with backwards-compatible default settings
postfix: See http://www.postfix.org/COMPATIBILITY_README.html for details
postfix: To disable backwards compatibility use "postconf compatibility_level=2" and "postfix reload"
postfix/postfix-tls-script: fatal: no private key found in file:

This bug was fixed in the package postfix - 3.3.0-1ubuntu0.3

---------------
postfix (3.3.0-1ubuntu0.3) bionic; urgency=medium

  * d/p/fix_tls_deploy-server-cert.patch: handle missing opt arg (LP: #1881196)

 -- Lucas Kanashiro <email address hidden> Fri, 10 Jul 2020 17:08:22 -0300

This bug was fixed in the package postfix - 3.1.0-3ubuntu0.4

---------------
postfix (3.1.0-3ubuntu0.4) xenial; urgency=medium

  * d/p/fix_tls_deploy-server-cert.patch: handle missing opt arg (LP: #1881196)

 -- Lucas Kanashiro <email address hidden> Fri, 10 Jul 2020 18:14:41 -0300