Page MenuHomePhabricator

API Gateway: add security headers
Closed, ResolvedPublic

Description

The API gateway should add security headers to API responses, based on some simple rules.

For reference, see the security header filter implemented in RESTbase: https://phabricator.wikimedia.org/diffusion/GRES/browse/master/lib/security_response_header_filter.js

See PCS related task: T321194: Implement Security Response Header Filter logic outside RESTBase for PCS

Event Timeline

Some of the content security policy rules are not so simple to implement downstream of the service answering - for example the gateway does not have access to the variables used for mobile-html, and things like hyper._rootReq.params.domain. It can approximate some of them, but perhaps it would be useful to make restbase or other services pass up a header (which could be removed) to indicate which behaviour the gateway should implement? The style-src header in particularly becomes quite messy as there is no header in the response containing the actual domain with which we're concerned and the domain will have to be parsed out of the content-location header and then mangled.

For the time being, I have access to the response returned by the service - I am considering checking content-type (and possibly also content-location ) to see if they contain the corresponding mobile-html string. This works more or less, but seems a little brittle. I've implemented this in a lua filter so we can do this globally and quickly without needing to configure this in each match object.

More generally, are we sure we need to set the headers in this manner? The comments seem somewhat ambiguous as to whether everything is needed or correct - I know we're trying to lift and shift as much behaviour as is possible of course, but implementing some of this in envoy seems brittle.

Adding a bunch of folks per API Gateway meeting this morning

Seems like there's some question about where certain headers live: AQSAssist vs. APIGateway

Change 890887 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/deployment-charts@master] api-gateway: add REST gateway Lua CSP handler

https://gerrit.wikimedia.org/r/890887

Change 890887 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/deployment-charts@master] api-gateway: add REST gateway Lua CSP handler

https://gerrit.wikimedia.org/r/890887

This change contains a Lua implementation of much of the restbase security header implementation, but is not complete (and, as far as I understand, cannot be complete). I'd love some feedback on this approach and the larger questions before proceeding further with it.

hnowlan changed the task status from Open to In Progress.Feb 22 2023, 11:45 AM
hnowlan claimed this task.

Change 890887 merged by jenkins-bot:

[operations/deployment-charts@master] api-gateway: add REST gateway Lua CSP handler

https://gerrit.wikimedia.org/r/890887

Change 907928 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/deployment-charts@master] rest-gateway: fix lua handler

https://gerrit.wikimedia.org/r/907928

Change 907928 merged by jenkins-bot:

[operations/deployment-charts@master] rest-gateway: fix lua handler

https://gerrit.wikimedia.org/r/907928

Change 908242 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/deployment-charts@master] rest-gateway: fix direct_response

https://gerrit.wikimedia.org/r/908242

Change 908242 merged by jenkins-bot:

[operations/deployment-charts@master] rest-gateway: fix direct_response

https://gerrit.wikimedia.org/r/908242