diff --git a/builtin/providers/aws/data_source_aws_iam_policy_document.go b/builtin/providers/aws/data_source_aws_iam_policy_document.go index 5bea111eec60..1a820508b328 100644 --- a/builtin/providers/aws/data_source_aws_iam_policy_document.go +++ b/builtin/providers/aws/data_source_aws_iam_policy_document.go @@ -112,12 +112,12 @@ func dataSourceAwsIamPolicyDocumentRead(d *schema.ResourceData, meta interface{} } if resources := cfgStmt["resources"].(*schema.Set).List(); len(resources) > 0 { - stmt.Resources = dataSourceAwsIamPolicyDocumentReplaceVarsInList( + stmt.Resources = dataSourceAwsIamPolicyDocumentReplaceVarsInSet( iamPolicyDecodeConfigStringList(resources), ) } if resources := cfgStmt["not_resources"].(*schema.Set).List(); len(resources) > 0 { - stmt.NotResources = dataSourceAwsIamPolicyDocumentReplaceVarsInList( + stmt.NotResources = dataSourceAwsIamPolicyDocumentReplaceVarsInSet( iamPolicyDecodeConfigStringList(resources), ) } @@ -150,52 +150,45 @@ func dataSourceAwsIamPolicyDocumentRead(d *schema.ResourceData, meta interface{} return nil } -func dataSourceAwsIamPolicyDocumentReplaceVarsInList(in interface{}) interface{} { - switch v := in.(type) { - case string: - return dataSourceAwsIamPolicyDocumentVarReplacer.Replace(v) - case []string: - out := make([]string, len(v)) - for i, item := range v { - out[i] = dataSourceAwsIamPolicyDocumentVarReplacer.Replace(item) - } - return out - default: - panic("dataSourceAwsIamPolicyDocumentReplaceVarsInList: input not string nor []string") +func dataSourceAwsIamPolicyDocumentReplaceVarsInSet(in IAMPolicyStringSet) IAMPolicyStringSet { + out := make(IAMPolicyStringSet, len(in)) + for i, item := range in { + out[i] = dataSourceAwsIamPolicyDocumentVarReplacer.Replace(item) } + return out } func dataSourceAwsIamPolicyDocumentMakeConditions(in []interface{}) IAMPolicyStatementConditionSet { - out := make([]IAMPolicyStatementCondition, len(in)) + out := make(IAMPolicyStatementConditionSet, len(in)) for i, itemI := range in { item := itemI.(map[string]interface{}) out[i] = IAMPolicyStatementCondition{ Test: item["test"].(string), Variable: item["variable"].(string), - Values: dataSourceAwsIamPolicyDocumentReplaceVarsInList( + Values: dataSourceAwsIamPolicyDocumentReplaceVarsInSet( iamPolicyDecodeConfigStringList( item["values"].(*schema.Set).List(), ), ), } } - return IAMPolicyStatementConditionSet(out) + return out } func dataSourceAwsIamPolicyDocumentMakePrincipals(in []interface{}) IAMPolicyStatementPrincipalSet { - out := make([]IAMPolicyStatementPrincipal, len(in)) + out := make(IAMPolicyStatementPrincipalSet, len(in)) for i, itemI := range in { item := itemI.(map[string]interface{}) out[i] = IAMPolicyStatementPrincipal{ Type: item["type"].(string), - Identifiers: dataSourceAwsIamPolicyDocumentReplaceVarsInList( + Identifiers: dataSourceAwsIamPolicyDocumentReplaceVarsInSet( iamPolicyDecodeConfigStringList( item["identifiers"].(*schema.Set).List(), ), ), } } - return IAMPolicyStatementPrincipalSet(out) + return out } func dataSourceAwsIamPolicyPrincipalSchema() *schema.Schema { diff --git a/builtin/providers/aws/data_source_aws_iam_policy_document_test.go b/builtin/providers/aws/data_source_aws_iam_policy_document_test.go index a50a8ae29ec3..fffa74bf941d 100644 --- a/builtin/providers/aws/data_source_aws_iam_policy_document_test.go +++ b/builtin/providers/aws/data_source_aws_iam_policy_document_test.go @@ -77,6 +77,7 @@ data "aws_iam_policy_document" "test" { values = [ "home/", "home/&{aws:username}/", + "", ] } @@ -117,13 +118,12 @@ var testAccAWSIAMPolicyDocumentExpectedJSON = `{ "Sid": "1", "Effect": "Allow", "Action": [ - "s3:ListAllMyBuckets", - "s3:GetBucketLocation" + "s3:GetBucketLocation", + "s3:ListAllMyBuckets" ], "Resource": "arn:aws:s3:::*" }, { - "Sid": "", "Effect": "Allow", "Action": "s3:ListBucket", "Resource": "arn:aws:s3:::foo", @@ -133,26 +133,25 @@ var testAccAWSIAMPolicyDocumentExpectedJSON = `{ "Condition": { "StringLike": { "s3:prefix": [ - "home/${aws:username}/", - "home/" + "", + "home/", + "home/${aws:username}/" ] } } }, { - "Sid": "", "Effect": "Allow", "Action": "s3:*", "Resource": [ - "arn:aws:s3:::foo/home/${aws:username}/*", - "arn:aws:s3:::foo/home/${aws:username}" + "arn:aws:s3:::foo/home/${aws:username}", + "arn:aws:s3:::foo/home/${aws:username}/*" ], "Principal": { "AWS": "arn:blahblah:example" } }, { - "Sid": "", "Effect": "Deny", "NotAction": "s3:*", "NotResource": "arn:aws:s3:::*" diff --git a/builtin/providers/aws/iam_policy_model.go b/builtin/providers/aws/iam_policy_model.go index 59192fbf11c0..b4e9a6cfb2f5 100644 --- a/builtin/providers/aws/iam_policy_model.go +++ b/builtin/providers/aws/iam_policy_model.go @@ -2,22 +2,32 @@ package aws import ( "encoding/json" + "fmt" "sort" + + "github.com/hashicorp/terraform/helper/schema" ) +// IAMPolicyDoc is an in-memory representation of an IAM policy document +// with annotations to marshal to and unmarshal from JSON policy syntax. +// +// Round-tripping through this struct will normalize a policy, but it +// is only guaranteed to work with policy versions 2012-10-17 or earlier. +// Newer policies may be silently corrupted by round-tripping through +// this structure. (At the time of writing there is no newer policy version.) type IAMPolicyDoc struct { Version string `json:",omitempty"` Id string `json:",omitempty"` - Statements []*IAMPolicyStatement `json:"Statement"` + Statements []*IAMPolicyStatement `json:"Statement,omitempty"` } type IAMPolicyStatement struct { - Sid string + Sid string `json:",omitempty"` Effect string `json:",omitempty"` - Actions interface{} `json:"Action,omitempty"` - NotActions interface{} `json:"NotAction,omitempty"` - Resources interface{} `json:"Resource,omitempty"` - NotResources interface{} `json:"NotResource,omitempty"` + Actions IAMPolicyStringSet `json:"Action,omitempty"` + NotActions IAMPolicyStringSet `json:"NotAction,omitempty"` + Resources IAMPolicyStringSet `json:"Resource,omitempty"` + NotResources IAMPolicyStringSet `json:"NotResource,omitempty"` Principals IAMPolicyStatementPrincipalSet `json:"Principal,omitempty"` NotPrincipals IAMPolicyStatementPrincipalSet `json:"NotPrincipal,omitempty"` Conditions IAMPolicyStatementConditionSet `json:"Condition,omitempty"` @@ -25,71 +35,234 @@ type IAMPolicyStatement struct { type IAMPolicyStatementPrincipal struct { Type string - Identifiers interface{} + Identifiers IAMPolicyStringSet } type IAMPolicyStatementCondition struct { Test string Variable string - Values interface{} + Values IAMPolicyStringSet } +// IAMPolicyStringSet is a specialization of []string that has special normalization +// rules for unmarshalling from JSON. Specifically, a single-item list is considered +// equivalent to a plain string, and a multi-item list is sorted into lexographical +// order to reflect that the ordering is not meaningful. +// +// When marshalling, the set is again ordered lexographically (in case it has been +// modified by code that didn't preserve the order) and single-item lists are serialized +// as primitive strings, since IAM considers this to be the normalized form. +type IAMPolicyStringSet []string + type IAMPolicyStatementPrincipalSet []IAMPolicyStatementPrincipal type IAMPolicyStatementConditionSet []IAMPolicyStatementCondition +func (ss *IAMPolicyStringSet) UnmarshalJSON(data []byte) error { + var single string + err := json.Unmarshal(data, &single) + if err == nil { + *ss = IAMPolicyStringSet{single} + return nil + } + + var set []string + err = json.Unmarshal(data, &set) + if err == nil { + *ss = IAMPolicyStringSet(set) + sort.Strings(*ss) + return nil + } + + return fmt.Errorf("must be string or array of strings") +} + +func (ss IAMPolicyStringSet) MarshalJSON() ([]byte, error) { + if len(ss) == 1 { + return json.Marshal(ss[0]) + } + + sort.Strings([]string(ss)) + return json.Marshal([]string(ss)) +} + func (ps IAMPolicyStatementPrincipalSet) MarshalJSON() ([]byte, error) { - raw := map[string]interface{}{} + raw := map[string]IAMPolicyStringSet{} for _, p := range ps { - switch i := p.Identifiers.(type) { - case []string: - if _, ok := raw[p.Type]; !ok { - raw[p.Type] = make([]string, 0, len(i)) - } - sort.Sort(sort.Reverse(sort.StringSlice(i))) - raw[p.Type] = append(raw[p.Type].([]string), i...) - case string: - raw[p.Type] = i - default: - panic("Unsupported data type for IAMPolicyStatementPrincipalSet") + if _, ok := raw[p.Type]; !ok { + raw[p.Type] = make(IAMPolicyStringSet, 0, len(p.Identifiers)) } + raw[p.Type] = append(raw[p.Type], p.Identifiers...) } - return json.Marshal(&raw) + return json.Marshal(raw) +} + +func (ps *IAMPolicyStatementPrincipalSet) UnmarshalJSON(data []byte) error { + var wildcard string + err := json.Unmarshal(data, &wildcard) + if err == nil { + if wildcard != "*" { + return fmt.Errorf( + "Principal and NotPrincipal must be either object or the string \"*\"", + ) + } + + // This wildcard is an alias for all/anonymous AWS principals. + // After round-tripping, this will normalize as an explicit wildcard under + // the "AWS" provider. + *ps = IAMPolicyStatementPrincipalSet{ + IAMPolicyStatementPrincipal{ + Type: "AWS", + Identifiers: IAMPolicyStringSet{wildcard}, + }, + } + return nil + } + + var raw map[string]IAMPolicyStringSet + err = json.Unmarshal(data, &raw) + if err != nil { + return err + } + + principalTypes := make([]string, 0, len(raw)) + for k := range raw { + principalTypes = append(principalTypes, k) + } + sort.Strings(principalTypes) + + *ps = make(IAMPolicyStatementPrincipalSet, 0, len(raw)) + for _, principalType := range principalTypes { + *ps = append(*ps, IAMPolicyStatementPrincipal{ + Type: principalType, + Identifiers: raw[principalType], + }) + } + return nil } func (cs IAMPolicyStatementConditionSet) MarshalJSON() ([]byte, error) { - raw := map[string]map[string]interface{}{} + raw := map[string]map[string]IAMPolicyStringSet{} for _, c := range cs { if _, ok := raw[c.Test]; !ok { - raw[c.Test] = map[string]interface{}{} + raw[c.Test] = map[string]IAMPolicyStringSet{} } - switch i := c.Values.(type) { - case []string: - if _, ok := raw[c.Test][c.Variable]; !ok { - raw[c.Test][c.Variable] = make([]string, 0, len(i)) - } - sort.Sort(sort.Reverse(sort.StringSlice(i))) - raw[c.Test][c.Variable] = append(raw[c.Test][c.Variable].([]string), i...) - case string: - raw[c.Test][c.Variable] = i - default: - panic("Unsupported data type for IAMPolicyStatementConditionSet") + if _, ok := raw[c.Test][c.Variable]; !ok { + raw[c.Test][c.Variable] = make(IAMPolicyStringSet, 0, len(c.Values)) } + raw[c.Test][c.Variable] = append(raw[c.Test][c.Variable], c.Values...) } return json.Marshal(&raw) } -func iamPolicyDecodeConfigStringList(lI []interface{}) interface{} { - if len(lI) == 1 { - return lI[0].(string) +func (cs *IAMPolicyStatementConditionSet) UnmarshalJSON(data []byte) error { + var raw map[string]map[string]IAMPolicyStringSet + err := json.Unmarshal(data, &raw) + if err != nil { + return err + } + + tests := make([]string, 0, len(raw)) + count := 0 + for k, v := range raw { + tests = append(tests, k) + count += len(v) + } + sort.Strings(tests) + + *cs = make(IAMPolicyStatementConditionSet, 0, count) + for _, test := range tests { + variables := make([]string, 0, len(raw[test])) + for k := range raw[test] { + variables = append(variables, k) + } + sort.Strings(variables) + + for _, variable := range variables { + *cs = append(*cs, IAMPolicyStatementCondition{ + Test: test, + Variable: variable, + Values: raw[test][variable], + }) + } + } + return nil +} + +// NormalizeIAMPolicyJSON takes an IAM policy in JSON format and produces +// an equivalent JSON document with normalizations applied. In particular, +// single-element string lists are serialized as standalone strings, +// multi-element string lists are sorted lexographically, and the +// policy element attributes are written in a predictable order. +// +// In the event of an error, the result is the input buffer, verbatim. +func NormalizeIAMPolicyJSON(in []byte, cb IAMPolicyStatementNormalizer) ([]byte, error) { + doc := &IAMPolicyDoc{} + err := json.Unmarshal([]byte(in), doc) + if err != nil { + return in, err + } + + if cb != nil && doc.Statements != nil { + // Caller wants to do some additional normalization + for i, stmt := range doc.Statements { + // Callback modifies statement data in-place + err := cb(stmt) + if err != nil { + return in, fmt.Errorf("statement #%d: %s", i + 1, err) + } + } + } + + resultBytes, err := json.Marshal(doc) + if err != nil { + return in, err + } + + return resultBytes, nil +} + +type IAMPolicyStatementNormalizer func (*IAMPolicyStatement) error + +// iamPolicyJSONStateFunc can be used as a StateFunc for attributes that +// take IAM policies in JSON format. +// Should usually be used in conjunction with iamPolicyJSONValidateFunc. +func iamPolicyJSONStateFunc(jsonSrcI interface{}) string { + // Safe to ignore the error because NormalizeIAMPolicyJSON will pass through + // the given string verbatim if it's not valid. + result, _ := NormalizeIAMPolicyJSON([]byte(jsonSrcI.(string)), nil) + return string(result) +} + +// iamPolicyJSONCustomStateFunc produces a function that can be used as a StateFunc +// for attributes that take IAM policies in JSON format and that need further +// resource-specific normalization via a normalization callback. +func iamPolicyJSONCustomStateFunc(cb IAMPolicyStatementNormalizer) schema.SchemaStateFunc { + return func (jsonSrcI interface{}) string { + result, _ := NormalizeIAMPolicyJSON([]byte(jsonSrcI.(string)), cb) + return string(result) + } +} + +// Can be used as a ValidateFunc for attributes that take IAM policies in JSON format. +// Does simple syntactic validation. +func iamPolicyJSONValidateFunc(jsonSrcI interface{}, _ string) ([]string, []error) { + _, err := NormalizeIAMPolicyJSON([]byte(jsonSrcI.(string)), nil) + if err != nil { + return nil, []error{err} } - ret := make([]string, len(lI)) - for i, vI := range lI { - ret[i] = vI.(string) + + return nil, nil +} + +func iamPolicyDecodeConfigStringList(configList []interface{}) IAMPolicyStringSet { + ret := make([]string, len(configList)) + for i, valueI := range configList { + ret[i] = valueI.(string) } - sort.Sort(sort.Reverse(sort.StringSlice(ret))) + sort.Strings(ret) return ret } diff --git a/builtin/providers/aws/iam_policy_model_test.go b/builtin/providers/aws/iam_policy_model_test.go new file mode 100644 index 000000000000..dd444b57c195 --- /dev/null +++ b/builtin/providers/aws/iam_policy_model_test.go @@ -0,0 +1,178 @@ +package aws + +import ( + "testing" +) + +func TestNormalizeIAMPolicyJSON(t *testing.T) { + type testCase struct { + Input string + Expected string + Normalizer IAMPolicyStatementNormalizer + ExpectError bool + } + + tests := []testCase{ + { + `{}`, + `{}`, + nil, + false, + }, + { + `{"Statement":[]}`, + `{}`, + nil, + false, + }, + { + // Single-item action set becomes single string + `{"Statement":[{"Action":["foo:Baz"]}]}`, + `{"Statement":[{"Action":"foo:Baz"}]}`, + nil, + false, + }, + { + // Multiple actions are sorted + `{"Statement":[{"Action":["foo:Zeek","foo:Baz"]}]}`, + `{"Statement":[{"Action":["foo:Baz","foo:Zeek"]}]}`, + nil, + false, + }, + { + `{"Statement":[{"NotAction":["foo:Zeek"]}]}`, + `{"Statement":[{"NotAction":"foo:Zeek"}]}`, + nil, + false, + }, + { + `{"Statement":[{"Resource":["foo:Zeek"]}]}`, + `{"Statement":[{"Resource":"foo:Zeek"}]}`, + nil, + false, + }, + { + `{"Statement":[{"NotResource":["foo:Zeek"]}]}`, + `{"Statement":[{"NotResource":"foo:Zeek"}]}`, + nil, + false, + }, + { + `{"Statement":[{"Principal":{"AWS":["12345"]}}]}`, + `{"Statement":[{"Principal":{"AWS":"12345"}}]}`, + nil, + false, + }, + { + `{"Statement":[{"NotPrincipal":{"AWS":["12345"]}}]}`, + `{"Statement":[{"NotPrincipal":{"AWS":"12345"}}]}`, + nil, + false, + }, + { + `{"Statement":[{"Condition":{"DataGreaterThan":{"aws:CurrentTime":["abc123"]}}}]}`, + `{"Statement":[{"Condition":{"DataGreaterThan":{"aws:CurrentTime":"abc123"}}}]}`, + nil, + false, + }, + { + // Statement attribute order is normalized + `{"Statement":[{"NotAction":"foo:Zeek","Action":"foo:Baz"}]}`, + `{"Statement":[{"Action":"foo:Baz","NotAction":"foo:Zeek"}]}`, + nil, + false, + }, + { + // Unrecognized attributes are discarded + `{"Statement":[{"Baz":"Nope"}]}`, + `{"Statement":[{}]}`, + nil, + false, + }, + { + // Special shorthand for "all AWS accounts and anonymous" + `{"Statement":[{"Principal":"*"}]}`, + `{"Statement":[{"Principal":{"AWS":"*"}}]}`, + nil, + false, + }, + { + // Custom normalizer to eliminate "Resource" + // Some AWS services insert the implied "Resource" value + // describing the object the policy is attached to when + // returning a policy document from the API. This is + // redundant and can't be meaningfully set to any other + // value in config, so we can remove it as part of + // normalization to avoid spurious diffs. + `{"Statement":[{"Effect":"Allow","Resource":"arn:fake:something"}]}`, + `{"Statement":[{"Effect":"Allow"}]}`, + func (stmt *IAMPolicyStatement) error { + stmt.Resources = IAMPolicyStringSet{} + return nil + }, + false, + }, + { + // Invalid JSON suntax + `{"Sta`, + ``, + nil, + true, + }, + { + // Inappropriate statement type + `{"Statement":[true]}`, + ``, + nil, + true, + }, + { + // Inappropriate type for string set + `{"Statement":[{"Action":[true]}]}`, + ``, + nil, + true, + }, + { + // Inappropriate type for string set redux + `{"Statement":[{"Action":true}]}`, + ``, + nil, + true, + }, + { + // Principal string shorthand may only be used for wildcard + `{"Statement":[{"Principal":"1234"}]}`, + ``, + nil, + true, + }, + } + + for _, test := range tests { + resultBytes, err := NormalizeIAMPolicyJSON([]byte(test.Input), test.Normalizer) + + if test.ExpectError { + if err == nil { + t.Errorf("%s normalized successfully; want error", test.Input) + continue + } + + result := string(resultBytes) + if result != test.Input { + t.Errorf("%s\nproduced %s\nshould match input", test.Input, result) + continue + } + } else { + if err != nil { + t.Errorf("%s returned error; want success\n%s", test.Input, err) + continue + } + + result := string(resultBytes) + if result != test.Expected { + t.Errorf("%s\nproduced %s\n want %s", test.Input, result, test.Expected) + } + } + } +}