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

Allow users to customize the CA of impersonation proxy to use with ingress #1104

Open
antoineozenne opened this issue Mar 31, 2022 · 4 comments
Labels
enhancement New feature or request priority/undecided Not yet prioritized

Comments

@antoineozenne
Copy link

Is your feature request related to a problem? Please describe.

Currently, we cannot use the impersonation proxy with an ingress controller and a custom DNS name. Here is my use case:

I have an Ingress with the host impersonation-proxy.example.com, and a wildcard certificate for *.example.com signed by a Certificate Authority. The Ingress is pointing to the service pinniped-concierge-proxy. Then here is my CredentialIssuer.

apiVersion: config.concierge.pinniped.dev/v1alpha1
kind: CredentialIssuer
metadata:
    name: using-custom-certificates
spec:
  impersonationProxy:
    # Enable the proxy
    mode: enabled
    # Provision a service
    service:
      type: ClusterIP
    # Advertise an external DNS name that points at a custom ingress
    externalEndpoint: impersonation-proxy.example.com

With this configuration, pinniped get kubeconfig complains that the certificate is signed by an unknown authority. I believe this is because the TLS endpoint in the concierge pod on port 8444 is served with an internal generated certificate.

Describe the solution you'd like

So I need something like:

apiVersion: config.concierge.pinniped.dev/v1alpha1
kind: CredentialIssuer
metadata:
    name: using-custom-certificates
spec:
  impersonationProxy:
    # Enable the proxy
    mode: enabled
    # Provision a service
    service:
      type: ClusterIP
    # Advertise an external DNS name that points at a custom ingress
    externalEndpoint: impersonation-proxy.example.com
    tls:
      # Point to some certificate/key to serve with, this would be my wildcard certificate for `*.example.com`.
      secretName: my-tls-cert

Or we could allow serving concierge in a non-TLS port, because the internal traffic between the ingress controller and the concierge could be non-TLS, with something like:

apiVersion: config.concierge.pinniped.dev/v1alpha1
kind: CredentialIssuer
metadata:
    name: using-custom-certificates
spec:
  impersonationProxy:
    # Enable the proxy
    mode: enabled
    # Provision a service
    service:
      type: ClusterIP
    tls:
      # Disable TLS
      enabled: false

This is already discussed in #617 (last use case) and #363 (comment).

@anjaltelang
Copy link
Contributor

anjaltelang commented Apr 6, 2022

Hi @antoineozenne, Thank you for submitting this issue. The maintainer team has other feature work that we're prioritizing right now, so we're not likely to pick this up soon. We'll let you know if that changes, but in the meantime if you're interested in making a PR, we'd be happy to review it. Here's how to contribute to the project https://github.com/vmware-tanzu/pinniped/blob/main/CONTRIBUTING.md. Do reach out to us on slack in the Kubernetes/Pinniped channel incase you have any questions.

@pinniped-ci-bot pinniped-ci-bot added enhancement New feature or request priority/undecided Not yet prioritized labels Apr 11, 2022
@enj
Copy link
Contributor

enj commented Apr 11, 2022

In case anyone decides to pick this up:

The impersonation proxy uses client certificates for authentication, thus any form TLS termination is incompatible with it (i.e. if using Kube ingress, it must support TLS passthrough). This of course disallows any non-TLS based approach.

We would need two new config knobs in the credential issuer: a required (when ImpersonationProxyTLS is not nil) secret with a serving certificate and an optional CA bundle to use for TLS verification - this will later get embedded in the generated kubeconfig. We may want to perform validations to help the admin know if they have misconfigured the serving certificate (i.e. invalid SAN).

diff --git a/apis/concierge/config/v1alpha1/types_credentialissuer.go.tmpl b/apis/concierge/config/v1alpha1/types_credentialissuer.go.tmpl
index d1cb160b..51a4692f 100644
--- a/apis/concierge/config/v1alpha1/types_credentialissuer.go.tmpl
+++ b/apis/concierge/config/v1alpha1/types_credentialissuer.go.tmpl
@@ -5,6 +5,8 @@ package v1alpha1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"go.pinniped.dev/GENERATED_PKG/apis/concierge/authentication/v1alpha1"
 )
 
 // StrategyType enumerates a type of "strategy" used to implement credential access on a cluster.
@@ -100,6 +102,14 @@ type ImpersonationProxySpec struct {
 	//
 	// +optional
 	ExternalEndpoint string `json:"externalEndpoint,omitempty"`
+
+	TLS *ImpersonationProxyTLS `json:"tls,omitempty"`
+}
+
+type ImpersonationProxyTLS struct {
+	v1alpha1.TLSSpec `json:",inline"`
+
+	SecretName string `json:"secretName"`
 }
 
 // ImpersonationProxyServiceSpec describes how the Concierge should provision a Service to expose the impersonation proxy.

@antoineozenne
Copy link
Author

Are you saying that the solution of exposing the proxy on a non-TLS port is not an option? It may be simpler, even if it is not the initial request.

@enj
Copy link
Contributor

enj commented Apr 11, 2022

Are you saying that the solution of exposing the proxy on a non-TLS port is not an option? It may be simpler, even if it is not the initial request.

Correct, this is not an option as client certificates are used for user authentication, i.e. mTLS.

The mTLS requirement aside, all existing non-TLS configuration for the supervisor is being deprecated and removed, i.e. #1094. I do not see us adding such configuration for the concierge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority/undecided Not yet prioritized
Projects
Status: No status
Development

No branches or pull requests

4 participants