Skip to content

Commit

Permalink
Check ARN for user ID strict check (kubernetes-sigs#660)
Browse files Browse the repository at this point in the history
  • Loading branch information
nckturner committed Dec 7, 2023
1 parent a8ee0d9 commit dbcce7f
Show file tree
Hide file tree
Showing 10 changed files with 201 additions and 52 deletions.
25 changes: 25 additions & 0 deletions pkg/config/mapper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package config

import (
"strings"

"sigs.k8s.io/aws-iam-authenticator/pkg/token"
)

// IdentityMapping converts the RoleMapping into a generic IdentityMapping object
func (m *RoleMapping) IdentityMapping(identity *token.Identity) *IdentityMapping {
return &IdentityMapping{
IdentityARN: strings.ToLower(identity.CanonicalARN),
Username: m.Username,
Groups: m.Groups,
}
}

// IdentityMapping converts the UserMapping into a generic IdentityMapping object
func (m *UserMapping) IdentityMapping(identity *token.Identity) *IdentityMapping {
return &IdentityMapping{
IdentityARN: strings.ToLower(identity.CanonicalARN),
Username: m.Username,
Groups: m.Groups,
}
}
9 changes: 9 additions & 0 deletions pkg/errutil/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package errutil

import (
"errors"
)

var ErrNotMapped = errors.New("Identity is not mapped")

var ErrIDAndARNMismatch = errors.New("ARN does not match User ID")
6 changes: 4 additions & 2 deletions pkg/mapper/configmap/mapper.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package configmap

import (
"sigs.k8s.io/aws-iam-authenticator/pkg/token"
"strings"

"sigs.k8s.io/aws-iam-authenticator/pkg/errutil"
"sigs.k8s.io/aws-iam-authenticator/pkg/token"

"sigs.k8s.io/aws-iam-authenticator/pkg/config"
"sigs.k8s.io/aws-iam-authenticator/pkg/mapper"
)
Expand Down Expand Up @@ -53,7 +55,7 @@ func (m *ConfigMapMapper) Map(identity *token.Identity) (*config.IdentityMapping
}, nil
}

return nil, mapper.ErrNotMapped
return nil, errutil.ErrNotMapped
}

func (m *ConfigMapMapper) IsAccountAllowed(accountID string) bool {
Expand Down
6 changes: 4 additions & 2 deletions pkg/mapper/crd/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package crd

import (
"fmt"
"sigs.k8s.io/aws-iam-authenticator/pkg/token"
"strings"
"time"

"sigs.k8s.io/aws-iam-authenticator/pkg/errutil"
"sigs.k8s.io/aws-iam-authenticator/pkg/token"

"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -114,7 +116,7 @@ func (m *CRDMapper) Map(identity *token.Identity) (*config.IdentityMapping, erro
}
}

return nil, mapper.ErrNotMapped
return nil, errutil.ErrNotMapped
}

func (m *CRDMapper) IsAccountAllowed(accountID string) bool {
Expand Down
20 changes: 7 additions & 13 deletions pkg/mapper/dynamicfile/dynamicfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ package dynamicfile

import (
"encoding/json"
"errors"
"fmt"
"strings"
"sync"

"github.com/sirupsen/logrus"
"sigs.k8s.io/aws-iam-authenticator/pkg/arn"
"sigs.k8s.io/aws-iam-authenticator/pkg/config"
"sigs.k8s.io/aws-iam-authenticator/pkg/errutil"
)

type DynamicFileMapStore struct {
Expand Down Expand Up @@ -81,27 +81,21 @@ func (ms *DynamicFileMapStore) saveMap(
}
}

// UserNotFound is the error returned when the user is not found in the config map.
var UserNotFound = errors.New("User not found in dynamic file")

// RoleNotFound is the error returned when the role is not found in the config map.
var RoleNotFound = errors.New("Role not found in dynamic file")

func (ms *DynamicFileMapStore) UserMapping(arn string) (config.UserMapping, error) {
func (ms *DynamicFileMapStore) UserMapping(key string) (config.UserMapping, error) {
ms.mutex.RLock()
defer ms.mutex.RUnlock()
if user, ok := ms.users[arn]; !ok {
return config.UserMapping{}, UserNotFound
if user, ok := ms.users[key]; !ok {
return config.UserMapping{}, errutil.ErrNotMapped
} else {
return user, nil
}
}

func (ms *DynamicFileMapStore) RoleMapping(arn string) (config.RoleMapping, error) {
func (ms *DynamicFileMapStore) RoleMapping(key string) (config.RoleMapping, error) {
ms.mutex.RLock()
defer ms.mutex.RUnlock()
if role, ok := ms.roles[arn]; !ok {
return config.RoleMapping{}, RoleNotFound
if role, ok := ms.roles[key]; !ok {
return config.RoleMapping{}, errutil.ErrNotMapped
} else {
return role, nil
}
Expand Down
133 changes: 121 additions & 12 deletions pkg/mapper/dynamicfile/dynamicfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,48 @@ import (
"testing"
"time"

"github.com/google/go-cmp/cmp"
"sigs.k8s.io/aws-iam-authenticator/pkg/config"
"sigs.k8s.io/aws-iam-authenticator/pkg/errutil"
"sigs.k8s.io/aws-iam-authenticator/pkg/fileutil"
"sigs.k8s.io/aws-iam-authenticator/pkg/token"
)

var (
testUser = config.UserMapping{UserARN: "arn:aws:iam::012345678912:user/matt", Username: "matlan", Groups: []string{"system:master", "dev"}}
testRole = config.RoleMapping{RoleARN: "arn:aws:iam::012345678912:role/computer", Username: "computer", Groups: []string{"system:nodes"}}
)

func makeStore() DynamicFileMapStore {
func makeStore(users map[string]config.UserMapping, roles map[string]config.RoleMapping, filename string, userIDStrict bool) DynamicFileMapStore {
ms := DynamicFileMapStore{
users: make(map[string]config.UserMapping),
roles: make(map[string]config.RoleMapping),
awsAccounts: make(map[string]interface{}),
filename: "test.txt",
users: users,
roles: roles,
awsAccounts: make(map[string]interface{}),
filename: filename,
userIDStrict: userIDStrict,
}
ms.users["arn:aws:iam::012345678912:user/matt"] = testUser
ms.roles["UserId001"] = testRole

ms.awsAccounts["123"] = nil
return ms
}

func makeDefaultStore() DynamicFileMapStore {
users := make(map[string]config.UserMapping)
roles := make(map[string]config.RoleMapping)
users["arn:aws:iam::012345678912:user/matt"] = testUser
roles["UserId001"] = testRole
return makeStore(users, roles, "test.txt", false)
}

func makeMapper(users map[string]config.UserMapping, roles map[string]config.RoleMapping, filename string, userIDStrict bool) *DynamicFileMapper {
store := makeStore(users, roles, filename, userIDStrict)
return &DynamicFileMapper{
DynamicFileMapStore: &store,
}
}

func TestUserMapping(t *testing.T) {
ms := makeStore()
ms := makeDefaultStore()
user, err := ms.UserMapping("arn:aws:iam::012345678912:user/matt")
if err != nil {
t.Errorf("Could not find user 'matt' in map")
Expand All @@ -39,7 +57,7 @@ func TestUserMapping(t *testing.T) {
}

user, err = ms.UserMapping("nic")
if err != UserNotFound {
if err != errutil.ErrNotMapped {
t.Errorf("UserNotFound error was not returned for user 'nic'")
}
if !reflect.DeepEqual(user, config.UserMapping{}) {
Expand All @@ -48,7 +66,7 @@ func TestUserMapping(t *testing.T) {
}

func TestRoleMapping(t *testing.T) {
ms := makeStore()
ms := makeDefaultStore()
role, err := ms.RoleMapping("UserId001")
if err != nil {
t.Errorf("Could not find user 'instance in map")
Expand All @@ -58,7 +76,7 @@ func TestRoleMapping(t *testing.T) {
}

role, err = ms.RoleMapping("borg")
if err != RoleNotFound {
if err != errutil.ErrNotMapped {
t.Errorf("RoleNotFound error was not returend for role 'borg'")
}
if !reflect.DeepEqual(role, config.RoleMapping{}) {
Expand All @@ -67,7 +85,7 @@ func TestRoleMapping(t *testing.T) {
}

func TestAWSAccount(t *testing.T) {
ms := makeStore()
ms := makeDefaultStore()
if !ms.AWSAccount("123") {
t.Errorf("Expected aws account '123' to be in accounts list: %v", ms.awsAccounts)
}
Expand Down Expand Up @@ -447,3 +465,94 @@ func TestCallBackForFileLoad(t *testing.T) {
}
}
}

func TestMap(t *testing.T) {

tests := []struct {
description string
identity *token.Identity
users map[string]config.UserMapping
expectedIDMapping *config.IdentityMapping
expectedError error
}{
{
description: "UserID strict: ARNs match.",
identity: &token.Identity{
ARN: "arn:aws:iam::012345678912:user/matt",
CanonicalARN: "arn:aws:iam::012345678912:user/matt",
UserID: "1234",
},
users: map[string]config.UserMapping{
"1234": {
UserARN: "arn:aws:iam::012345678912:user/matt",
UserId: "1234",
Username: "asdf",
Groups: []string{"asdf"},
},
},
expectedIDMapping: &config.IdentityMapping{
IdentityARN: "arn:aws:iam::012345678912:user/matt",
Username: "asdf",
Groups: []string{"asdf"},
},
expectedError: nil,
},
{
description: "UserID strict: ARNs do not match but UserIDs do.",
identity: &token.Identity{
ARN: "arn:aws:iam::012345678912:user/alice",
CanonicalARN: "arn:aws:iam::012345678912:user/alice",
UserID: "1234",
},
users: map[string]config.UserMapping{
"1234": {
UserARN: "arn:aws:iam::012345678912:user/bob",
UserId: "1234",
},
},
expectedIDMapping: nil,
expectedError: errutil.ErrIDAndARNMismatch,
},
{
description: "UserID strict: No ARN provided.",
identity: &token.Identity{
ARN: "arn:aws:iam::012345678912:user/matt",
CanonicalARN: "arn:aws:iam::012345678912:user/matt",
UserID: "1234",
},
users: map[string]config.UserMapping{
"1234": {
UserId: "1234",
Username: "asdf",
Groups: []string{"asdf"},
},
},
expectedIDMapping: &config.IdentityMapping{
IdentityARN: "arn:aws:iam::012345678912:user/matt",
Username: "asdf",
Groups: []string{"asdf"},
},
expectedError: nil,
},
}

for _, tc := range tests {
t.Run(tc.description, func(t *testing.T) {

mapper := makeMapper(tc.users, map[string]config.RoleMapping{}, "test.txt", true)
identityMapping, err := mapper.Map(tc.identity)

if tc.expectedError != nil {
if err == nil {
t.Errorf("expected error %v but didn't get one", tc.expectedError)
} else if err != tc.expectedError {
t.Errorf("expected error %v but got %v", tc.expectedError, err)
}
}

if diff := cmp.Diff(tc.expectedIDMapping, identityMapping); diff != "" {
t.Errorf("Result mismatch (-want +got):\n%s", diff)
}
})
}
}
41 changes: 25 additions & 16 deletions pkg/mapper/dynamicfile/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"strings"

"sigs.k8s.io/aws-iam-authenticator/pkg/config"
"sigs.k8s.io/aws-iam-authenticator/pkg/errutil"
"sigs.k8s.io/aws-iam-authenticator/pkg/fileutil"
"sigs.k8s.io/aws-iam-authenticator/pkg/mapper"
"sigs.k8s.io/aws-iam-authenticator/pkg/token"
Expand Down Expand Up @@ -37,31 +38,39 @@ func (m *DynamicFileMapper) Start(stopCh <-chan struct{}) error {

func (m *DynamicFileMapper) Map(identity *token.Identity) (*config.IdentityMapping, error) {
canonicalARN := strings.ToLower(identity.CanonicalARN)

key := canonicalARN
if m.userIDStrict {
key = identity.UserID
}

rm, err := m.RoleMapping(key)
// TODO: Check for non Role/UserNotFound errors
if err == nil {
return &config.IdentityMapping{
IdentityARN: canonicalARN,
Username: rm.Username,
Groups: rm.Groups,
}, nil
if roleMapping, err := m.RoleMapping(key); err == nil {
if err := m.match(identity, roleMapping.RoleARN, roleMapping.UserId); err != nil {
return nil, err
}
return roleMapping.IdentityMapping(identity), nil
}

um, err := m.UserMapping(key)
if err == nil {
return &config.IdentityMapping{
IdentityARN: canonicalARN,
Username: um.Username,
Groups: um.Groups,
}, nil
if userMapping, err := m.UserMapping(key); err == nil {
if err := m.match(identity, userMapping.UserARN, userMapping.UserId); err != nil {
return nil, err
}
return userMapping.IdentityMapping(identity), nil
}

return nil, mapper.ErrNotMapped
return nil, errutil.ErrNotMapped
}

func (m *DynamicFileMapper) match(token *token.Identity, mappedARN, mappedUserID string) error {
if m.userIDStrict {
// If ARN is provided, ARN must be validated along with UserID. This avoids having to
// support IAM user name/ARN changes. Without preventing this the mapping would look
// invalid but still work and auditing would be difficult/impossible.
if mappedARN != "" && token.ARN != mappedARN {
return errutil.ErrIDAndARNMismatch
}
}
return nil
}

func (m *DynamicFileMapper) IsAccountAllowed(accountID string) bool {
Expand Down
Loading

0 comments on commit dbcce7f

Please sign in to comment.