feat(kuma-cp) add service identity injection

[jewertow]

Signed-off-by: Jacek Ewertowski jacek.ewertowski1@gmail.com

Summary

This pull request is a continuation of #1941. This change introduces forwarding headers:

• X-Kuma-Forwarded-Client-Cert
• X-Kuma-Forwarded-Client-Service
• X-Kuma-Forwarded-Client-Zone

These headers contain data extracted from a downstream SSL connection details in the Lua filter.

Issues resolved

Fix #1256

Testing

I created a repository with a set of configurations that allow to quickly test this feature manually. To do this, follow instructions defined here.

[jpeach]

I haven't reviewed this, but a couple of notes

• x-headers are deprecated in RFC 6648 so I'd suggest using Kuma- for Kuma-specific things
• This looks like it forwards the SPIFFE identity URLS; is there already a relevant convention in the SPIFFE or Envoy communities?
• If envoy is already sending this info in the XFCC header, what's the use case for breaking this info out? Just making it easier to consume on the server side?

[jewertow]

• X-Kuma-* headers were suggested by @subnetmarco.
• What do you mean by a relevant convention? Is there anything wrong in forwarding SPIFFE?
• The use case is to make it easier to consume service name or spiffe. Otherwise server would have to parse XFCC header to retrieve these information. It could be out of the box.

[jpeach]

• X-Kuma-* headers were suggested by @subnetmarco.

• What do you mean by a relevant convention? Is there anything wrong in forwarding SPIFFE?

Is there already a convention for this that other Envoy-based proxies use? It's better to be consistent with the broader ecosystem if there's something to be consistent with :)

[jpeach]

• X-Kuma-* headers were suggested by @subnetmarco.

• What do you mean by a relevant convention? Is there anything wrong in forwarding SPIFFE?

Is there already a convention for this that other Envoy-based proxies use? It's better to be consistent with the broader ecosystem if there's something to be consistent with :)

For example, X-Forwarded-Client-Cert seems to be the common convention. Is there a good reason to be different?

[jakubdyszkiewicz]

This looks like good direction. Is there a chance to extract all uri sans from the cert (by not using x-forwarded-client-cert but some other structure)?

[jewertow]

• X-Kuma-* headers were suggested by @subnetmarco.
• What do you mean by a relevant convention? Is there anything wrong in forwarding SPIFFE?

Is there already a convention for this that other Envoy-based proxies use? It's better to be consistent with the broader ecosystem if there's something to be consistent with :)

I don't know, but I will investigate how is it solved in other projects.

[jewertow]

This looks like good direction. Is there a chance to extract all uri sans from the cert (by not using x-forwarded-client-cert but some other structure)?

Cool. I don't know, but I suspect it can be done somehow... I will dive deeper into capabilities of the Lua filters.

[jewertow]

I found a way to extract all URI SANs from a cert:
https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/lua_filter#urisanlocalcertificate

I tested this function and it returned a spiffe entry and all kuma tags. So it will be possible to forward X-Kuma-Forwarded-Client-Zone header.

[bartsmykla]

It looks good to me

[jewertow]

It looks good to me

I have to fix e2e tests. It seems that my changes affected existing tests.

[jpeach]

OK, let me summarize the slack discussion :)

1. This is about service identity, so the semantics we want are for these headers to contain information about the calling Kuma service, and for that information to be trustworthy.
2. Following from (1), we aren't forwarding these headers across multiple hops.
3. The current patch implements X-Kuma-Forwarded-Client-Cert with the SPIFFE ID as its value. However, since this header isn't intuitively named and we don't know of a use case that needs the SPIFFE ID, we propose to drop this header.

So I think that this means we have consensus for

• Naming these headers with the pattern Kuma-Client-XXXX
• Implementing Kuma-Client-Service and Kuma-Client-Zone with service identity semantics.

I'd like to see a very prominent comment in the implementation that the service identity semantics depends on Kuma configuring SANITIZE_SET for the Envoy XFCC header. This is potentially fragile and these headers ought to be trustworthy. At least the comment can remind maintainers in the future.

I'd also like to see explicit config to remove these headers in the virtual host headers_to_remove, and also in the route headers_to_remove. I think that this should guarantee that applications can't spoof these headers.

@jakubdyszkiewicz suggested that all the service tags should be reflected into header. While I think this is a good idea, I don't see an Envoy API to remove headers that match a prefix, so I'm not sure how we can make the guarantees we need if we do this. We could file a separate issue for this and tackle it with a custom Envoy filter though.

[jpeach]

I'd also like to see explicit config to remove these headers in the virtual host headers_to_remove, and also in the route headers_to_remove. I think that this should guarantee that applications can't spoof these headers.

I checked that Envoy always applies headers_to_remove before headers_to_add, but I'm not sure what the relative ordering of the Lua plugin would be.

[jakubdyszkiewicz]

Ok, lets start with just kuma-client-service and kuma-client-zone

[lahabana]

@jewertow is this something you are still planning on working on?

[jewertow]

I'm sorry that I didn't close it yet. Currently I have no time to work on this pull request.