mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
fix: supply aws:userid on the gateway authorization path (#987)
This commit is contained in:
@@ -21,7 +21,7 @@ require (
|
||||
github.com/jackc/pgx/v5 v5.10.0
|
||||
github.com/klauspost/cpuid/v2 v2.4.0
|
||||
github.com/miekg/dns v1.1.73
|
||||
github.com/mulgadc/bluebottle v1.18.1-0.20260902235515-6023971275c0
|
||||
github.com/mulgadc/bluebottle v1.18.1-0.20260903003911-3d26c3a8def8
|
||||
github.com/mulgadc/northstar v1.18.0
|
||||
github.com/mulgadc/predastore v1.18.1-0.20260831064140-ff6a1dc42441
|
||||
github.com/mulgadc/viperblock v1.18.0
|
||||
|
||||
@@ -1327,10 +1327,8 @@ github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELU
|
||||
github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo=
|
||||
github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc=
|
||||
github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk=
|
||||
github.com/mulgadc/bluebottle v1.18.1-0.20260901051501-b00bc7828992 h1:ss5/vEr5STIZXBWAZZthlAS6q9i44aOMbaFjZL9ediY=
|
||||
github.com/mulgadc/bluebottle v1.18.1-0.20260901051501-b00bc7828992/go.mod h1:XBIMI+G2s3wuJNiEdYtlGDvCacw82OXEw50RIQhowYg=
|
||||
github.com/mulgadc/bluebottle v1.18.1-0.20260902235515-6023971275c0 h1:img64+hlZeHkjPbF/l9FPhiPNW+p0/9sqDpv8PopbpE=
|
||||
github.com/mulgadc/bluebottle v1.18.1-0.20260902235515-6023971275c0/go.mod h1:XBIMI+G2s3wuJNiEdYtlGDvCacw82OXEw50RIQhowYg=
|
||||
github.com/mulgadc/bluebottle v1.18.1-0.20260903003911-3d26c3a8def8 h1:eTOKjTcXmX6i9CSzrFhjtc/bpfy6bRizmIiPoVmVask=
|
||||
github.com/mulgadc/bluebottle v1.18.1-0.20260903003911-3d26c3a8def8/go.mod h1:XBIMI+G2s3wuJNiEdYtlGDvCacw82OXEw50RIQhowYg=
|
||||
github.com/mulgadc/northstar v1.18.0 h1:ksRjIKE6qtrw/ZoYaJh8JRBh+xdnuDw8FsmFLFCiXbg=
|
||||
github.com/mulgadc/northstar v1.18.0/go.mod h1:z6iol77R1vsIQyIBc1JQboUPhSJ6eG4bipzfbZZbLEM=
|
||||
github.com/mulgadc/predastore v1.18.1-0.20260831064140-ff6a1dc42441 h1:5t+iuKpnRWXkKOmh5lLBoziBaWD+H71edWO/O4ceMp8=
|
||||
|
||||
@@ -163,6 +163,15 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler {
|
||||
return
|
||||
}
|
||||
|
||||
// Resolved here, once, rather than per policy check: a fault must fail
|
||||
// the request rather than authorize it against a context missing the key.
|
||||
userID, err := gw.principalUserID(principal)
|
||||
if err != nil {
|
||||
gw.writeSigV4Error(w, r, err.Error())
|
||||
return
|
||||
}
|
||||
principal.userID = userID
|
||||
|
||||
// Parse rewound the body; re-read it for query-arg parsing, then rewind
|
||||
// again for the downstream handler.
|
||||
body, err := io.ReadAll(r.Body)
|
||||
@@ -191,6 +200,9 @@ func (gw *GatewayConfig) SigV4AuthMiddleware() func(http.Handler) http.Handler {
|
||||
if principal.underlyingRoleARN != "" {
|
||||
ctx = context.WithValue(ctx, ctxUnderlyingRoleARN, principal.underlyingRoleARN)
|
||||
}
|
||||
if principal.userID != "" {
|
||||
ctx = context.WithValue(ctx, ctxUserID, principal.userID)
|
||||
}
|
||||
|
||||
// Parse once; dispatchers reuse via ctxQueryArgs. On error the
|
||||
// dispatcher re-parses and returns MalformedQueryString.
|
||||
@@ -267,6 +279,9 @@ type principalContext struct {
|
||||
assumedRoleARN string
|
||||
assumedRoleID string
|
||||
underlyingRoleARN string
|
||||
// userID is aws:userid, resolved where the principal is so the value cannot
|
||||
// differ between two policy checks in the same request.
|
||||
userID string
|
||||
}
|
||||
|
||||
// resolveLongLivedAKID handles the AKIA path: IAM lookup, status check, secret decrypt.
|
||||
|
||||
@@ -18,6 +18,8 @@ import (
|
||||
|
||||
"github.com/aws/aws-sdk-go-v2/aws"
|
||||
v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
|
||||
awsv1 "github.com/aws/aws-sdk-go/aws"
|
||||
"github.com/aws/aws-sdk-go/service/iam"
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/mulgadc/bluebottle/pkg/sigv4"
|
||||
"github.com/mulgadc/spinifex/spinifex/awserrors"
|
||||
@@ -39,7 +41,7 @@ const (
|
||||
|
||||
// mockIAMService implements handlers_iam.IAMService for auth tests. It embeds
|
||||
// the interface so it satisfies the full contract; the SigV4 middleware drives
|
||||
// only LookupAccessKey and DecryptSecret, so those are the only methods wired.
|
||||
// LookupAccessKey, DecryptSecret and GetUser, so those are the methods wired.
|
||||
// Any other method nil-panics, surfacing an unexpected call.
|
||||
type mockIAMService struct {
|
||||
handlers_iam.IAMService
|
||||
@@ -51,6 +53,10 @@ type mockIAMService struct {
|
||||
// every account is ACTIVE, which is what most tests here are asserting
|
||||
// around rather than asserting about.
|
||||
accounts map[string]*handlers_iam.Account
|
||||
|
||||
// getUserFn overrides the synthetic user record below, for a test that cares
|
||||
// what aws:userid resolves to or that it fails to resolve at all.
|
||||
getUserFn func(accountID string, input *iam.GetUserInput) (*iam.GetUserOutput, error)
|
||||
}
|
||||
|
||||
func (m *mockIAMService) LookupAccessKey(accessKeyID string) (*handlers_iam.AccessKey, error) {
|
||||
@@ -72,6 +78,19 @@ func (m *mockIAMService) GetAccount(accountID string) (*handlers_iam.Account, er
|
||||
return &handlers_iam.Account{AccountID: accountID, Status: handlers_iam.AccountStatusActive}, nil
|
||||
}
|
||||
|
||||
// GetUser answers with a unique ID derived from the name, so aws:userid resolves
|
||||
// on the authorization path without every test defining a user record.
|
||||
func (m *mockIAMService) GetUser(accountID string, input *iam.GetUserInput) (*iam.GetUserOutput, error) {
|
||||
if m.getUserFn != nil {
|
||||
return m.getUserFn(accountID, input)
|
||||
}
|
||||
name := awsv1.StringValue(input.UserName)
|
||||
return &iam.GetUserOutput{User: &iam.User{
|
||||
UserName: awsv1.String(name),
|
||||
UserId: awsv1.String("AIDA" + strings.ToUpper(name)),
|
||||
}}, nil
|
||||
}
|
||||
|
||||
// testMasterKey is a fixed 32-byte key for deterministic tests.
|
||||
var testMasterKey []byte
|
||||
|
||||
@@ -143,6 +162,69 @@ func setupTestApp(accessKey, secretKey string) http.Handler {
|
||||
return r
|
||||
}
|
||||
|
||||
// userIDApp signs requests for alice on a real account, so the middleware takes
|
||||
// the aws:userid branch that a user principal reaches in production.
|
||||
func userIDApp(t *testing.T, svc *mockIAMService) (http.Handler, *string) {
|
||||
t.Helper()
|
||||
encryptedSecret, err := handlers_iam.EncryptSecret(testSecretKey, testMasterKey)
|
||||
require.NoError(t, err)
|
||||
svc.masterKey = testMasterKey
|
||||
svc.accessKeys = map[string]*handlers_iam.AccessKey{
|
||||
testAccessKey: {
|
||||
AccessKeyID: testAccessKey,
|
||||
SecretAccessKey: encryptedSecret,
|
||||
UserName: "alice",
|
||||
AccountID: "000000000001",
|
||||
Status: "Active",
|
||||
},
|
||||
}
|
||||
|
||||
gw := &GatewayConfig{DisableLogging: true, Region: testRegion, IAMService: svc}
|
||||
seen := new(string)
|
||||
r := chi.NewRouter()
|
||||
r.Use(gw.SigV4AuthMiddleware())
|
||||
r.HandleFunc("/*", func(w http.ResponseWriter, r *http.Request) {
|
||||
*seen = mustCtxString(r, ctxUserID)
|
||||
w.Write([]byte("OK"))
|
||||
})
|
||||
return r, seen
|
||||
}
|
||||
|
||||
// aws:userid is resolved once, by the middleware, so every policy check in the
|
||||
// request evaluates the same value rather than re-reading the user record.
|
||||
func TestSigV4Auth_StashesUserID(t *testing.T) {
|
||||
handler, seen := userIDApp(t, &mockIAMService{})
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Host = "localhost:9999"
|
||||
signTestRequest(t, req, nil, testAccessKey, testSecretKey)
|
||||
|
||||
resp := doRequest(handler, req)
|
||||
|
||||
require.Equal(t, http.StatusOK, resp.StatusCode)
|
||||
assert.Equal(t, "AIDAALICE", *seen)
|
||||
}
|
||||
|
||||
// An IAM fault must fail the request, not authorize it against a context missing
|
||||
// aws:userid: that silently narrows an Allow and widens a Deny to everything.
|
||||
func TestSigV4Auth_UserIDDependencyFaultFailsClosed(t *testing.T) {
|
||||
handler, _ := userIDApp(t, &mockIAMService{
|
||||
getUserFn: func(string, *iam.GetUserInput) (*iam.GetUserOutput, error) {
|
||||
return nil, errors.New("nats: no responders available for request")
|
||||
},
|
||||
})
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Host = "localhost:9999"
|
||||
signTestRequest(t, req, nil, testAccessKey, testSecretKey)
|
||||
|
||||
resp := doRequest(handler, req)
|
||||
|
||||
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
assert.Contains(t, string(body), awserrors.ErrorInternalError)
|
||||
}
|
||||
|
||||
func TestSigV4Auth_NoAuthorizationHeader(t *testing.T) {
|
||||
handler := setupTestApp(testAccessKey, testSecretKey)
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ import (
|
||||
var gatewayDoorKeys = []string{
|
||||
iampolicy.KeySecureTransport,
|
||||
iampolicy.KeyUsername,
|
||||
iampolicy.KeyUserID,
|
||||
iampolicy.KeyPrincipalAccount,
|
||||
iampolicy.KeySourceIP,
|
||||
}
|
||||
@@ -71,14 +72,21 @@ func emittedKeys(t *testing.T) map[string]string {
|
||||
accountID: "000000000001",
|
||||
principalType: principalTypeAssumedRole,
|
||||
assumedRoleARN: "arn:aws:sts::000000000001:assumed-role/SharedOps/session",
|
||||
assumedRoleID: "AROASHAREDOPS:session",
|
||||
},
|
||||
}
|
||||
|
||||
gw := &GatewayConfig{DisableLogging: true, IAMService: &mockIAMService{}}
|
||||
union := make(map[string]string)
|
||||
for name, principal := range principals {
|
||||
r := httptest.NewRequest(http.MethodPost, "/?prefix=home/", nil)
|
||||
r.TLS = &tls.ConnectionState{}
|
||||
r.RemoteAddr = "10.4.1.9:52344"
|
||||
// Resolved the way the middleware resolves it, so the gate cannot pass on
|
||||
// a value only the test supplies.
|
||||
userID, err := gw.principalUserID(principal)
|
||||
require.NoError(t, err)
|
||||
principal.userID = userID
|
||||
for key := range requestConditionKeys(r, principal) {
|
||||
union[key] = name
|
||||
}
|
||||
|
||||
@@ -7,10 +7,13 @@ package gateway
|
||||
import (
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/aws/aws-sdk-go/aws"
|
||||
"github.com/aws/aws-sdk-go/service/iam"
|
||||
"github.com/mulgadc/bluebottle/pkg/iampolicy"
|
||||
"github.com/mulgadc/spinifex/spinifex/awserrors"
|
||||
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
|
||||
@@ -26,6 +29,7 @@ func TestRequestConditionKeys_PopulatesAvailableKeys(t *testing.T) {
|
||||
identity: "alice",
|
||||
accountID: "000000000001",
|
||||
principalType: principalTypeUser,
|
||||
userID: "AIDAALICE",
|
||||
}
|
||||
|
||||
keys := requestConditionKeys(r, principal)
|
||||
@@ -34,10 +38,115 @@ func TestRequestConditionKeys_PopulatesAvailableKeys(t *testing.T) {
|
||||
iampolicy.KeySourceIP: "10.4.1.9",
|
||||
iampolicy.KeySecureTransport: "true",
|
||||
iampolicy.KeyUsername: "alice",
|
||||
iampolicy.KeyUserID: "AIDAALICE",
|
||||
iampolicy.KeyPrincipalAccount: "000000000001",
|
||||
}, keys)
|
||||
}
|
||||
|
||||
// A role session's aws:userid is the ID STS minted for it. Both halves come from
|
||||
// the resolved role, so unlike aws:username it can carry a decision.
|
||||
func TestRequestConditionKeys_UserIDForAssumedRole(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPost, "/", nil)
|
||||
|
||||
keys := requestConditionKeys(r, principalContext{
|
||||
identity: "session",
|
||||
accountID: "000000000001",
|
||||
principalType: principalTypeAssumedRole,
|
||||
assumedRoleID: "AROASHAREDOPS:session",
|
||||
userID: "AROASHAREDOPS:session",
|
||||
})
|
||||
|
||||
assert.Equal(t, "AROASHAREDOPS:session", keys[iampolicy.KeyUserID])
|
||||
assert.NotContains(t, keys, iampolicy.KeyUsername)
|
||||
}
|
||||
|
||||
// An ID the door could not resolve is omitted, not set empty: a policy naming it
|
||||
// then selects nothing on an Allow rather than selecting the empty-string path.
|
||||
func TestRequestConditionKeys_OmitsUnresolvableUserID(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPost, "/", nil)
|
||||
|
||||
keys := requestConditionKeys(r, principalContext{
|
||||
identity: "alice", accountID: "000000000001", principalType: principalTypeUser,
|
||||
})
|
||||
assert.NotContains(t, keys, iampolicy.KeyUserID)
|
||||
}
|
||||
|
||||
// principalUserID resolves the ID once per request, at the point the principal
|
||||
// is resolved. A user with no record and a session minted before the ID was
|
||||
// recorded both yield an omitted key, not a failed request.
|
||||
func TestPrincipalUserID_ResolvesAndOmits(t *testing.T) {
|
||||
gw := &GatewayConfig{DisableLogging: true, IAMService: &mockIAMService{}}
|
||||
|
||||
userID, err := gw.principalUserID(principalContext{
|
||||
identity: "alice", accountID: "000000000001", principalType: principalTypeUser,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "AIDAALICE", userID)
|
||||
|
||||
userID, err = gw.principalUserID(principalContext{
|
||||
identity: "session", accountID: "000000000001", principalType: principalTypeAssumedRole,
|
||||
assumedRoleID: "AROASHAREDOPS:session",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "AROASHAREDOPS:session", userID)
|
||||
|
||||
// Root's aws:userid is the account ID, matching what GetCallerIdentity
|
||||
// reports for the same principal.
|
||||
userID, err = gw.principalUserID(principalContext{
|
||||
identity: "root", accountID: "000000000000", principalType: principalTypeUser,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "000000000000", userID)
|
||||
|
||||
missing := &GatewayConfig{DisableLogging: true, IAMService: &mockIAMService{
|
||||
getUserFn: func(string, *iam.GetUserInput) (*iam.GetUserOutput, error) {
|
||||
return nil, errors.New(awserrors.ErrorIAMNoSuchEntity)
|
||||
},
|
||||
}}
|
||||
userID, err = missing.principalUserID(principalContext{
|
||||
identity: "alice", accountID: "000000000001", principalType: principalTypeUser,
|
||||
})
|
||||
require.NoError(t, err, "a deleted user omits the key rather than failing the request")
|
||||
assert.Empty(t, userID)
|
||||
|
||||
// A record predating the field resolves to no ID, which omits the key.
|
||||
legacy := &GatewayConfig{DisableLogging: true, IAMService: &mockIAMService{
|
||||
getUserFn: func(string, *iam.GetUserInput) (*iam.GetUserOutput, error) {
|
||||
return &iam.GetUserOutput{User: &iam.User{UserName: aws.String("alice")}}, nil
|
||||
},
|
||||
}}
|
||||
userID, err = legacy.principalUserID(principalContext{
|
||||
identity: "alice", accountID: "000000000001", principalType: principalTypeUser,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, userID)
|
||||
|
||||
// A session minted before assumed_role_id was recorded takes the same arm.
|
||||
userID, err = gw.principalUserID(principalContext{
|
||||
identity: "session", accountID: "000000000001", principalType: principalTypeAssumedRole,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, userID)
|
||||
}
|
||||
|
||||
// A dependency fault must not read as "the principal has no ID": the key would
|
||||
// be omitted, silently narrowing an Allow and widening a Deny to everything.
|
||||
func TestPrincipalUserID_DependencyFaultFailsClosed(t *testing.T) {
|
||||
faulty := &GatewayConfig{DisableLogging: true, IAMService: &mockIAMService{
|
||||
getUserFn: func(string, *iam.GetUserInput) (*iam.GetUserOutput, error) {
|
||||
return nil, errors.New("nats: no responders available for request")
|
||||
},
|
||||
}}
|
||||
|
||||
userID, err := faulty.principalUserID(principalContext{
|
||||
identity: "alice", accountID: "000000000001", principalType: principalTypeUser,
|
||||
})
|
||||
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, awserrors.ErrorInternalError, err.Error())
|
||||
assert.Empty(t, userID)
|
||||
}
|
||||
|
||||
// RoleSessionName is chosen by the caller of AssumeRole, so aws:username must
|
||||
// stay absent for a role session — otherwise anyone permitted to assume the
|
||||
// role satisfies an aws:username condition just by naming their session.
|
||||
|
||||
@@ -47,6 +47,9 @@ func (gw *GatewayConfig) ecrOperationAuthorization(next http.Handler) http.Handl
|
||||
return
|
||||
}
|
||||
|
||||
// Hoisted: the keys depend on the request and the principal, neither of
|
||||
// which changes across requirements.
|
||||
keys := requestConditionKeys(r, principal)
|
||||
for _, req := range op.Requirements {
|
||||
resource, err := ecrResourceARN(gw.Region, principal.accountID, req.Scope, op)
|
||||
if err != nil {
|
||||
@@ -54,8 +57,7 @@ func (gw *GatewayConfig) ecrOperationAuthorization(next http.Handler) http.Handl
|
||||
return
|
||||
}
|
||||
if err := gw.evaluatePrincipalPolicyResources(
|
||||
principal, policy.IAMAction("ecr", req.Action), []string{resource},
|
||||
requestConditionKeys(r, principal),
|
||||
principal, policy.IAMAction("ecr", req.Action), []string{resource}, keys,
|
||||
); err != nil {
|
||||
if err.Error() == awserrors.ErrorInternalError {
|
||||
// IAM/STS/NATS dependency failure: fail closed, never dispatch.
|
||||
|
||||
@@ -135,6 +135,7 @@ func (gw *GatewayConfig) resolveECRLongLivedPrincipal(claims *gateway_ecrauth.Cl
|
||||
identity: identity,
|
||||
accountID: ak.AccountID,
|
||||
principalType: principalTypeUser,
|
||||
userID: aws.StringValue(userOut.User.UserId),
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -195,6 +196,7 @@ func (gw *GatewayConfig) resolveECRSessionPrincipal(claims *gateway_ecrauth.Clai
|
||||
identity: identity,
|
||||
accountID: cred.AccountID,
|
||||
principalType: principalTypeUser,
|
||||
userID: aws.StringValue(userOut.User.UserId),
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -240,6 +242,7 @@ func (gw *GatewayConfig) resolveECRSessionPrincipal(claims *gateway_ecrauth.Clai
|
||||
assumedRoleARN: cred.AssumedRoleARN,
|
||||
assumedRoleID: cred.AssumedRoleID,
|
||||
underlyingRoleARN: cred.UnderlyingRoleARN,
|
||||
userID: cred.AssumedRoleID,
|
||||
}, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package gateway
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -137,6 +138,7 @@ func seedECRTestUser(iamSvc *ecrMockIAMService, accountID, userName, akid string
|
||||
}
|
||||
iamSvc.users[accountID+"|"+userName] = &iam.User{
|
||||
UserName: aws.String(userName),
|
||||
UserId: aws.String("AIDA" + strings.ToUpper(userName)),
|
||||
Arn: aws.String("arn:aws:iam::" + accountID + ":user/" + userName),
|
||||
}
|
||||
iamSvc.accounts[accountID] = &handlers_iam.Account{AccountID: accountID, Status: handlers_iam.AccountStatusActive}
|
||||
@@ -156,7 +158,8 @@ func TestResolveECRPrincipal_LongLivedUser(t *testing.T) {
|
||||
|
||||
got, err := gw.resolveECRPrincipal(claims)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, principalContext{identity: "dev", accountID: ecrPrincipalTestAccount, principalType: principalTypeUser}, got)
|
||||
assert.Equal(t, principalContext{identity: "dev", accountID: ecrPrincipalTestAccount,
|
||||
principalType: principalTypeUser, userID: "AIDADEV"}, got)
|
||||
}
|
||||
|
||||
func TestResolveECRPrincipal_GlobalRoot(t *testing.T) {
|
||||
@@ -275,6 +278,7 @@ func TestResolveECRPrincipal_LongLivedUser_Rejections(t *testing.T) {
|
||||
func seedECRSessionUser(iamSvc *ecrMockIAMService, stsSvc *ecrMockSTSService, accountID, userName, asid string, expiresAt time.Time) {
|
||||
iamSvc.users[accountID+"|"+userName] = &iam.User{
|
||||
UserName: aws.String(userName),
|
||||
UserId: aws.String("AIDA" + strings.ToUpper(userName)),
|
||||
Arn: aws.String("arn:aws:iam::" + accountID + ":user/" + userName),
|
||||
}
|
||||
iamSvc.accounts[accountID] = &handlers_iam.Account{AccountID: accountID, Status: handlers_iam.AccountStatusActive}
|
||||
@@ -302,7 +306,8 @@ func TestResolveECRPrincipal_SessionToken_User(t *testing.T) {
|
||||
|
||||
got, err := gw.resolveECRPrincipal(claims)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, principalContext{identity: "dev", accountID: ecrPrincipalTestAccount, principalType: principalTypeUser}, got)
|
||||
assert.Equal(t, principalContext{identity: "dev", accountID: ecrPrincipalTestAccount,
|
||||
principalType: principalTypeUser, userID: "AIDADEV"}, got)
|
||||
}
|
||||
|
||||
func TestResolveECRPrincipal_SessionToken_Expired(t *testing.T) {
|
||||
|
||||
@@ -30,6 +30,7 @@ import (
|
||||
gateway_ecr "github.com/mulgadc/spinifex/spinifex/gateway/ecr"
|
||||
gateway_ecrauth "github.com/mulgadc/spinifex/spinifex/gateway/ecrauth"
|
||||
"github.com/mulgadc/spinifex/spinifex/gateway/policy"
|
||||
gateway_sts "github.com/mulgadc/spinifex/spinifex/gateway/sts"
|
||||
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
|
||||
handlers_ochrevector "github.com/mulgadc/spinifex/spinifex/handlers/ochrevector"
|
||||
handlers_quota "github.com/mulgadc/spinifex/spinifex/handlers/quota"
|
||||
@@ -57,6 +58,9 @@ const (
|
||||
ctxPrincipalType contextKey = "sigv4.principalType"
|
||||
ctxAssumedRoleARN contextKey = "sigv4.assumedRoleARN"
|
||||
ctxAssumedRoleID contextKey = "sigv4.assumedRoleID"
|
||||
// ctxUserID carries aws:userid, resolved once by the SigV4 middleware so
|
||||
// every policy check in a request evaluates the same value.
|
||||
ctxUserID contextKey = "sigv4.userID"
|
||||
// ctxUnderlyingRoleARN carries the IAM role ARN backing an assumed-role session.
|
||||
// Policy enforcement resolves the role name from this, never from ctxIdentity
|
||||
// (attacker-influenced RoleSessionName).
|
||||
@@ -586,6 +590,7 @@ func (gw *GatewayConfig) checkPolicyResources(r *http.Request, service, action s
|
||||
assumedRoleARN: mustCtxString(r, ctxAssumedRoleARN),
|
||||
assumedRoleID: mustCtxString(r, ctxAssumedRoleID),
|
||||
underlyingRoleARN: mustCtxString(r, ctxUnderlyingRoleARN),
|
||||
userID: mustCtxString(r, ctxUserID),
|
||||
}
|
||||
return gw.evaluatePrincipalPolicyResources(principal, policy.IAMAction(service, action), resources,
|
||||
requestConditionKeys(r, principal))
|
||||
@@ -611,6 +616,11 @@ func requestConditionKeys(r *http.Request, principal principalContext) iampolicy
|
||||
if principal.accountID != "" {
|
||||
keys[iampolicy.KeyPrincipalAccount] = principal.accountID
|
||||
}
|
||||
// Resolved once where the principal was, so a per-check lookup cannot leave
|
||||
// one check in a request evaluating against a different context than the next.
|
||||
if principal.userID != "" {
|
||||
keys[iampolicy.KeyUserID] = principal.userID
|
||||
}
|
||||
// Derived from the request rather than read from the context: the OCI
|
||||
// registry chain never runs SigV4AuthMiddleware, so a context-carried
|
||||
// address would be absent there and every aws:SourceIp condition inert.
|
||||
@@ -620,6 +630,39 @@ func requestConditionKeys(r *http.Request, principal principalContext) iampolicy
|
||||
return keys
|
||||
}
|
||||
|
||||
// principalUserID resolves aws:userid: an IAM user's unique ID, the role ID and
|
||||
// session name STS minted for a role session, or the account ID for root. Both
|
||||
// halves of a session's ID come from the resolved role, so unlike aws:username
|
||||
// it is not caller-chosen.
|
||||
//
|
||||
// A principal with no ID on record returns empty and the door omits the key. A
|
||||
// dependency fault returns InternalError instead: authorizing against a context
|
||||
// missing the key silently narrows an Allow and widens a Deny.
|
||||
func (gw *GatewayConfig) principalUserID(principal principalContext) (string, error) {
|
||||
if principal.identity == "" || principal.accountID == "" {
|
||||
return "", nil
|
||||
}
|
||||
userID, err := gateway_sts.ResolveCallerUserID(principal.accountID, principal.principalType,
|
||||
principal.identity, principal.assumedRoleID, gw.IAMService)
|
||||
switch {
|
||||
case err == nil:
|
||||
if userID == "" {
|
||||
slog.Warn("aws:userid unavailable: principal record carries no user ID",
|
||||
"accountID", principal.accountID, "identity", principal.identity,
|
||||
"principalType", principal.principalType)
|
||||
}
|
||||
return userID, nil
|
||||
case strings.Contains(err.Error(), awserrors.ErrorIAMNoSuchEntity):
|
||||
slog.Warn("aws:userid unavailable: no such IAM user",
|
||||
"accountID", principal.accountID, "user", principal.identity)
|
||||
return "", nil
|
||||
default:
|
||||
slog.Error("aws:userid: IAM dependency fault, refusing to authorize on a degraded context",
|
||||
"accountID", principal.accountID, "identity", principal.identity, "err", err)
|
||||
return "", errors.New(awserrors.ErrorInternalError)
|
||||
}
|
||||
}
|
||||
|
||||
// mustCtxString reads a string context value, defaulting to "" for an absent
|
||||
// or wrong-typed key rather than panicking.
|
||||
func mustCtxString(r *http.Request, key contextKey) string {
|
||||
|
||||
@@ -35,14 +35,28 @@ func GetCallerIdentity(
|
||||
iamSvc handlers_iam.IAMService,
|
||||
stsSvc handlers_sts.STSService,
|
||||
) (*sts.GetCallerIdentityOutput, error) {
|
||||
userID, err := resolveCallerUserID(accountID, callerPrincipalType, identity, assumedRoleID, iamSvc)
|
||||
userID, err := ResolveCallerUserID(accountID, callerPrincipalType, identity, assumedRoleID, iamSvc)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if userID == "" {
|
||||
if callerPrincipalType == PrincipalTypeAssumedRole {
|
||||
// Session vanished between auth and dispatch (expired record); surface as
|
||||
// InvalidClientTokenId rather than a 500.
|
||||
slog.Warn("GetCallerIdentity: assumed-role session vanished between auth and dispatch")
|
||||
return nil, errors.New(awserrors.ErrorInvalidClientTokenId)
|
||||
}
|
||||
slog.Error("GetCallerIdentity: IAM GetUser returned empty UserId", "user", identity)
|
||||
return nil, errors.New(awserrors.ErrorInternalError)
|
||||
}
|
||||
return stsSvc.GetCallerIdentity(accountID, callerARN, userID, input)
|
||||
}
|
||||
|
||||
func resolveCallerUserID(
|
||||
// ResolveCallerUserID resolves the UserId of an authenticated principal, which
|
||||
// is also aws:userid at the authorization door. An error is a dependency fault;
|
||||
// a principal with no ID to report returns empty, which each caller reads on its
|
||||
// own terms.
|
||||
func ResolveCallerUserID(
|
||||
accountID, callerPrincipalType, identity, assumedRoleID string,
|
||||
iamSvc handlers_iam.IAMService,
|
||||
) (string, error) {
|
||||
@@ -55,28 +69,21 @@ func resolveCallerUserID(
|
||||
return accountID, nil
|
||||
}
|
||||
if iamSvc == nil {
|
||||
slog.Error("GetCallerIdentity: IAM service not initialized")
|
||||
slog.Error("ResolveCallerUserID: IAM service not initialized")
|
||||
return "", errors.New(awserrors.ErrorInternalError)
|
||||
}
|
||||
out, err := iamSvc.GetUser(accountID, &iam.GetUserInput{UserName: aws.String(identity)})
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if out == nil || out.User == nil || aws.StringValue(out.User.UserId) == "" {
|
||||
slog.Error("GetCallerIdentity: IAM GetUser returned empty UserId", "user", identity)
|
||||
return "", errors.New(awserrors.ErrorInternalError)
|
||||
if out == nil || out.User == nil {
|
||||
return "", nil
|
||||
}
|
||||
return aws.StringValue(out.User.UserId), nil
|
||||
case PrincipalTypeAssumedRole:
|
||||
if assumedRoleID == "" {
|
||||
// Session vanished between auth and dispatch (expired record); surface as
|
||||
// InvalidClientTokenId rather than a 500.
|
||||
slog.Warn("GetCallerIdentity: assumed-role session vanished between auth and dispatch")
|
||||
return "", errors.New(awserrors.ErrorInvalidClientTokenId)
|
||||
}
|
||||
return assumedRoleID, nil
|
||||
default:
|
||||
slog.Error("GetCallerIdentity: unknown principal type", "principalType", callerPrincipalType)
|
||||
slog.Error("ResolveCallerUserID: unknown principal type", "principalType", callerPrincipalType)
|
||||
return "", errors.New(awserrors.ErrorInternalError)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1871,14 +1871,14 @@ func TestValidatePolicyDocument_RejectsUnresolvableVariables(t *testing.T) {
|
||||
stmt string
|
||||
wantErr string
|
||||
}{
|
||||
{"userid in a resource", `"Action":"s3:*","Resource":"arn:aws:s3:::home/${aws:userid}/*"`,
|
||||
`references policy variable "aws:userid"`},
|
||||
{"userid in a StringEquals value",
|
||||
`"Action":"s3:*","Resource":"*","Condition":{"StringEquals":{"aws:username":"${aws:userid}"}}`,
|
||||
`references policy variable "aws:userid"`},
|
||||
{"userid in a StringLike value",
|
||||
`"Action":"s3:*","Resource":"*","Condition":{"StringLike":{"s3:prefix":"${aws:userid}/*"}}`,
|
||||
`references policy variable "aws:userid"`},
|
||||
{"MFA in a resource", `"Action":"s3:*","Resource":"arn:aws:s3:::home/${aws:MultiFactorAuthPresent}/*"`,
|
||||
`references policy variable "aws:MultiFactorAuthPresent"`},
|
||||
{"MFA in a StringEquals value",
|
||||
`"Action":"s3:*","Resource":"*","Condition":{"StringEquals":{"aws:username":"${aws:MultiFactorAuthPresent}"}}`,
|
||||
`references policy variable "aws:MultiFactorAuthPresent"`},
|
||||
{"MFA in a StringLike value",
|
||||
`"Action":"s3:*","Resource":"*","Condition":{"StringLike":{"s3:prefix":"${aws:MultiFactorAuthPresent}/*"}}`,
|
||||
`references policy variable "aws:MultiFactorAuthPresent"`},
|
||||
{"unknown key", `"Action":"s3:*","Resource":"arn:aws:s3:::home/${aws:bogus}/*"`,
|
||||
`references policy variable "aws:bogus"`},
|
||||
{"condition key that is not substitutable",
|
||||
@@ -1889,8 +1889,8 @@ func TestValidatePolicyDocument_RejectsUnresolvableVariables(t *testing.T) {
|
||||
{"empty reference", `"Action":"s3:*","Resource":"arn:aws:s3:::home/${}/*"`,
|
||||
"references policy variable"},
|
||||
{"second resource carries it",
|
||||
`"Action":"s3:*","Resource":["arn:aws:s3:::a/*","arn:aws:s3:::${aws:userid}/*"]`,
|
||||
`references policy variable "aws:userid"`},
|
||||
`"Action":"s3:*","Resource":["arn:aws:s3:::a/*","arn:aws:s3:::${aws:bogus}/*"]`,
|
||||
`references policy variable "aws:bogus"`},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
||||
@@ -112,14 +112,14 @@ func TestPutUserPolicy_UnresolvableVariableNamesTheCause(t *testing.T) {
|
||||
UserName: aws.String("variable-user"),
|
||||
PolicyName: aws.String("Inert"),
|
||||
PolicyDocument: aws.String(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow",
|
||||
"Action":"s3:GetObject","Resource":"arn:aws:s3:::home/${aws:userid}/*"}]}`),
|
||||
"Action":"s3:GetObject","Resource":"arn:aws:s3:::home/${aws:teamname}/*"}]}`),
|
||||
})
|
||||
require.Error(t, err)
|
||||
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
require.True(t, ok, "the error carries no registered AWS code, so it reaches the client as a 500")
|
||||
assert.Equal(t, awserrors.ErrorIAMMalformedPolicyDocument, code)
|
||||
assert.Contains(t, message, "aws:userid", "the message does not name the offending variable")
|
||||
assert.Contains(t, message, "aws:teamname", "the message does not name the offending variable")
|
||||
assert.Contains(t, message, "${aws:username}", "the message does not name the variables that do resolve")
|
||||
|
||||
// The rejection is also a rejection: nothing was stored.
|
||||
|
||||
Reference in New Issue
Block a user