djpbessems/ContainerImage.Pinniped
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Impersonator checks cert addresses when endpoint config is a hostname

Browse Source 
Also update concierge_impersonation_proxy_test.go integration test
to use real TLS when calling the impersonator.

Signed-off-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Margo Crawford 2021-02-25 10:27:19 -08:00 committed by Ryan Richard
parent 8fc68a4b21
commit 9a8c80f20a
3 changed files with 161 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
internal/controller/impersonatorconfig/impersonator_config.go
View File

@ -359,10 +359,10 @@ func (c *impersonatorConfigController) deleteTLSCertificateWithWrongName(ctx con
// The certPEM is not valid.
return secret, nil // TODO what should we do?
}
parsed, _ := x509.ParseCertificate(block.Bytes)
actualCertFromSecret, _ := x509.ParseCertificate(block.Bytes)
// TODO handle err
desiredIPs, _, nameIsReady, err := c.findTLSCertificateName(config) // TODO check this for hostnames too, not just ips
desiredIPs, desiredHostnames, nameIsReady, err := c.findTLSCertificateName(config) // TODO check this for hostnames too, not just ips
//nolint:staticcheck // TODO remove this nolint when we fix the TODO below
if err != nil {
// TODO return err
@ -377,12 +377,24 @@ func (c *impersonatorConfigController) deleteTLSCertificateWithWrongName(ctx con
return nil, nil
}
actualIPs := parsed.IPAddresses
// TODO handle multiple IPs, and handle when there is no IP
if desiredIPs[0].Equal(actualIPs[0]) {
actualIPs := actualCertFromSecret.IPAddresses
actualHostnames := actualCertFromSecret.DNSNames
plog.Info("Checking TLS certificate names",
"desiredIPs", desiredIPs,
"desiredHostnames", desiredHostnames,
"actualIPs", actualIPs,
"actualHostnames", actualHostnames,
"secret", c.tlsSecretName,
"namespace", c.namespace)
// TODO handle multiple IPs
if len(desiredIPs) == len(actualIPs) && len(desiredIPs) == 1 && desiredIPs[0].Equal(actualIPs[0]) { //nolint:gocritic
// The cert matches the desired state, so we do not need to delete it.
return secret, nil
}
if len(desiredHostnames) == len(actualHostnames) && len(desiredHostnames) == 1 && desiredHostnames[0] == actualHostnames[0] { //nolint:gocritic
return secret, nil
}
err = c.ensureTLSSecretIsRemoved(ctx)
if err != nil {
@ -430,18 +442,28 @@ func (c *impersonatorConfigController) ensureTLSSecretIsCreatedAndLoaded(ctx con
}
func (c *impersonatorConfigController) findTLSCertificateName(config *impersonator.Config) ([]net.IP, []string, bool, error) {
if config.Endpoint != "" {
return c.findTLSCertificateNameFromEndpointConfig(config)
}
return c.findTLSCertificateNameFromLoadBalancer()
}
func (c *impersonatorConfigController) findTLSCertificateNameFromEndpointConfig(config *impersonator.Config) ([]net.IP, []string, bool, error) {
var ips []net.IP
var hostnames []string
if config.Endpoint != "" {
parsedAsIP := net.ParseIP(config.Endpoint)
if parsedAsIP != nil {
ips = []net.IP{parsedAsIP}
} else {
hostnames = []string{config.Endpoint}
}
// TODO Endpoint could be a hostname
// TODO Endpoint could have a port number in it, which we should parse out and ignore for this purpose
} else {
return ips, hostnames, true, nil
}
func (c *impersonatorConfigController) findTLSCertificateNameFromLoadBalancer() ([]net.IP, []string, bool, error) {
var ips []net.IP
var hostnames []string
lb, err := c.servicesInformer.Lister().Services(c.namespace).Get(c.generatedLoadBalancerServiceName)
notFound := k8serrors.IsNotFound(err)
if notFound {
@ -461,7 +483,6 @@ func (c *impersonatorConfigController) findTLSCertificateName(config *impersonat
// TODO get all IPs and all hostnames from ingresses and put them in the cert
ip := ingresses[0].IP
ips = []net.IP{net.ParseIP(ip)}
}
return ips, hostnames, true, nil
}
@ -487,6 +508,11 @@ func (c *impersonatorConfigController) createNewTLSSecret(ctx context.Context, c
v1.TLSCertKey: certPEM,
},
}
plog.Info("Creating TLS certificates for impersonation proxy",
"ips", ips,
"hostnames", hostnames,
"secret", c.tlsSecretName,
"namespace", c.namespace)
_, _ = c.k8sClient.CoreV1().Secrets(c.namespace).Create(ctx, newTLSSecret, metav1.CreateOptions{})
// TODO handle error on create
return newTLSSecret, nil
@ -505,6 +531,10 @@ func (c *impersonatorConfigController) loadTLSCertFromSecret(tlsSecret *v1.Secre
// TODO clear the secret if it was already set previously... c.setTLSCert(nil)
return fmt.Errorf("could not parse TLS cert PEM data from Secret: %w", err)
}
plog.Info("Loading TLS certificates for impersonation proxy",
"certPEM", certPEM,
"secret", c.tlsSecretName,
"namespace", c.namespace)
c.setTLSCert(&tlsCert)
return nil
}
@ -518,7 +548,7 @@ func (c *impersonatorConfigController) ensureTLSSecretIsRemoved(ctx context.Cont
return nil
}
plog.Info("Deleting TLS certificates for impersonation proxy",
"service", c.tlsSecretName,
"secret", c.tlsSecretName,
"namespace", c.namespace)
err = c.k8sClient.CoreV1().Secrets(c.namespace).Delete(ctx, c.tlsSecretName, metav1.DeleteOptions{})
if err != nil {

54
internal/controller/impersonatorconfig/impersonator_config_test.go
View File

@ -1017,10 +1017,62 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
r.Len(kubeAPIClient.Actions(), 2)
requireNodesListed(kubeAPIClient.Actions()[0])
ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1])
// Check that the server is running and that TLS certs that are being served are are for fakeIP.
// Check that the server is running and that TLS certs that are being served are are for fakeHostname.
requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + ":443": startedTLSListener.Addr().String()})
})
})
when("switching from ip address endpoint to hostname endpoint and back to ip address", func() {
const fakeHostname = "fake.example.com"
const fakeIP = "127.0.0.42"
var hostnameYAML = fmt.Sprintf("{mode: enabled, endpoint: %s}", fakeHostname)
var ipAddressYAML = fmt.Sprintf("{mode: enabled, endpoint: %s}", fakeIP)
it.Before(func() {
addImpersonatorConfigMapToTracker(configMapResourceName, ipAddressYAML)
addNodeWithRoleToTracker("worker")
})
it("regenerates the cert for the hostname, then regenerates it for the IP again", func() {
startInformersAndController()
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
r.Len(kubeAPIClient.Actions(), 2)
requireNodesListed(kubeAPIClient.Actions()[0])
ca := requireTLSSecretWasCreated(kubeAPIClient.Actions()[1])
// Check that the server is running and that TLS certs that are being served are are for fakeIP.
requireTLSServerIsRunning(ca, fakeIP, map[string]string{fakeIP + ":443": startedTLSListener.Addr().String()})
// update manually because the kubeAPIClient isn't connected to the informer in the tests
addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[1], kubeInformerClient, "1")
waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "1")
// Switch the endpoint config to a hostname.
updateImpersonatorConfigMapInTracker(configMapResourceName, hostnameYAML, "1")
waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1")
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
r.Len(kubeAPIClient.Actions(), 4)
requireTLSSecretDeleted(kubeAPIClient.Actions()[2])
ca = requireTLSSecretWasCreated(kubeAPIClient.Actions()[3])
// Check that the server is running and that TLS certs that are being served are are for fakeHostname.
requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + ":443": startedTLSListener.Addr().String()})
// update manually because the kubeAPIClient isn't connected to the informer in the tests
deleteTLSCertSecretFromTracker(tlsSecretName, kubeInformerClient)
addSecretFromCreateActionToTracker(kubeAPIClient.Actions()[3], kubeInformerClient, "2")
waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Secrets().Informer(), "2")
// Switch the endpoint config back to an IP.
updateImpersonatorConfigMapInTracker(configMapResourceName, ipAddressYAML, "2")
waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2")
r.NoError(controllerlib.TestSync(t, subject, *syncContext))
r.Len(kubeAPIClient.Actions(), 6)
requireTLSSecretDeleted(kubeAPIClient.Actions()[4])
ca = requireTLSSecretWasCreated(kubeAPIClient.Actions()[5])
// Check that the server is running and that TLS certs that are being served are are for fakeIP.
requireTLSServerIsRunning(ca, fakeIP, map[string]string{fakeIP + ":443": startedTLSListener.Addr().String()})
})
})
})
when("the configuration switches from enabled to disabled mode", func() {

40
test/integration/concierge_impersonation_proxy_test.go
View File

@ -29,8 +29,11 @@ import (
"go.pinniped.dev/test/library"
)
// TODO don't hard code "pinniped-concierge-" in this string. It should be constructed from the env app name.
const impersonationProxyConfigMapName = "pinniped-concierge-impersonation-proxy-config"
const (
// TODO don't hard code "pinniped-concierge-" in these strings. It should be constructed from the env app name.
impersonationProxyConfigMapName = "pinniped-concierge-impersonation-proxy-config"
impersonationProxyTLSSecretName = "pinniped-concierge-impersonation-proxy-tls-serving-certificate" //nolint:gosec // this is not a credential
)
func TestImpersonationProxy(t *testing.T) {
env := library.IntegrationEnv(t)
@ -49,12 +52,14 @@ func TestImpersonationProxy(t *testing.T) {
authenticator := library.CreateTestWebhookAuthenticator(ctx, t)
// The address of the ClusterIP service that points at the impersonation proxy's port
proxyServiceURL := fmt.Sprintf("https://%s-proxy.%s.svc.cluster.local", env.ConciergeAppName, env.ConciergeNamespace)
proxyServiceEndpoint := fmt.Sprintf("%s-proxy.%s.svc.cluster.local", env.ConciergeAppName, env.ConciergeNamespace)
proxyServiceURL := fmt.Sprintf("https://%s", proxyServiceEndpoint)
t.Logf("making kubeconfig that points to %q", proxyServiceURL)
getImpersonationProxyClient := func(caData []byte) *kubernetes.Clientset {
kubeconfig := &rest.Config{
Host: proxyServiceURL,
TLSClientConfig: rest.TLSClientConfig{Insecure: true},
TLSClientConfig: rest.TLSClientConfig{Insecure: caData == nil, CAData: caData},
BearerToken: impersonationtoken.Make(t, env.TestUser.Token, &authenticator, env.APIGroupSuffix),
Proxy: func(req *http.Request) (*url.URL, error) {
proxyURL, err := url.Parse(env.Proxy)
@ -66,6 +71,8 @@ func TestImpersonationProxy(t *testing.T) {
impersonationProxyClient, err := kubernetes.NewForConfig(kubeconfig)
require.NoError(t, err, "unexpected failure from kubernetes.NewForConfig()")
return impersonationProxyClient
}
oldConfigMap, err := adminClient.CoreV1().ConfigMaps(env.ConciergeNamespace).Get(ctx, impersonationProxyConfigMapName, metav1.GetOptions{})
if oldConfigMap.Data != nil {
@ -74,6 +81,8 @@ func TestImpersonationProxy(t *testing.T) {
}
serviceUnavailableError := fmt.Sprintf(`Get "%s/api/v1/namespaces": Service Unavailable`, proxyServiceURL)
insecureImpersonationProxyClient := getImpersonationProxyClient(nil)
if env.HasCapability(library.HasExternalLoadBalancerProvider) {
// Check that load balancer has been created
require.Eventually(t, func() bool {
@ -86,13 +95,13 @@ func TestImpersonationProxy(t *testing.T) {
}, 10*time.Second, 500*time.Millisecond)
// Check that we can't use the impersonation proxy to execute kubectl commands yet
_, err = impersonationProxyClient.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
_, err = insecureImpersonationProxyClient.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
require.EqualError(t, err, serviceUnavailableError)
// Create configuration to make the impersonation proxy turn on with a hard coded endpoint (without a LoadBalancer)
configMap := configMapForConfig(t, impersonator.Config{
Mode: impersonator.ModeEnabled,
Endpoint: proxyServiceURL,
Endpoint: proxyServiceEndpoint,
TLS: nil,
})
t.Logf("creating configmap %s", configMap.Name)
@ -117,6 +126,16 @@ func TestImpersonationProxy(t *testing.T) {
})
}
// Wait for ca data to be available at the secret location.
var caSecret *corev1.Secret
require.Eventually(t,
func() bool {
caSecret, err = adminClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName, metav1.GetOptions{})
return caSecret != nil && caSecret.Data["ca.crt"] != nil
}, 5*time.Minute, 250*time.Millisecond)
// Create an impersonation proxy client with that ca data.
impersonationProxyClient := getImpersonationProxyClient(caSecret.Data["ca.crt"])
t.Run(
"access as user",
library.AccessAsUserTest(ctx, env.TestUser.ExpectedUsername, impersonationProxyClient),
@ -296,9 +315,9 @@ func TestImpersonationProxy(t *testing.T) {
require.Eventually(t, func() bool {
// It's okay if this returns RBAC errors because this user has no role bindings.
// What we want to see is that the proxy eventually shuts down entirely.
_, err = impersonationProxyClient.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
_, err = insecureImpersonationProxyClient.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
return err.Error() == serviceUnavailableError
}, 10*time.Second, 500*time.Millisecond)
}, 20*time.Second, 500*time.Millisecond)
if env.HasCapability(library.HasExternalLoadBalancerProvider) {
// The load balancer should not exist after we disable the impersonation proxy.
@ -307,6 +326,11 @@ func TestImpersonationProxy(t *testing.T) {
return !hasLoadBalancerService(ctx, t, adminClient, env.ConciergeNamespace)
}, time.Minute, 500*time.Millisecond)
}
require.Eventually(t, func() bool {
caSecret, err = adminClient.CoreV1().Secrets(env.ConciergeNamespace).Get(ctx, impersonationProxyTLSSecretName, metav1.GetOptions{})
return k8serrors.IsNotFound(err)
}, 10*time.Second, 250*time.Millisecond)
}
func configMapForConfig(t *testing.T, config impersonator.Config) corev1.ConfigMap {