Fix two security bugs

Related: rhbz#1691771 - CVE-2019-3877 mod_auth_mellon: open redirect in
                        logout url when using URLs with backslashes
Related: rhbz#1691136 - CVE-2019-3878 mod_auth_mellon: authentication
                        bypass in ECP flow

.gitignore

@@ -5,3 +5,4 @@
 /mod_auth_mellon-0.11.0.tar.gz
 /mod_auth_mellon-0.12.0.tar.gz
 /mod_auth_mellon-0.13.1.tar.gz
+/mod_auth_mellon-0.14.0.tar.gz

0001-Modify-am_handler-setup-to-run-before-mod_proxy.patch

@@ -0,0 +1,80 @@
+From e09a28a30e13e5c22b481010f26b4a7743a09280 Mon Sep 17 00:00:00 2001
+From: John Dennis <jdennis@redhat.com>
+Date: Tue, 5 Mar 2019 10:15:48 +0100
+Subject: [PATCH] Modify am_handler setup to run before mod_proxy
+
+The way the ECP flow works is that when a client initiates the flow, the
+SP's response is HTTP 200, but not the requested content, but a signed XML
+document that contains the "samlp:AuthnRequest" element. The idea is that
+the ECP client would then determine the IDP and send the document to the
+IDP, get a samlp:Response and convey that to the SP to get access to the
+protected resource.
+
+Internally, the auth check which is normally done with am_check_uid() set to
+apache's ap_hook_check_user_id() hook, just responds with OK, so it pretends
+to authenticate the user. Then in the usual flow, the request reaches the
+ap_hook_handler which handles the request. There in the pipeline, mellon
+registers functions am_handler() which should run first (APR_HOOK_FIRST),
+determine that this request is an ECP one and return the ECP AuthnRequest
+document. But in case the proxy module is also in the picture, the proxy
+module "races" for who gets to be the first to handle the request in the
+pipeline and wins. Therefore, the request reaches the protected resource
+via mod_proxy and returns it.
+
+This fix modifies the ap_hook_handler() call to explicitly run before
+handlers from mod_proxy.c
+
+To reproduce the bug:
+0) Have a SP with mellon connected to a Keycloak IDP (or any other IDP I
+   guess). In the example below, my SAML SP is saml.federation.test
+1) Set a Location protected by mellon that proxies requests to another
+   URL. For example:
+
+    ProxyPass         /sp-proxy  http://app.federation.test/example_app/
+    <Location /sp-proxy>
+        AuthType Mellon
+        MellonEnable auth
+        Require valid-user
+    </Location>
+
+2) call:
+ curl -L -H "Accept: application/vnd.paos+xml" \
+         -H 'PAOS: ver="urn:liberty:paos:2003-08";"urn:oasis:names:tc:SAML:2.0:profiles:SSO:ecp"' \
+          http://saml.federation.test/sp-proxy
+
+Before the patch, you would see whatever is served from the proxied
+page. With the patch, you should get back a XML document with a
+samlp:AuthnRequest.
+---
+ mod_auth_mellon.c | 8 +++++++-
+ 1 file changed, 7 insertions(+), 1 deletion(-)
+
+diff --git a/mod_auth_mellon.c b/mod_auth_mellon.c
+index 74bd328..5330f48 100644
+--- a/mod_auth_mellon.c
++++ b/mod_auth_mellon.c
+@@ -207,6 +207,12 @@ static int am_create_request(request_rec *r)
+ 
+ static void register_hooks(apr_pool_t *p)
+ {
++    /* Our handler needs to run before mod_proxy so that it can properly
++     * return ECP AuthnRequest messages when running as a reverse proxy.
++     * See: https://github.com/Uninett/mod_auth_mellon/pull/196
++     */
++    static const char * const run_handler_before[]={ "mod_proxy.c", NULL };
++
+     ap_hook_access_checker(am_auth_mellon_user, NULL, NULL, APR_HOOK_MIDDLE);
+     ap_hook_check_user_id(am_check_uid, NULL, NULL, APR_HOOK_MIDDLE);
+     ap_hook_post_config(am_global_init, NULL, NULL, APR_HOOK_MIDDLE);
+@@ -222,7 +228,7 @@ static void register_hooks(apr_pool_t *p)
+      * Therefore this hook must run before any handler that may check
+      * r->handler and decide that it is the only handler for this URL.
+      */
+-    ap_hook_handler(am_handler, NULL, NULL, APR_HOOK_FIRST);
++    ap_hook_handler(am_handler, NULL, run_handler_before, APR_HOOK_FIRST);
+ 
+ #ifdef ENABLE_DIAGNOSTICS
+     ap_hook_open_logs(am_diag_log_init,NULL,NULL,APR_HOOK_MIDDLE);
+-- 
+2.19.2
+

0002-Fix-redirect-URL-validation-bypass.patch

@@ -0,0 +1,44 @@
+From 62041428a32de402e0be6ba45fe12df6a83bedb8 Mon Sep 17 00:00:00 2001
+From: Olav Morken <olav.morken@uninett.no>
+Date: Tue, 19 Mar 2019 13:42:22 +0100
+Subject: [PATCH] Fix redirect URL validation bypass
+
+It turns out that browsers silently convert backslash characters into
+forward slashes, while apr_uri_parse() does not.
+
+This mismatch allows an attacker to bypass the redirect URL validation
+by using an URL like:
+
+  https://sp.example.org/mellon/logout?ReturnTo=https:%5c%5cmalicious.example.org/
+
+mod_auth_mellon will assume that it is a relative URL and allow the
+request to pass through, while the browsers will use it as an absolute
+url and redirect to https://malicious.example.org/ .
+
+This patch fixes this issue by rejecting all redirect URLs with
+backslashes.
+---
+ auth_mellon_util.c | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/auth_mellon_util.c b/auth_mellon_util.c
+index 0fab309..fd442f9 100644
+--- a/auth_mellon_util.c
++++ b/auth_mellon_util.c
+@@ -927,6 +927,13 @@ int am_check_url(request_rec *r, const char *url)
+                           "Control character detected in URL.");
+             return HTTP_BAD_REQUEST;
+         }
++        if (*i == '\\') {
++            /* Reject backslash character, as it can be used to bypass
++             * redirect URL validation. */
++            AM_LOG_RERROR(APLOG_MARK, APLOG_ERR, HTTP_BAD_REQUEST, r,
++                          "Backslash character detected in URL.");
++            return HTTP_BAD_REQUEST;
++        }
+     }
+ 
+     return OK;
+-- 
+2.19.2
+

README.redhat.rst

@@ -0,0 +1,83 @@
+Red Hat Specific mod_auth_mellon Information
+============================================
+
+This README contains information specific to Red Hat's distribution of
+``mod_auth_mellon``.
+
+Diagnostic Logging
+------------------
+
+Diagnostic logging can be used to collect run time information to help
+diagnose problems with your ``mod_auth_mellon`` deployment. Please see
+the "Mellon Diagnostics" section in the Mellon User Guide for more
+details.
+
+How to enable diagnostic logging on Red Hat systems
+```````````````````````````````````````````````````
+
+Diagnostic logging adds overhead to the execution of
+``mod_auth_mellon``. The code to emit diagnostic logging must be
+compiled into ``mod_auth_mellon`` at build time. In addition the
+diagnostic log file may contain security sensitive information which
+should not normally be written to a log file. If you have a
+version of ``mod_auth_mellon`` which was built with diagnostics you
+can disable diagnostic logging via the ``MellonDiagnosticsEnable``
+configuration directive. However given human nature the potential to
+enable diagnostic logging while resolving a problem and then forget to
+disable it is not a situation that should exist by default. Therefore
+given the overhead consideration and the desire to avoid enabling
+diagnostic logging by mistake the Red Hat ``mod_auth_mellon`` RPM's
+ship with two versions of the ``mod_auth_mellon`` Apache module.
+
+1. The ``mod_auth_mellon`` RPM contains the normal Apache module
+   ``/usr/lib*/httpd/modules/mod_auth_mellon.so`` 
+
+2. The ``mod_auth_mellon-diagnostics`` RPM contains the diagnostic
+   version of the Apache module
+   ``/usr/lib*/httpd/modules/mod_auth_mellon-diagnostics.so``
+
+Because each version of the module has a different name both the
+normal and diagnostic modules can be installed simultaneously without
+conflict. But Apache will only load one of the two modules. Which
+module is loaded is controlled by the
+``/etc/httpd/conf.modules.d/10-auth_mellon.conf`` config file which
+has a line in it which looks like this::
+
+    LoadModule auth_mellon_module modules/mod_auth_mellon.so
+
+To load the diagnostics version of the module you need to change the
+module name so it looks like this::
+
+    LoadModule auth_mellon_module modules/mod_auth_mellon-diagnostics.so
+
+**Don't forget to change it back again when you're done debugging.**
+
+You'll also need to enable the collection of diagnostic information,
+do this by adding this directive at the top of your Mellon conf.d
+config file or inside your virtual host config (diagnostics are per
+server instance)::
+
+    MellonDiagnosticsEnable On
+
+.. NOTE::
+   Some versions of the Mellon User Guide have a typo in the name of
+   this directive, it incorrectly uses ``MellonDiagnosticEnable``
+   instead of ``MellonDiagnosticsEnable``. The difference is
+   Diagnostics is plural.
+
+The Apache ``error_log`` will contain a message indicating how it
+processed the ``MellonDiagnosticsEnable`` directive. If you loaded the
+standard module without diagnostics you'll see a message like this::
+
+    MellonDiagnosticsEnable has no effect because Mellon was not
+    compiled with diagnostics enabled, use
+    ./configure --enable-diagnostics at build time to turn this
+    feature on.
+
+If you've loaded the diagnostics version of the module you'll see a
+message in the ``error_log`` like this::
+
+    mellon diagnostics enabled for virtual server *:443
+    (/etc/httpd/conf.d/my_server.conf:7)
+    ServerName=https://my_server.example.com:443, diagnostics
+    filename=logs/mellon_diagnostics

mod_auth_mellon.spec

@@ -1,24 +1,29 @@
 Summary: A SAML 2.0 authentication module for the Apache Httpd Server
 Name: mod_auth_mellon
-Version: 0.13.1
-Release: 2%{?dist}
+Version: 0.14.0
+Release: 4%{?dist}
 Group: System Environment/Daemons
 Source0: https://github.com/UNINETT/mod_auth_mellon/releases/download/v%{version}/%{name}-%{version}.tar.gz
 Source1: auth_mellon.conf
 Source2: 10-auth_mellon.conf
 Source3: mod_auth_mellon.conf
 Source4: mellon_create_metadata.sh
+Source5: README.redhat.rst
 License: GPLv2+
 BuildRequires: curl-devel
 BuildRequires: glib2-devel
 BuildRequires: httpd-devel
-BuildRequires: lasso-devel >= 2.5.0
+BuildRequires: lasso-devel >= 2.5.1-13
 BuildRequires: openssl-devel
 BuildRequires: xmlsec1-devel
+BuildRequires: rubygem-asciidoctor
 Requires: httpd-mmn = %{_httpd_mmn}
-Requires: lasso >= 2.5.0
+Requires: lasso >= 2.5.1-13
 Url: https://github.com/UNINETT/mod_auth_mellon
 
+Patch0001: 0001-Modify-am_handler-setup-to-run-before-mod_proxy.patch
+Patch0002: 0002-Fix-redirect-URL-validation-bypass.patch
+
 
 %description
 The mod_auth_mellon module is an authentication service that implements the
@@ -27,16 +32,28 @@ received in assertions generated by a IdP server.
 
 %prep
 %setup -q -n %{name}-%{version}
+%patch0001 -p1
+%patch0002 -p1
 
 %build
 export APXS=%{_httpd_apxs}
-%configure
+%configure --enable-diagnostics
+make clean
 make %{?_smp_mflags}
+cp .libs/%{name}.so %{name}-diagnostics.so
+
+%configure
+make clean
+make %{?_smp_mflags}
+pushd doc/user_guide
+asciidoctor -a data-uri mellon_user_guide.adoc
+popd
 
 %install
 # install module
 mkdir -p %{buildroot}%{_httpd_moddir}
 install -m 755 .libs/%{name}.so %{buildroot}%{_httpd_moddir}
+install -m 755 %{name}-diagnostics.so %{buildroot}%{_httpd_moddir}
 
 # install module configuration
 mkdir -p %{buildroot}%{_httpd_confdir}
@@ -52,6 +69,26 @@ mkdir -p %{buildroot}/run/%{name}
 mkdir -p %{buildroot}/%{_libexecdir}/%{name}
 install -m 755 %{SOURCE4} %{buildroot}/%{_libexecdir}/%{name}
 
+#install documentation
+mkdir -p %{buildroot}/%{_pkgdocdir}
+
+# install Red Hat README
+install %{SOURCE5} %{buildroot}/%{_pkgdocdir}
+
+# install user guide
+cp -r doc/user_guide %{buildroot}/%{_pkgdocdir}
+
+%package diagnostics
+Summary: Build of mod_auth_mellon with diagnostic logging
+Requires: %{name} = %{version}-%{release}
+
+%description diagnostics
+Build of mod_auth_mellon with diagnostic logging. See README.redhat.rst
+in the doc directory for instructions on using the diagnostics build.
+
+%files diagnostics
+%{_httpd_moddir}/%{name}-diagnostics.so
+
 %files
 %defattr(-,root,root)
 %if 0%{?rhel} && 0%{?rhel} < 7
@@ -59,7 +96,9 @@ install -m 755 %{SOURCE4} %{buildroot}/%{_libexecdir}/%{name}
 %else
 %license COPYING
 %endif
-%doc README NEWS ECP.rst
+%doc README.md NEWS ECP.rst
+%doc %{_pkgdocdir}/README.redhat.rst
+%doc %{_pkgdocdir}/user_guide
 %config(noreplace) %{_httpd_modconfdir}/10-auth_mellon.conf
 %config(noreplace) %{_httpd_confdir}/auth_mellon.conf
 %{_httpd_moddir}/mod_auth_mellon.so
@@ -68,6 +107,22 @@ install -m 755 %{SOURCE4} %{buildroot}/%{_libexecdir}/%{name}
 %dir /run/%{name}/
 
 %changelog
+* Fri Mar 22 2019 Jakub Hrozek <jhrozek@redhat.com> - 0.14.0-4
+- Related: rhbz#1691771 - CVE-2019-3877 mod_auth_mellon: open redirect in
+                          logout url when using URLs with backslashes
+- Related: rhbz#1691136 - CVE-2019-3878 mod_auth_mellon: authentication
+                          bypass in ECP flow
+
+* Wed May  2 2018 John Dennis <jdennis@redhat.com> - 0.14.0-3
+- update lasso version dependency
+
+* Tue May  1 2018 John Dennis <jdennis@redhat.com> - 0.14.0-2
+- clean diagnostics build prior to normal build
+
+* Thu Apr 19 2018 John Dennis <jdennis@redhat.com> - 0.14.0-1
+- Upgrade to new upstream release
+- Add README.redhat.rst doc explaining packaging of this module.
+
 * Thu Feb 08 2018 Fedora Release Engineering <releng@fedoraproject.org> - 0.13.1-2
 - Rebuilt for https://fedoraproject.org/wiki/Fedora_28_Mass_Rebuild
 

sources

@@ -1 +1 @@
-SHA512 (mod_auth_mellon-0.13.1.tar.gz) = ad0479be8aa94404a832d11f7ead1f704d86cab2f11aa6f90b895be9b4028026f15ec8ee85260ca76f4a001c115ff14b4b7c9e8da74676a1f0295f6c2f0a1341
+SHA512 (mod_auth_mellon-0.14.0.tar.gz) = db1bf70c234fe89914b1bb34fc6afb5b901193a8c8c7e9946485a3e20a7d129c36427717eab53764edf5a5cff5c45dfe412e400cb1f50c49ef24dbbfd6ecbf25