Unicon / shibboleth-idp-dockerized

A Shibboleth Identity Provider (IdP) base-image
Apache License 2.0
108 stars 54 forks source link

Headers configured in jetty-rewrite.xml not compatable with SLO #64

Open netscruff opened 5 years ago

netscruff commented 5 years ago

The Content-Security-Policy and X-Frame-Options headers set in jetty-rewrite.xml conflict with single logout. At a minimum the PropagateLogout url need to be excluded from having these headers configured or SLO will fail. I just removed the jetty-rewrite.xml file from the image and rebuilt and was able to successfully test SLO under our deployment.

I think the IdP now has functionality to set these headers itself so I don't think this should be done here anymroe.

jtgasper3 commented 5 years ago

Wait?!? Someone actually uses SAML SLO? j/k.

The image is using the guidance at https://wiki.shibboleth.net/confluence/display/IDP30/Jetty93#Jetty93-ClickjackMitigation. It looks like it is still documented. I'm torn... Do you have a link to the documentation where the IdP natively supports the functionality?

BTW, pull requests are always considered (eventually... sometimes I'm really slow about it.)

netscruff commented 5 years ago

This was new to me in the idp.properties:

# HSTS/CSP response headers
#idp.hsts = max-age=0
# X-Frame-Options value, set to DENY or SAMEORIGIN to block framing`
#idp.frameoptions = DENY
# Content-Security-Policy value, set to match X-Frame-Options default
#idp.csp = frame-ancestors 'none';

I didn't have these options in my idp.properties but they exist in the idp.properties for new installs.

I'm not sure which release this was added to and haven't tried uncommenting the options and looking at http headers so I can't confirm yet. There was a discussion in the shibboleth users mailing list about this and that they are not set in the logout urls because of the use of iframes in SLO.

I'll do a few more tests and then create a PR if I see the header appearing when uncommented in the configuration.

jtgasper3 commented 5 years ago

Good enough for me... I'll revert that portion of the file jetty-rewrite.xml, (or the whole thing, if it was only for this functionality).