From 360fbc0fa52d82721a7fd5433cebc7a8423632fa Mon Sep 17 00:00:00 2001 From: Zahari Dichev Date: Fri, 6 Aug 2021 13:26:39 +0300 Subject: [PATCH] Extract CP roots from config map (#25) When running with the latest Linkerd edge bloud agent is sending a certificate message with the Roots being empty. The reason for that is that in the bcloud agent, we look for the `EnvVar.Value` field rather than the `EnvVar.ValueFrom`. In https://github.com/linkerd/linkerd2/pull/6455 the proxy manifest was changed to load the trust anchors from a config map. This causes the described above problem. This can be fixed by making the agent consider this value when inspecting the env vars. It is important to make this backwards compatible - check the `Value` field and only if not present, check the `ValueFrom` one. This might require RBAC changes. Note to reviewers: In order to test, run this agent with the latest stable to ensure the change is backwards compatible. After that you can run it with the latest edge to convince yourself that the problem has been fixed. In both cases you need to make sure that the certificates in the control plane page are populated. Signed-off-by: Zahari Dichev --- agent/pkg/handler/linkerd_info.go | 7 +- agent/pkg/k8s/certificates.go | 33 ++++++++-- agent/pkg/k8s/certificates_test.go | 100 ++++++++++++++++++++++++++++- 3 files changed, 130 insertions(+), 10 deletions(-) diff --git a/agent/pkg/handler/linkerd_info.go b/agent/pkg/handler/linkerd_info.go index a2ef62b..4874d8a 100644 --- a/agent/pkg/handler/linkerd_info.go +++ b/agent/pkg/handler/linkerd_info.go @@ -1,6 +1,7 @@ package handler import ( + "context" "time" "github.com/buoyantio/linkerd-buoyant/agent/pkg/api" @@ -43,7 +44,7 @@ func (h *LinkerdInfo) Start() { for { select { case <-ticker.C: - h.handleCertsInfo() + h.handleCertsInfo(context.Background()) case <-h.stopCh: return } @@ -56,8 +57,8 @@ func (h *LinkerdInfo) Stop() { close(h.stopCh) } -func (h *LinkerdInfo) handleCertsInfo() { - certs, err := h.k8s.GetControlPlaneCerts() +func (h *LinkerdInfo) handleCertsInfo(ctx context.Context) { + certs, err := h.k8s.GetControlPlaneCerts(ctx) if err != nil { h.log.Errorf("error getting control plane certs: %s", err) return diff --git a/agent/pkg/k8s/certificates.go b/agent/pkg/k8s/certificates.go index fc31136..8de6947 100644 --- a/agent/pkg/k8s/certificates.go +++ b/agent/pkg/k8s/certificates.go @@ -1,6 +1,7 @@ package k8s import ( + "context" "crypto/tls" "fmt" "net" @@ -11,6 +12,7 @@ import ( l5dk8s "github.com/linkerd/linkerd2/pkg/k8s" ldTls "github.com/linkerd/linkerd2/pkg/tls" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" ) @@ -18,9 +20,11 @@ const ( identityComponentName = "identity" linkerdNsEnvVarName = "_l5d_ns" linkerdTrustDomainEnvVarName = "_l5d_trustdomain" + trustRootsConfigMapName = "linkerd-identity-trust-roots" + trustRootsConfigMapKeyName = "ca-bundle.crt" ) -func (c *Client) GetControlPlaneCerts() (*pb.ControlPlaneCerts, error) { +func (c *Client) GetControlPlaneCerts(ctx context.Context) (*pb.ControlPlaneCerts, error) { identityPod, err := c.getControlPlaneComponentPod(identityComponentName) if err != nil { return nil, err @@ -31,7 +35,7 @@ func (c *Client) GetControlPlaneCerts() (*pb.ControlPlaneCerts, error) { return nil, err } - rootCerts, err := extractRootsCerts(container) + rootCerts, err := c.extractRootsCerts(ctx, container, identityPod.Namespace) if err != nil { return nil, err } @@ -95,15 +99,36 @@ func getServerName(podsa string, podns string, container *v1.Container) (string, return fmt.Sprintf("%s.%s.serviceaccount.identity.%s.%s", podsa, podns, l5dns, l5dtrustdomain), nil } -func extractRootsCerts(container *v1.Container) ([]*pb.CertData, error) { +func (c *Client) extractRootsCerts(ctx context.Context, container *v1.Container, namespace string) ([]*pb.CertData, error) { for _, ev := range container.Env { if ev.Name != identity.EnvTrustAnchors { continue } - certificates, err := ldTls.DecodePEMCertificates(ev.Value) + + rootsValue := ev.Value + if rootsValue == "" { + // in this case we need to check for a reference to a config map + if ev.ValueFrom == nil || ev.ValueFrom.ConfigMapKeyRef == nil { + return nil, fmt.Errorf("neither a Value nor a ConfigMapKeyRef for the %s env var are present on proxy container [%s]", identity.EnvTrustAnchors, container.Name) + } + cmName := ev.ValueFrom.ConfigMapKeyRef.Name + cm, err := c.k8sClient.CoreV1().ConfigMaps(namespace).Get(ctx, cmName, metav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("cannot obtain config map %s/%s", namespace, cmName) + } + + var ok bool + rootsValue, ok = cm.Data[trustRootsConfigMapKeyName] + if !ok { + return nil, fmt.Errorf("config map %s/%s does not have %s key", namespace, cmName, trustRootsConfigMapKeyName) + } + } + + certificates, err := ldTls.DecodePEMCertificates(rootsValue) if err != nil { return nil, err } + certsData := make([]*pb.CertData, len(certificates)) for i, crt := range certificates { encoded := ldTls.EncodeCertificatesPEM(crt) diff --git a/agent/pkg/k8s/certificates_test.go b/agent/pkg/k8s/certificates_test.go index 0712db1..c8c2206 100644 --- a/agent/pkg/k8s/certificates_test.go +++ b/agent/pkg/k8s/certificates_test.go @@ -1,6 +1,7 @@ package k8s import ( + "context" "crypto/x509" "errors" "fmt" @@ -317,11 +318,12 @@ AiAtuoI5XuCtrGVRzSmRTl2ra28aV9MyTU7d5qnTAFHKSgIgRKCvluOSgA5O21p5 fixtures := []*struct { testName string container *v1.Container + crtConfigMap *v1.ConfigMap expectedCerts string expectedErr error }{ { - "gets correct cert", + "gets correct cert from env value", &v1.Container{ Name: l5dk8s.ProxyContainerName, Env: []v1.EnvVar{ @@ -331,15 +333,99 @@ AiAtuoI5XuCtrGVRzSmRTl2ra28aV9MyTU7d5qnTAFHKSgIgRKCvluOSgA5O21p5 }, }, }, + nil, + expectedRoots, + nil, + }, + { + "gets correct cert from config map", + &v1.Container{ + Name: l5dk8s.ProxyContainerName, + Env: []v1.EnvVar{ + { + Name: identity.EnvTrustAnchors, + ValueFrom: &v1.EnvVarSource{ + ConfigMapKeyRef: &v1.ConfigMapKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: trustRootsConfigMapName, + }, + Key: trustRootsConfigMapKeyName, + }, + }, + }, + }, + }, + &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: trustRootsConfigMapName, + Namespace: "linkerd", + }, + Data: map[string]string{ + trustRootsConfigMapKeyName: expectedRoots, + }, + }, expectedRoots, nil, }, { - "no roots", + "errors when config map not present", + &v1.Container{ + Name: l5dk8s.ProxyContainerName, + Env: []v1.EnvVar{ + { + Name: identity.EnvTrustAnchors, + ValueFrom: &v1.EnvVarSource{ + ConfigMapKeyRef: &v1.ConfigMapKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: trustRootsConfigMapName, + }, + Key: trustRootsConfigMapKeyName, + }, + }, + }, + }, + }, + nil, + expectedRoots, + fmt.Errorf("cannot obtain config map linkerd/%s", trustRootsConfigMapName), + }, + { + "errors when config map key not present", + &v1.Container{ + Name: l5dk8s.ProxyContainerName, + Env: []v1.EnvVar{ + { + Name: identity.EnvTrustAnchors, + ValueFrom: &v1.EnvVarSource{ + ConfigMapKeyRef: &v1.ConfigMapKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: trustRootsConfigMapName, + }, + Key: trustRootsConfigMapKeyName, + }, + }, + }, + }, + }, + &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: trustRootsConfigMapName, + Namespace: "linkerd", + }, + Data: map[string]string{ + "nonsense.key": expectedRoots, + }, + }, + expectedRoots, + fmt.Errorf("config map linkerd/%s does not have %s key", trustRootsConfigMapName, trustRootsConfigMapKeyName), + }, + { + "no roots env var", &v1.Container{ Name: l5dk8s.ProxyContainerName, Env: []v1.EnvVar{}, }, + nil, "", fmt.Errorf("could not find env var with name %s on proxy container [linkerd-proxy]", identity.EnvTrustAnchors), }, @@ -348,7 +434,15 @@ AiAtuoI5XuCtrGVRzSmRTl2ra28aV9MyTU7d5qnTAFHKSgIgRKCvluOSgA5O21p5 for _, tc := range fixtures { tc := tc t.Run(tc.testName, func(t *testing.T) { - roots, err := extractRootsCerts(tc.container) + c := fakeClient() + if tc.crtConfigMap != nil { + c = fakeClient(tc.crtConfigMap) + } + + c.Sync(nil, time.Second) + client := NewClient(c.k8sClient, c.sharedInformers, nil) + + roots, err := client.extractRootsCerts(context.Background(), tc.container, "linkerd") if tc.expectedErr != nil { if tc.expectedErr.Error() != err.Error() { t.Fatalf("exepected err %s, got %s", tc.expectedErr, err)