skipper icon indicating copy to clipboard operation
skipper copied to clipboard

Published 20 hours ago • verified publisher icon zalando

feat: added sigv4 filter

Open Anurag252 opened this issue 9 months ago • 7 comments

https://github.com/zalando/skipper/issues/2911

Example 1 with default body {}

actual sdk code

see https://go.dev/play/p/nPiTzwAl6Ut for a signed sample request with sdk. response : -

Signed Request:
AWS4-HMAC-SHA256 Credential=AKID/19700101/us-east-1/dynamodb/aws4_request, SignedHeaders=accept;content-length;content-type;host;x-amz-date;x-amz-meta-other-header;x-amz-meta-other-header_with_underscore;x-amz-security-token;x-amz-target, Signature=0dabdcda8c405b6f3cdd34c7a63a4d1a701942bd78fcd535ccd49de33be2a229

skipper

./bin/skipper -inline-routes='r: * -> status(200) -> sigv4( "us-east-1","dynamodb", "false", "false", "false") -> "http://httpbin.org"'

curl -

curl -v  POST \
  http://localhost:9090/anything \
  -H 'X-Amz-Target: prefix.Operation' \
  -H 'Content-Type: application/x-amz-json-1.0' \
  -H 'X-Amz-Meta-Other-Header: some-value=!@#$%^&* (+)' \
  -H 'X-Amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)' \
  -H 'X-amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)' \
  -H 'x-amz-accesskey: AKID' \
  -H 'x-amz-secret: SECRET' \
  -H 'x-amz-session: SESSION' \
  -H 'x-amz-time: 1970-01-01T00:00:00Z' \
  -H 'Host: dynamodb.us-east-1.amazonaws.com' -d '{}'

Response from curl -

*   Trying 52.20.143.163:80...
* Connected to POST (52.20.143.163) port 80 (#0)
> POST / HTTP/1.1
> Host: dynamodb.us-east-1.amazonaws.com
> User-Agent: curl/8.1.2
> Accept: */*
> X-Amz-Target: prefix.Operation
> Content-Type: application/x-amz-json-1.0
> X-Amz-Meta-Other-Header: some-value=!@#$%^&* (+)
> X-Amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)
> X-amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)
> x-amz-accesskey: AKID
> x-amz-secret: SECRET
> x-amz-session: SESSION
> x-amz-time: 1970-01-01T00:00:00Z
> Content-Length: 2
> 
< HTTP/1.1 404 Not Found
< Date: Thu, 09 May 2024 04:37:17 GMT
< Content-Type: text/html
< Content-Length: 146
< Connection: keep-alive
< 
<html>
<head><title>404 Not Found</title></head>
<body>
<center><h1>404 Not Found</h1></center>
<hr><center>nginx</center>
</body>
</html>
* Connection #0 to host POST left intact
*   Trying 127.0.0.1:9090...
* Connected to localhost (127.0.0.1) port 9090 (#1)
> POST /anything HTTP/1.1
> Host: dynamodb.us-east-1.amazonaws.com
> User-Agent: curl/8.1.2
> Accept: */*
> X-Amz-Target: prefix.Operation
> Content-Type: application/x-amz-json-1.0
> X-Amz-Meta-Other-Header: some-value=!@#$%^&* (+)
> X-Amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)
> X-amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)
> x-amz-accesskey: AKID
> x-amz-secret: SECRET
> x-amz-session: SESSION
> x-amz-time: 1970-01-01T00:00:00Z
> Content-Length: 2
> 
< HTTP/1.1 200 OK
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Origin: *
< Connection: keep-alive
< Content-Length: 1071
< Content-Type: application/json
< Date: Thu, 09 May 2024 04:37:17 GMT
< Server: gunicorn/19.9.0
< 
{
  "args": {}, 
  "data": "{}", 
  "files": {}, 
  "form": {}, 
  "headers": {
    "Accept": "*/*", 
    "Accept-Encoding": "gzip", 
    "Authorization": "AWS4-HMAC-SHA256 Credential=AKID/19700101/us-east-1/dynamodb/aws4_request, SignedHeaders=accept;content-length;content-type;host;x-amz-date;x-amz-meta-other-header;x-amz-meta-other-header_with_underscore;x-amz-security-token;x-amz-target, Signature=0dabdcda8c405b6f3cdd34c7a63a4d1a701942bd78fcd535ccd49de33be2a229", 
    "Content-Length": "2", 
    "Content-Type": "application/x-amz-json-1.0", 
    "Host": "httpbin.org", 
    "User-Agent": "curl/8.1.2", 
    "X-Amz-Date": "19700101T000000Z", 
    "X-Amz-Meta-Other-Header": "some-value=!@#$%^&* (+)", 
    "X-Amz-Meta-Other-Header-With-Underscore": "some-value=!@#$%^&* (+),some-value=!@#$%^&* (+)", 
    "X-Amz-Security-Token": "SESSION", 
    "X-Amz-Target": "prefix.Operation", 
    "X-Amzn-Trace-Id": "Root=1-663c52fd-1c443e565cba09c323349703"
  }, 
  "json": {}, 
  "method": "POST", 
  "origin": "158.181.74.224", 
  "url": "http://httpbin.org/anything"
}

Example 2 with non-default body

skipper

./bin/skipper -inline-routes='r: * -> status(200) ->  sigv4( "us-east-1","dynamodb",  "false", "false", "false") -> "http://httpbin.org"'

curl

anuragdubey@Anurags-Air skipper % curl -v  POST \
  http://localhost:9090/anything \
  -H 'X-Amz-Target: prefix.Operation' \
  -H 'Content-Type: application/x-amz-json-1.0' \
  -H 'X-Amz-Meta-Other-Header: some-value=!@#$%^&* (+)' \
  -H 'X-Amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)' \
  -H 'X-amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)' \
  -H 'x-amz-accesskey: AKID' \
  -H 'x-amz-secret: SECRET' \
  -H 'x-amz-session: SESSION' \
  -H 'x-amz-time: 1970-01-01T00:00:00Z' \
  -H 'Host: dynamodb.us-east-1.amazonaws.com' -d '{key:value}'   
*   Trying 34.232.152.68:80...
* Connected to POST (34.232.152.68) port 80 (#0)
> POST / HTTP/1.1
> Host: dynamodb.us-east-1.amazonaws.com
> User-Agent: curl/8.1.2
> Accept: */*
> X-Amz-Target: prefix.Operation
> Content-Type: application/x-amz-json-1.0
> X-Amz-Meta-Other-Header: some-value=!@#$%^&* (+)
> X-Amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)
> X-amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)
> x-amz-accesskey: AKID
> x-amz-secret: SECRET
> x-amz-session: SESSION
> x-amz-time: 1970-01-01T00:00:00Z
> Content-Length: 11
> 
< HTTP/1.1 404 Not Found
< Date: Thu, 09 May 2024 09:12:02 GMT
< Content-Type: text/html
< Content-Length: 146
< Connection: keep-alive
< 
<html>
<head><title>404 Not Found</title></head>
<body>
<center><h1>404 Not Found</h1></center>
<hr><center>nginx</center>
</body>
</html>
* Connection #0 to host POST left intact
*   Trying 127.0.0.1:9090...
* Connected to localhost (127.0.0.1) port 9090 (#1)
> POST /anything HTTP/1.1
> Host: dynamodb.us-east-1.amazonaws.com
> User-Agent: curl/8.1.2
> Accept: */*
> X-Amz-Target: prefix.Operation
> Content-Type: application/x-amz-json-1.0
> X-Amz-Meta-Other-Header: some-value=!@#$%^&* (+)
> X-Amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)
> X-amz-Meta-Other-Header_With_Underscore: some-value=!@#$%^&* (+)
> x-amz-accesskey: AKID
> x-amz-secret: SECRET
> x-amz-session: SESSION
> x-amz-time: 1970-01-01T00:00:00Z
> Content-Length: 11
> 
< HTTP/1.1 200 OK
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Origin: *
< Connection: keep-alive
< Content-Length: 1125
< Content-Type: application/json
< Date: Thu, 09 May 2024 09:12:03 GMT
< Server: gunicorn/19.9.0
< 
{
  "args": {}, 
  "data": "{key:value}", 
  "files": {}, 
  "form": {}, 
  "headers": {
    "Accept": "*/*", 
    "Accept-Encoding": "gzip", 
    "Authorization": "AWS4-HMAC-SHA256 Credential=AKID/19700101/us-east-1/dynamodb/aws4_request, SignedHeaders=accept;content-length;content-type;host;x-amz-date;x-amz-meta-other-header;x-amz-meta-other-header_with_underscore;x-amz-security-token;x-amz-target, Signature=8ff94a60e916faca5238078181d0858d37945640ce2e967ab8361b639252593c", 
    "Content-Length": "11", 
    "Content-Type": "application/x-amz-json-1.0", 
    "Host": "dynamodb.us-east-1.amazonaws.com", 
    "User-Agent": "curl/8.1.2", 
    "X-Amz-Date": "19700101T000000Z", 
    "X-Amz-Meta-Other-Header": "some-value=!@#$%^&* (+)", 
    "X-Amz-Meta-Other-Header-With-Underscore": "some-value=!@#$%^&* (+),some-value=!@#$%^&* (+)", 
    "X-Amz-Security-Token": "SESSION", 
    "X-Amz-Target": "prefix.Operation", 
    "X-Amzn-Trace-Id": "Root=1-663c9363-20d7b8ee264d1bfa56373aa7"
  }, 
  "json": null, 
  "method": "POST", 
  "origin": "158.181.74.224", 
  "url": "http://dynamodb.us-east-1.amazonaws.com/anything"
}
* Connection #1 to host localhost left intact

actual sdk code

https://go.dev/play/p/Ixx-ozxeWfv Response

Signed Request:
AWS4-HMAC-SHA256 Credential=AKID/19700101/us-east-1/dynamodb/aws4_request, SignedHeaders=accept;content-length;content-type;host;x-amz-date;x-amz-meta-other-header;x-amz-meta-other-header_with_underscore;x-amz-security-token;x-amz-target, Signature=8ff94a60e916faca5238078181d0858d37945640ce2e967ab8361b639252593c

Anurag252 avatar May 07 '24 16:05 Anurag252

This is quite a lot of new code and I am concerned about supporting this.

@szuecs should we also consider

* adding AWS sdk as a dependency

* copy-paste AWS signer code (vendor-in)

* use AWS sdk with a build tag to exclude for lib users by default

Good question, I thought it makes sense to not put AWS SDK into dependency because this is really huge. The contributor copies basically code (option 2) that won't be able to be changed anyways into the repository, because AWS can't really change the SDK signing code, because if so they likely need to change validation, too and then you would break half the internet that uses AWS SDK to have to update.

Adding an AWS build tag would be something we can do to have this feature as a more "experimental" implementation.

szuecs avatar May 14 '24 12:05 szuecs

Added review comments except the unresolved open discussions.

  • keeping or disabling caching
  • +3 to store lookup key in cache
  • incorrect exported functions/types

will add changes for these once I have more information

Anurag252 avatar May 28 '24 09:05 Anurag252

@Anurag252 I tried to answer all comments that seem to have an open question. If there is something missing please point us to it.

szuecs avatar May 29 '24 15:05 szuecs

@Anurag252 I tried to answer all comments that seem to have an open question. If there is something missing please point us to it.

Thanks @szuecs . A comment about keeping/removing caching layer is still remaining. That was copied from sdk code and I do not have any opinion on keeping or removing it. I am open to both.

Can you also suggest next steps, as PR appears feature complete to me ( if test and check-race passes)?

Anurag252 avatar May 30 '24 09:05 Anurag252

re-requesting review. Added fixes

  • default body is empty string and not {}. see
  • bug due to typo disableURIPathEscaping
  • more test to verify disableSessionToken behaviour

Anurag252 avatar Jun 06 '24 15:06 Anurag252

I didn't check but please check the CodeQL warning. I think we should fix it.

szuecs avatar Jun 10 '24 15:06 szuecs

I didn't check but please check the CodeQL warning. I think we should fix it.

removed logging, so CodeQL warning should go away

Anurag252 avatar Jun 30 '24 12:06 Anurag252

@szuecs @AlexanderYastrebov This PR is feature complete. Could we approve the workflow ? If there are any more issues in workflow, I can take a look at them.

Anurag252 avatar Jul 08 '24 10:07 Anurag252

From my side looks good, only some godoc related errors

szuecs avatar Jul 08 '24 13:07 szuecs

:+1:

szuecs avatar Jul 09 '24 09:07 szuecs

:+1:

AlexanderYastrebov avatar Jul 17 '24 08:07 AlexanderYastrebov