Skip to content

Commit

Permalink
[s3 gateway] Handle all path characters in SigV2 and add Nessie S3-is…
Browse files Browse the repository at this point in the history
…h test (#2464)

* Change AWS S3 sigV2 verification to use path as documented by AWS

* Add Nessie S3-ish test

Use the minIO client to test S3 uploads and downloads with both V2 and V4
signatures.  (Unlike the AWS S3 client, the minIO client supports signing
using SigV2.)

* Parallelise S3 gateway test

(Timed out on GCP)

* Add S3 SigV2, SigV4 component tests

Sign using minio, verify using gateway code.

* SigV4: Test bothV4 path and host styles

* Make S3 gateway test pathnames (just) shorter than 1023 chars

* Close channel in Nessie S3 gateway test

Also increase parallelism somewhat.

* [CR] Use t.Errorf.  Kill test if minio client create fails.
  • Loading branch information
arielshaqed committed Sep 14, 2021
1 parent 78078b7 commit 4713534
Show file tree
Hide file tree
Showing 6 changed files with 326 additions and 18 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ require (
github.com/magiconair/properties v1.8.4 // indirect
github.com/manifoldco/promptui v0.8.0
github.com/matoous/go-nanoid/v2 v2.0.0
github.com/minio/minio-go/v7 v7.0.13 // indirect
github.com/mitchellh/go-homedir v1.1.0
github.com/mitchellh/mapstructure v1.4.1
github.com/ory/dockertest/v3 v3.6.3
Expand Down
11 changes: 11 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,9 @@ github.com/klauspost/compress v1.11.12 h1:famVnQVu7QwryBN4jNseQdUKES71ZAOnB6UQQJ
github.com/klauspost/compress v1.11.12/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
github.com/klauspost/cpuid v1.2.1 h1:vJi+O/nMdFt0vqm8NZBI6wzALWdA2X+egi0ogNyrC/w=
github.com/klauspost/cpuid v1.2.1/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek=
github.com/klauspost/cpuid v1.2.3/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek=
github.com/klauspost/cpuid v1.3.1 h1:5JNjFYYQrZeKRJ0734q51WCEEn2huer72Dc7K+R/b6s=
github.com/klauspost/cpuid v1.3.1/go.mod h1:bYW4mA6ZgKPob1/Dlai2LviZJO7KGI3uoWLd42rAQw4=
github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
github.com/konsorten/go-windows-terminal-sequences v1.0.3 h1:CE8S1cTafDpPvMhIxNJKvHsGVBgn1xWYf1NbHQhywc8=
Expand Down Expand Up @@ -1116,6 +1119,12 @@ github.com/microcosm-cc/bluemonday v1.0.2/go.mod h1:iVP4YcDBq+n/5fb23BhYFvIMq/le
github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
github.com/miekg/dns v1.1.17 h1:BhJxdA7bH51vKFZSY8Sn9pR7++LREvg0eYFzHA452ew=
github.com/miekg/dns v1.1.17/go.mod h1:WgzbA6oji13JREwiNsRDNfl7jYdPnmz+VEuLrA+/48M=
github.com/minio/md5-simd v1.1.0 h1:QPfiOqlZH+Cj9teu0t9b1nTBfPbyTl16Of5MeuShdK4=
github.com/minio/md5-simd v1.1.0/go.mod h1:XpBqgZULrMYD3R+M28PcmP0CkI7PEMzB3U77ZrKZ0Gw=
github.com/minio/minio-go/v7 v7.0.13 h1:rYCca0+8ciW4wFY/vsO5CEMBVL0iabA2D0iq9gOWDjM=
github.com/minio/minio-go/v7 v7.0.13/go.mod h1:S23iSP5/gbMwtxeY5FM71R+TkAYyzEdoNEDDwpt8yWs=
github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKUJU=
github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=
github.com/mitchellh/cli v1.0.0 h1:iGBIsUe3+HZ/AD/Vd7DErOt5sU9fa8Uj7A2s1aggv1Y=
github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc=
github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db h1:62I3jR2EmQ4l5rM/4FEfDWcRD+abF5XlKShorW5LRoQ=
Expand Down Expand Up @@ -1630,6 +1639,7 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh
golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20201216223049-8b5274cf687f/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 h1:/ZScEX8SfEmUGRHs0gxpqteO5nfNW6axyZbBdw9A12g=
golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
Expand Down Expand Up @@ -2120,6 +2130,7 @@ gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec/go.mod h1:a
gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=
gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/ini.v1 v1.57.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/ini.v1 v1.62.0 h1:duBzk771uxoUuOlyRLkHsygud9+5lrlGjdFBb4mSKDU=
gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/jcmturner/aescts.v1 v1.0.1 h1:cVVZBK2b1zY26haWB4vbBiZrfFQnfbTVrE3xZq6hrEw=
Expand Down
106 changes: 106 additions & 0 deletions nessie/s3_gateway_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package nessie

import (
"bytes"
"io"
"math/rand"
"strings"
"sync"
"testing"

"github.com/minio/minio-go/v7"
"github.com/minio/minio-go/v7/pkg/credentials"
"github.com/spf13/viper"
"github.com/treeverse/lakefs/pkg/testutil"
)

var sigs = []struct {
Name string
GetCredentials func(id, secret, token string) *credentials.Credentials
}{
{"V2", credentials.NewStaticV2},
{"V4", credentials.NewStaticV4},
}

const (
numUploads = 100
randomDataPathLength = 1020
prefix = "main/data/"
)

func TestS3UploadAndDownload(t *testing.T) {
const parallelism = 10

ctx, _, repo := setupTest(t)

accessKeyID := viper.GetString("access_key_id")
secretAccessKey := viper.GetString("secret_access_key")
endpoint := viper.GetString("s3_endpoint")
opts := minio.PutObjectOptions{}

for _, sig := range sigs {
t.Run("Sig"+sig.Name, func(t *testing.T) {
// Use same sequence of pathnames to test each sig.
rand := rand.New(rand.NewSource(17))

creds := sig.GetCredentials(accessKeyID, secretAccessKey, "")

type Object struct {
Path, Content string
}

var (
wg sync.WaitGroup
objects = make(chan Object, parallelism*2)
)

for i := 0; i < parallelism; i++ {
client, err := minio.New(endpoint, &minio.Options{
Creds: creds,
Secure: false,
})
if err != nil {
t.Fatalf("minio.New: %s", err)
}

wg.Add(1)
go func() {
for o := range objects {
_, err := client.PutObject(
ctx, repo, o.Path, strings.NewReader(o.Content), int64(len(o.Content)), opts)
if err != nil {
t.Errorf("minio.Client.PutObject(%s): %s", o.Path, err)
}

download, err := client.GetObject(
ctx, repo, o.Path, minio.GetObjectOptions{})
if err != nil {
t.Errorf("minio.Client.GetObject(%s): %s", o.Path, err)
}
contents := bytes.NewBuffer(nil)
_, err = io.Copy(contents, download)
if err != nil {
t.Errorf("download %s: %s", o.Path, err)
}
if strings.Compare(contents.String(), o.Content) != 0 {
t.Errorf(
"Downloaded bytes %v from uploaded bytes %v", contents.Bytes(), o.Content)
}
}
wg.Done()
}()
}

for i := 0; i < numUploads; i++ {
objects <- Object{
Content: testutil.RandomString(rand, randomDataContentLength),
// lakeFS supports _any_ path, even if its
// byte sequence is not legal UTF-8 string.
Path: prefix + testutil.RandomString(rand, randomDataPathLength-len(prefix)),
}
}
close(objects)
wg.Wait()
})
}
}
177 changes: 169 additions & 8 deletions pkg/gateway/sig/sig_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
package sig_test

import (
"errors"
"fmt"
"math/rand"
"net/http"
"net/url"
"testing"
"time"

"github.com/minio/minio-go/v7/pkg/s3utils"
"github.com/minio/minio-go/v7/pkg/signer"
"github.com/treeverse/lakefs/pkg/auth/model"
gwErrors "github.com/treeverse/lakefs/pkg/gateway/errors"
"github.com/treeverse/lakefs/pkg/gateway/sig"
"github.com/treeverse/lakefs/pkg/testutil"
)

func makeRequest(t *testing.T, headers map[string]string, query map[string]string) *http.Request {
Expand All @@ -26,10 +36,10 @@ func makeRequest(t *testing.T, headers map[string]string, query map[string]strin
func TestIsAWSSignedRequest(t *testing.T) {
type KV map[string]string
cases := []struct {
name string
want bool
headers map[string]string
query map[string]string
Name string
Want bool
Headers map[string]string
Query map[string]string
}{
{"no sig", false, nil, nil},
{"non aws auth header", false, KV{"Authorization": "Basic dXNlcjpwYXNzd29yZA=="}, nil},
Expand All @@ -40,11 +50,162 @@ func TestIsAWSSignedRequest(t *testing.T) {
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
r := makeRequest(t, tc.headers, tc.query)
t.Run(tc.Name, func(t *testing.T) {
r := makeRequest(t, tc.Headers, tc.Query)
got := sig.IsAWSSignedRequest(r)
if got != tc.want {
t.Fatalf("IsAWSSignedRequest with %s: got %v, expected %v", tc.name, got, tc.want)
if got != tc.Want {
t.Fatalf("IsAWSSignedRequest with %s: got %v, expected %v", tc.Name, got, tc.Want)
}
})
}
}

type Signer func(req http.Request) *http.Request
type Verifier func(req *http.Request) error

type Style string

const (
PathStyle = "path"
HostStyle = "host"
)

func MakeV2Signer(keyID, secretKey string, style Style) Signer {
return func(req http.Request) *http.Request {
return signer.SignV2(req, keyID, secretKey, style == "host")
}
}

func MakeV4Signer(keyID, secretKey, location string) Signer {
return func(req http.Request) *http.Request {
return signer.SignV4(req, keyID, secretKey, "", location)
}
}

func MakeV2Verifier(keyID, secretKey, bareDomain string) Verifier {
return func(req *http.Request) error {
authenticator := sig.NewV2SigAuthenticator(req)
_, err := authenticator.Parse()
if err != nil {
return fmt.Errorf("sigV2 parse failed: %w", err)
}
return authenticator.Verify(
&model.Credential{AccessKeyID: keyID, SecretAccessKey: secretKey},
bareDomain,
)
}
}

func MakeV4Verifier(keyID, secretKey, bareDomain string) Verifier {
return func(req *http.Request) error {
authenticator := sig.NewV4Authenticator(req)
_, err := authenticator.Parse()
if err != nil {
return fmt.Errorf("sigV4 parse failed: %w", err)
}
return authenticator.Verify(
&model.Credential{AccessKeyID: keyID, SecretAccessKey: secretKey},
bareDomain,
)
}
}

func MakeHeader(m map[string]string) http.Header {
ret := http.Header{}
for k, v := range m {
ret.Add(k, v)
}
return ret
}

const (
keyID = "AKIAIOSFODNN7EXAMPLE"
secretKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
domain = "s3.lakefs.test"
location = "lu-alpha-1"
)

var date = time.Unix(1631523198, 0)

type SignCase struct {
Name string
Signer Signer
Verifier Verifier
Style Style
}

var signatures = []SignCase{
{
Name: "V2Host",
Signer: MakeV2Signer(keyID, secretKey, "host"),
Verifier: MakeV2Verifier(keyID, secretKey, domain),
Style: "host",
}, {
Name: "V2Path",
Signer: MakeV2Signer(keyID, secretKey, "path"),
Verifier: MakeV2Verifier(keyID, secretKey, domain),
Style: "path",
}, {
Name: "V4Host",
Signer: MakeV4Signer(keyID, secretKey, location),
Verifier: MakeV4Verifier(keyID, secretKey, domain),
Style: "host",
}, {
Name: "V4Path",
Signer: MakeV4Signer(keyID, secretKey, location),
Verifier: MakeV4Verifier(keyID, secretKey, domain),
Style: "path",
},
}

func TestAWSSigVerify(t *testing.T) {
const (
numRounds = 1000
seed = 20210913
pathLength = 900
bucket = "my-bucket"
)
methods := []string{"GET", "PUT", "DELETE", "PATCH"}

for _, s := range signatures {
t.Run("Sig"+s.Name, func(t *testing.T) {
host := domain
if s.Style == "host" {
host = fmt.Sprintf("%s.%s", bucket, domain)
}

rand := rand.New(rand.NewSource(seed))
for i := 0; i < numRounds; i++ {
path := s3utils.EncodePath("my-branch/ariels/x/" + testutil.RandomString(rand, pathLength))
url := &url.URL{
Scheme: "s3",
Host: bucket,
Path: path,
// No way to construct possibly-equivalent forms, so let
// URL construct The Right RawPath.
RawPath: "",
}
req := http.Request{
Method: methods[rand.Intn(len(methods))],
Host: host,
URL: url,
Header: MakeHeader(map[string]string{
"Date": date.Format(http.TimeFormat),
"x-amz-date": date.Format("20060102T150405Z"),
"Content-Md5": "deadbeef",
"Content-Type": "application/binary",
}),
}
signedReq := s.Signer(req)
err := s.Verifier(signedReq)
if err != nil {
errText := err.Error()
var apiErr gwErrors.APIErrorCode
if errors.As(err, &apiErr) {
errText = apiErr.ToAPIErr().Description
}
t.Errorf("Sign and verify %s: %s", url.String(), errText)
}
}
})
}
Expand Down
14 changes: 4 additions & 10 deletions pkg/gateway/sig/v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,17 +238,11 @@ func (a *V2SigAuthenticator) Verify(creds *model.Credential, bareDomain string)
- QSA(Query String Arguments) - query arguments are searched for "interesting Resources".
*/

/*
URI encoding requirements for aws signature are different from what GO does.
This logic is taken from https://docs.aws.amazon.com/AWSECommerceService/latest/DG/Query_QueryAuth.html
These replacements are necessary for Java. There is no description about GO, but I found the '=' needs treatment as well
*/
// Prefer the raw path if it exists -- *this* is what SigV2 signs
url := a.r.URL
rawPath := url.EscapedPath()

patchedPath := strings.ReplaceAll(a.r.URL.Path, "=", "%3D")
patchedPath = strings.ReplaceAll(patchedPath, "+", "%20")
patchedPath = strings.ReplaceAll(patchedPath, "*", "%2A")
patchedPath = strings.ReplaceAll(patchedPath, "%7E", "~")
path := buildPath(a.r.Host, bareDomain, patchedPath)
path := buildPath(a.r.Host, bareDomain, rawPath)
stringToSign := canonicalString(a.r.Method, a.r.URL.Query(), path, a.r.Header)
digest := signCanonicalString(stringToSign, []byte(creds.SecretAccessKey))
if !Equal(digest, a.ctx.signature) {
Expand Down

0 comments on commit 4713534

Please sign in to comment.