Merge ~ahasenack/ubuntu/+source/apache2:bionic-client-cert-auth-fix into ubuntu/+source/apache2:ubuntu/bionic-devel

[Andreas Hasenack]

PPA with test packages: https://launchpad.net/~ahasenack/+archive/ubuntu/apache2-client-cert-1833039

sudo add-apt-repository ppa:ahasenack/apache2-client-cert-1833039 -y -u

The bug has the SRU template filled out, including a test case. It may sound daunting at first, but the certificates are all pre-made and are attached to the bug.

About the actual patch, I kept it as "upstream" as possible, just removing the hunk that is not needed to fix this bug. I therefore made no backport claim, but I did change the patch by removing that hunk and its explanation. It applies, but is unneeded as far as I can tell (only relevant for TLSv1.3). If you feel I should keep it as well, or change the patch to be an actual backport (and therefore refresh it, remove diff noise, apply all our dep3 header lines, etc), please let me know.

[Christian Ehrhardt  (paelzer)]

I think the choice on the removed hunk is fine.

[Christian Ehrhardt  (paelzer)]

Patch LGTM +1, but I have no time to give it a try.
If someone else could to give this a full ack then that would be great.

[Andreas Hasenack (ahasenack)]

DEP8 tests passed:
$ tail -n 20 dep8-bionic/log
+ a2enconf chroot
Enabling conf chroot.
To activate the new configuration, you need to run:
  systemctl reload apache2
+ echo Hello, world!
+ service apache2 restart
+ wget -qO- http://localhost/hello.txt
+ result=Hello, world!
+ [ Hello, world! != Hello, world! ]
autopkgtest [17:52:45]: test chroot: -----------------------]
autopkgtest [17:52:45]: test chroot: - - - - - - - - - - results - - - - - - - - - -
chroot PASS
autopkgtest [17:52:46]: @@@@@@@@@@@@@@@@@@@@ summary
run-test-suite PASS
duplicate-module-load PASS
htcacheclean PASS
default-mods PASS
ssl-passphrase PASS
check-http2 PASS
chroot PASS

[Bryce Harrington (bryce)]

LGTM +1

* I ran through the full test case in the bug (thanks for preparing it!)
  - Red/Green testing worked as expected.
  - http://paste.ubuntu.com/p/tzVmRKK7Dh/
* Debdiff: looks good, one minor spelling error in comment (see inline)
* Review of code: no obvious errors spotted

SSL_CTX_clear_mode() removes the SSL_MODE_AUTO_RETRY mode via the ctx bitmask. From the description of this flag, I gather that during the initialization process there is unexpected handshake data, and the code is confused and keeps retrying looking for application data. With this patch, if it encounters handshake data it skips the retry and continues with processing. For regression potential I take it there is no chance that skipping the retry could cause issues if transmission errors were to occur, but perhaps that would be something to watch for.

review: Approve (code/debdiff)

[Andreas Hasenack (ahasenack)]

Thanks for the reviews and tests guys!

Tagged and uploaded:
$ git push pkg upload/2.4.29-1ubuntu4.7
Enumerating objects: 22, done.
Counting objects: 100% (22/22), done.
Delta compression using up to 2 threads
Compressing objects: 100% (14/14), done.
Writing objects: 100% (14/14), 2.34 KiB | 45.00 KiB/s, done.
Total 14 (delta 10), reused 0 (delta 0)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/apache2
 * [new tag] upload/2.4.29-1ubuntu4.7 -> upload/2.4.29-1ubuntu4.7

$ dput ubuntu ../apache2_2.4.29-1ubuntu4.7_source.changes
Checking signature on .changes
gpg: ../apache2_2.4.29-1ubuntu4.7_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../apache2_2.4.29-1ubuntu4.7.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading apache2_2.4.29-1ubuntu4.7.dsc: done.
  Uploading apache2_2.4.29-1ubuntu4.7.debian.tar.xz: done.
  Uploading apache2_2.4.29-1ubuntu4.7_source.buildinfo: done.
  Uploading apache2_2.4.29-1ubuntu4.7_source.changes: done.
Successfully uploaded packages.

[Andreas Hasenack (ahasenack)]

This was released already.

diff --git a/debian/changelog b/debian/changelog
index a9ece4e..522f32f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+apache2 (2.4.29-1ubuntu4.7) bionic; urgency=medium
+
+  * d/p/disable-ssl-1.1.1-auto-retry.patch: fix client certificate
+    authentication when built with openssl 1.1.1 (LP: #1833039)
+
+ -- Andreas Hasenack <[e-mail]>  Fri, 28 Jun 2019 13:49:35 -0300
+
 apache2 (2.4.29-1ubuntu4.6) bionic-security; urgency=medium
 
   * SECURITY UPDATE: slowloris DoS in mod_http2
diff --git a/debian/patches/disable-ssl-1.1.1-auto-retry.patch b/debian/patches/disable-ssl-1.1.1-auto-retry.patch
new file mode 100644
index 0000000..3465113
--- /dev/null
+++ b/debian/patches/disable-ssl-1.1.1-auto-retry.patch
@@ -0,0 +1,41 @@
+From bbedd8b80e50647e09f2937455cc57565d94a844 Mon Sep 17 00:00:00 2001
+From: Joe Orton <[e-mail]>
+Date: Wed, 12 Sep 2018 15:54:59 +0000
+Subject: [PATCH] Merge r1840710 from trunk:
+
+* modules/ssl/ssl_engine_init.c (ssl_init_ctx_protocol):
+  Disable AUTO_RETRY mode for OpenSSL 1.1.1, which fixes
+  post-handshake authentication.
+
+git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/tlsv1.3-for-2.4.x@1840711 13f79535-47bb-0310-9956-ffa450edef68
+---
+ modules/ssl/ssl_engine_init.c | 14 ++++++++++++++
+ 1 file changed, 14 insertions(+)
+
+ Ubuntu note: the second hunk of the patch, modifying ssl_init_proxy_certs(),
+ was dropped from this patch because the post-handshake authentication is only
+ available in TLSv1.3, which is not available in this version of Apache. The hunk
+ that was kept does affect TLSv1.2, however.
+
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/apache2/+bug/1833039
+Origin: upstream, https://github.com/apache/httpd/commit/bbedd8b80e50647e09f2937455cc57565d94a844
+Last-Update: 2019-06-28
+
+diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c
+index 85d43376b2a..b7b2be796c2 100644
+--- a/modules/ssl/ssl_engine_init.c
++++ b/modules/ssl/ssl_engine_init.c
+@@ -761,6 +761,13 @@ static apr_status_t ssl_init_ctx_protocol(server_rec *s,
+         SSL_CTX_set_mode(ctx, SSL_MODE_RELEASE_BUFFERS);
+ #endif
+ 
++#if OPENSSL_VERSION_NUMBER >= 0x1010100fL
++    /* For OpenSSL >=1.1.1, disable auto-retry mode so it's possible
++     * to consume handshake records without blocking for app-data.
++     * https://github.com/openssl/openssl/issues/7178 */
++    SSL_CTX_clear_mode(ctx, SSL_MODE_AUTO_RETRY);
++#endif
++    
+     return APR_SUCCESS;
+ }
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 84868ea..0a4cf1a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -36,3 +36,4 @@ CVE-2019-0217.patch
 CVE-2019-0220-1.patch
 CVE-2019-0220-2.patch
 CVE-2019-0220-3.patch
+disable-ssl-1.1.1-auto-retry.patch