Bug#1050997: bookworm-pu: package lemonldap-ng/2.16.1+ds-deb12u1
Package: release.debian.org
Severity: normal
Tags: bookworm
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: lemonldap-ng@packages.debian.org
Control: affects -1 + src:lemonldap-ng
[ Reason ]
Version 2.17.0 of lemonldap-ng fixes two low-level security issues:
* the "login" security regex wasn't applied when using AuthSlave
* lemonldap-ng portal can be used as open-redirection due to incorrect
escape handling
This proposal includes these 2 patches for Bookworm
[ Impact ]
Low security issues
[ Tests ]
Test updated, passed both with autopkgtest and build
[ Risks ]
No risk, patch is trivial
[ Checklist ]
[X] *all* changes are documented in the d/changelog
[X] I reviewed all changes and I approve them
[X] attach debdiff against the package in (old)stable
[X] the issue is verified as fixed in unstable
[ Changes ]
* check if login value respects the config when login comes from
AuthSlave
* Sanitize URLs used in redirections
* Tests
Cheers,
Yadd
diff --git a/debian/changelog b/debian/changelog
index 8de0d083f..268c0d993 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+lemonldap-ng (2.16.1+ds-deb12u1) UNRELEASED; urgency=medium
+
+ * Apply login control to auth-slave requests
+ * Fix open redirection due to incorrect escape handling
+
+ -- Yadd <yadd@debian.org> Fri, 01 Sep 2023 10:11:50 +0400
+
lemonldap-ng (2.16.1+ds-2) unstable; urgency=medium
* Fix incorrect parsing of OP-provided acr
diff --git a/debian/gitlab-ci.yml b/debian/gitlab-ci.yml
index 33c3a640d..756ccd252 100644
--- a/debian/gitlab-ci.yml
+++ b/debian/gitlab-ci.yml
@@ -1,4 +1,6 @@
---
+variables:
+ RELEASE: 'bookworm'
include:
- https://salsa.debian.org/salsa-ci-team/pipeline/raw/master/salsa-ci.yml
- https://salsa.debian.org/salsa-ci-team/pipeline/raw/master/pipeline-jobs.yml
diff --git a/debian/patches/apply-user-control-to-authslave.patch b/debian/patches/apply-user-control-to-authslave.patch
new file mode 100644
index 000000000..df0ceca39
--- /dev/null
+++ b/debian/patches/apply-user-control-to-authslave.patch
@@ -0,0 +1,83 @@
+Description: [Security] apply user-control to authSlave
+Author: Christophe Maudoux <chrmdx@gmail.com>
+Origin: upstream, https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/-/merge_requests/351/diffs
+Bug: https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/-/issues/2946
+Forwarded: not-needed
+Applied-Upstream: 2.17.0, https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/-/merge_requests/351
+Reviewed-By: Yadd <yadd@debian.org>
+Last-Update: 2023-09-01
+
+--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Auth/Slave.pm
++++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Auth/Slave.pm
+@@ -8,6 +8,7 @@
+ PE_OK
+ PE_FORBIDDENIP
+ PE_USERNOTFOUND
++ PE_MALFORMEDUSER
+ );
+
+ our $VERSION = '2.0.12';
+@@ -37,11 +38,15 @@
+ $user_header = 'HTTP_' . uc($user_header);
+ $user_header =~ s/\-/_/g;
+
+- unless ( $req->{user} = $req->env->{$user_header} ) {
++ unless ( $req->env->{$user_header} ) {
+ $self->userLogger->error(
+ "No header " . $self->conf->{slaveUserHeader} . " found" );
+ return PE_USERNOTFOUND;
+ }
++ return PE_MALFORMEDUSER
++ unless ( $req->env->{$user_header} =~ /$self->{conf}->{userControl}/o );
++
++ $req->{user} = $req->env->{$user_header};
+ return PE_OK;
+ }
+
+--- a/lemonldap-ng-portal/t/25-AuthSlave-with-Credentials.t
++++ b/lemonldap-ng-portal/t/25-AuthSlave-with-Credentials.t
+@@ -2,7 +2,7 @@
+ use Test::More;
+ use strict;
+ use JSON;
+-use Lemonldap::NG::Portal::Main::Constants qw(PE_FORBIDDENIP PE_USERNOTFOUND);
++use Lemonldap::NG::Portal::Main::Constants qw(PE_FORBIDDENIP PE_USERNOTFOUND PE_MALFORMEDUSER);
+
+ require 't/test-lib.pm';
+
+@@ -17,6 +17,7 @@
+ securedCookie => 3,
+ authentication => 'Slave',
+ userDB => 'Same',
++ userControl => '^\w{4}$',
+ slaveUserHeader => 'My-Test',
+ slaveHeaderName => 'Check-Slave',
+ slaveHeaderContent => 'Password',
+@@ -91,6 +92,27 @@
+ or explain( $json, "error => 4" );
+ count(4);
+
++# Good credentials with an unauthorized login
++ok(
++ $res = $client->_get(
++ '/',
++ ip => '127.0.0.1',
++ custom => {
++ HTTP_MY_TEST => 'dwhoo',
++ HTTP_NAME => 'Dr Who',
++ HTTP_CHECK_SLAVE => 'Password',
++ }
++
++ ),
++ 'Auth query'
++);
++ok( $res->[0] == 401, 'Get 401' ) or explain( $res->[0], 401 );
++ok( $json = eval { from_json( $res->[2]->[0] ) }, 'Response is JSON' )
++ or print STDERR "$@\n" . Dumper($res);
++ok( $json->{error} == PE_MALFORMEDUSER, 'Response is PE_MALFORMEDUSER' )
++ or explain( $json, "error => 40" );
++count(4);
++
+ # Good credentials with acredited IP
+ ok(
+ $res = $client->_get(
diff --git a/debian/patches/fix-open-redirection.patch b/debian/patches/fix-open-redirection.patch
new file mode 100644
index 000000000..96850a2a4
--- /dev/null
+++ b/debian/patches/fix-open-redirection.patch
@@ -0,0 +1,291 @@
+Description: fix open redirection
+Author: Yadd <yadd@debian.org>
+ Maxime Besson <maxime.besson@worteks.com>
+Origin: upstream, https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/-/merge_requests/342/diffs
+Bug: https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/-/issues/2931
+Forwarded: not-needed
+Applied-Upstream: 2.17.0, https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/-/merge_requests/342
+Last-Update: 2023-09-01
+
+--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/ApacheMP2/Main.pm
++++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/ApacheMP2/Main.pm
+@@ -16,6 +16,7 @@
+ use APR::Table;
+ use Apache2::Const -compile =>
+ qw(FORBIDDEN HTTP_UNAUTHORIZED REDIRECT OK DECLINED DONE SERVER_ERROR AUTH_REQUIRED HTTP_SERVICE_UNAVAILABLE);
++use URI;
+ use base 'Lemonldap::NG::Handler::Main';
+
+ use constant FORBIDDEN => Apache2::Const::FORBIDDEN;
+@@ -166,7 +167,7 @@
+ $f->r->status( $class->REDIRECT );
+ $f->r->status_line("303 See Other");
+ $f->r->headers_out->unset('Location');
+- $f->r->err_headers_out->set( 'Location' => $url );
++ $f->r->err_headers_out->set( 'Location' => URI->new($url)->as_string );
+ $f->ctx(1);
+ }
+ while ( $f->read( my $buffer, 1024 ) ) {
+--- a/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Main/Run.pm
++++ b/lemonldap-ng-handler/lib/Lemonldap/NG/Handler/Main/Run.pm
+@@ -9,6 +9,7 @@
+
+ #use AutoLoader 'AUTOLOAD';
+ use MIME::Base64;
++use URI;
+ use URI::Escape;
+ use Lemonldap::NG::Common::Session;
+
+@@ -690,7 +691,7 @@
+ ) ? '' : ":$portString";
+ my $url = "http" . ( $_https ? "s" : "" ) . "://$realvhost$portString$s";
+ $class->logger->debug("Build URL $url");
+- return $url;
++ return URI->new($url)->as_string;
+ }
+
+ ## @rmethod protected int isUnprotected()
+--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/CDC.pm
++++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/CDC.pm
+@@ -9,6 +9,7 @@
+ use Mouse;
+ use MIME::Base64;
+ use Lemonldap::NG::Common::FormEncode;
++use URI;
+
+ our $VERSION = '2.0.6';
+
+@@ -163,7 +164,10 @@
+ );
+
+ # Redirect
+- return [ 302, [ Location => $urldc, $req->spliceHdrs ], [] ];
++ return [
++ 302, [ Location => URI->new($urldc)->as_string, $req->spliceHdrs ],
++ []
++ ];
+
+ }
+
+--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Issuer/CAS.pm
++++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Issuer/CAS.pm
+@@ -17,6 +17,7 @@
+ PE_UNAUTHORIZEDPARTNER
+ PE_CAS_SERVICE_NOT_ALLOWED
+ );
++use URI;
+
+ our $VERSION = '2.0.15';
+
+@@ -93,7 +94,8 @@
+ if ( $self->_gatewayAllowedRedirect( $req, $service ) ) {
+ $self->logger->debug(
+ "Gateway mode requested, redirect without authentication");
+- $req->response( [ 302, [ Location => $service ], [] ] );
++ $req->response(
++ [ 302, [ Location => URI->new($service)->as_string ], [] ] );
+ for my $s ( $self->ipath, $self->ipath . 'Path' ) {
+ $self->logger->debug("Removing $s from pdata")
+ if delete $req->pdata->{$s};
+--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm
++++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/OpenIDConnect.pm
+@@ -24,6 +24,7 @@
+ use URI::QueryParam;
+ use Mouse;
+ use Crypt::URandom;
++use URI;
+
+ use Lemonldap::NG::Portal::Main::Constants qw(PE_OK PE_REDIRECT PE_ERROR);
+
+@@ -2288,7 +2289,7 @@
+ $response_url .= build_urlencoded( state => $state );
+ }
+
+- return $response_url;
++ return URI->new($response_url)->as_string;
+ }
+
+ # Create session_state parameter
+--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/SAML.pm
++++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/SAML.pm
+@@ -2658,7 +2658,7 @@
+
+ # Redirect user to response URL
+ my $slo_url = $logout->msg_url;
+- return [ 302, [ Location => $slo_url ], [] ];
++ return [ 302, [ Location => URI->new($slo_url)->as_string ], [] ];
+ }
+
+ # HTTP-POST
+--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Process.pm
++++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Process.pm
+@@ -144,6 +144,7 @@
+ $req->{urldc} =~ s/[\r\n]//sg;
+ }
+ }
++ $req->{urldc} = URI->new( $req->{urldc} )->as_string;
+
+ # For logout request, test if Referer comes from an authorized site
+ my $tmp = (
+--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm
++++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm
+@@ -440,7 +440,14 @@
+ $req->data->{redirectFormMethod} = "get";
+ }
+ else {
+- return [ 302, [ Location => $req->{urldc}, $req->spliceHdrs ], [] ];
++ return [
++ 302,
++ [
++ Location => URI->new( $req->{urldc} )->as_string,
++ $req->spliceHdrs
++ ],
++ []
++ ];
+ }
+ }
+ my ( $tpl, $prms ) = $self->display($req);
+@@ -1308,7 +1315,7 @@
+ my ( $self, $url ) = @_;
+ return unless $url;
+ my $uri = $url;
+- unless (blessed($uri) && $uri->isa("URI") ) {
++ unless ( blessed($uri) && $uri->isa("URI") ) {
+ $uri = URI->new($uri);
+ }
+ my $scheme = $uri->scheme || "";
+--- a/lemonldap-ng-portal/t/03-XSS-protection.t
++++ b/lemonldap-ng-portal/t/03-XSS-protection.t
+@@ -5,8 +5,7 @@
+
+ require 't/test-lib.pm';
+
+-my $client = LLNG::Manager::Test->new(
+- {
++my $client = LLNG::Manager::Test->new( {
+ ini => {
+ logLevel => 'error',
+ useSafeJail => 1,
+@@ -21,21 +20,25 @@
+ '' => 0, 'Empty',
+
+ # 2 http://test1.example.com/
+- 'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tLw==' => 1, 'Protected virtual host',
++ 'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tLw==' => 'http://test1.example.com/',
++ 'Protected virtual host',
+
+ # 3 http://test1.example.com
+- 'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29t' => 1, 'Missing / in URL',
++ 'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29t' => 'http://test1.example.com',
++ 'Missing / in URL',
+
+ # 4 http://test1.example.com:8000/test
+- 'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tOjgwMDAvdGVzdA==' => 1,
++ 'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tOjgwMDAvdGVzdA==' =>
++ 'http://test1.example.com:8000/test',
+ 'Non default port',
+
+ # 5 http://test1.example.com:8000/
+- 'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tOjgwMDAv' => 1,
++ 'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tOjgwMDAv' =>
++ 'http://test1.example.com:8000/',
+ 'Non default port with missing /',
+
+ # 6 http://t.example2.com/test
+- 'aHR0cDovL3QuZXhhbXBsZTIuY29tL3Rlc3Q=' => 1,
++ 'aHR0cDovL3QuZXhhbXBsZTIuY29tL3Rlc3Q=' => 'http://t.example2.com/test',
+ 'Undeclared virtual host in trusted domain',
+
+ # 7 http://testexample2.com/
+@@ -49,7 +52,7 @@
+ . ' "example3.com" is trusted, but domain "*.example3.com" not)',
+
+ # 9 http://example3.com/
+- 'aHR0cDovL2V4YW1wbGUzLmNvbS8K' => 1,
++ 'aHR0cDovL2V4YW1wbGUzLmNvbS8K' => 'http://example3.com/',
+ 'Undeclared virtual host with trusted domain name',
+
+ # 10 http://t.example.com/test
+@@ -87,6 +90,21 @@
+ 'aHR0cHM6Ly90ZXN0MS5leGFtcGxlLmNvbTp0ZXN0QGhhY2tlci5jb20=' => 0,
+ 'userinfo trick',
+
++ # 22 url=https://hacker.com\@@test1.example.com/
++ 'aHR0cHM6Ly9oYWNrZXIuY29tXEBAdGVzdDEuZXhhbXBsZS5jb20v' =>
++ 'https://hacker.com%5C@@test1.example.com/',
++ 'Good reencoding (2931)',
++
++ # 23 url=https://hacker.com:\@@test1.example.com/
++ 'aHR0cHM6Ly9oYWNrZXIuY29tOlxAQHRlc3QxLmV4YW1wbGUuY29tLw==' =>
++ 'https://hacker.com:%5C@@test1.example.com/',
++ 'Good reencoding (2931)',
++
++ # 24 url='https://hacker.com\anything@test1.example.com/'
++ 'aHR0cHM6Ly9oYWNrZXIuY29tXGFueXRoaW5nQHRlc3QxLmV4YW1wbGUuY29tLw==' =>
++ 'https://hacker.com%5Canything@test1.example.com/',
++ 'Good reencoding (2931)',
++
+ # LOGOUT TESTS
+ 'LOGOUT',
+
+@@ -97,7 +115,7 @@
+
+ # 19 url=http://www.toto.com/, good referer
+ 'aHR0cDovL3d3dy50b3RvLmNvbS8=',
+- 'http://test1.example.com/' => 1,
++ 'http://test1.example.com/' => 'http://www.toto.com/',
+ 'Logout required by good site',
+
+ # 20 url=http://www?<script>, good referer
+@@ -107,7 +125,7 @@
+
+ # 21 url=http://www.toto.com/, no referer
+ 'aHR0cDovL3d3dy50b3RvLmNvbS8=',
+- '' => 1,
++ '' => 'http://www.toto.com/',
+ 'Logout required by good site, empty referer',
+ );
+
+@@ -137,10 +155,13 @@
+ ),
+ $detail
+ );
+- ok( ( $res->[0] == ( $redir ? 302 : 200 ) ),
+- ( $redir ? 'Get redirection' : 'Redirection dropped' ) )
+- or explain( $res->[0], ( $redir ? 302 : 200 ) );
+- count(2);
++ if ($redir) {
++ expectRedirection( $res, $redir );
++ }
++ else {
++ expectOK($res);
++ }
++ count(1);
+ }
+
+ while ( defined( my $url = shift(@tests) ) ) {
+@@ -158,9 +179,12 @@
+ ),
+ $detail
+ );
+- ok( ( $res->[0] == ( $redir ? 302 : 200 ) ),
+- ( $redir ? 'Get redirection' : 'Redirection dropped' ) )
+- or explain( $res->[0], ( $redir ? 302 : 200 ) );
++ if ($redir) {
++ expectRedirection( $res, $redir );
++ }
++ else {
++ expectOK($res);
++ }
+ ok(
+ $res = $client->_post(
+ '/',
+@@ -171,7 +195,7 @@
+ );
+ expectOK($res);
+ $id = expectCookie($res);
+- count(3);
++ count(2);
+ }
+
+ clean_sessions();
diff --git a/debian/patches/series b/debian/patches/series
index 0fe038944..14369dfd8 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,3 +5,5 @@ replace-api-doc-by-link.diff
drop-network-test.patch
fix-OP-acr-parsing.patch
fix-viewer-endpoint.patch
+apply-user-control-to-authslave.patch
+fix-open-redirection.patch
Reply to: