Package: release.debian.org Severity: normal Tags: bookworm User: release.debian....@packages.debian.org Usertags: pu X-Debbugs-Cc: lemonldap...@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 <y...@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 <chr...@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 <y...@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 <y...@debian.org> + Maxime Besson <maxime.bes...@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\anyth...@test1.example.com/' ++ 'aHR0cHM6Ly9oYWNrZXIuY29tXGFueXRoaW5nQHRlc3QxLmV4YW1wbGUuY29tLw==' => ++ 'https://hacker.com%5canyth...@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