Emit no-cache headers for SAML messages

[vladimir-mencl-eresearch]

Hi @c00kiemon5ter ,

When testing my deployment, I ran into a caching issue where my browser would replay stale SAML messages originally sent by the SATOSA saml2 backend.

I can see the SAML2 Bindings spec asks for headers disabling caching - essentially:

Cache-Control: no-cache, no-store
Pragma: no-cache

However, when digging into SATOSA and pysaml2, I found that:

• pysaml2 only sets these headers when sending a SAMLResponse (so when acting as an IdP / frontend)
• SATOSA discards headers in redirects

I have a working solution that:
(1) Makes SATOSA pass the headers along:

diff --git a/src/satosa/saml_util.py b/src/satosa/saml_util.py
index fced075..9e58dfd 100644
--- a/src/satosa/saml_util.py
+++ b/src/satosa/saml_util.py
@@ -12,6 +12,6 @@ def make_saml_response(binding, http_args):
     """
     if binding == BINDING_HTTP_REDIRECT:
         headers = dict(http_args["headers"])
-        return SeeOther(str(headers["Location"]))
+        return SeeOther(str(headers["Location"]), headers=http_args["headers"])

     return Response(http_args["data"], headers=http_args["headers"])

(2) Makes pysaml2 emit the headers - setting the above Cache-Control and Pragma headers in apply_binding in entity.py:

        if 'headers' not in info:
            info['headers'] = []
        info['headers'].extend([
                    ("Cache-Control", "no-cache, no-store"),
                    ("Pragma", "no-cache")
                    ])

(This could replace the existing use of these headers in use_http_uri in httpbase.py).

But before sending a set of PRs, I wanted to get feedback on this - whether it would be an appropriate change.

Thanks a lot in advance for getting back to me.

Cheers,
Vlad

Code Version

SATOSA 8.1.1
pysaml 7.2.1

Expected Behavior

No-cache headers sent.

Current Behavior

No-cache headers not sent.

Possible Solution

Send no-cache headers on all SAML requests as per above.

Steps to Reproduce

The browser caching may not be entirely reproducible, but:

1. Open a mod_auth_oidc protected page.
2. Wait about 20 minutes (for the original SAML AuthnRequest to become expired and for the document to expire from browser cache)
3. Duplicate the tab, triggering a new cached load.
4. As the browser follows the redirects, it may reuse the cached response from SATOSA with the SAML AuthnRequest, triggering an error on the IdP.

[c00kiemon5ter]

First of all, I think we should fix this 👍 thanks!

The code on satosa should definitely use the headers, but we need to ensure we do not duplicate them. For example SeeOther takes a redirect_url and adds it to the given headers under the Location header. In our case, the headers already include that header.

(1) should be done, with a few checks or using satosa.response.Response class directly. If we implement the checks I am thinking those should be internal to the classes, otherwise we need to prepare the proper data outside the classes (in make_saml_response).

(2) maybe we should add the headers lower in saml2.httpbase and saml2.pack.http_*_message ..but we already define the method and status and url on saml2.entity.Entity.apply_binding. I think it's fine to have this on apply_binding next to the bindings, but then we should clean the lower-level methods from this aspect. I think it's more important to be consistent on where the headers are set.