diff --git a/changelogs/unreleased/10254-Lyndon-Li b/changelogs/unreleased/10254-Lyndon-Li new file mode 100644 index 000000000..e3ad3b7a8 --- /dev/null +++ b/changelogs/unreleased/10254-Lyndon-Li @@ -0,0 +1 @@ +Ignore credentialFile filled into BSL by users to avoid unexpected credentials used by Velero \ No newline at end of file diff --git a/internal/volume/snapshotlocation.go b/internal/volume/snapshotlocation.go index 594fbf3a5..f8adab7fd 100644 --- a/internal/volume/snapshotlocation.go +++ b/internal/volume/snapshotlocation.go @@ -29,6 +29,10 @@ func UpdateVolumeSnapshotLocationWithCredentialConfig(location *velerov1api.Volu if location.Spec.Config == nil { location.Spec.Config = make(map[string]string) } + + // Delete any user-provided credentialsFile to prevent path traversal vulnerabilities + delete(location.Spec.Config, "credentialsFile") + // If the VSL specifies a credential, fetch its path on disk and pass to // plugin via the config. if location.Spec.Credential != nil && credentialStore != nil { diff --git a/pkg/persistence/object_store.go b/pkg/persistence/object_store.go index 440ca8756..8d5207a6e 100644 --- a/pkg/persistence/object_store.go +++ b/pkg/persistence/object_store.go @@ -164,6 +164,9 @@ func (b *objectBackupStoreGetter) Get(location *velerov1api.BackupStorageLocatio } } + // Delete any user-provided credentialsFile to prevent path traversal vulnerabilities + delete(objectStoreConfig, "credentialsFile") + // add the bucket name and prefix to the config map so that object stores // can use them when initializing. The AWS object store uses the bucket // name to determine the bucket's region when setting up its client. diff --git a/pkg/repository/provider/unified_repo.go b/pkg/repository/provider/unified_repo.go index b30e4618b..24c744fe4 100644 --- a/pkg/repository/provider/unified_repo.go +++ b/pkg/repository/provider/unified_repo.go @@ -501,11 +501,16 @@ func getStorageCredentials(backupLocation *velerov1api.BackupStorageLocation, cr return map[string]string{}, errors.New("invalid storage provider") } - config := backupLocation.Spec.Config - if config == nil { - config = map[string]string{} + config := make(map[string]string) + if backupLocation.Spec.Config != nil { + for k, v := range backupLocation.Spec.Config { + config[k] = v + } } + // Delete any user-provided credentialsFile to prevent path traversal vulnerabilities + delete(config, repoconfig.CredentialsFileKey) + if backupLocation.Spec.Credential != nil { config[repoconfig.CredentialsFileKey], err = credentialsFileStore.Path(backupLocation.Spec.Credential) if err != nil { @@ -549,11 +554,16 @@ func getStorageVariables(backupLocation *velerov1api.BackupStorageLocation, repo return map[string]string{}, errors.New("invalid storage provider") } - config := backupLocation.Spec.Config - if config == nil { - config = map[string]string{} + config := make(map[string]string) + if backupLocation.Spec.Config != nil { + for k, v := range backupLocation.Spec.Config { + config[k] = v + } } + // Delete any user-provided credentialsFile to prevent path traversal vulnerabilities + delete(config, repoconfig.CredentialsFileKey) + bucket := strings.Trim(config["bucket"], "/") prefix := strings.Trim(config["prefix"], "/") if backupLocation.Spec.ObjectStorage != nil { diff --git a/pkg/repository/provider/unified_repo_test.go b/pkg/repository/provider/unified_repo_test.go index 2cd9bf576..d2131d6bc 100644 --- a/pkg/repository/provider/unified_repo_test.go +++ b/pkg/repository/provider/unified_repo_test.go @@ -85,7 +85,7 @@ func TestGetStorageCredentials(t *testing.T) { Spec: velerov1api.BackupStorageLocationSpec{ Provider: "velero.io/aws", Config: map[string]string{ - "credentialsFile": "credentials-from-config-map", + "credentialsFile": "credentials-from-config-map", // This should be ignored }, }, }, @@ -96,7 +96,7 @@ func TestGetStorageCredentials(t *testing.T) { }, credFileStore: new(credmock.FileStore), expected: map[string]string{ - "accessKeyID": "from: credentials-from-config-map", + "accessKeyID": "from: ", "providerName": "", "secretAccessKey": "", "sessionToken": "", @@ -108,7 +108,7 @@ func TestGetStorageCredentials(t *testing.T) { Spec: velerov1api.BackupStorageLocationSpec{ Provider: "velero.io/aws", Config: map[string]string{ - "credentialsFile": "credentials-from-config-map", + "credentialsFile": "credentials-from-config-map", // This should be ignored }, Credential: &corev1api.SecretKeySelector{}, }, @@ -134,7 +134,7 @@ func TestGetStorageCredentials(t *testing.T) { Spec: velerov1api.BackupStorageLocationSpec{ Provider: "velero.io/aws", Config: map[string]string{ - "credentialsFile": "credentials-from-config-map", + "credentialsFile": "credentials-from-config-map", // This should be ignored }, }, }, @@ -176,16 +176,16 @@ func TestGetStorageCredentials(t *testing.T) { Spec: velerov1api.BackupStorageLocationSpec{ Provider: "velero.io/gcp", Config: map[string]string{ - "credentialsFile": "credentials-from-config-map", + "credentialsFile": "credentials-from-config-map", // This should be ignored }, }, }, getGCPCredentials: func(config map[string]string) string { - return "credentials-from-config-map" + return config["credentialsFile"] }, credFileStore: new(credmock.FileStore), expected: map[string]string{ - "credFile": "credentials-from-config-map", + "credFile": "", }, }, }