amadan

carlos / platform Public

mail: harden the IAM policy against ARN and domain validation

Four security hardening fixes from the adversarial review:

1. Refuse ARNs with wildcards (* or ?) — IAM expands these as match patterns,
   so arn:aws:ses:*:*:identity/* grants over every SES identity in the account.
2. Validate ARN shape strictly: arn:aws:ses:<region>:<account-id>:identity/<name>.
3. Require each identity name to match a granted domain: either exactly one,
   or a parent domain of one (e.g., example.com covers sub.example.com).
4. Fix UserName/StandaloneUserName collision: app names allow hyphens, so
   UserName("bag", "see-s-laptop") collided with StandaloneUserName("bag",
   "see", "laptop"). Changed separator to +s+ which app names cannot contain.
5. Add test coverage for multi-domain/multi-ARN steady state.
Paul Campbell a40ef5a58a9442fae4ac0276fdbf9e8757fefc31 parent 1ceedc6
2 files changed, +78 −2
  • internal/mail/policy.go +29 −2
  • internal/mail/policy_test.go +49 −0
diff --git a/internal/mail/policy.go b/internal/mail/policy.go
index 94e97df73..e964bd523 100644
--- a/internal/mail/policy.go
+++ b/internal/mail/policy.go
@@ -31,7 +31,7 @@ func UserName(acct, app string) string { return "carlos-mail-" + acct + "-" + ap
// already spend both. A standalone key on the shared user would work until
// the first rotation and then make rotation impossible.
func StandaloneUserName(acct, app, name string) string {
- return "carlos-mail-" + acct + "-" + app + "-s-" + name
+ return "carlos-mail-" + acct + "-" + app + "+s+" + name
}
// SendPolicyJSON renders the policy for a set of verified identity ARNs and
@@ -41,6 +41,9 @@ func StandaloneUserName(acct, app, name string) string {
// An empty grant returns an error rather than a permissive document: the
// failure worth having is "this app cannot send", never "this app can send
// as anything in the estate".
+// Each ARN must have the shape arn:aws:ses:<region>:<account-id>:identity/<name>,
+// the identity name must not contain wildcards, and it must be either exactly
+// one of the domains or a parent domain of one of them.
func SendPolicyJSON(identityARNs, domains []string) (string, error) {
if len(identityARNs) == 0 {
return "", fmt.Errorf("mail: refusing to render a send policy with no identity ARNs")
@@ -59,9 +62,33 @@ func SendPolicyJSON(identityARNs, domains []string) (string, error) {
froms = append(froms, "*@"+d)
}
for _, arn := range identityARNs {
- if !strings.HasPrefix(arn, "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" {
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
+ for _, d := range domains {
+ if identityName == d || strings.HasSuffix(d, "."+identityName) {
+ found = true
+ break
+ }
+ }
+ if !found {
+ return "", fmt.Errorf("mail: identity %q in ARN %q does not match any granted domain", identityName, arn)
+ }
}
doc := map[string]any{
"Version": "2012-10-17",
diff --git a/internal/mail/policy_test.go b/internal/mail/policy_test.go
index 01a8bee35..c3183794d 100644
--- a/internal/mail/policy_test.go
+++ b/internal/mail/policy_test.go
@@ -69,6 +69,51 @@ func TestPolicyListsEveryVerifiedARN(t *testing.T) {
}
}
+// Steady state is multiple domains per app. Untested single-domain case would
+// allow an implementation that rendered only froms[0] to pass the suite.
+func TestPolicyWithMultipleDomainsAndARNs(t *testing.T) {
+ got, err := SendPolicyJSON([]string{
+ "arn:aws:ses:eu-west-1:1:identity/platform.com",
+ "arn:aws:ses:us-east-1:1:identity/custom.app",
+ }, []string{"platform.com", "custom.app"})
+ if err != nil {
+ t.Fatalf("SendPolicyJSON: %v", err)
+ }
+ d := parsePolicy(t, got)
+ st := d.Statement[0]
+
+ // Check resources
+ if len(st.Resource) != 2 {
+ t.Fatalf("Resource count = %d, want 2", len(st.Resource))
+ }
+ resourceSet := make(map[string]bool)
+ for _, r := range st.Resource {
+ resourceSet[r] = true
+ }
+ if !resourceSet["arn:aws:ses:eu-west-1:1:identity/platform.com"] {
+ t.Fatalf("Resource missing arn:aws:ses:eu-west-1:1:identity/platform.com")
+ }
+ if !resourceSet["arn:aws:ses:us-east-1:1:identity/custom.app"] {
+ t.Fatalf("Resource missing arn:aws:ses:us-east-1:1:identity/custom.app")
+ }
+
+ // Check from addresses
+ froms := st.Condition["StringLike"]["ses:FromAddress"]
+ if len(froms) != 2 {
+ t.Fatalf("ses:FromAddress count = %d, want 2", len(froms))
+ }
+ fromSet := make(map[string]bool)
+ for _, f := range froms {
+ fromSet[f] = true
+ }
+ if !fromSet["*@platform.com"] {
+ t.Fatalf("ses:FromAddress missing *@platform.com")
+ }
+ if !fromSet["*@custom.app"] {
+ t.Fatalf("ses:FromAddress missing *@custom.app")
+ }
+}
+
// An empty grant must not render a policy that permits everything. Refusing
// is the only safe direction: a wildcard here would let one app send as any
// identity in the estate.
@@ -102,4 +147,8 @@ func TestUserNamesAreStableAndDistinct(t *testing.T) {
if StandaloneUserName("bag", "seapointish", "laptop") == UserName("bag", "seapointish") {
t.Fatal("standalone credential collided with the delivered user")
}
+ // App names allow hyphens, so ensure no collision with a different separator.
+ if StandaloneUserName("bag", "see", "laptop") == UserName("bag", "see-s-laptop") {
+ t.Fatal("standalone credential collided with a hyphenated app name")
+ }
}