Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v15] [kube] fix greedy deny rule blocking namespace list when blocking other resources #44975

Merged
merged 1 commit into from
Aug 1, 2024
Merged

[v15] [kube] fix greedy deny rule blocking namespace list when blocking other resources #44975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/kube/proxy/forwarder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,14 +1109,14 @@ func matchKubernetesResource(resource types.KubernetesResource, allowed, denied
// utils.KubeResourceMatchesRegex checks if the resource.Kind is strictly equal
// to each entry and validates if the Name and Namespace fields matches the
// regex allowed by each entry.
result, err := utils.KubeResourceMatchesRegex(resource, denied)
result, err := utils.KubeResourceMatchesRegex(resource, denied, types.Deny)
if err != nil {
return false, trace.Wrap(err)
} else if result {
return false, nil
}

result, err = utils.KubeResourceMatchesRegex(resource, allowed)
result, err = utils.KubeResourceMatchesRegex(resource, allowed, types.Allow)
if err != nil {
return false, trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/services/role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2398,7 +2398,7 @@ func NewKubernetesResourceMatcher(resource types.KubernetesResource) *Kubernetes

// Match matches a Kubernetes Resource against provided role and condition.
func (m *KubernetesResourceMatcher) Match(role types.Role, condition types.RoleConditionType) (bool, error) {
result, err := utils.KubeResourceMatchesRegex(m.resource, role.GetKubeResources(condition))
result, err := utils.KubeResourceMatchesRegex(m.resource, role.GetKubeResources(condition), condition)

return result, trace.Wrap(err)
}
Expand Down
6 changes: 3 additions & 3 deletions lib/utils/replace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,14 @@ const (
// input is the resource we are checking for access.
// resources is a list of resources that the user has access to - collected from
// their roles that match the Kubernetes cluster where the resource is defined.
func KubeResourceMatchesRegex(input types.KubernetesResource, resources []types.KubernetesResource) (bool, error) {
// cond is the deny or allow condition of the role that we are evaluating.
func KubeResourceMatchesRegex(input types.KubernetesResource, resources []types.KubernetesResource, cond types.RoleConditionType) (bool, error) {
if len(input.Verbs) != 1 {
return false, trace.BadParameter("only one verb is supported, input: %v", input.Verbs)
}
// isClusterWideResource is true if the resource is cluster-wide, e.g. a
// namespace resource or a clusterrole.
isClusterWideResource := slices.Contains(types.KubernetesClusterWideResourceKinds, input.Kind)

verb := input.Verbs[0]
// If the user is list/read/watch a namespace, they should be able to see the
// namespace they have resources defined for.
Expand Down Expand Up @@ -191,7 +191,7 @@ func KubeResourceMatchesRegex(input types.KubernetesResource, resources []types.
if ok, err := MatchString(input.Namespace, resource.Name); err != nil || ok {
return ok, trace.Wrap(err)
}
case targetsReadOnlyNamespace && resource.Kind != types.KindKubeNamespace && resource.Namespace != "":
case targetsReadOnlyNamespace && cond == types.Allow && resource.Kind != types.KindKubeNamespace && resource.Namespace != "":
// If the user requests a read-only namespace get/list/watch, they should
// be able to see the list of namespaces they have resources defined in.
// This means that if the user has access to pods in the "foo" namespace,
Expand Down
116 changes: 115 additions & 1 deletion lib/utils/replace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
name string
input types.KubernetesResource
resources []types.KubernetesResource
action types.RoleConditionType
matches bool
assert require.ErrorAssertionFunc
}{
Expand All @@ -180,8 +181,102 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.Error,
action: types.Allow,
matches: false,
},
{
name: "list namespace matches resource",
input: types.KubernetesResource{
Kind: types.KindNamespace,
Verbs: []string{types.KubeVerbList},
},
resources: []types.KubernetesResource{
{
Kind: types.KindKubeSecret,
Namespace: "*",
Name: "*",
Verbs: []string{types.Wildcard},
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
name: "list namespace doesn't match denying secrets",
input: types.KubernetesResource{
Kind: types.KindNamespace,
Verbs: []string{types.KubeVerbList},
},
resources: []types.KubernetesResource{
{
Kind: types.KindKubeSecret,
Namespace: "*",
Name: "*",
Verbs: []string{types.Wildcard},
},
},
assert: require.NoError,
action: types.Deny,
matches: false,
},
{
name: "get namespace match denying everything",
input: types.KubernetesResource{
Kind: types.KindNamespace,
Name: "default",
Verbs: []string{types.KubeVerbGet},
},
resources: []types.KubernetesResource{
{
Kind: types.Wildcard,
Namespace: types.Wildcard,
Name: types.Wildcard,
Verbs: []string{types.Wildcard},
},
},
assert: require.NoError,
action: types.Deny,
matches: true,
},
{
name: "get namespace doesn't match denying secrets",
input: types.KubernetesResource{
Kind: types.KindNamespace,
Name: "default",
Verbs: []string{types.KubeVerbGet},
},
resources: []types.KubernetesResource{
{
Kind: types.KindKubeSecret,
Namespace: "*",
Name: "*",
Verbs: []string{types.Wildcard},
},
},
assert: require.NoError,
action: types.Deny,
matches: false,
},
{
name: "get secret matches denying secrets",
input: types.KubernetesResource{
Kind: types.KindKubeSecret,
Name: "default",
Verbs: []string{types.KubeVerbGet},
},
resources: []types.KubernetesResource{
{
Kind: types.KindKubeSecret,
Namespace: "*",
Name: "*",
Verbs: []string{types.Wildcard},
},
},
assert: require.NoError,
action: types.Deny,
matches: true,
},
{
name: "input matches single resource with wildcard verb",
input: types.KubernetesResource{
Expand All @@ -199,6 +294,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
Expand All @@ -218,6 +314,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
Expand All @@ -237,6 +334,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: false,
},
{
Expand All @@ -255,6 +353,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: false,
},
{
Expand Down Expand Up @@ -286,6 +385,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
Expand All @@ -305,6 +405,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
Expand All @@ -324,6 +425,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: false,
},
{
Expand All @@ -342,6 +444,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
Verbs: []string{types.Wildcard},
},
},
action: types.Allow,
assert: require.Error,
},
{
Expand All @@ -359,6 +462,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
Name: "podname",
},
},
action: types.Allow,
assert: require.NoError,
},
{
Expand All @@ -376,6 +480,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
Expand All @@ -394,6 +499,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
Expand All @@ -412,6 +518,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: false,
},
{
Expand All @@ -430,6 +537,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},

Expand All @@ -449,6 +557,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: false,
},

Expand All @@ -468,6 +577,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
Expand All @@ -486,6 +596,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: false,
},

Expand All @@ -505,6 +616,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: true,
},
{
Expand All @@ -523,6 +635,7 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: false,
},
{
Expand All @@ -547,12 +660,13 @@ func TestKubeResourceMatchesRegex(t *testing.T) {
},
},
assert: require.NoError,
action: types.Allow,
matches: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := KubeResourceMatchesRegex(tt.input, tt.resources)
got, err := KubeResourceMatchesRegex(tt.input, tt.resources, tt.action)
tt.assert(t, err)
require.Equal(t, tt.matches, got)
})
Expand Down
Loading