diff --git a/changelog/26135.txt b/changelog/26135.txt new file mode 100644 index 0000000000..75cc1dc750 --- /dev/null +++ b/changelog/26135.txt @@ -0,0 +1,3 @@ +```release-note:improvement +storage/azure: Perform validation on Azure account name and container name +``` diff --git a/physical/azure/azure.go b/physical/azure/azure.go index 251d2301c0..a4eb29f899 100644 --- a/physical/azure/azure.go +++ b/physical/azure/azure.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "net/url" "os" + "regexp" "sort" "strconv" "strings" @@ -56,6 +57,10 @@ func NewAzureBackend(conf map[string]string, logger log.Logger) (physical.Backen } } + if err := validateContainerName(name); err != nil { + return nil, fmt.Errorf("invalid container name %s: %w", name, err) + } + accountName := os.Getenv("AZURE_ACCOUNT_NAME") if accountName == "" { accountName = conf["accountName"] @@ -63,6 +68,9 @@ func NewAzureBackend(conf map[string]string, logger log.Logger) (physical.Backen return nil, fmt.Errorf("'accountName' must be set") } } + if err := validateAccountName(name); err != nil { + return nil, fmt.Errorf("invalid account name %s: %w", name, err) + } accountKey := os.Getenv("AZURE_ACCOUNT_KEY") if accountKey == "" { @@ -188,6 +196,35 @@ func NewAzureBackend(conf map[string]string, logger log.Logger) (physical.Backen return a, nil } +// validation rules for containers are defined here: +// https://learn.microsoft.com/en-us/rest/api/storageservices/Naming-and-Referencing-Containers--Blobs--and-Metadata#container-names +var containerNameRegex = regexp.MustCompile("^[a-z0-9]+(-[a-z0-9]+)*$") + +func validateContainerName(name string) error { + if len(name) < 3 || len(name) > 63 { + return errors.New("name must be between 3 and 63 characters long") + } + + if !containerNameRegex.MatchString(name) { + return errors.New("name is invalid") + } + return nil +} + +// validation rules are defined here: +// https://learn.microsoft.com/en-us/azure/azure-resource-manager/troubleshooting/error-storage-account-name?tabs=bicep#cause +var accountNameRegex = regexp.MustCompile("^[a-z0-9]+$") + +func validateAccountName(name string) error { + if len(name) < 3 || len(name) > 24 { + return errors.New("name must be between 3 and 24 characters long") + } + if !accountNameRegex.MatchString(name) { + return errors.New("name is invalid") + } + return nil +} + // Put is used to insert or update an entry func (a *AzureBackend) Put(ctx context.Context, entry *physical.Entry) error { defer metrics.MeasureSince([]string{"azure", "put"}, time.Now()) diff --git a/physical/azure/azure_test.go b/physical/azure/azure_test.go index 13f4e301cd..22344a6bca 100644 --- a/physical/azure/azure_test.go +++ b/physical/azure/azure_test.go @@ -9,6 +9,7 @@ import ( "net" "os" "strconv" + "strings" "testing" "time" @@ -17,6 +18,7 @@ import ( "github.com/hashicorp/vault/helper/testhelpers/azurite" "github.com/hashicorp/vault/sdk/helper/logging" "github.com/hashicorp/vault/sdk/physical" + "github.com/stretchr/testify/require" ) /// These tests run against an Azurite docker container, unless AZURE_ACCOUNT_NAME is given. @@ -33,7 +35,7 @@ func testFixture(t *testing.T) (*AzureBackend, func()) { t.Helper() ts := time.Now().UnixNano() - name := fmt.Sprintf("vault-test-%d", ts) + name := fmt.Sprintf("vlt%d", ts) _ = os.Setenv("AZURE_BLOB_CONTAINER", name) cleanup := func() {} @@ -128,3 +130,116 @@ func isIMDSReachable(t *testing.T) bool { return true } + +// TestAzureBackend_validateContainerName validates that the given container +// names meet the Azure restrictions for container names +func TestAzureBackend_validateContainerName(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + containerName string + wantError bool + }{ + { + name: "success", + containerName: "abcd-1234-efgh", + wantError: false, + }, + { + name: "uppercase", + containerName: "Abcd-1234-efgh", + wantError: true, + }, + { + name: "hyphen start", + containerName: "-abcd-1234-efgh", + wantError: true, + }, + { + name: "hyphen end", + containerName: "abcd-1234-efgh-", + wantError: true, + }, + { + name: "double hyphen", + containerName: "abcd-1234--efgh", + wantError: true, + }, + { + name: "too short", + containerName: "ab", + wantError: true, + }, + { + name: "too long", + containerName: strings.Repeat("a", 64), + wantError: true, + }, + { + name: "other character", + containerName: "abcd-1234-e!gh", + wantError: true, + }, + } + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + err := validateContainerName(tc.containerName) + if tc.wantError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + +// TestAzureBackend_validateAccountName validates that the given account names +// meet the Azure restrictions for account names +func TestAzureBackend_validateAccountName(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + accountName string + wantError bool + }{ + { + name: "success", + accountName: "abcd1234", + wantError: false, + }, + { + name: "uppercase", + accountName: "Abcd0123", + wantError: true, + }, + { + name: "hyphen", + accountName: "abcd-1234", + wantError: true, + }, + { + name: "too short", + accountName: "ab", + wantError: true, + }, + { + name: "too long", + accountName: strings.Repeat("a", 25), + wantError: true, + }, + } + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + err := validateAccountName(tc.accountName) + if tc.wantError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +}