From 69f5abdc910231b48b4ac2b2da71dbdb73de7e9a Mon Sep 17 00:00:00 2001 From: Robin Walsh Date: Mon, 24 Aug 2015 16:12:45 -0700 Subject: [PATCH 1/4] spaces in displayName break AWS IAM --- builtin/logical/aws/secret_access_keys.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/builtin/logical/aws/secret_access_keys.go b/builtin/logical/aws/secret_access_keys.go index 1d566b41b..a342c741b 100644 --- a/builtin/logical/aws/secret_access_keys.go +++ b/builtin/logical/aws/secret_access_keys.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "math/rand" + "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -46,7 +47,7 @@ func (b *backend) secretAccessKeysCreate( // Generate a random username. We don't put the policy names in the // username because the AWS console makes it pretty easy to see that. - username := fmt.Sprintf("vault-%s-%d-%d", displayName, time.Now().Unix(), rand.Int31n(10000)) + username := fmt.Sprintf("vault-%s-%d-%d", normalizeDisplayName(displayName), time.Now().Unix(), rand.Int31n(10000)) // Write to the WAL that this user will be created. We do this before // the user is created because if switch the order then the WAL put @@ -141,3 +142,7 @@ func secretAccessKeysRevoke( return nil, nil } + +func normalizeDisplayName(displayName string) string { + return strings.Replace(displayName, " ", "_", -1) +} From 8530f14fee9d7054bf7d85afc24d74a286555b3d Mon Sep 17 00:00:00 2001 From: Robin Walsh Date: Mon, 24 Aug 2015 17:00:54 -0700 Subject: [PATCH 2/4] s/string replacement/regexp replacement --- builtin/logical/aws/secret_access_keys.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/logical/aws/secret_access_keys.go b/builtin/logical/aws/secret_access_keys.go index a342c741b..53d5f80f7 100644 --- a/builtin/logical/aws/secret_access_keys.go +++ b/builtin/logical/aws/secret_access_keys.go @@ -3,7 +3,7 @@ package aws import ( "fmt" "math/rand" - "strings" + "regexp" "time" "github.com/aws/aws-sdk-go/aws" @@ -144,5 +144,6 @@ func secretAccessKeysRevoke( } func normalizeDisplayName(displayName string) string { - return strings.Replace(displayName, " ", "_", -1) + re := regexp.MustCompile("[^a-zA-Z+=,.@_-]") + return re.ReplaceAllString(displayName, "_") } From 4b7c2cc114a18eade1a9a2a05df71bad275a2014 Mon Sep 17 00:00:00 2001 From: Robin Walsh Date: Wed, 26 Aug 2015 09:23:33 -0700 Subject: [PATCH 3/4] Adding unit test for normalizeDisplayName() --- builtin/logical/aws/secret_access_keys_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 builtin/logical/aws/secret_access_keys_test.go diff --git a/builtin/logical/aws/secret_access_keys_test.go b/builtin/logical/aws/secret_access_keys_test.go new file mode 100644 index 000000000..e755d391d --- /dev/null +++ b/builtin/logical/aws/secret_access_keys_test.go @@ -0,0 +1,15 @@ +package aws + +import ( + "testing" +) + +func TestNormalizeDisplayName(t *testing.T) { + invalidName := "^#$test name\nshould be normalized)(*" + expectedName := "___test_name_should_be_normalized___" + normalizedName := normalizeDisplayName(invalidName) + if normalizedName != expectedName { + t.Fatalf("normalizeDisplayName does not normalize AWS name correctly: %s", normalizedName) + } + +} From 34b84367b53b1a2f11883d79f5ff6012a31babc4 Mon Sep 17 00:00:00 2001 From: Robin Walsh Date: Wed, 26 Aug 2015 09:26:20 -0700 Subject: [PATCH 4/4] Adding one more test (for no-op case) --- builtin/logical/aws/secret_access_keys_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/builtin/logical/aws/secret_access_keys_test.go b/builtin/logical/aws/secret_access_keys_test.go index e755d391d..156fbd028 100644 --- a/builtin/logical/aws/secret_access_keys_test.go +++ b/builtin/logical/aws/secret_access_keys_test.go @@ -9,7 +9,17 @@ func TestNormalizeDisplayName(t *testing.T) { expectedName := "___test_name_should_be_normalized___" normalizedName := normalizeDisplayName(invalidName) if normalizedName != expectedName { - t.Fatalf("normalizeDisplayName does not normalize AWS name correctly: %s", normalizedName) + t.Fatalf( + "normalizeDisplayName does not normalize AWS name correctly: %s", + normalizedName) + } + + validName := "test_name_should_normalize_to_itself@example.com" + normalizedValidName := normalizeDisplayName(validName) + if normalizedValidName != validName { + t.Fatalf( + "normalizeDisplayName erroneously normalizes valid names: %s", + normalizedName) } }