diff --git a/spinifex/gateway/gateway.go b/spinifex/gateway/gateway.go index 44f5302d4..bf84bf9cf 100644 --- a/spinifex/gateway/gateway.go +++ b/spinifex/gateway/gateway.go @@ -16,6 +16,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/google/uuid" "github.com/mulgadc/predastore/auth" + "github.com/mulgadc/predastore/pkg/iampolicy" "github.com/mulgadc/predastore/ratelimit" "github.com/mulgadc/spinifex/spinifex/awserrors" gateway_ecr "github.com/mulgadc/spinifex/spinifex/gateway/ecr" @@ -475,7 +476,7 @@ func (gw *GatewayConfig) checkPolicyResource(r *http.Request, service, action, r return errors.New(awserrors.ErrorInternalError) } - if policy.EvaluateAccess(logIdentity, iamAction, resource, policies) == policy.Deny { + if iampolicy.Evaluate(iamAction, resource, policies) == iampolicy.Deny { slog.Info("checkPolicy: access denied", "identity", logIdentity, "action", iamAction, "resource", resource) return errors.New(awserrors.ErrorAccessDenied) } diff --git a/spinifex/gateway/policy/evaluator.go b/spinifex/gateway/policy/evaluator.go deleted file mode 100644 index 83f88f2f7..000000000 --- a/spinifex/gateway/policy/evaluator.go +++ /dev/null @@ -1,72 +0,0 @@ -// Package policy implements IAM policy evaluation for access control decisions. -package policy - -import ( - "log/slog" - "strings" - - "github.com/mulgadc/spinifex/spinifex/filterutil" - handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" -) - -// Decision represents the outcome of a policy evaluation. -type Decision int - -const ( - // Deny is the default — no matching Allow, or an explicit Deny. - Deny Decision = iota - // Allow means an explicit Allow was found with no overriding Deny. - Allow -) - -// EvaluateAccess checks whether the given identity is permitted to perform -// the specified action on the specified resource, based on the supplied -// policy documents. It follows AWS's evaluation order: -// -// 1. Explicit Deny in any statement → Deny (wins immediately). -// 2. Explicit Allow in any statement → Allow. -// 3. No matching statement → Deny (implicit default). -// -// Root bypass is handled by the gateway before this function is called. -func EvaluateAccess(identity, action, resource string, policies []handlers_iam.PolicyDocument) Decision { - hasAllow := false - for i := range policies { - for j := range policies[i].Statement { - stmt := &policies[i].Statement[j] //nolint:gosec // G602 false positive: j is bounded by range - - if !matchesAny(stmt.Action, action) { - continue - } - if !matchesAny(stmt.Resource, resource) { - continue - } - switch stmt.Effect { - case handlers_iam.PolicyEffectDeny: - return Deny - case handlers_iam.PolicyEffectAllow: - hasAllow = true - default: - slog.Warn("EvaluateAccess: unrecognized Effect, treating as Deny", - "effect", stmt.Effect, "action", action, "identity", identity) - return Deny - } - } - } - - if hasAllow { - return Allow - } - return Deny -} - -// matchesAny reports whether any pattern matches value using AWS IAM wildcard -// semantics. Comparison is case-insensitive. -func matchesAny(patterns []string, value string) bool { - lv := strings.ToLower(value) - for _, p := range patterns { - if filterutil.MatchWildcard(strings.ToLower(p), lv) { - return true - } - } - return false -} diff --git a/spinifex/gateway/policy/evaluator_test.go b/spinifex/gateway/policy/evaluator_test.go index 26e0fa84d..43b716817 100644 --- a/spinifex/gateway/policy/evaluator_test.go +++ b/spinifex/gateway/policy/evaluator_test.go @@ -3,10 +3,11 @@ package policy import ( "testing" + "github.com/mulgadc/predastore/pkg/iampolicy" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" ) -// helper to build a single-statement policy document. +// doc builds a single-statement policy document. func doc(effect, action, resource string) handlers_iam.PolicyDocument { return handlers_iam.PolicyDocument{ Version: "2012-10-17", @@ -16,303 +17,15 @@ func doc(effect, action, resource string) handlers_iam.PolicyDocument { } } -// Root bypass is tested at the gateway layer (checkPolicy), not in the evaluator. +// The deny-wins algorithm and wildcard matcher are unit-tested in predastore's +// pkg/iampolicy. These tests pin spinifex's own action-string conventions end to +// end through iampolicy.Evaluate, exercising the aliased handlers_iam DTOs. -func TestEvaluateAccess_DefaultDeny(t *testing.T) { - // Non-root with no policies → default deny. - got := EvaluateAccess("alice", "ec2:RunInstances", "*", nil) - if got != Deny { - t.Fatalf("expected default Deny with no policies, got %v", got) - } -} - -func TestEvaluateAccess_DefaultDenyEmptyPolicies(t *testing.T) { - // Non-root with empty policy list → default deny. - got := EvaluateAccess("alice", "ec2:RunInstances", "*", []handlers_iam.PolicyDocument{}) - if got != Deny { - t.Fatalf("expected default Deny with empty policies, got %v", got) - } -} - -func TestEvaluateAccess_ExplicitAllow(t *testing.T) { - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "ec2:RunInstances", "*"), - } - got := EvaluateAccess("alice", "ec2:RunInstances", "*", policies) - if got != Allow { - t.Fatalf("expected Allow, got %v", got) - } -} - -func TestEvaluateAccess_ExplicitDenyWins(t *testing.T) { - // Explicit deny overrides an explicit allow. - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "ec2:*", "*"), - doc("Deny", "ec2:TerminateInstances", "*"), - } - got := EvaluateAccess("alice", "ec2:TerminateInstances", "*", policies) - if got != Deny { - t.Fatalf("expected Deny (explicit deny wins), got %v", got) - } -} - -func TestEvaluateAccess_ExplicitDenyWinsSamePolicy(t *testing.T) { - // Deny and Allow in the same policy document — Deny wins. - policies := []handlers_iam.PolicyDocument{ - { - Version: "2012-10-17", - Statement: []handlers_iam.Statement{ - {Effect: "Allow", Action: handlers_iam.StringOrArr{"ec2:*"}, Resource: handlers_iam.StringOrArr{"*"}}, - {Effect: "Deny", Action: handlers_iam.StringOrArr{"ec2:TerminateInstances"}, Resource: handlers_iam.StringOrArr{"*"}}, - }, - }, - } - got := EvaluateAccess("alice", "ec2:TerminateInstances", "*", policies) - if got != Deny { - t.Fatalf("expected Deny (same-policy explicit deny), got %v", got) - } -} - -func TestEvaluateAccess_NoMatchingAction(t *testing.T) { - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "s3:GetObject", "*"), - } - got := EvaluateAccess("alice", "ec2:RunInstances", "*", policies) - if got != Deny { - t.Fatalf("expected Deny (no matching action), got %v", got) - } -} - -func TestEvaluateAccess_WildcardAllActions(t *testing.T) { - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "*", "*"), - } - got := EvaluateAccess("alice", "ec2:RunInstances", "*", policies) - if got != Allow { - t.Fatalf("expected Allow with wildcard *, got %v", got) - } -} - -func TestEvaluateAccess_ServiceWildcard(t *testing.T) { - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "ec2:*", "*"), - } - - tests := []struct { - action string - want Decision - }{ - {"ec2:RunInstances", Allow}, - {"ec2:DescribeInstances", Allow}, - {"s3:GetObject", Deny}, - {"iam:CreateUser", Deny}, - } - - for _, tt := range tests { - got := EvaluateAccess("alice", tt.action, "*", policies) - if got != tt.want { - t.Errorf("ec2:* policy, action=%s: expected %v, got %v", tt.action, tt.want, got) - } - } -} - -func TestEvaluateAccess_PrefixWildcard(t *testing.T) { - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "s3:Get*", "*"), - } - - tests := []struct { - action string - want Decision - }{ - {"s3:GetObject", Allow}, - {"s3:GetBucketPolicy", Allow}, - {"s3:PutObject", Deny}, - {"s3:DeleteObject", Deny}, - } - - for _, tt := range tests { - got := EvaluateAccess("alice", tt.action, "*", policies) - if got != tt.want { - t.Errorf("s3:Get* policy, action=%s: expected %v, got %v", tt.action, tt.want, got) - } - } -} - -func TestEvaluateAccess_MultipleActions(t *testing.T) { - // A statement with multiple actions. - policies := []handlers_iam.PolicyDocument{ - { - Version: "2012-10-17", - Statement: []handlers_iam.Statement{ - { - Effect: "Allow", - Action: handlers_iam.StringOrArr{"ec2:DescribeInstances", "ec2:RunInstances"}, - Resource: handlers_iam.StringOrArr{"*"}, - }, - }, - }, - } - - tests := []struct { - action string - want Decision - }{ - {"ec2:DescribeInstances", Allow}, - {"ec2:RunInstances", Allow}, - {"ec2:TerminateInstances", Deny}, - } - - for _, tt := range tests { - got := EvaluateAccess("alice", tt.action, "*", policies) - if got != tt.want { - t.Errorf("multi-action policy, action=%s: expected %v, got %v", tt.action, tt.want, got) - } - } -} - -func TestEvaluateAccess_MultiplePolicies(t *testing.T) { - // Permissions spread across multiple policy documents. - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "ec2:DescribeInstances", "*"), - doc("Allow", "s3:GetObject", "*"), - } - - tests := []struct { - action string - want Decision - }{ - {"ec2:DescribeInstances", Allow}, - {"s3:GetObject", Allow}, - {"iam:CreateUser", Deny}, - } - - for _, tt := range tests { - got := EvaluateAccess("alice", tt.action, "*", policies) - if got != tt.want { - t.Errorf("multi-policy, action=%s: expected %v, got %v", tt.action, tt.want, got) - } - } -} - -func TestEvaluateAccess_ResourceMismatch(t *testing.T) { - // Allow only on a specific resource, request uses "*". - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "s3:GetObject", "arn:aws:s3:::my-bucket/*"), - } - got := EvaluateAccess("alice", "s3:GetObject", "*", policies) - if got != Deny { - t.Fatalf("expected Deny (resource mismatch), got %v", got) - } -} - -func TestEvaluateAccess_CaseInsensitiveAction(t *testing.T) { - // Action matching should be case-insensitive for exact matches. - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "EC2:RunInstances", "*"), - } - got := EvaluateAccess("alice", "ec2:RunInstances", "*", policies) - if got != Allow { - t.Fatalf("expected Allow (case-insensitive match), got %v", got) - } -} - -// --- wildcard matching tests (via matchesAny) --- - -func TestMatchesAny_Wildcard(t *testing.T) { - tests := []struct { - pattern string - value string - want bool - }{ - // Global wildcard - {"*", "anything", true}, - {"*", "", true}, - - // Service wildcard - {"ec2:*", "ec2:RunInstances", true}, - {"ec2:*", "ec2:DescribeInstances", true}, - {"ec2:*", "s3:GetObject", false}, - {"EC2:*", "ec2:RunInstances", true}, - - // Prefix wildcard - {"s3:Get*", "s3:GetObject", true}, - {"s3:Get*", "s3:GetBucketPolicy", true}, - {"s3:Get*", "s3:PutObject", false}, - {"S3:get*", "s3:GetObject", true}, - - // Exact match - {"ec2:RunInstances", "ec2:RunInstances", true}, - {"ec2:RunInstances", "ec2:StopInstances", false}, - - // Case insensitive exact match - {"EC2:RunInstances", "ec2:RunInstances", true}, - {"ec2:runinstances", "ec2:RunInstances", true}, - - // Embedded wildcards (AWS IAM-style — required for iam:PassRole ARN matching). - {"arn:aws:iam::*:role/app-*", "arn:aws:iam::123456789012:role/app-foo", true}, - {"arn:aws:iam::*:role/app-*", "arn:aws:iam::999999999999:role/app-bar", true}, - {"arn:aws:iam::*:role/*", "arn:aws:iam::123456789012:role/anything", true}, - {"arn:aws:iam::123456789012:role/app-*", "arn:aws:iam::123456789012:role/app-foo", true}, - {"arn:aws:iam::*:role/app-*", "arn:aws:iam::123456789012:role/admin-foo", false}, - {"arn:aws:iam::*:role/app-*", "arn:aws:iam::123456789012:user/app-foo", false}, - {"arn:aws:iam::*:role/app-*", "arn:aws:iam::123456789012:role/app-", true}, - {"a*b*c", "axxbyyc", true}, - {"a*b*c", "axxbyy", false}, - - // Edge cases - {"", "", true}, - {"", "something", false}, - } - - for _, tt := range tests { - got := matchesAny([]string{tt.pattern}, tt.value) - if got != tt.want { - t.Errorf("matchesAny([%q], %q) = %v, want %v", tt.pattern, tt.value, got, tt.want) - } - } -} - -// TestEvaluateAccess_PassRoleResourceARN exercises the resource-ARN matching -// path used by iam:PassRole enforcement. The caller's policy will typically -// scope PassRole to a wildcard ARN; the evaluator must match a concrete role -// ARN against it. -func TestEvaluateAccess_PassRoleResourceARN(t *testing.T) { - policies := []handlers_iam.PolicyDocument{ - { - Version: "2012-10-17", - Statement: []handlers_iam.Statement{ - { - Effect: "Allow", - Action: handlers_iam.StringOrArr{"iam:PassRole"}, - Resource: handlers_iam.StringOrArr{"arn:aws:iam::*:role/app-*"}, - }, - }, - }, - } - - tests := []struct { - resource string - want Decision - }{ - {"arn:aws:iam::123456789012:role/app-foo", Allow}, - {"arn:aws:iam::999999999999:role/app-bar", Allow}, - {"arn:aws:iam::123456789012:role/admin-foo", Deny}, - {"arn:aws:iam::123456789012:user/app-foo", Deny}, - } - for _, tt := range tests { - got := EvaluateAccess("alice", "iam:PassRole", tt.resource, policies) - if got != tt.want { - t.Errorf("PassRole on %s: expected %v, got %v", tt.resource, tt.want, got) - } - } -} - -// TestEvaluateAccess_IAMInstanceProfileActionStrings pins the action strings -// for iam:PassRole + the four EC2 instance-profile association actions so a -// rename of any ec2Actions key or checkPolicyResource call site is caught at -// the policy layer. Strings are produced dynamically via policy.IAMAction. -func TestEvaluateAccess_IAMInstanceProfileActionStrings(t *testing.T) { +// TestEvaluate_IAMInstanceProfileActionStrings pins the action strings for +// iam:PassRole + the four EC2 instance-profile association actions so a rename of +// any ec2Actions key or checkPolicyResource call site is caught at the policy +// layer. Strings are produced dynamically via IAMAction. +func TestEvaluate_IAMInstanceProfileActionStrings(t *testing.T) { actions := []string{ "iam:PassRole", "ec2:AssociateIamInstanceProfile", @@ -324,7 +37,7 @@ func TestEvaluateAccess_IAMInstanceProfileActionStrings(t *testing.T) { doc("Allow", "*", "*"), } for _, a := range actions { - if got := EvaluateAccess("alice", a, "*", policies); got != Allow { + if got := iampolicy.Evaluate(a, "*", policies); got != iampolicy.Allow { t.Errorf("expected Allow for action %q under wildcard policy, got %v", a, got) } } @@ -336,18 +49,18 @@ func TestEvaluateAccess_IAMInstanceProfileActionStrings(t *testing.T) { scopedTests := []struct { action string resource string - want Decision + want iampolicy.Decision }{ - {"ec2:AssociateIamInstanceProfile", "*", Allow}, - {"ec2:DisassociateIamInstanceProfile", "*", Allow}, - {"ec2:ReplaceIamInstanceProfileAssociation", "*", Allow}, - {"ec2:DescribeIamInstanceProfileAssociations", "*", Allow}, - {"iam:PassRole", "arn:aws:iam::123456789012:role/app-foo", Allow}, - {"iam:PassRole", "arn:aws:iam::123456789012:user/app-foo", Deny}, - {"ec2:RunInstances", "*", Deny}, + {"ec2:AssociateIamInstanceProfile", "*", iampolicy.Allow}, + {"ec2:DisassociateIamInstanceProfile", "*", iampolicy.Allow}, + {"ec2:ReplaceIamInstanceProfileAssociation", "*", iampolicy.Allow}, + {"ec2:DescribeIamInstanceProfileAssociations", "*", iampolicy.Allow}, + {"iam:PassRole", "arn:aws:iam::123456789012:role/app-foo", iampolicy.Allow}, + {"iam:PassRole", "arn:aws:iam::123456789012:user/app-foo", iampolicy.Deny}, + {"ec2:RunInstances", "*", iampolicy.Deny}, } for _, tt := range scopedTests { - got := EvaluateAccess("alice", tt.action, tt.resource, scoped) + got := iampolicy.Evaluate(tt.action, tt.resource, scoped) if got != tt.want { t.Errorf("scoped policy, action=%s resource=%s: expected %v, got %v", tt.action, tt.resource, tt.want, got) @@ -355,12 +68,12 @@ func TestEvaluateAccess_IAMInstanceProfileActionStrings(t *testing.T) { } } -// TestEvaluateAccess_STSActionStrings pins the action strings emitted by the -// STS gateway dispatcher (gateway/sts.go stsActions + checkPolicy(r, "sts", -// action) call site). Locks in that every STS verb the dispatcher accepts is -// matchable by the evaluator under both wildcard and service-scoped policies, -// so a future rename of any stsActions key surfaces here. -func TestEvaluateAccess_STSActionStrings(t *testing.T) { +// TestEvaluate_STSActionStrings pins the action strings emitted by the STS +// gateway dispatcher (gateway/sts.go stsActions + checkPolicy(r, "sts", action) +// call site). Locks in that every STS verb the dispatcher accepts is matchable +// by the evaluator under both wildcard and service-scoped policies, so a future +// rename of any stsActions key surfaces here. +func TestEvaluate_STSActionStrings(t *testing.T) { actions := []string{ "sts:AssumeRole", "sts:GetCallerIdentity", @@ -370,17 +83,17 @@ func TestEvaluateAccess_STSActionStrings(t *testing.T) { scoped := []handlers_iam.PolicyDocument{doc("Allow", "sts:*", "*")} for _, a := range actions { - if got := EvaluateAccess("alice", a, "*", wildcard); got != Allow { + if got := iampolicy.Evaluate(a, "*", wildcard); got != iampolicy.Allow { t.Errorf("wildcard policy: expected Allow for %q, got %v", a, got) } - if got := EvaluateAccess("alice", a, "*", scoped); got != Allow { + if got := iampolicy.Evaluate(a, "*", scoped); got != iampolicy.Allow { t.Errorf("sts:* policy: expected Allow for %q, got %v", a, got) } } // Non-STS action must NOT match an sts:*-scoped policy — guards against a // pattern regression that would over-allow. - if got := EvaluateAccess("alice", "ec2:RunInstances", "*", scoped); got != Deny { + if got := iampolicy.Evaluate("ec2:RunInstances", "*", scoped); got != iampolicy.Deny { t.Errorf("sts:* policy: expected Deny for ec2:RunInstances, got %v", got) } } @@ -393,66 +106,3 @@ func TestIAMAction(t *testing.T) { t.Fatalf("IAMAction(ec2, RunInstances) = %q, want %q", got, "ec2:RunInstances") } } - -// --- Realistic scenario tests --- - -func TestEvaluateAccess_ReadOnlyUser(t *testing.T) { - // Simulate a read-only user: allow all Describe* actions, deny everything else. - policies := []handlers_iam.PolicyDocument{ - { - Version: "2012-10-17", - Statement: []handlers_iam.Statement{ - { - Effect: "Allow", - Action: handlers_iam.StringOrArr{"ec2:Describe*"}, - Resource: handlers_iam.StringOrArr{"*"}, - }, - }, - }, - } - - tests := []struct { - action string - want Decision - }{ - {"ec2:DescribeInstances", Allow}, - {"ec2:DescribeVolumes", Allow}, - {"ec2:DescribeVpcs", Allow}, - {"ec2:RunInstances", Deny}, - {"ec2:TerminateInstances", Deny}, - {"iam:CreateUser", Deny}, - } - - for _, tt := range tests { - got := EvaluateAccess("viewer", tt.action, "*", policies) - if got != tt.want { - t.Errorf("read-only user, action=%s: expected %v, got %v", tt.action, tt.want, got) - } - } -} - -func TestEvaluateAccess_AdminWithDenyTerminate(t *testing.T) { - // Admin that can do everything except terminate instances. - policies := []handlers_iam.PolicyDocument{ - doc("Allow", "*", "*"), - doc("Deny", "ec2:TerminateInstances", "*"), - } - - tests := []struct { - action string - want Decision - }{ - {"ec2:RunInstances", Allow}, - {"ec2:DescribeInstances", Allow}, - {"s3:GetObject", Allow}, - {"iam:CreateUser", Allow}, - {"ec2:TerminateInstances", Deny}, // explicit deny - } - - for _, tt := range tests { - got := EvaluateAccess("admin", tt.action, "*", policies) - if got != tt.want { - t.Errorf("admin-no-terminate, action=%s: expected %v, got %v", tt.action, tt.want, got) - } - } -} diff --git a/spinifex/handlers/iam/account_test.go b/spinifex/handlers/iam/account_test.go index a9b226d8f..1382446ad 100644 --- a/spinifex/handlers/iam/account_test.go +++ b/spinifex/handlers/iam/account_test.go @@ -350,7 +350,7 @@ func TestCrossAccount_LookupAccessKeyReturnsCorrectAccount(t *testing.T) { func TestSeedBootstrap_AccountScoped(t *testing.T) { svc := setupTestIAMService(t) - encryptedSecret, err := EncryptSecret("root-secret", svc.masterKey) + encryptedSecret, err := svc.key.EncryptBase64("root-secret") require.NoError(t, err) err = svc.SeedBootstrap(&BootstrapData{ diff --git a/spinifex/handlers/iam/crypto.go b/spinifex/handlers/iam/crypto.go index 849d4f520..360704edf 100644 --- a/spinifex/handlers/iam/crypto.go +++ b/spinifex/handlers/iam/crypto.go @@ -1,13 +1,12 @@ package handlers_iam import ( - "crypto/aes" - "crypto/cipher" "crypto/rand" - "encoding/base64" "encoding/json" "fmt" "os" + + "github.com/mulgadc/predastore/pkg/masterkey" ) const masterKeySize = 32 // AES-256 @@ -48,75 +47,24 @@ func SaveMasterKey(path string, key []byte) error { return nil } -// EncryptSecret encrypts a plaintext secret with AES-256-GCM. -// Returns base64(nonce + ciphertext + tag). +// EncryptSecret encrypts a plaintext secret with AES-256-GCM, returning +// base64(nonce + ciphertext + tag). It delegates to predastore's masterkey so +// the wire format stays byte-for-byte identical across services. func EncryptSecret(plaintext string, key []byte) (string, error) { - block, err := aes.NewCipher(key) + k, err := masterkey.New(key) if err != nil { - return "", fmt.Errorf("create cipher: %w", err) + return "", err } - - gcm, err := cipher.NewGCM(block) - if err != nil { - return "", fmt.Errorf("create GCM: %w", err) - } - - nonce := make([]byte, gcm.NonceSize()) - if _, err := rand.Read(nonce); err != nil { - return "", fmt.Errorf("generate nonce: %w", err) - } - - // Seal appends ciphertext+tag to nonce - ciphertext := gcm.Seal(nonce, nonce, []byte(plaintext), nil) - return base64.StdEncoding.EncodeToString(ciphertext), nil -} - -// Decrypter holds a pre-computed AES-256-GCM cipher for repeated decryption. -// Create once at startup with NewDecrypter and reuse across requests. -type Decrypter struct { - gcm cipher.AEAD -} - -// NewDecrypter creates a Decrypter with a pre-computed AES-GCM cipher from the given key. -func NewDecrypter(key []byte) (*Decrypter, error) { - block, err := aes.NewCipher(key) - if err != nil { - return nil, fmt.Errorf("create cipher: %w", err) - } - gcm, err := cipher.NewGCM(block) - if err != nil { - return nil, fmt.Errorf("create GCM: %w", err) - } - return &Decrypter{gcm: gcm}, nil -} - -// Decrypt decrypts a base64-encoded AES-256-GCM ciphertext using the pre-computed cipher. -func (d *Decrypter) Decrypt(ciphertext string) (string, error) { - data, err := base64.StdEncoding.DecodeString(ciphertext) - if err != nil { - return "", fmt.Errorf("base64 decode: %w", err) - } - - nonceSize := d.gcm.NonceSize() - if len(data) < nonceSize { - return "", fmt.Errorf("ciphertext too short") - } - - nonce, sealed := data[:nonceSize], data[nonceSize:] - plaintext, err := d.gcm.Open(nil, nonce, sealed, nil) - if err != nil { - return "", fmt.Errorf("decrypt: %w", err) - } - return string(plaintext), nil + return k.EncryptBase64(plaintext) } // DecryptSecret decrypts a base64-encoded AES-256-GCM ciphertext. func DecryptSecret(ciphertext string, key []byte) (string, error) { - d, err := NewDecrypter(key) + k, err := masterkey.New(key) if err != nil { return "", err } - return d.Decrypt(ciphertext) + return k.DecryptBase64(ciphertext) } // SaveBootstrapData writes bootstrap data as JSON to disk with 0600 permissions. diff --git a/spinifex/handlers/iam/service_impl.go b/spinifex/handlers/iam/service_impl.go index 9aaac94cf..47a1e50eb 100644 --- a/spinifex/handlers/iam/service_impl.go +++ b/spinifex/handlers/iam/service_impl.go @@ -14,6 +14,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/predastore/pkg/masterkey" "github.com/mulgadc/spinifex/spinifex/admin" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/migrate" @@ -85,8 +86,7 @@ type IAMServiceImpl struct { rolesBucket nats.KeyValue instanceProfilesBucket nats.KeyValue groupsBucket nats.KeyValue - masterKey []byte - decrypter *Decrypter + key *masterkey.Key // replicas is the JetStream replication factor for lazily-created per-account buckets. replicas int } @@ -171,9 +171,9 @@ func NewIAMServiceImpl(natsConn *nats.Conn, masterKey []byte, clusterSize int) ( return nil, fmt.Errorf("migrate %s: %w", KVBucketGroups, err) } - decrypter, err := NewDecrypter(masterKey) + key, err := masterkey.New(masterKey) if err != nil { - return nil, fmt.Errorf("init decrypter: %w", err) + return nil, fmt.Errorf("init master key: %w", err) } slog.Info("IAM service initialized", @@ -197,8 +197,7 @@ func NewIAMServiceImpl(natsConn *nats.Conn, masterKey []byte, clusterSize int) ( rolesBucket: rolesBucket, instanceProfilesBucket: instanceProfilesBucket, groupsBucket: groupsBucket, - masterKey: masterKey, - decrypter: decrypter, + key: key, replicas: replicas, }, nil } @@ -428,7 +427,7 @@ func (s *IAMServiceImpl) CreateAccessKey(accountID string, input *iam.CreateAcce return nil, fmt.Errorf("generate secret key: %w", err) } - encryptedSecret, err := EncryptSecret(secretAccessKey, s.masterKey) + encryptedSecret, err := s.key.EncryptBase64(secretAccessKey) if err != nil { return nil, fmt.Errorf("encrypt secret: %w", err) } @@ -625,9 +624,9 @@ func (s *IAMServiceImpl) LookupAccessKey(accessKeyID string) (*AccessKey, error) } // DecryptSecret decrypts a base64-encoded AES-256-GCM ciphertext using the -// pre-computed cipher initialized at startup. +// master key loaded at startup. func (s *IAMServiceImpl) DecryptSecret(ciphertext string) (string, error) { - return s.decrypter.Decrypt(ciphertext) + return s.key.DecryptBase64(ciphertext) } // SeedBootstrap seeds the system root user and optional admin account into NATS KV. diff --git a/spinifex/handlers/iam/service_impl_test.go b/spinifex/handlers/iam/service_impl_test.go index 06e3ee3d7..d7fc1c1e0 100644 --- a/spinifex/handlers/iam/service_impl_test.go +++ b/spinifex/handlers/iam/service_impl_test.go @@ -330,7 +330,7 @@ func TestCreateAccessKey_SecretIsDecryptable(t *testing.T) { ak, err := svc.LookupAccessKey(accessKeyID) require.NoError(t, err) - decrypted, err := DecryptSecret(ak.SecretAccessKey, svc.masterKey) + decrypted, err := svc.key.DecryptBase64(ak.SecretAccessKey) require.NoError(t, err) assert.Equal(t, plaintextSecret, decrypted) } @@ -649,7 +649,7 @@ func TestLookupAccessKey_InactiveKey(t *testing.T) { func TestSeedBootstrap(t *testing.T) { svc := setupTestIAMService(t) - encryptedSecret, err := EncryptSecret("test-secret-key", svc.masterKey) + encryptedSecret, err := svc.key.EncryptBase64("test-secret-key") require.NoError(t, err) err = svc.SeedBootstrap(&BootstrapData{ @@ -676,7 +676,7 @@ func TestSeedBootstrap(t *testing.T) { assert.Equal(t, "Active", ak.Status) // Verify secret is decryptable - decrypted, err := DecryptSecret(ak.SecretAccessKey, svc.masterKey) + decrypted, err := svc.key.DecryptBase64(ak.SecretAccessKey) require.NoError(t, err) assert.Equal(t, "test-secret-key", decrypted) @@ -691,7 +691,7 @@ func TestSeedBootstrap(t *testing.T) { func TestSeedBootstrap_Idempotent(t *testing.T) { svc := setupTestIAMService(t) - encryptedSecret, _ := EncryptSecret("test-secret", svc.masterKey) + encryptedSecret, _ := svc.key.EncryptBase64("test-secret") data := &BootstrapData{ AccessKeyID: "AKIAEXAMPLE123456789", EncryptedSecret: encryptedSecret, @@ -717,9 +717,9 @@ func TestSeedBootstrap_Idempotent(t *testing.T) { func TestSeedBootstrap_WithAdmin(t *testing.T) { svc := setupTestIAMService(t) - systemSecret, err := EncryptSecret("system-secret", svc.masterKey) + systemSecret, err := svc.key.EncryptBase64("system-secret") require.NoError(t, err) - adminSecret, err := EncryptSecret("admin-secret", svc.masterKey) + adminSecret, err := svc.key.EncryptBase64("admin-secret") require.NoError(t, err) err = svc.SeedBootstrap(&BootstrapData{ @@ -764,7 +764,7 @@ func TestSeedBootstrap_WithAdmin(t *testing.T) { assert.Equal(t, "admin", ak.UserName) assert.Equal(t, "000000000001", ak.AccountID) - decrypted, err := DecryptSecret(ak.SecretAccessKey, svc.masterKey) + decrypted, err := svc.key.DecryptBase64(ak.SecretAccessKey) require.NoError(t, err) assert.Equal(t, "admin-secret", decrypted) @@ -785,7 +785,7 @@ func TestSeedBootstrap_WithAdmin(t *testing.T) { func TestSeedBootstrap_AdminNil_BackwardCompat(t *testing.T) { svc := setupTestIAMService(t) - encryptedSecret, err := EncryptSecret("test-secret", svc.masterKey) + encryptedSecret, err := svc.key.EncryptBase64("test-secret") require.NoError(t, err) // No Admin field — backward compatibility with old bootstrap.json @@ -1595,14 +1595,18 @@ func TestSensitiveDataNotLogged_MasterKey(t *testing.T) { slog.SetDefault(slog.New(slog.DiscardHandler)) }) - svc := setupTestIAMService(t) + _, nc, _ := testutil.StartTestJetStream(t) + masterKey, err := GenerateMasterKey() + require.NoError(t, err) + _, err = NewIAMServiceImpl(nc, masterKey, 1) + require.NoError(t, err) logOutput := buf.String() - masterKeyHex := hex.EncodeToString(svc.masterKey) + masterKeyHex := hex.EncodeToString(masterKey) assert.NotContains(t, logOutput, masterKeyHex, "master key hex must not appear in log output") - assert.NotContains(t, logOutput, string(svc.masterKey), + assert.NotContains(t, logOutput, string(masterKey), "raw master key bytes must not appear in log output") } diff --git a/spinifex/handlers/iam/types.go b/spinifex/handlers/iam/types.go index bf3b1bbdc..8e450108e 100644 --- a/spinifex/handlers/iam/types.go +++ b/spinifex/handlers/iam/types.go @@ -1,6 +1,10 @@ package handlers_iam -import "encoding/json" +import ( + "encoding/json" + + "github.com/mulgadc/predastore/pkg/iampolicy" +) const ( AccessKeyStatusActive = "Active" @@ -9,8 +13,17 @@ const ( AccountStatusActive = "ACTIVE" AccountStatusSuspended = "SUSPENDED" - PolicyEffectAllow = "Allow" - PolicyEffectDeny = "Deny" + PolicyEffectAllow = iampolicy.EffectAllow + PolicyEffectDeny = iampolicy.EffectDeny +) + +// The IAM policy-document DTOs are owned by predastore's pkg/iampolicy so +// predastore and spinifex share one definition. These aliases keep every +// existing spinifex reference (and its JSON (un)marshalling) working unchanged. +type ( + PolicyDocument = iampolicy.PolicyDocument + Statement = iampolicy.Statement + StringOrArr = iampolicy.StringOrArr ) // User represents an IAM user stored in JetStream KV. @@ -73,20 +86,6 @@ type Policy struct { Tags []Tag `json:"tags"` } -// PolicyDocument is the parsed IAM policy JSON structure. -type PolicyDocument struct { - Version string `json:"Version"` - Statement []Statement `json:"Statement"` -} - -// Statement is a single statement within a policy document. -type Statement struct { - Sid string `json:"Sid,omitempty"` - Effect string `json:"Effect"` - Action StringOrArr `json:"Action"` - Resource StringOrArr `json:"Resource"` -} - // Role is an assumable IAM identity stored in JetStream KV. // AssumeRolePolicyDocument is stored opaque; parsed for shape validation only. type Role struct { @@ -138,33 +137,6 @@ type TrustStatement struct { Condition json.RawMessage `json:"Condition,omitempty"` } -// StringOrArr handles JSON fields that can be either a string or an array of strings. -type StringOrArr []string - -// UnmarshalJSON implements custom unmarshaling for string-or-array fields. -func (s *StringOrArr) UnmarshalJSON(data []byte) error { - var single string - if err := json.Unmarshal(data, &single); err == nil { - *s = []string{single} - return nil - } - - var arr []string - if err := json.Unmarshal(data, &arr); err != nil { - return err - } - *s = arr - return nil -} - -// MarshalJSON marshals as a string if single element, otherwise as an array. -func (s StringOrArr) MarshalJSON() ([]byte, error) { - if len(s) == 1 { - return json.Marshal(s[0]) - } - return json.Marshal([]string(s)) -} - // BootstrapData is the on-disk JSON file consumed on first gateway start // to seed the root IAM user into NATS KV. type BootstrapData struct { diff --git a/spinifex/handlers/iam/types_test.go b/spinifex/handlers/iam/types_test.go index 6e2b3dcb0..00c6625a6 100644 --- a/spinifex/handlers/iam/types_test.go +++ b/spinifex/handlers/iam/types_test.go @@ -23,11 +23,11 @@ func TestStringOrArr_UnmarshalArray(t *testing.T) { } func TestStringOrArr_UnmarshalNull(t *testing.T) { - // null JSON unmarshals as empty string via the string path → [""] + // A JSON null yields a nil slice (an inert field), not [""]. var s StringOrArr err := json.Unmarshal([]byte(`null`), &s) require.NoError(t, err) - assert.Equal(t, StringOrArr{""}, s) + assert.Nil(t, s) } func TestStringOrArr_UnmarshalEmptyArray(t *testing.T) {