s3: improve credentials validation

access secret can now be empty, so check if not empty before encrypting
the secret

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

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 {

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
 		}
 	}

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)

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 /")