mail: close ARN wildcard bypass — validate all segments
The original wildcard check only validated the identity name, missing wildcards in region and account-id fields. arn:aws:ses:*:*:identity/x.com rendered successfully because the identity name was clean. FIX: 1. Check the WHOLE ARN string for * and ? before parsing, not per-segment. 2. Tighten len(parts) < 6 to len(parts) != 6 to reject trailing segments. 3. Require region and account-id to be non-empty. Add comprehensive test cases for all bypass routes, plus a test that legitimate production ARNs (real 12-digit account IDs) still pass.
2 files changed,
+55
−5
internal/mail/policy.go+14 −5internal/mail/policy_test.go+41 −0
diff --git a/internal/mail/policy.go b/internal/mail/policy.go| index e964bd523..d05cec36a 100644 |
| --- a/internal/mail/policy.go |
| +++ b/internal/mail/policy.go |
| @@ -62,21 +62,30 @@ func SendPolicyJSON(identityARNs, domains []string) (string, error) { |
| froms = append(froms, "*@"+d) |
| } |
| for _, arn := range identityARNs { |
| + // Refuse wildcards anywhere in the ARN before parsing. |
| + // IAM expands * and ? as match patterns in all segments, not just identity name. |
| + if strings.Contains(arn, "*") || strings.Contains(arn, "?") { |
| + return "", fmt.Errorf("mail: invalid identity ARN %q in send policy", arn) |
| + } |
| + |
| // Validate ARN shape: arn:aws:ses:<region>:<account-id>:identity/<name> |
| parts := strings.Split(arn, ":") |
| - if len(parts) < 6 || parts[0] != "arn" || parts[1] != "aws" || parts[2] != "ses" { |
| + if len(parts) != 6 || parts[0] != "arn" || parts[1] != "aws" || parts[2] != "ses" { |
| + return "", fmt.Errorf("mail: invalid identity ARN %q in send policy", arn) |
| + } |
| + |
| + // Require region and account-id to be non-empty |
| + if parts[3] == "" || parts[4] == "" { |
| return "", fmt.Errorf("mail: invalid identity ARN %q in send policy", arn) |
| } |
| + |
| // Extract identity name from the resource part |
| resourceParts := strings.SplitN(parts[5], "/", 2) |
| if len(resourceParts) != 2 || resourceParts[0] != "identity" { |
| return "", fmt.Errorf("mail: invalid identity ARN %q in send policy", arn) |
| } |
| identityName := resourceParts[1] |
| - // Refuse wildcards in the identity name |
| - if strings.Contains(identityName, "*") || strings.Contains(identityName, "?") { |
| - return "", fmt.Errorf("mail: invalid identity ARN %q in send policy", arn) |
| - } |
| + |
| // Check that identity matches domains: either exactly one of them, |
| // or a parent domain of one of them. |
| found := false |
diff --git a/internal/mail/policy_test.go b/internal/mail/policy_test.go| index c3183794d..bc8fec51d 100644 |
| --- a/internal/mail/policy_test.go |
| +++ b/internal/mail/policy_test.go |
| @@ -137,6 +137,47 @@ func TestPolicyRefusesAnInvalidDomain(t *testing.T) { |
| } |
| } |
| +// IAM expands * and ? as match patterns in ALL ARN segments, not just the |
| +// identity name. Reject wildcards anywhere in the ARN, and validate the |
| +// entire structure: exactly 6 colon-delimited parts, region and account-id |
| +// non-empty, no trailing segments, no bypasses. |
| +func TestPolicyRefusesARNWildcards(t *testing.T) { |
| + tests := []struct { |
| + name string |
| + arn string |
| + }{ |
| + // Wildcards in different segments |
| + {"wildcard in region", "arn:aws:ses:*:1:identity/example.com"}, |
| + {"wildcard in account", "arn:aws:ses:eu-west-1:*:identity/example.com"}, |
| + {"wildcard in both", "arn:aws:ses:*:*:identity/example.com"}, |
| + {"question mark in region", "arn:aws:ses:?:1:identity/example.com"}, |
| + |
| + // Malformed structure |
| + {"trailing segment", "arn:aws:ses:eu-west-1:1:identity/x.com:extra"}, |
| + {"empty region", "arn:aws:ses::1:identity/example.com"}, |
| + {"empty account", "arn:aws:ses:eu-west-1::identity/example.com"}, |
| + } |
| + |
| + for _, tt := range tests { |
| + t.Run(tt.name, func(t *testing.T) { |
| + _, err := SendPolicyJSON([]string{tt.arn}, []string{"example.com"}) |
| + if err == nil { |
| + t.Fatalf("SendPolicyJSON accepted invalid ARN: %s", tt.arn) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// Legitimate production shape must pass: real 12-digit account ID, valid region. |
| +func TestPolicyAcceptsLegitimateARN(t *testing.T) { |
| + _, err := SendPolicyJSON( |
| + []string{"arn:aws:ses:eu-west-1:123456789012:identity/seapointish.bag.oncarlos.com"}, |
| + []string{"seapointish.bag.oncarlos.com"}) |
| + if err != nil { |
| + t.Fatalf("SendPolicyJSON rejected legitimate ARN: %v", err) |
| + } |
| +} |
| + |
| func TestUserNamesAreStableAndDistinct(t *testing.T) { |
| if got, want := UserName("bag", "seapointish"), "carlos-mail-bag-seapointish"; got != want { |
| t.Fatalf("UserName = %q, want %q", got, want) |