Merge pull request #888 from lsst-sqre/tickets/DM-41424

DM-41424: Always mask all Gafaelfawr response headers

changelog.d/20231027_125137_rra_DM_41424.md

@@ -0,0 +1,3 @@
+### Bug fixes
+
+- Always mask all headers to which Gafaelfawr gives special meaning when passing requests to a service downstream of a `GafaelfawrIngress`, instead of only masking the ones Gafaelfawr might set in that configuration. This ensures that no service behind a `GafaelfawrIngress` sees, e.g., `X-Auth-Request-User` unless it truly is authenticated by Gafaelfawr.

src/gafaelfawr/constants.py

@@ -90,6 +90,20 @@
 MINIMUM_LIFETIME = timedelta(minutes=5)
 """Minimum expiration lifetime for a token."""
 
+NGINX_RESPONSE_HEADERS = (
+    "Authorization",
+    "Cookie",
+    "X-Auth-Request-Email",
+    "X-Auth-Request-Token",
+    "X-Auth-Request-User",
+)
+"""Headers to lift from the Gafaelfawr response to the backend request.
+
+Any of these headers in the incoming request will be overwritten with the
+versions of these headers returned by the Gafaelfawr auth subrequest, or
+deleted entirely if the subrequest doesn't return one of these headers.
+"""
+
 NGINX_SNIPPET = """\
 auth_request_set $auth_www_authenticate $upstream_http_www_authenticate;
 auth_request_set $auth_status $upstream_http_x_error_status;

src/gafaelfawr/services/kubernetes.py

@@ -15,7 +15,7 @@
 from structlog.stdlib import BoundLogger
 
 from ..config import Config
-from ..constants import NGINX_SNIPPET
+from ..constants import NGINX_RESPONSE_HEADERS, NGINX_SNIPPET
 from ..exceptions import (
     InputValidationError,
     InvalidScopesError,
@@ -105,9 +105,7 @@ async def update(
             raise
         return await self._update_ingress(old_ingress, new_ingress, parent)
 
-    def _build_annotations(  # noqa: C901
-        self, ingress: GafaelfawrIngress
-    ) -> dict[str, str]:
+    def _build_annotations(self, ingress: GafaelfawrIngress) -> dict[str, str]:
         """Build annotations for an ``Ingress``."""
         base_url = ingress.config.base_url.rstrip("/")
 
@@ -137,11 +135,7 @@ def _build_annotations(  # noqa: C901
         if snippet and not snippet.endswith("\n"):
             snippet += "\n"
         snippet += NGINX_SNIPPET
-        headers = (
-            "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-User"
-        )
-        if ingress.config.delegate:
-            headers += ",X-Auth-Request-Token"
+        headers = ",".join(NGINX_RESPONSE_HEADERS)
         annotations = {
             **ingress.template.metadata.annotations,
             "nginx.ingress.kubernetes.io/auth-method": "GET",
@@ -161,7 +155,7 @@ def _build_anonymous_annotations(
         """Build annotations for an anonymous ``Ingress``."""
         base_url = ingress.config.base_url.rstrip("/")
         auth_url = f"{base_url}/auth/anonymous"
-        headers = "Authorization,Cookie"
+        headers = ",".join(NGINX_RESPONSE_HEADERS)
         return {
             **ingress.template.metadata.annotations,
             "nginx.ingress.kubernetes.io/auth-method": "GET",

tests/data/kubernetes/output/ingresses.yaml

@@ -5,7 +5,7 @@ metadata:
   namespace: {namespace}
   annotations:
     nginx.ingress.kubernetes.io/auth-method: GET
-    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-User"
+    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Token,X-Auth-Request-User"
     nginx.ingress.kubernetes.io/auth-url: "https://foo.example.com/auth?scope=read%3Aall"
     nginx.ingress.kubernetes.io/configuration-snippet: |
       {snippet}
@@ -44,7 +44,7 @@ metadata:
   annotations:
     another.annotation.example.com: bar
     nginx.ingress.kubernetes.io/auth-method: GET
-    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-User,X-Auth-Request-Token"
+    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Token,X-Auth-Request-User"
     nginx.ingress.kubernetes.io/auth-signin: "https://foo.example.com/login"
     nginx.ingress.kubernetes.io/auth-url: "https://foo.example.com/auth?scope=read%3Aall&satisfy=any&notebook=true&minimum_lifetime=600"
     nginx.ingress.kubernetes.io/configuration-snippet: |
@@ -89,7 +89,7 @@ metadata:
   namespace: {namespace}
   annotations:
     nginx.ingress.kubernetes.io/auth-method: GET
-    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-User,X-Auth-Request-Token"
+    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Token,X-Auth-Request-User"
     nginx.ingress.kubernetes.io/auth-url: "https://foo.example.com/auth?scope=read%3Aall&scope=read%3Asome&delegate_to=some-service&delegate_scope=read%3Aall%2Cread%3Asome&auth_type=basic"
     nginx.ingress.kubernetes.io/configuration-snippet: |
       {snippet}
@@ -127,7 +127,7 @@ metadata:
   namespace: {namespace}
   annotations:
     nginx.ingress.kubernetes.io/auth-method: GET
-    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-User,X-Auth-Request-Token"
+    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Token,X-Auth-Request-User"
     nginx.ingress.kubernetes.io/auth-url: "https://foo.example.com/auth?scope=read%3Aall&delegate_to=some-service&delegate_scope=read%3Aall&use_authorization=true"
     nginx.ingress.kubernetes.io/configuration-snippet: |
       add_header "X-Foo" "bar";
@@ -166,7 +166,7 @@ metadata:
   namespace: {namespace}
   annotations:
     nginx.ingress.kubernetes.io/auth-method: GET
-    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie"
+    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Token,X-Auth-Request-User"
     nginx.ingress.kubernetes.io/auth-url: "https://foo.example.com/auth/anonymous"
     some.annotation: foo
   creationTimestamp: {any}