Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

Support AWS style hmac signatures on requests to upstream servers. #310

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support AWS style hmac signatures on requests to upstream servers. #310

wants to merge 6 commits into from

Conversation

alexhornbake
Copy link

Hey, first PR against this repo... lmk if there is a better way to contribute or propose this change. Thanks! - Alex

@mbland would be curious to get your feedback since it looks like you added the current hmac signing.

Proposal to add support for more granular upstreams (backends)

Why?

  • Currently all upstream servers are specified in the "--upstreams" flag.
  • There is also a specific implementation of "GAP-Auth" signatures specified using the "--signature-data" flag. This signature is applied to all upstreams when specified. While it's unlikely that an upstream server will be outside your control, it seems odd to leak this data to upstreams that do not need the signature.
  • The "GAP-Auth" header doesn't help if you want to sign requests for AWS services. The specific use case for me is AWS's managed ElasticSearch (useful for things like kibana), which can only be protected using AWS style signatures.

How?

  • Use the same pattern as "providers" to create a set of "backends".
  • Currently split in to two sets:
    • "Default" uses the existing logic to handle all the servers passed in as --upstreams, and maintain retro-compatibility.
    • "Aws" uses a new "--aws-upstreams" flag to specify a list of upstreams that need AWS style hmac signatures.

Possible follow ups to this

  • Things like "GAP-Auth" could use a "--gap-auth-upstreams" flag and be handled with a specific gap-auth backend.
  • Serving static files could use "--file-upstreams" instead of parsing "file://" from the url.
  • "backends" as implemented currently are using a catch all "options" struct. Each backend uses the "options" that it knows/cares about, since there are only 3 use cases so far it seems reasonable, but I'm all ears if anyone sees a better way.

@alexhornbake
Copy link
Author

Also... I'm working on fixing/adding tests now. Just realized this change breaks the existing tests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant