zalando/skipper

AWS sigv4 auth filter

szuecs opened this issue · 7 comments

szuecs commented

Sometimes you want to proxy to an aws service and aws uses sigv4 to do authnz.
It would be great to be able to sign with sigv4 the request with https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/aws/signer/v4#HTTPSigner to be able to call AWS services from the proxy itself.

Maybe leverage a similar kind of roundtripper which depends also on aws sdk https://github.com/prometheus/common/blob/main/sigv4/sigv4.go

Anurag252 commented

Hey @szuecs
I can take a look at this one. My thoughts around this

  • Create a new request filter named AWSSignV4
  • add a dependency to aws-sdk-go-v2 and call Sign.
  • Allow createfilter to accept credentials, take service and region from eskip file, and keep signtime as now documentation.

I believe we need to consider that

  • An instance of skipper would be able to sign requests for a single AWS account , but may target different services as credentials are being passed to create filter. Maybe there are other ways to generalize the credentials.

  • SignHTTP accepts payloadHash which is essentially hex encoded SHA-256 hash of the request payload. This would mean request body would need to be read completely in the filter and then a new ReadCloser would need to be assigned to request so that it can be propagated further. This could increase memory usage of skipper significantly. Although as docs says there are a few AWS services that do not need body to be signed.

  • Since request is signed here, it makes sense if this is the last filter before proxy as any other filter changing headers, query params or body may invalidate the signing.

szuecs commented

@Anurag252 I am not sure if I want to have the AWS sdk as dependency, rather not. I think it seems to be open enough to build this without AWS SDK, but I am not sure.

Anurag252 commented

@Anurag252 I am not sure if I want to have the AWS sdk as dependency, rather not. I think it seems to be open enough to build this without AWS SDK, but I am not sure.

@szuecs makes sense to me. I can try and implement this . What are your thoughts around reading the whole body in filter ( as described in point 2 of considerations) ?

szuecs commented

@Anurag252 I am not sure if I want to have the AWS sdk as dependency, rather not. I think it seems to be open enough to build this without AWS SDK, but I am not sure.

@szuecs makes sense to me. I can try and implement this . What are your thoughts around reading the whole body in filter ( as described in point 2 of considerations) ?

Sounds like we have to do it. Not sure if it makes sense to have 2 kind of filters, 1 that requires body and the other which does not.

Similar to https://opensource.zalando.com/skipper/reference/filters/#opaauthorizerequest and https://opensource.zalando.com/skipper/reference/filters/#opaauthorizerequestwithbody

What do you think?

Anurag252 commented

@Anurag252 I am not sure if I want to have the AWS sdk as dependency, rather not. I think it seems to be open enough to build this without AWS SDK, but I am not sure.

@szuecs makes sense to me. I can try and implement this . What are your thoughts around reading the whole body in filter ( as described in point 2 of considerations) ?

Sounds like we have to do it. Not sure if it makes sense to have 2 kind of filters, 1 that requires body and the other which does not.

Similar to https://opensource.zalando.com/skipper/reference/filters/#opaauthorizerequest and https://opensource.zalando.com/skipper/reference/filters/#opaauthorizerequestwithbody

What do you think?

Okay then I can probably take a param in CreateFilter which would denote the maximum body size (like opaAuthorizeRequestWithBody ) that could be read and keep default as 8kb (? or any other size) .

I could not think of a reason when having two filters would be better, but I maybe missing out some case 🤔 .
Even with no body present documentation mentions including empty body in signature generation like so
Hex(SHA256Hash(""))

szuecs commented

Okay then I can probably take a param in CreateFilter which would denote the maximum body size (like opaAuthorizeRequestWithBody ) that could be read and keep default as 8kb (? or any other size) .

I could not think of a reason when having two filters would be better, but I maybe missing out some case 🤔 . Even with no body present documentation mentions including empty body in signature generation like so Hex(SHA256Hash(""))

Then I would ignore it and handle it inside the filter.
For example you can use ContentLength to detect a request without body.
So basically:

var data string
if ContentLength != 0 {
   data = ...read body and make it string..
}
..Hex(SHA256Hash(data)) ..
  • 👍1
Anurag252 commented

Okay then I can probably take a param in CreateFilter which would denote the maximum body size (like opaAuthorizeRequestWithBody ) that could be read and keep default as 8kb (? or any other size) .
I could not think of a reason when having two filters would be better, but I maybe missing out some case 🤔 . Even with no body present documentation mentions including empty body in signature generation like so Hex(SHA256Hash(""))

Then I would ignore it and handle it inside the filter. For example you can use ContentLength to detect a request without body. So basically:

var data string
if ContentLength != 0 {
   data = ...read body and make it string..
}
..Hex(SHA256Hash(data)) ..

makes sense. I will try to submit a PR 👍