Skip to content

Inject Tap service name into proxy PodSpec#3155

Merged
kleimkuhler merged 4 commits intomasterfrom
kleimkuhler/inject-tap-svc-name
Jul 29, 2019
Merged

Inject Tap service name into proxy PodSpec#3155
kleimkuhler merged 4 commits intomasterfrom
kleimkuhler/inject-tap-svc-name

Conversation

@kleimkuhler
Copy link
Contributor

Summary

In order for Pods' tap servers to start authorizing tap clients, the tap server
must be able to check client names against the expected tap service name.

This change injects the LINKERD2_PROXY_TAP_SVC_NAME into proxy PodSpecs.

Details

The tap servers on the individual resources being tapped should be able to
verify that the client is the tap service. The LINKERD2_PROXY_TAP_SVC_NAME is
now injected as an environment variable in the proxies so that it can check this
value against the client name of the TLS connection. Currently, this environment
will go unused. There is an open PR (linkerd2-proxy#290) to use this variable in
the proxy, but this is not dependent on that merging first.

Note: The variable is not injected if tap is disabled.

Testing

Test output has been updated with the newly injected environment variable.

Signed-off-by: Kevin Leimkuhler kleimkuhler@icloud.com

Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
@kleimkuhler
Copy link
Contributor Author

This is not dependent on #3154

@l5d-bot
Copy link
Collaborator

l5d-bot commented Jul 28, 2019

Integration test results for 47c5d47: fail 😕
Log output: https://gist.github.com/0a0fcdab5504918dfb5780454a8a65dc

Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
@l5d-bot
Copy link
Collaborator

l5d-bot commented Jul 28, 2019

Integration test results for 81516b9: success 🎉
Log output: https://gist.github.com/6332081fbd10aa7bf97bd2d91177aed9

@kleimkuhler kleimkuhler requested review from adleong and removed request for alpeb July 29, 2019 16:33
Copy link
Member

@adleong adleong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⭐ 💉 🐳

Copy link
Member

@siggy siggy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one nit otherwise looks good 🚢 👍

Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
@l5d-bot
Copy link
Collaborator

l5d-bot commented Jul 29, 2019

Integration test results for 0c73637: success 🎉
Log output: https://gist.github.com/9ee2a1f80de14b60d48f3cc663325721

@kleimkuhler
Copy link
Contributor Author

@siggy After our discussion, I added a comment to clarify that the order of setting tapSvcName is significant because it depends on _l5d_ns and _l5d_trustdomain.

I opened #3157 to note on the proxy work to take place to couple spawning the tap server only when identity has not been disabled. There will be a similar issue for control plane as well.

@kleimkuhler kleimkuhler merged commit 8d9cfbf into master Jul 29, 2019
@kleimkuhler kleimkuhler deleted the kleimkuhler/inject-tap-svc-name branch July 29, 2019 22:05
kleimkuhler added a commit to linkerd/linkerd2-proxy that referenced this pull request Aug 7, 2019
### Motivation

With linkerd/linkerd2#3155 merging, proxy containers will now always have the `LINKERD2_PROXY_TAP_SVC_NAME` variable in their environment. This variable can now be reliably used to check that the client names of incoming tap requests match this value.

If the values do not match, we will always return the gRPC Unauthenticated status code. If the values do match, we will accept the connection.

If there is no client identity, we will similarly return the gRPC Unauthenticated status code.

Closes linkerd/linkerd2#3157
Closes linkerd/linkerd2#3163

Signed-off by: Kevin Leimkuhler <kleimkuhler@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants