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.

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

Plugin attachment to a group of resources from different namespaces #5355

Closed
1 of 2 tasks
liyangau opened this issue Dec 18, 2023 · 5 comments · Fixed by #5965
Closed
1 of 2 tasks

Plugin attachment to a group of resources from different namespaces #5355

liyangau opened this issue Dec 18, 2023 · 5 comments · Fixed by #5965
Assignees
Labels
area/feature New feature or request priority/medium
Milestone

Comments

@liyangau
Copy link

liyangau commented Dec 18, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Problem Statement

Currently I am using kong gateway to create plugins that is scoped at BOTH service and consumer.

A deck state file snippet as below.

- consumer: test
  name: rate-limiting-advanced
  service: loopback-route-svc
- consumer: test
  name: rate-limiting-advanced
  service: echo-svc

I am trying to achieve the same with KIC but it seems it is not possible to do this with KongPlugin or KongClusterPlugin because KongConsumer is scoped Namespaced and the services are in different namespaces.

  names:
    categories:
      - kong-ingress-controller
    kind: KongConsumer
    listKind: KongConsumerList
    plural: kongconsumers
    shortNames:
      - kc
    singular: kongconsumer
  scope: Namespaced

Proposed Solution

No response

Additional information

My goal is to implement per-consumer, per-service rate limiting with the rate limiting plugin. This means each consumer would have dedicated rate limits for each service they access.

This can be achieved with gateway easily. This feature request aims to enhance KIC's applicability to support this gateway feature.

Acceptance Criteria

  • if is possible to define a plugin that is attached to a combination of resources (e.g. service and consumer) that live in different namespaces
@mflendrich mflendrich changed the title Feature to support referencing consumer object globally Plugin attachment to a group of resources from different namespaces Dec 18, 2023
@mflendrich mflendrich modified the milestones: KIC v3.1.x, KIC v3.2.x Dec 18, 2023
@randmonkey randmonkey added area/feature New feature or request priority/medium labels Apr 24, 2024
@rainest
Copy link
Contributor

rainest commented Apr 24, 2024

tl;dr and next steps

  • There is currently no way to achieve the requested behavior.
  • Implementing the requested behavior leaves a way to roughly approximate the old behavior.
  • The new behavior would, in addition to possibly breaking configurations that rely on existing behavior, introduce an attack vector for administrators to effectively strip protections from resources in namespaces they do not control.
  • A viable, if ugly PoC of new behavior is available in feat(plugins) allow cross-namespace KCP plugins #5913.
  • Releasing this probably requires some as-yet unknown safeguard against the malicious use.

Current behavior

The current behavior is from KongState.getPluginRelations(). Generated plugins collect attached entities in a map keyed by <annotated resource namespace>:<plugin name>. These are then fed into GetCombinations() by getPlugins() to generate multiple plugins when the same Kong(Cluster)Plugin is used on multiple resources of the same type.

Because the namespace is derived from the annotated resource, KongClusterPlugins attached to multiple resources in different namespaces will split into multiple generated plugins, one each for all of the resources in a namespace.

To implement the requested behavior we'd presumably need to strike the namespace from keys for KongClusterPlugins. This should still be compatible with GetCombinations() (functionally it's the same as having all resources in the same namespace). Refactoring this is not trivial though, because getPluginRelations() doesn't actually know whether the requested plugin will be from a namespaced or cluster-scoped resource; it only knows the resource name.

Breaking changes

Configuration relying on existing behavior

This constitutes a breaking change. Users that want the old split behavior could still get it by creating two separate KongClusterPlugins, at the cost of having to apply configuration updates to each separately. You could pull some shenanigans with configuration in Secrets, but that feature isn't really intended for that purpose.

Existing configurations that fit the criteria for this would see changes to when plugins are applied. In the original example, while the plugin would have previously been applied to any request sent by the consumer in the non-Service namespace, it will now only be applied to that consumer's requests if they go through the service also.

Similarly, in a case where the KongClusterPlugin was not also applied to a KongConsumer in the Service's namespace, the plugin would have applied to any request through that service, whereas after the change it would not apply to anonymous or other consumer's requests through the service.

Stateful plugins

I'd originally considered additional implications for stateful plugins (which are uncommon and do not have consistent means of managing state, so they're hard to discuss in aggregate), but on further review I think any implications for them are mostly irrelevant. They may matter when state keys are generated randomly but IIRC we don't really allow you to use random defaults due to their inherent inconsistency when used with KIC versus direct admin API access.

Security concerns

New attack vector

KongClusterPlugin itself already had the capability to modify resources in different namespaces, and control over it still requires cluster-scope permissions. This would not change that aspect of the security model.

This does (maybe) introduce a new problem involving attached resources, however: because attaching a KongClusterPlugin to another type of resource in a different namespace creates a Kong plugin that only applies to requests matching both, someone in namespace A can effectively strip plugins from resources in namespace B. For example, if adminX had attached a rate-limiting KongClusterPlugin to a Service in namespaceA, adminY, who has no permission to namespaceA, can attach the same KongClusterPlugin to a KongConsumer in namespaceB, effectively removing most of the existing protections from the Service.

I believe (but did not test) that this affects Ingresses also, even though a malicious Ingress would need to attach to a Service in its own namespace--AFAIK the plugin binding system on Kong's side doesn't care that a plugin is useless because it wants to attach to a route+service where the route does not actually point to the service. If this is wrong, auth plugins are (probably) safe because they cannot attach to consumers.

ReferenceGrant-able fields that allow other cross-namespace behavior may complicate matters. I'm not sure and didn't think about it much, since IMO the above breaks things more than enough when admins have not permitted any cross-namespace interaction.

Possible safeguards

The attack vector presumably necessitates opt-in configuration that explicitly permits specific other namespaces to bind other types of configuration to a plugin. This would make for clunkier UX and significantly complicate the implementation.

ReferenceGrant is the closest equivalent to this, though it is not fully mature. It furthermore operates at the GVK level and wouldn't necessarily be suitable for our use case, as we may want to limit bindings by plugin, not simply allow KongClusterPlugins in general. We may be able to abuse annotations on ReferenceGrants for this.

Test configuration

For reference, plugin-demo.yaml.txt generates result.json.txt. Applying the same KongClusterPlugin to a Service and KongConsumer in one namespace and a KongConsumer in another namespace results in two plugins, attached to the annotated resources in each namespace.

@rainest
Copy link
Contributor

rainest commented Apr 24, 2024

Unfortunately I don't think the namespace field is a viable workaround here.

Although rate-limiting-advanced instances sharing the same namespace effectively act as a single plugin instance (they share counters), you're still limited to a single instance per consumer. While you could create a same-namespace set of plugins on a consumer and service independently, you wouldn't be able to limit that consumer's access to any other service, which probably renders that approach useless for the original case.

@rainest
Copy link
Contributor

rainest commented Apr 25, 2024

I thought over some different options for providing this functionality. I'm leaning towards ReferenceGrant with KongPlugin because of its clearer implementation path and simpler security config.

@liyangau do you think that'd work for you? You'd have a way to create plugins bound to a service from one namespace and a consumer from another, but they'd be generated from KongPlugins instead of KongClusterPlugins.

Caveat for the two ReferenceGrant options is that we don't own that CRD. You'd need to install (at least part of) the GWAPI CRDs to use it, and managing installation for third-party CRDs is annoying. However, I don't think we can manage without it--we'd need to either make our own clone CRD or abuse labels as an alternative.

ReferenceGrant with annotations

We use ReferenceGrant and apply the same criteria we'd use for GWAPI objects: the target namespace must have a grant from some namespace+G(V)K to a G(V)K and optional name to allow binding. A "foonamespace:KongConsumer can reference Services" grant allows plugins to attach to service+consumer combos derived from Services in the local namespace and KongConsumers in foonamespace.

Because the relationship is symmetrical and there's no way to discern who initiated it, we probably actually need grants in both resources' namespace.

Because policy may vary by plugin, ReferenceGrants will need an additional label. We could either handle this by plugin type (key-auth, rate-limiting, etc.) or specific named KongClusterPlugin instances. I'd lean toward type, though I'm not particularly concerned with a cluster admin changing the type of a KCP and thus changing the effective grant meaning.

Implementation feels difficult on initial review of the involved functions, particularly if we need to support both existing (no ReferenceGrant means you get separate generated plugins for each resource) and new behavior.

ReferenceGrant with KongPlugin

We preserve the existing KongClusterPlugin behavior and add a new behavior: a ReferenceGrant from remote namespace resources to a KongPlugin allows those resources to specify konghq.com/plugins: "namespace:name" and use that KongPlugin in that namespace, and by extension can create multi-entity plugins using resources in the target namespace.

This introduces no breaking changes and doesn't have the bidirectionality issue. Same caveat about maybe needing a ReferenceGrant annotation applies.

This would not provide the ability to limit which types of resources we can create multi-entity plugins with. We could require separate grants (from both Service to KongPlugin and Service to KongConsumer) to make this explicit, but it'd probably complicate implementation and I'd recommend against it to start.

Implementation should be easy: you just stick the requested namespace in when running getPluginRelations() and update the lookup to check for the grant.

KongClusterPlugin Toggle

We add a boolean field to KongClusterPlugins. When true, those KongClusterPlugins can create multi-entity plugins across namespaces. When false, they follow the existing behavior.

This is not great for security, since there are no explicit permissions granted by the namespaces involved, and it applies to all namespaces equally. It's a little better than nothing.

If you don't want to allow this and your KongClusterPlugin has it enabled, you need to ask your cluster admin to create another where it's disabled. Namespace admins need to proactively review the KongClusterPlugins they use. It's effectively an opt out rather than opt in, and the risky setting can be enabled after some resource starts using a plugin.

Implementation is trivial. You do 3101d02 but have an if clause when building relations, to either use empty namespace (will cross-namespace bind) or the attached resource namespace (won't) when processing those plugins.

@rainest
Copy link
Contributor

rainest commented Apr 29, 2024

For @mheap's request, example-cross-plugin-grant.tar.gz includes example manifests and outputs for some different scenarios.

One of the scenarios (all resources in separate namespaces) would have produced no output originally and is otherwise mostly the same as the some resources in the same namesapace case--it just needs an additional grant that's like the first--so it's not shown separately.

The example with all resources in the same namespace doesn't change at all, so I've only included one output.

Modified output assumes the ReferenceGrants are in place. Without them, output should match current behavior, i.e. multi-entity plugins only for resources in the same namespace.

@mheap
Copy link
Member

mheap commented May 3, 2024

Thanks @rainest. The concrete examples really helped. I'd opt for the KongPlugin + ReferenceGrant option today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment