Merge ~ahasenack/ubuntu/+source/libapache2-mod-perl2:xenial-modperl2-fix-dep8-1779400 into ubuntu/+source/libapache2-mod-perl2:ubuntu/xenial-devel

[Andreas Hasenack]

PPA with test packages: https://launchpad.net/~ahasenack/+archive/ubuntu/libapache2-mod-perl2-dep8/+packages (ppa:ahasenack/libapache2-mod-perl2-dep8)

This branch brings to xenial the Debian DEP8 fixes from libapache2-mod-perl2-2.0.10-2: https://git.launchpad.net/ubuntu/+source/libapache2-mod-perl2/commit/?h=debian/buster&id=1a0e6c046960f816f0089d8d164f7fa9b077719f

I slightly augmented the two patches with a reference to the Ubuntu bug, but that's it.

The two test failures we are fixing here are:
t/apache/read.t (Wstat: 0 Tests: 1 Failed: 1)
  Failed test: 1
t/filter/in_bbs_inject_header.t (Wstat: 512 Tests: 0 Failed: 0)
  Non-zero exit status: 2
  Parse errors: Bad plan. You planned 36 tests but ran 0.

Here is the xenial/amd64 test history: http://autopkgtest.ubuntu.com/packages/liba/libapache2-mod-perl2/xenial/amd64

Run this to verify the fix and run the dep8 tests locally (adjust the path to the xenial autopkgtest image):

autopkgtest -o dep8-xenial-ppa -U -s --apt-pocket=proposed --setup-commands="sudo add-apt-repository -s -y -u ppa:ahasenack/libapache2-mod-perl2-dep8" -B libapache2-mod-perl2 -- qemu /var/lib/adt-images/autopkgtest-xenial-amd64.img

Here is an output run with the package from the test ppa: http://people.ubuntu.com/~ahasenack/deb8-modperl2-xenial-ppa/

[Christian Ehrhardt  (paelzer)]

Thanks for adding the Bug refs, from a packaging POV this LGTM.

It leaves open the convincing of the SRU Team to push an update for it, but I think it might be ok.
Worst case they will ask you to do a force-badtest hint, but then the coverage of the tests would be lost and since you have the fix ready, why not.

Two minor comments that you might include if you want (maybe only in the bug for the second).
- this is a chain of thanks, you mention Niko, and in the Debian change he mentions Stefan Fritsch.
  Not sure, but if you want to update you might want to mention the original Author instead of the
  Debian packager?
- The Bug and Comments mention this is a compat needed for 2.4.24 but Xenial has apache2/2.4.18-2ubuntu3.9 it seems - I assume that some security update might have brought the changes in, it might be worth to find what exactly and mention it in the changelog/patch-headers

review: Approve

[Andreas Hasenack (ahasenack)]

I think it's less confusing to drop the thanks from d/changelog. That was put in there by the dep3changelog script. The patch files have the authorship explicitly stated.

2.4.24 was unreleased, and our apache was affected by this test failure due to a change there that became a security fix we backported ("HTTP strict parsing changes"):

Changes with Apache 2.4.26

...

  *) SECURITY: CVE-2017-7668 (cve.mitre.org)
     The HTTP strict parsing changes added in 2.2.32 and 2.4.24 introduced a
     bug in token list parsing, which allows ap_find_token() to search past
     the end of its input string. By maliciously crafting a sequence of
     request headers, an attacker may be able to cause a segmentation fault,
     or to force ap_find_token() to return an incorrect value.
     [Jacob Champion]

[Andreas Hasenack (ahasenack)]

I believe it was this change in 2.4.18-2ubuntu3.2:

  * SECURITY UPDATE: response splitting and cache pollution issue via
    incomplete RFC7230 HTTP request grammar enforcing
    - debian/patches/CVE-2016-8743.patch: enfore stricter parsing in
      include/http_core.h, include/http_protocol.h, include/httpd.h,
      modules/http/http_filters.c, server/core.c, server/gen_test_char.c,
      server/protocol.c, server/util.c, server/vhost.c.
    - debian/patches/hostnames_with_underscores.diff: relax hostname
      restrictions in server/vhost.c.
    - CVE-2016-8743
  * WARNING: The fix for CVE-2016-8743 introduces a behavioural change and
    may introduce compatibility issues with clients that do not strictly
    follow specifications. A new configuration directive,
    "HttpProtocolOptions Unsafe" can be used to re-enable some of the less
    strict parsing restrictions, at the expense of security.

[Andreas Hasenack (ahasenack)]

I updated d/changelog with this information, instead of the patches. Let me know if you would rather have this info in the patches.

[Christian Ehrhardt  (paelzer)]

changelog is fine - any place is ok for that IMHO, thanks for adding it - I think you are good to go now.

[Andreas Hasenack (ahasenack)]

Getting bored during your PTO? :)

Thanks, please set the MP to approved and push the upload tag. Or just change the MP status, Robie can push the tag tomorrow. I can then dput the package.

[Robie Basak (racb)]

upload/2.0.9-4ubuntu1.1 pushed as afe63fa

[Christian Ehrhardt  (paelzer)]

Setting to merged as it was called uploaded.
It is there https://launchpadlibrarian.net/378238062/libapache2-mod-perl2_2.0.9-4ubuntu1.1_source.changes

While this needs SRU team handling we can take it out of the review queue for now.

diff --git a/debian/changelog b/debian/changelog
index 1f1d278..f449fe1 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+libapache2-mod-perl2 (2.0.9-4ubuntu1.1) xenial; urgency=medium
+
+  * Pull fixes from Debian's package 2.0.10-2 for the DEP8 tests that started
+    failing after the CVE-2016-8743 fix for apache2 (LP: #1779400):
+    - debian/patches/370_http_syntax.patch: [PATCH 1/2] Fix
+      t/apache/read.t HTTP syntax for Apache 2.4.24.
+    - debian/patches/380_inject_header_line_terminators.patch: [PATCH 2/2]
+      Fix in_bbs_inject_header line terminators for Apache.
+
+ -- Andreas Hasenack <[e-mail]>  Fri, 29 Jun 2018 15:48:39 -0300
+
 libapache2-mod-perl2 (2.0.9-4ubuntu1) xenial; urgency=low
 
   * Resynchronise with Debian. Remaining changes:
diff --git a/debian/patches/370_http_syntax.patch b/debian/patches/370_http_syntax.patch
new file mode 100644
index 0000000..a173da3
--- /dev/null
+++ b/debian/patches/370_http_syntax.patch
@@ -0,0 +1,34 @@
+From 4a803fdb4c9eae8538293fe31c9222eecb6465be Mon Sep 17 00:00:00 2001
+From: Niko Tyni <[e-mail]>
+Date: Fri, 23 Dec 2016 18:27:23 +0200
+Subject: [PATCH 1/2] Fix t/apache/read.t HTTP syntax for Apache 2.4.24
+ compatibility
+
+HTTP/1.1 RFC 7230, section 2.6. "Protocol Versioning" says the HTTP name
+is case sensitive. Starting with Apache 2.4.24, using lower case will
+make the server issue a 400 Bad request response, causing a test failure.
+
+https://tools.ietf.org/html/rfc7230#section-2.6
+
+Bug-Debian: https://bugs.debian.org/849082
+Bug-Ubuntu: https://launchpad.net/bugs/1779400
+---
+ t/apache/read.t | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/t/apache/read.t b/t/apache/read.t
+index 83670c9..9f7f504 100644
+--- a/t/apache/read.t
++++ b/t/apache/read.t
+@@ -24,7 +24,7 @@ close $fh;
+ 
+ my $size = length $data;
+ 
+-for my $string ("POST $location http/1.0",
++for my $string ("POST $location HTTP/1.0",
+                 "Content-length: $size",
+                 "") {
+     my $line = "$string\r\n";
+-- 
+2.11.0
+
diff --git a/debian/patches/380_inject_header_line_terminators.patch b/debian/patches/380_inject_header_line_terminators.patch
new file mode 100644
index 0000000..286d7be
--- /dev/null
+++ b/debian/patches/380_inject_header_line_terminators.patch
@@ -0,0 +1,46 @@
+From d59229cf4f5b91ed58e25e27977e76f59096b72d Mon Sep 17 00:00:00 2001
+From: Niko Tyni <[e-mail]>
+Date: Sat, 24 Dec 2016 23:07:28 +0200
+Subject: [PATCH 2/2] Fix in_bbs_inject_header line terminators for Apache
+ 2.4.24 compatibility
+
+rfc7230 3.5 says:
+
+  Although the line terminator for the start-line and header fields is
+   the sequence CRLF, a recipient MAY recognize a single LF as a line
+   terminator and ignore any preceding CR.
+
+Apache with strict enabled chooses not to implement the MAY.
+
+Author: Stefan Fritsch <[e-mail]>
+Bug-Debian: https://bugs.debian.org/849082
+Bug-Ubuntu: https://launchpad.net/bugs/1779400
+---
+ t/filter/TestFilter/in_bbs_inject_header.pm | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/t/filter/TestFilter/in_bbs_inject_header.pm b/t/filter/TestFilter/in_bbs_inject_header.pm
+index b09d6f9..5380c65 100644
+--- a/t/filter/TestFilter/in_bbs_inject_header.pm
++++ b/t/filter/TestFilter/in_bbs_inject_header.pm
+@@ -181,7 +181,7 @@ sub handler : FilterConnectionHandler {
+ 
+         if ($data and $data =~ /^POST/) {
+             # demonstrate how to add a header while processing other headers
+-            my $header = "$header1_key: $header1_val\n";
++            my $header = "$header1_key: $header1_val\r\n";
+             push @{ $ctx->{buckets} }, APR::Bucket->new($c->bucket_alloc, $header);
+             debug "queued header [$header]";
+         }
+@@ -199,7 +199,7 @@ sub handler : FilterConnectionHandler {
+             # we hit the headers and body separator, which is a good
+             # time to add extra headers:
+             for my $key (keys %headers) {
+-                my $header = "$key: $headers{$key}\n";
++                my $header = "$key: $headers{$key}\r\n";
+                 push @{ $ctx->{buckets} }, APR::Bucket->new($c->bucket_alloc, $header);
+                 debug "queued header [$header]";
+             }
+-- 
+2.11.0
+
diff --git a/debian/patches/series b/debian/patches/series
index 9d123a8..ab36e55 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -14,3 +14,5 @@ avoid-db-linkage.patch
 0001-Fix-t-api-uri.t-t-apr-uri.t-and-t-apr-ext-uri.t-unde.patch
 0002-Fix-t-modperl-print.t-t-modperl-printf.t-and-t-direc.patch
 svn-r1717474-perl-5.22.patch
+370_http_syntax.patch
+380_inject_header_line_terminators.patch