Merge ~bryce/ubuntu/+source/fetchmail:eoan-merge-6.4.0-beta4-3 into ubuntu/+source/fetchmail:debian/sid

[Bryce Harrington]

Merge with debian's package. Carried forward one patch in the delta.

Added two stabilization fixes from upstream's git tree; these can be dropped next time we merge an new upstream version.

Also added are some basic dep8 tests to check installation and service operation, to try and address LP: #1677818. A complete set of tests would include actual fetching of an email, but that felt too out of scope for the merge so can be tackled later.

PPA available for testing is at
https://launchpad.net/~bryce/+archive/ubuntu/fetchmail-merge-6.4.0-beta4-3

Usual tags are pushed.

[Christian Ehrhardt  (paelzer)]

some changelog bikeshedding

[Christian Ehrhardt  (paelzer)]

some incomplete patch headers and a few questions on the added tests (all inline)

review: Needs Fixing

[Christian Ehrhardt  (paelzer)]

Ack on the added stabilization patches

[Bryce Harrington (bryce)]

On Wed, Jun 26, 2019 at 07:21:25AM -0000, Christian Ehrhardt  wrote:
> some changelog bikeshedding
>
> Diff comments:
>
> > diff --git a/debian/changelog b/debian/changelog
> > index 6aaef1d..0bcbcad 100644
> > --- a/debian/changelog
> > +++ b/debian/changelog
> > @@ -1,3 +1,18 @@
> > +fetchmail (6.4.0~beta4-3ubuntu1) eoan; urgency=medium
> > +
> > + * Merge with Debian unstable. Remaining changes:
> > + + d/p/sni-support.patch: TLS: set hostname for SNI. Thanks to Matthias
>
> IIRC on this level this would usually be a "-".
> This is style which is meant to be flexible as long as it is understandable but usually we have
> * foo
> - bar
> + foobar
> in most cases, unless a changelog always uses a different syntax, then we follow that.

Thanks, fixed

> > + Andree <email address hidden> (LP #1798786)
> > + * d/p/clang-robustness.patch: Backport clang null pointer checks from upstream
>
> Added changes in the context of a merge go under a top entry
> * Added Changes:
>
> I saw no drops, but if there would be to be complete on this info those would list after
> * Dropped Change:
> It turned out on these drops to list reasons in brackets like
> [now upstream]
> [accepted by Debian]
> [no more needed becasue foo]

Thanks, was wondering about this, I've rebased with the changelog
reformats, and will repush the MP.

Bryce

> > + to prevent potential segfaults.
> > + * d/p/set-umask-for-fetchids.patch: Backport upstream fix to set umask
> > + properly before writing the .fetchids file.
> > + * d/p/tests: Add preliminary/basic dep8 tests to check installation and
> > + service operation.
> > + (Partial fix for LP: #1677818)
> > +
> > + -- Bryce Harrington <email address hidden> Sun, 16 Jun 2019 18:45:16 -0700
> > +
> > fetchmail (6.4.0~beta4-3) unstable; urgency=medium
> >
> > * Backport fix potential SIGSEGV in pop3_delete (closes: #921450).
>
>
> --
> https://code.launchpad.net/~bryce/ubuntu/+source/fetchmail/+git/fetchmail/+merge/369322
> You are the owner of ~bryce/ubuntu/+source/fetchmail:eoan-merge-6.4.0-beta4-3.

[Bryce Harrington (bryce)]

Thanks Christian, I've updated the branch with the review feedback, I
think it is ready for re-review.

On Wed, Jun 26, 2019 at 07:33:07AM -0000, Christian Ehrhardt  wrote:
> Ack on the added stabilization patches
> --
> https://code.launchpad.net/~bryce/ubuntu/+source/fetchmail/+git/fetchmail/+merge/369322
> You are the owner of ~bryce/ubuntu/+source/fetchmail:eoan-merge-6.4.0-beta4-3.

[Bryce Harrington (bryce)]

Is there something else I need to do or set before this can get another review? Should I resubmit it?

[Christian Ehrhardt  (paelzer)]

Hey, just as so many others as mentioned yesterday this was lost it seems.
I think nothing is missing, let me re-check it.

[Christian Ehrhardt  (paelzer)]

Yeah LGTM now, thanks for adding all the stuff I asked for!

review: Approve

[Bryce Harrington (bryce)]

Thanks Christian, uploaded.

[Andreas Hasenack (ahasenack)]

I pushed the tag for upload/6.4.0_beta4-3ubuntu1 on 4a4c066fbf5722714f6e6f44d508597f1b42f4a6?:
$ git push pkg upload/6.4.0_beta4-3ubuntu1
Enumerating objects: 56, done.
Counting objects: 100% (56/56), done.
Delta compression using up to 2 threads
Compressing objects: 100% (50/50), done.
Writing objects: 100% (50/50), 7.93 KiB | 1.13 MiB/s, done.
Total 50 (delta 32), reused 0 (delta 0)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/fetchmail
 * [new tag] upload/6.4.0_beta4-3ubuntu1 -> upload/6.4.0_beta4-3ubuntu1

diff --git a/debian/changelog b/debian/changelog
index 6aaef1d..5b412e9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,19 @@
+fetchmail (6.4.0~beta4-3ubuntu1) eoan; urgency=medium
+
+  * Merge with Debian unstable. Remaining changes:
+    - d/p/sni-support.patch: TLS: set hostname for SNI. Thanks to Matthias
+      Andree <[e-mail]> (LP #1798786)
+  * Added Changes:
+    - d/p/clang-robustness.patch: Backport clang null pointer checks from upstream
+      to prevent potential segfaults.
+    - d/p/set-umask-for-fetchids.patch: Backport upstream fix to set umask
+      properly before writing the .fetchids file.
+    - d/p/tests: Add preliminary/basic dep8 tests to check installation and
+      service operation.
+      (Partial fix for LP: #1677818)
+
+ -- Bryce Harrington <[e-mail]>  Sun, 16 Jun 2019 18:45:16 -0700
+
 fetchmail (6.4.0~beta4-3) unstable; urgency=medium
 
   * Backport fix potential SIGSEGV in pop3_delete (closes: #921450).
@@ -52,6 +68,19 @@ fetchmail (6.4.0~beta4-1) experimental; urgency=medium
 
  -- Laszlo Boszormenyi (GCS) <[e-mail]>  Sat, 23 Jun 2018 15:52:22 +0000
 
+fetchmail (6.3.26-3ubuntu1) disco; urgency=medium
+
+  * d/p/sni-support.patch: TLS: set hostname for SNI. Thanks to Matthias
+    Andree <[e-mail]> (LP: #1798786)
+
+ -- Karl Stenerud <[e-mail]>  Wed, 24 Oct 2018 05:12:24 -0700
+
+fetchmail (6.3.26-3build1) bionic; urgency=high
+
+  * No change rebuild against openssl1.1.
+
+ -- Dimitri John Ledkov <[e-mail]>  Mon, 05 Feb 2018 23:21:22 +0000
+
 fetchmail (6.3.26-3) unstable; urgency=low
 
   * Use short debhelper rules format.
diff --git a/debian/control b/debian/control
index 77824a7..6b8827a 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: fetchmail
 Section: mail
 Priority: optional
-Maintainer: Laszlo Boszormenyi (GCS) <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Laszlo Boszormenyi (GCS) <[e-mail]>
 Build-Depends: debhelper (>= 11), libssl-dev (>= 1.0.2), bison, python (>= 2.7), dh-python, libkrb5-dev, libgssglue-dev
 Standards-Version: 4.3.0
 Homepage: http://www.fetchmail.info
diff --git a/debian/patches/clang-robustness.patch b/debian/patches/clang-robustness.patch
new file mode 100644
index 0000000..125e1ee
--- /dev/null
+++ b/debian/patches/clang-robustness.patch
@@ -0,0 +1,59 @@
+Description: Backport clang null pointer checks from upstream
+ to prevent potential segfaults.
+Author: Matthias Andree <[e-mail]>
+Origin: upstream, https://gitlab.com/fetchmail/fetchmail/commit/124518ae5fba9e352a9cf8a8d7c1a8160475d10b
+Last-Update: 2019-06-27
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+index 6176bcb9..74e1b28a 100644
+--- a/driver.c
++++ b/driver.c
+@@ -542,8 +542,9 @@ static int fetch_messages(int mailserver_socket, struct query *ctl,
+ 	    lastnum = num + fetchsizelimit - 1;
+ 	    if (lastnum > count)
+ 		lastnum = count;
+-	    for (i = 0; i < fetchsizelimit; i++)
+-		(*msgsizes)[i] = 0;
++	    if (*msgsizes)
++		for (i = 0; i < fetchsizelimit; i++)
++		    (*msgsizes)[i] = 0;
+ 
+ 	    stage = STAGE_GETSIZES;
+ 	    err = (ctl->server.base_protocol->getpartialsizes)(mailserver_socket, num, lastnum, *msgsizes);
+diff --git a/report.c b/report.c
+index 5d9abb73..1466802a 100644
+--- a/report.c
++++ b/report.c
+@@ -362,7 +362,7 @@ report_at_line (FILE *errfp, errnum, file_name, line_number, message, va_alist)
+ 	static unsigned int old_line_number;
+ 
+ 	if (old_line_number == line_number &&
+-	    (file_name == old_file_name || !strcmp (old_file_name, file_name)))
++	    (file_name == old_file_name || (old_file_name != NULL && 0 == strcmp (old_file_name, file_name))))
+ 	    /* Simply return and print nothing.  */
+ 	    return;
+ 
+diff --git a/sink.c b/sink.c
+index e8dd8355..164c36d8 100644
+--- a/sink.c
++++ b/sink.c
+@@ -1189,14 +1189,14 @@ static int open_mda_sink(struct query *ctl, struct msgblk *msg,
+ 	    ** the worst case (end of string) sp[1] == '\0' */
+ 	    if (sp[1] == 's' || sp[1] == 'T') {
+ 		*dp++ = '\'';
+-		strcpy(dp, names);
++		if (names) strcpy(dp, names);
+ 		dp += nameslen;
+ 		*dp++ = '\'';
+ 		sp++;	/* position sp over [sT] */
+ 		dp--;	/* adjust dp */
+ 	    } else if (sp[1] == 'F') {
+ 		*dp++ = '\'';
+-		strcpy(dp, from);
++		if (from) strcpy(dp, from);
+ 		dp += fromlen;
+ 		*dp++ = '\'';
+ 		sp++;	/* position sp over F */
+-- 
+2.17.1
+
diff --git a/debian/patches/series b/debian/patches/series
index 6747c0a..ce42e10 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -4,3 +4,7 @@
 04_invoke-rc.d.diff
 05_Fix_potential_SIGSEGV_in_pop3_delete.patch
 06_Enable_OpenSSL_native_name_verification.patch
+
+sni-support.patch
+clang-robustness.patch
+set-umask-for-fetchids.patch
diff --git a/debian/patches/set-umask-for-fetchids.patch b/debian/patches/set-umask-for-fetchids.patch
new file mode 100644
index 0000000..5f2811b
--- /dev/null
+++ b/debian/patches/set-umask-for-fetchids.patch
@@ -0,0 +1,49 @@
+Description: Set umask properly before writing the .fetchids file,
+ to avoid failing the security check on the next run.
+Author: Matthias Andree <[e-mail]>
+Origin: upstream, https://gitlab.com/fetchmail/fetchmail/commit/295ff6340f665d58e369035ffa4257f2f84553d6
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=831611
+Last-Update: 2019-06-27
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+index d910c19d..af2e77ba 100644
+--- a/NEWS
++++ b/NEWS
+@@ -131,6 +131,8 @@ fetchmail-6.4.0 (not yet released):
+ * Fetchmail no longer adds a NUL byte to the username in GSSAPI authentication.
+   This was reported to break Kerberos-based authentication with Microsoft
+   Exchange 2013 by Greg Hudson.
++* Set umask properly before writing the .fetchids file, to avoid failing the
++  security check on the next run.  Reported by Fabian Raab, Debian Bug#831611.
+ 
+ # KNOWN BUGS AND WORKAROUNDS
+   (This section floats upwards through the NEWS file so it stays with the
+diff --git a/uid.c b/uid.c
+index 7ee702b9..2db06733 100644
+--- a/uid.c
++++ b/uid.c
+@@ -469,11 +469,13 @@ void write_saved_lists(struct query *hostlist, const char *idfile)
+ 	    report(stderr, GT_("Error deleting %s: %s\n"), idfile, strerror(errno));
+     } else {
+ 	char *newnam = (char *)xmalloc(strlen(idfile) + 2);
++	mode_t old_umask;
+ 	strcpy(newnam, idfile);
+ 	strcat(newnam, "_");
+ 	if (outlevel >= O_DEBUG)
+ 	    report(stdout, GT_("Writing fetchids file.\n"));
+ 	(void)unlink(newnam); /* remove file/link first */
++	old_umask = umask(S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH | S_IXOTH);
+ 	if ((tmpfp = fopen(newnam, "w")) != (FILE *)NULL) {
+ 	    struct write_saved_info info;
+ 	    int errflg = 0;
+@@ -517,6 +519,7 @@ bailout:
+ 	    report(stderr, GT_("Cannot open fetchids file %s for writing: %s\n"), newnam, strerror(errno));
+ 	}
+ 	free(newnam);
++	(void)umask(old_umask);
+     }
+ }
+ #endif /* POP3_ENABLE */
+-- 
+2.17.1
+
diff --git a/debian/patches/sni-support.patch b/debian/patches/sni-support.patch
new file mode 100644
index 0000000..e29ff88
--- /dev/null
+++ b/debian/patches/sni-support.patch
@@ -0,0 +1,32 @@
+Description: TLS: set hostname for SNI.
+Author: Matthias Andree <[e-mail]>
+Origin: upstream, https://gitlab.com/fetchmail/fetchmail/commit/9b8b634312f169fab872f3580c2febe5af031615
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/fetchmail/+bug/1798786
+Last-Update: 2018-10-24
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+Index: fetchmail-gu/socket.c
+===================================================================
+--- fetchmail-gu.orig/socket.c
++++ fetchmail-gu/socket.c
+@@ -1201,6 +1201,20 @@ int SSLOpen(int sock, char *mycert, char
+ 	    }
+ 	}
+ 
++	/*
++	 * Support SNI, some servers (googlemail) appear to require it.
++	 */
++	{
++	    long r;
++	    r = SSL_set_tlsext_host_name(_ssl_context[sock], servercname);
++
++	    if (0 == r) {
++		/* handle error */
++		report(stderr, GT_("Warning: SSL_set_tlsext_host_name(%p, \"%s\") failed (code %#lx), trying to continue.\n"), _ssl_context[sock], servercname, r);
++		ERR_print_errors_fp(stderr);
++	    }
++	}
++
+ 	if( mycert || mykey ) {
+ 
+ 	/* Ok...  He has a certificate file defined, so lets declare it.  If
diff --git a/debian/tests/control b/debian/tests/control
new file mode 100644
index 0000000..1ca0335
--- /dev/null
+++ b/debian/tests/control
@@ -0,0 +1,8 @@
+Tests: installation
+Depends: @
+Restrictions:
+
+Tests: service
+Depends: @, systemd-sysv
+Restrictions: needs-root
+
diff --git a/debian/tests/installation b/debian/tests/installation
new file mode 100755
index 0000000..271fc6c
--- /dev/null
+++ b/debian/tests/installation
@@ -0,0 +1,23 @@
+#!/bin/sh
+
+############################################
+### Check main installation requirements ###
+############################################
+
+set -e
+
+# Config file
+echo "Checking config file present"
+test -e /etc/default/fetchmail
+
+# Service
+echo "Checking service scripts are installed"
+test -e /etc/init.d/fetchmail
+test -e /etc/ppp/ip-down.d/fetchmail
+
+# Script
+echo "Checking fetchmail is present"
+test -e /usr/bin/fetchmail
+fetchmail --version > /dev/null 2>&1
+c=$(fetchmail --help 2>&1 | wc -l)
+test $c -gt 5
diff --git a/debian/tests/service b/debian/tests/service
new file mode 100755
index 0000000..95f9abc
--- /dev/null
+++ b/debian/tests/service
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+###############################
+### Check fetchmail service ###
+###############################
+
+set -e
+
+echo "Checking fetchmail service is enabled"
+systemctl is-enabled fetchmail.service
+
+echo "Checking fetchmail service is active"
+systemctl is-active fetchmail.service