From 93e02e1400c25a0172af9d3a14907cba7befa621 Mon Sep 17 00:00:00 2001 From: Xavier Guimard Date: Sat, 21 Jan 2017 09:17:24 +0000 Subject: [PATCH] Error in CSP (#1138) --- .../lib/Lemonldap/NG/Portal/Main/Run.pm | 46 +++++++++++-------- ...-Auth-and-issuer-SAML-POST-IdP-initiated.t | 9 ++-- ...h-and-issuer-SAML-Redirect-IdP-initiated.t | 11 +++-- .../t/31-Auth-and-issuer-CAS.t | 10 ++-- ...-Auth-and-issuer-OIDC-authorization_code.t | 5 +- 5 files changed, 47 insertions(+), 34 deletions(-) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm index a75256448..1510a11b0 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm @@ -637,27 +637,33 @@ sub sendHtml { push @{ $req->respHeaders }, 'X-XSS-Protection' => '1; mode=block', 'X-Content-Type-Options' => 'nosniff'; - unless ( $req->frame or $self->conf->{portalAntiFrame} == 0 ) { - my $csp = $self->csp . "form-action 'self'"; - my $url = $args{params}->{URL}; - if ( $url and $url =~ s#https?://([^/]+).*#$1# ) { - $csp .= " $url"; - } - $csp .= ';'; - my @url; - if ( $req->info ) { - @url = map { s#https?://([^/]+).*#$1#; $_ } - ( $req->info =~ /respHeaders }, 'X-Frame-Options' => 'DENY'; - $csp .= "frame-ancestors 'none';"; - } - push @{ $req->respHeaders }, 'Content-Security-Policy' => $csp; + + # Set authorizated URL for POST + my $csp = $self->csp . "form-action 'self'"; + my $url = $args{params}->{URL}; + if ( $url and $url =~ s#https?://([^/]+).*#$1# ) { + $csp .= " $url"; } + $csp .= ';'; + + # Deny using portal in frame except if it is required + unless ( $req->frame or $self->conf->{portalAntiFrame} == 0 ) { + push @{ $req->respHeaders }, 'X-Frame-Options' => 'DENY'; + $csp .= "frame-ancestors 'none';"; + } + + # Check if frames need to be embedded + my @url; + if ( $req->info ) { + @url = map { s#https?://([^/]+).*#$1#; $_ } + ( $req->info =~ /respHeaders }, 'Content-Security-Policy' => $csp; return $self->SUPER::sendHtml( $req, $template, %args ); } diff --git a/lemonldap-ng-portal/t/30-Auth-and-issuer-SAML-POST-IdP-initiated.t b/lemonldap-ng-portal/t/30-Auth-and-issuer-SAML-POST-IdP-initiated.t index b6d27cb37..bf04e05de 100644 --- a/lemonldap-ng-portal/t/30-Auth-and-issuer-SAML-POST-IdP-initiated.t +++ b/lemonldap-ng-portal/t/30-Auth-and-issuer-SAML-POST-IdP-initiated.t @@ -94,11 +94,11 @@ m#iframe src="http://auth.idp.com(/saml/relaySingleLogoutPOST)\?(relay=.*?)"#s, ( $url, $query ) = ( $1, $2 ); ok( getHeader( $res, 'Content-Security-Policy' ) =~ - /frame-ancestors auth.idp.com/, + /child-src auth.idp.com/, ' Frame is authorizated' ) or explain( $res->[1], - 'Content-Security-Policy => ...frame-ancestors auth.idp.com' ); + 'Content-Security-Policy => ...child-src auth.idp.com' ); ok( $res = $issuer->_get( @@ -109,8 +109,9 @@ m#iframe src="http://auth.idp.com(/saml/relaySingleLogoutPOST)\?(relay=.*?)"#s, ), 'Get iframe' ); - ok( !defined getHeader( $res, 'Content-Security-Policy' ), - ' No CSP header' ); + ok( getHeader( $res, 'Content-Security-Policy' ) !~ /frame-ancessor/, + ' Framing authorizated' ) + or explain( $res->[1], 'No frame-ancessor' ); ( $host, $url, $query ) = expectAutoPost( $res, 'auth.sp.com', '/saml/proxySingleLogout', 'SAMLRequest' ); diff --git a/lemonldap-ng-portal/t/30-Auth-and-issuer-SAML-Redirect-IdP-initiated.t b/lemonldap-ng-portal/t/30-Auth-and-issuer-SAML-Redirect-IdP-initiated.t index 5e6c1bb83..f0ddbce8b 100644 --- a/lemonldap-ng-portal/t/30-Auth-and-issuer-SAML-Redirect-IdP-initiated.t +++ b/lemonldap-ng-portal/t/30-Auth-and-issuer-SAML-Redirect-IdP-initiated.t @@ -103,12 +103,11 @@ m#iframe src="http://auth.sp.com(/saml/proxySingleLogout)\?(SAMLRequest=.*?)"#, $url = $1; my $query = $2; ok( - getHeader( $res, 'Content-Security-Policy' ) =~ - /frame-ancestors auth.sp.com/, + getHeader( $res, 'Content-Security-Policy' ) =~ /child-src auth.sp.com/, 'Frame is authorizated' ) or explain( $res->[1], - 'Content-Security-Policy => ...frame-ancestors auth.idp.com' ); + 'Content-Security-Policy => ...child-src auth.idp.com' ); switch ('sp'); ok( $res = $sp->_get( $url, query => $query, accept => 'text/html' ), @@ -121,8 +120,10 @@ m#iframe src="http://auth.sp.com(/saml/proxySingleLogout)\?(SAMLRequest=.*?)"#, ok( $res = $issuer->_get( $url, query => $query, accept => 'text/html' ), 'Push SAML response to IdP' ); expectOK($res); - ok( !defined getHeader( $res, 'Content-Security-Policy' ), - ' No CSP header' ); + ok( getHeader( $res, 'Content-Security-Policy' ) !~ /frame-ancessor/, + ' Frame can be embedded' ) + or explain( $res->[1], + 'Content-Security-Policy does not contain a frame-ancessor' ); # Test if logout is done switch ('issuer'); diff --git a/lemonldap-ng-portal/t/31-Auth-and-issuer-CAS.t b/lemonldap-ng-portal/t/31-Auth-and-issuer-CAS.t index 0b15724d9..1e35eb233 100644 --- a/lemonldap-ng-portal/t/31-Auth-and-issuer-CAS.t +++ b/lemonldap-ng-portal/t/31-Auth-and-issuer-CAS.t @@ -107,11 +107,11 @@ SKIP: { $query = $2; ok( getHeader( $res, 'Content-Security-Policy' ) =~ - /frame-ancestors auth.idp.com/, + /child-src auth.idp.com/, 'Frame is authorizated' ) or explain( $res->[1], - 'Content-Security-Policy => ...frame-ancestors auth.idp.com' ); + 'Content-Security-Policy => ...child-src auth.idp.com' ); switch ('issuer'); ok( @@ -124,8 +124,10 @@ SKIP: { 'Get iframe from IdP' ); expectOK($res); - ok( !defined getHeader( $res, 'Content-Security-Policy' ), - ' No CSP header' ); + ok( getHeader( $res, 'Content-Security-Policy' ) !~ /frame-ancessor/, + ' Frame can be embedded' ) + or explain( $res->[1], + 'Content-Security-Policy does not contain a frame-ancessor' ); # Verify that user has been disconnected ok( $res = $issuer->_get( '/', cookie => "lemonldap=$idpId" ), diff --git a/lemonldap-ng-portal/t/32-Auth-and-issuer-OIDC-authorization_code.t b/lemonldap-ng-portal/t/32-Auth-and-issuer-OIDC-authorization_code.t index f3e5b1bbc..5d43e1fa2 100644 --- a/lemonldap-ng-portal/t/32-Auth-and-issuer-OIDC-authorization_code.t +++ b/lemonldap-ng-portal/t/32-Auth-and-issuer-OIDC-authorization_code.t @@ -86,7 +86,10 @@ ok( ); count(1); expectOK($res); -ok( !defined getHeader( $res, 'Content-Security-Policy' ), ' No CSP header' ); +ok( getHeader( $res, 'Content-Security-Policy' ) !~ /frame-ancessor/, + ' Frame can be embedded' ) + or explain( $res->[1], + 'Content-Security-Policy does not contain a frame-ancessor' ); count(1); # Logout initiated by RP