From db45079d51e9f6d2129f9cd99537102044e29ca2 Mon Sep 17 00:00:00 2001 From: The Fox in the Shell Date: Tue, 27 Mar 2018 14:36:53 +0200 Subject: [PATCH 1/4] resource_aws_iam_user_login_profile/generatePassword(): Use a CSPRNG There are 2 major issues with the previous generatePassword() function: - It does not use a cryptographically-secure RNG, meaning that its output is not guaranteed to be secret; - It is seeded only from the current time, making it very easy to bruteforce the password for an adversary with (limited) information on the creation time of the account. resource_aws_iam_user_login_profile: Make uniformly random passwords This maximises the entropy of the resulting password, given a fixed charset and length; given that the charset contains 2*26 + 10 + 20 distinct charactes, using length-21 passwords results in password entropy above 128 bits. --- aws/resource_aws_iam_user_login_profile.go | 40 ++++++++++------------ 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/aws/resource_aws_iam_user_login_profile.go b/aws/resource_aws_iam_user_login_profile.go index ca4cf61efd43..121802d3ea15 100644 --- a/aws/resource_aws_iam_user_login_profile.go +++ b/aws/resource_aws_iam_user_login_profile.go @@ -1,10 +1,10 @@ package aws import ( + "crypto/rand" "fmt" "log" - "math/rand" - "time" + "math/big" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -59,28 +59,24 @@ func resourceAwsIamUserLoginProfile() *schema.Resource { // characters that are likely to satisfy any possible AWS password policy // (given sufficient length). func generatePassword(length int) string { - charsets := []string{ - "abcdefghijklmnopqrstuvwxyz", - "ABCDEFGHIJKLMNOPQRSTUVWXYZ", - "012346789", - "!@#$%^&*()_+-=[]{}|'", - } - - // Use all character sets - random := rand.New(rand.NewSource(time.Now().UTC().UnixNano())) - components := make(map[int]byte, length) - for i := 0; i < length; i++ { - charset := charsets[i%len(charsets)] - components[i] = charset[random.Intn(len(charset))] - } + charset := `abcdefghijklmnopqrstuvwxyz +ABCDEFGHIJKLMNOPQRSTUVWXYZ +0123456789 +!@#$%^&*()_+-=[]{}|'` - // Randomise the ordering so we don't end up with a predictable - // lower case, upper case, numeric, symbol pattern result := make([]byte, length) - i := 0 - for _, b := range components { - result[i] = b - i = i + 1 + charsetSize := big.NewInt(int64(len(charset))) + + for i := range result { + r, err := rand.Int(rand.Reader, charsetSize) + if err != nil { + panic(err) + } + if !r.IsInt64() { + panic("rand.Int() not representable as an Int64") + } + + result[i] = charset[r.Int64()] } return string(result) From 74334db1a727ab67ff8210096b113e5211a9b6bc Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 29 Mar 2018 14:36:26 -0400 Subject: [PATCH 2/4] add policy checking to iam password generation Check that generated IAM passwords pass any specified iam.PasswordPolicy. IAM password policies cannot set specific numbers of each characters classes, nor can the forbid specific characters. Since the policy restrictions are minimal, rejection sampling will quickly find even the shortest passwords containing all characters classes. This can be extended to lookup the account policy, but in practice that is probably not required. The only real unknown in this case is the minimum password length, which will be validated when setting the password, and catching it earlier won't do any good. Update the validation for password length, since the minimum is 6 characters. --- aws/resource_aws_iam_user_login_profile.go | 65 ++++++++++++++----- ...esource_aws_iam_user_login_profile_test.go | 12 ++++ 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/aws/resource_aws_iam_user_login_profile.go b/aws/resource_aws_iam_user_login_profile.go index 121802d3ea15..37bee75468d1 100644 --- a/aws/resource_aws_iam_user_login_profile.go +++ b/aws/resource_aws_iam_user_login_profile.go @@ -1,6 +1,7 @@ package aws import ( + "bytes" "crypto/rand" "fmt" "log" @@ -40,7 +41,7 @@ func resourceAwsIamUserLoginProfile() *schema.Resource { Type: schema.TypeInt, Optional: true, Default: 20, - ValidateFunc: validation.IntBetween(4, 128), + ValidateFunc: validation.IntBetween(5, 128), }, "key_fingerprint": { @@ -55,31 +56,62 @@ func resourceAwsIamUserLoginProfile() *schema.Resource { } } -// generatePassword generates a random password of a given length using -// characters that are likely to satisfy any possible AWS password policy -// (given sufficient length). +const ( + charLower = "abcdefghijklmnopqrstuvwxyz" + charUpper = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + charNumbers = "0123456789" + charSymbols = "!@#$%^&*()_+-=[]{}|'" +) + +// generatePassword generates a random password of a given length, matching the +// most restrictive iam password policy. func generatePassword(length int) string { - charset := `abcdefghijklmnopqrstuvwxyz -ABCDEFGHIJKLMNOPQRSTUVWXYZ -0123456789 -!@#$%^&*()_+-=[]{}|'` + const charset = charLower + charUpper + charNumbers + charSymbols result := make([]byte, length) charsetSize := big.NewInt(int64(len(charset))) - for i := range result { - r, err := rand.Int(rand.Reader, charsetSize) - if err != nil { - panic(err) + // rather than trying to artifically add specific characters from each + // class to the password to match the policy, we generate passwords + // randomly and reject those that don't match. + // + // Even in the worst case, this tends to take less than 10 tries to find a + // matching password. Any sufficiently long password is likely to succeed + // on the first try + for n := 0; n < 100000; n++ { + for i := range result { + r, err := rand.Int(rand.Reader, charsetSize) + if err != nil { + panic(err) + } + if !r.IsInt64() { + panic("rand.Int() not representable as an Int64") + } + + result[i] = charset[r.Int64()] } - if !r.IsInt64() { - panic("rand.Int() not representable as an Int64") + + if !checkPwdPolicy(result) { + continue } - result[i] = charset[r.Int64()] + return string(result) + } + + panic("failed to generate acceptable password") +} + +// Check the generated password contains all character classes listed in the +// IAM password policy. +func checkPwdPolicy(pass []byte) bool { + if !(bytes.ContainsAny(pass, charLower) && + bytes.ContainsAny(pass, charNumbers) && + bytes.ContainsAny(pass, charSymbols) && + bytes.ContainsAny(pass, charUpper)) { + return false } - return string(result) + return true } func resourceAwsIamUserLoginProfileCreate(d *schema.ResourceData, meta interface{}) error { @@ -110,6 +142,7 @@ func resourceAwsIamUserLoginProfileCreate(d *schema.ResourceData, meta interface } initialPassword := generatePassword(passwordLength) + fingerprint, encrypted, err := encryption.EncryptValue(encryptionKey, initialPassword, "Password") if err != nil { return err diff --git a/aws/resource_aws_iam_user_login_profile_test.go b/aws/resource_aws_iam_user_login_profile_test.go index 0e55ebfea557..bf948b134045 100644 --- a/aws/resource_aws_iam_user_login_profile_test.go +++ b/aws/resource_aws_iam_user_login_profile_test.go @@ -20,6 +20,18 @@ import ( "github.com/hashicorp/vault/helper/pgpkeys" ) +func TestGenerateIAMPassword(t *testing.T) { + p := generatePassword(6) + if len(p) != 6 { + t.Fatalf("expected a 6 character password, got: %q", p) + } + + p = generatePassword(128) + if len(p) != 128 { + t.Fatalf("expected a 128 character password, got: %q", p) + } +} + func TestAccAWSUserLoginProfile_basic(t *testing.T) { var conf iam.GetLoginProfileOutput From 78d50b1d8833ec72083701ae8e62a01a19a3760e Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 30 Mar 2018 11:30:09 -0400 Subject: [PATCH 3/4] add iam password policy test --- ...esource_aws_iam_user_login_profile_test.go | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/aws/resource_aws_iam_user_login_profile_test.go b/aws/resource_aws_iam_user_login_profile_test.go index bf948b134045..a85b8094b19e 100644 --- a/aws/resource_aws_iam_user_login_profile_test.go +++ b/aws/resource_aws_iam_user_login_profile_test.go @@ -32,6 +32,30 @@ func TestGenerateIAMPassword(t *testing.T) { } } +func TestIAMPasswordPolicyCheck(t *testing.T) { + for _, tc := range []struct { + pass string + valid bool + }{ + // no symbol + {pass: "abCD12", valid: false}, + // no number + {pass: "abCD%$", valid: false}, + // no upper + {pass: "abcd1#", valid: false}, + // no lower + {pass: "ABCD1#", valid: false}, + {pass: "abCD11#$", valid: true}, + } { + t.Run(tc.pass, func(t *testing.T) { + valid := checkPwdPolicy([]byte(tc.pass)) + if valid != tc.valid { + t.Fatalf("expected %q to be valid==%t, got %t", tc.pass, tc.valid, valid) + } + }) + } +} + func TestAccAWSUserLoginProfile_basic(t *testing.T) { var conf iam.GetLoginProfileOutput From e804aedbb31e77cbee8392e929cec5ec73a9e01e Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 30 Mar 2018 11:31:21 -0400 Subject: [PATCH 4/4] rename functions to be more descriptive --- aws/resource_aws_iam_user_login_profile.go | 10 +++++----- aws/resource_aws_iam_user_login_profile_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/aws/resource_aws_iam_user_login_profile.go b/aws/resource_aws_iam_user_login_profile.go index 37bee75468d1..98c0ac4e8f13 100644 --- a/aws/resource_aws_iam_user_login_profile.go +++ b/aws/resource_aws_iam_user_login_profile.go @@ -63,9 +63,9 @@ const ( charSymbols = "!@#$%^&*()_+-=[]{}|'" ) -// generatePassword generates a random password of a given length, matching the +// generateIAMPassword generates a random password of a given length, matching the // most restrictive iam password policy. -func generatePassword(length int) string { +func generateIAMPassword(length int) string { const charset = charLower + charUpper + charNumbers + charSymbols result := make([]byte, length) @@ -91,7 +91,7 @@ func generatePassword(length int) string { result[i] = charset[r.Int64()] } - if !checkPwdPolicy(result) { + if !checkIAMPwdPolicy(result) { continue } @@ -103,7 +103,7 @@ func generatePassword(length int) string { // Check the generated password contains all character classes listed in the // IAM password policy. -func checkPwdPolicy(pass []byte) bool { +func checkIAMPwdPolicy(pass []byte) bool { if !(bytes.ContainsAny(pass, charLower) && bytes.ContainsAny(pass, charNumbers) && bytes.ContainsAny(pass, charSymbols) && @@ -141,7 +141,7 @@ func resourceAwsIamUserLoginProfileCreate(d *schema.ResourceData, meta interface } } - initialPassword := generatePassword(passwordLength) + initialPassword := generateIAMPassword(passwordLength) fingerprint, encrypted, err := encryption.EncryptValue(encryptionKey, initialPassword, "Password") if err != nil { diff --git a/aws/resource_aws_iam_user_login_profile_test.go b/aws/resource_aws_iam_user_login_profile_test.go index a85b8094b19e..2ba687748419 100644 --- a/aws/resource_aws_iam_user_login_profile_test.go +++ b/aws/resource_aws_iam_user_login_profile_test.go @@ -21,12 +21,12 @@ import ( ) func TestGenerateIAMPassword(t *testing.T) { - p := generatePassword(6) + p := generateIAMPassword(6) if len(p) != 6 { t.Fatalf("expected a 6 character password, got: %q", p) } - p = generatePassword(128) + p = generateIAMPassword(128) if len(p) != 128 { t.Fatalf("expected a 128 character password, got: %q", p) } @@ -48,7 +48,7 @@ func TestIAMPasswordPolicyCheck(t *testing.T) { {pass: "abCD11#$", valid: true}, } { t.Run(tc.pass, func(t *testing.T) { - valid := checkPwdPolicy([]byte(tc.pass)) + valid := checkIAMPwdPolicy([]byte(tc.pass)) if valid != tc.valid { t.Fatalf("expected %q to be valid==%t, got %t", tc.pass, tc.valid, valid) } @@ -225,7 +225,7 @@ func testDecryptPasswordAndTest(nProfile, nAccessKey, key string) resource.TestC iamAsCreatedUser := iam.New(iamAsCreatedUserSession) _, err = iamAsCreatedUser.ChangePassword(&iam.ChangePasswordInput{ OldPassword: aws.String(decryptedPassword.String()), - NewPassword: aws.String(generatePassword(20)), + NewPassword: aws.String(generateIAMPassword(20)), }) if err != nil { if awserr, ok := err.(awserr.Error); ok && awserr.Code() == "InvalidClientTokenId" {