DemocracyClub / aggregator-api

https://developers.democracyclub.org.uk/
3 stars 3 forks source link

WIP reject fake host headers #545

Open chris48s opened 1 month ago

chris48s commented 1 month ago

OK. I'm a bit stuck here.

Currently this application allows anything in the host header. This makes the site vulnerable to a range of spoofing attacks. I want to tighten this up.

The changes in https://github.com/DemocracyClub/aggregator-api/commit/f680c9a8026cba86cc7a47951b646032ea675e66 achieve this. However, they cause another problem.

When we deploy this app, we bring the lambda up and then run some smoke tests against it before we finalise the deploy. Because at the stage we're running the smoke tests, we haven't pointed the DNS at it yet, we have to test this on a direct URL i.e: we have to call https://fig9r88dhk.execute-api.eu-west-2.amazonaws.com/Prod/ rather than https://developers.womblelabs.co.uk/ or whatever.

That means those URLs also have to be in ALLOWED_HOSTS, otherwise we raise 400 Bad Request in the smoke tests.

So my first bash at a fix for this is https://github.com/DemocracyClub/aggregator-api/commit/fe21b9649539d749704472dfbd32900698304379. In theory that should work. Unfortunately, when we try to run that code on lambda I get

ClientError: An error occurred (AccessDeniedException) when calling the GetRestApis operation: User: arn:aws:sts::489559689862:assumed-role/AggregatorApiLambdaExecutionRole/AggregatorApiApp-development-ApiFrontendFunction-KHzbY2EsfvyY is not authorized to perform: apigateway:GET on resource: arn:aws:apigateway:eu-west-2::/restapis because no identity-based policy allows the apigateway:GET action

so in order to get that to work, I need to grant our AggregatorApiLambdaExecutionRoles the ability to perform that action but I'm unclear on how permissions are managed in this application. It looks from the docs like this was probably done as a one-off rather than as part of the deploy process.

The other thing we could consider doing is just setting the IDs as env vars. We can't use the AggregatorApiFrontendFqdn output to set an env var because we have to define the env vars before that output is available. I'm slightly less clear on ServerlessRestApi. I don't think I can really explain where that comes from.

I think this would be a good one for us to catch up on next week @symroe