From 58253968fcef4432abdcccf8b412b346032dbbd2 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 16 Feb 2020 10:14:44 +0100 Subject: [PATCH] s3: improve credentials validation access secret can now be empty, so check if not empty before encrypting the secret --- README.md | 8 +++++--- dataprovider/dataprovider.go | 14 ++++++++------ httpd/api_user.go | 2 +- httpd/httpd_test.go | 23 +++++++++++++++++++++++ vfs/vfs.go | 6 ++++++ 5 files changed, 43 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 9fa08d88..19f2fcb9 100644 --- a/README.md +++ b/README.md @@ -532,11 +532,13 @@ The configured bucket must exist. To connect SFTPGo to AWS you need to specify credentials, and a `region` is required too, here is the list of available [AWS regions](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-regions-availability-zones.html#concepts-available-regions). For example if your bucket is at `Frankfurt` you have to set the region to `eu-central-1`. You can specify an AWS [storage class](https://docs.aws.amazon.com/AmazonS3/latest/dev/storage-class-intro.html) too, leave blank to use the default AWS storage class. An endpoint is required if you are connecting to a Compatible AWS Storage such as [MinIO](https://min.io/). -AWS SDK for Go has different options for credentials. [More Detail](https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html) -1. Providing [Access Keys](https://docs.aws.amazon.com/general/latest/gr/aws-sec-cred-types.html#access-keys-and-secret-access-keys). -2. Use IAM roles for Amazon EC2 +AWS SDK has different options for credentials. [More Detail](https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html). We support: +1. Providing [Access Keys](https://docs.aws.amazon.com/general/latest/gr/aws-sec-cred-types.html#access-keys-and-secret-access-keys). +2. Use IAM roles for Amazon EC2 3. Use IAM roles for tasks if your application uses an ECS task definition +So you need to provide access keys to activate option 1 or leave them blank to use the other ways to specify credentials. + Some SFTP commands doesn't work over S3: - `symlink` and `chtimes` will fail diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index c7f17019..d487ccbf 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -593,13 +593,15 @@ func validateFilesystemConfig(user *User) error { if err != nil { return &ValidationError{err: fmt.Sprintf("could not validate s3config: %v", err)} } - vals := strings.Split(user.FsConfig.S3Config.AccessSecret, "$") - if !strings.HasPrefix(user.FsConfig.S3Config.AccessSecret, "$aes$") || len(vals) != 4 { - accessSecret, err := utils.EncryptData(user.FsConfig.S3Config.AccessSecret) - if err != nil { - return &ValidationError{err: fmt.Sprintf("could not encrypt s3 access secret: %v", err)} + if len(user.FsConfig.S3Config.AccessSecret) > 0 { + vals := strings.Split(user.FsConfig.S3Config.AccessSecret, "$") + if !strings.HasPrefix(user.FsConfig.S3Config.AccessSecret, "$aes$") || len(vals) != 4 { + accessSecret, err := utils.EncryptData(user.FsConfig.S3Config.AccessSecret) + if err != nil { + return &ValidationError{err: fmt.Sprintf("could not encrypt s3 access secret: %v", err)} + } + user.FsConfig.S3Config.AccessSecret = accessSecret } - user.FsConfig.S3Config.AccessSecret = accessSecret } return nil } else if user.FsConfig.Provider == 2 { diff --git a/httpd/api_user.go b/httpd/api_user.go index 77ac9461..538814b4 100644 --- a/httpd/api_user.go +++ b/httpd/api_user.go @@ -127,7 +127,7 @@ func updateUser(w http.ResponseWriter, r *http.Request) { // we use the new access secret if different from the old one and not empty if user.FsConfig.Provider == 1 { if utils.RemoveDecryptionKey(currentS3AccessSecret) == user.FsConfig.S3Config.AccessSecret || - len(user.FsConfig.S3Config.AccessSecret) == 0 { + (len(user.FsConfig.S3Config.AccessSecret) == 0 && len(user.FsConfig.S3Config.AccessKey) > 0) { user.FsConfig.S3Config.AccessSecret = currentS3AccessSecret } } diff --git a/httpd/httpd_test.go b/httpd/httpd_test.go index d42c25f3..60703ba6 100644 --- a/httpd/httpd_test.go +++ b/httpd/httpd_test.go @@ -465,6 +465,29 @@ func TestUserS3Config(t *testing.T) { if err != nil { t.Errorf("unable to update user: %v", err) } + user.FsConfig.Provider = 0 + user.FsConfig.S3Config.Bucket = "" + user.FsConfig.S3Config.Region = "" + user.FsConfig.S3Config.AccessKey = "" + user.FsConfig.S3Config.AccessSecret = "" + user.FsConfig.S3Config.Endpoint = "" + user.FsConfig.S3Config.KeyPrefix = "" + user, _, err = httpd.UpdateUser(user, http.StatusOK) + if err != nil { + t.Errorf("unable to update user: %v", err) + } + // test user without access key and access secret (shared config state) + user.FsConfig.Provider = 1 + user.FsConfig.S3Config.Bucket = "test1" + user.FsConfig.S3Config.Region = "us-east-1" + user.FsConfig.S3Config.AccessKey = "" + user.FsConfig.S3Config.AccessSecret = "" + user.FsConfig.S3Config.Endpoint = "" + user.FsConfig.S3Config.KeyPrefix = "somedir/subdir" + user, _, err = httpd.UpdateUser(user, http.StatusOK) + if err != nil { + t.Errorf("unable to update user: %v", err) + } _, err = httpd.RemoveUser(user, http.StatusOK) if err != nil { t.Errorf("unable to remove: %v", err) diff --git a/vfs/vfs.go b/vfs/vfs.go index 862be90f..55da5303 100644 --- a/vfs/vfs.go +++ b/vfs/vfs.go @@ -77,6 +77,12 @@ func ValidateS3FsConfig(config *S3FsConfig) error { if len(config.Region) == 0 { return errors.New("region cannot be empty") } + if len(config.AccessKey) == 0 && len(config.AccessSecret) > 0 { + return errors.New("access_key cannot be empty with access_secret not empty") + } + if len(config.AccessSecret) == 0 && len(config.AccessKey) > 0 { + return errors.New("access_secret cannot be empty with access_key not empty") + } if len(config.KeyPrefix) > 0 { if strings.HasPrefix(config.KeyPrefix, "/") { return errors.New("key_prefix cannot start with /")