From 137686f4e428ec2f06ec9149950de3c6e99ad7bf Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Thu, 11 Sep 2025 20:59:24 +0100 Subject: [PATCH] feat(k8s-dynamic): add filter to exclude TLS secrets without client certs - Introduce ExcludeTLSSecretsWithoutClientCert filter for k8s-dynamic gatherer - Update config, docs, and examples to support and demonstrate filter usage - Implement filter logic to exclude TLS secrets lacking client certificates - Extend tests to cover filtering of TLS secrets based on certificate EKU - Improve logging for filter decisions and error cases Signed-off-by: Richard Wall --- .../templates/configmap.yaml | 2 + docs/datagatherers/k8s-dynamic.md | 23 ++ examples/machinehub.yaml | 2 + pkg/datagatherer/k8s/cache.go | 139 +++++++++- pkg/datagatherer/k8s/dynamic.go | 29 +- pkg/datagatherer/k8s/dynamic_test.go | 249 +++++++++++++++++- 6 files changed, 441 insertions(+), 3 deletions(-) diff --git a/deploy/charts/cyberark-disco-agent/templates/configmap.yaml b/deploy/charts/cyberark-disco-agent/templates/configmap.yaml index d9703168..5991d9bd 100644 --- a/deploy/charts/cyberark-disco-agent/templates/configmap.yaml +++ b/deploy/charts/cyberark-disco-agent/templates/configmap.yaml @@ -30,6 +30,8 @@ data: - type!=kubernetes.io/dockerconfigjson - type!=bootstrap.kubernetes.io/token - type!=helm.sh/release.v1 + filters: + - ExcludeTLSSecretsWithoutClientCert - kind: k8s-dynamic name: ark/serviceaccounts config: diff --git a/docs/datagatherers/k8s-dynamic.md b/docs/datagatherers/k8s-dynamic.md index da8767a8..a078c098 100644 --- a/docs/datagatherers/k8s-dynamic.md +++ b/docs/datagatherers/k8s-dynamic.md @@ -106,3 +106,26 @@ when listing Secrets. - type!=bootstrap.kubernetes.io/token - type!=helm.sh/release.v1 ``` + +## Filters + +You can use filters to drop certain resources based on custom logic. +For example, you can drop TLS secrets that do not contain any client certificates using the `ExcludeTLSSecretsWithoutClientCert` filter, as shown below: + +```yaml +- kind: "k8s-dynamic" + name: "k8s/secrets" + config: + resource-type: + version: v1 + resource: secrets + filters: + - ExcludeTLSSecretsWithoutClientCert +``` + +The available filters are: +* `ExcludeTLSSecretsWithoutClientCert`: Drops TLS secrets that do not contain any client certificates. + +If you find that the filters are not having the desired effect, you can enable +debug logging by setting the log-level to `debug`. This will log info about why +certain resources are not being gathered. diff --git a/examples/machinehub.yaml b/examples/machinehub.yaml index ea0b28e5..0634fe7a 100644 --- a/examples/machinehub.yaml +++ b/examples/machinehub.yaml @@ -28,6 +28,8 @@ data-gatherers: - type!=kubernetes.io/dockerconfigjson - type!=bootstrap.kubernetes.io/token - type!=helm.sh/release.v1 + filters: + - ExcludeTLSSecretsWithoutClientCert # Gather Kubernetes service accounts - name: ark/serviceaccounts diff --git a/pkg/datagatherer/k8s/cache.go b/pkg/datagatherer/k8s/cache.go index 5ecc1fac..3571dc4c 100644 --- a/pkg/datagatherer/k8s/cache.go +++ b/pkg/datagatherer/k8s/cache.go @@ -1,11 +1,16 @@ package k8s import ( + "crypto/x509" + "encoding/base64" + "encoding/pem" "fmt" "time" "github.com/go-logr/logr" "github.com/pmylund/go-cache" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" "github.com/jetstack/preflight/api" @@ -39,9 +44,141 @@ func logCacheUpdateFailure(log logr.Logger, obj interface{}, operation string) { log.Error(err, "Cache update failure", "operation", operation) } +// cacheFilterFunction is a function that can be used to filter out objects +// that should not be added to the cache. If the function returns true, the +// object is filtered out. +type cacheFilterFunction func(logr.Logger, interface{}) bool + +// excludeTLSSecretsWithoutClientCert filters out all TLS secrets that do not +// contain a client certificate in the `tls.crt` key. +// Secrets are obtained by a DynamicClient, so they have type +// *unstructured.Unstructured. +func excludeTLSSecretsWithoutClientCert(log logr.Logger, obj interface{}) bool { + // Fast path: type assertion and kind/type checks + unstructuredObj, ok := obj.(*unstructured.Unstructured) + if !ok { + log.V(4).Info("Object is not a Unstructured", "type", fmt.Sprintf("%T", obj)) + return false + } + if unstructuredObj.GetKind() != "Secret" || unstructuredObj.GetAPIVersion() != "v1" { + return false + } + + log = log.WithValues("namespace", unstructuredObj.GetNamespace(), "name", unstructuredObj.GetName()) + + secretType, found, err := unstructured.NestedString(unstructuredObj.Object, "type") + if err != nil || !found || secretType != string(corev1.SecretTypeTLS) { + log.V(4).Info("Object is not a TLS Secret", "type", secretType) + return false + } + + // Directly extract tls.crt from unstructured data (avoid conversion if possible) + dataMap, found, err := unstructured.NestedMap(unstructuredObj.Object, "data") + if err != nil || !found { + log.V(4).Info("Secret data missing or not a map") + return true + } + tlsCrtRaw, found := dataMap[corev1.TLSCertKey] + if !found { + log.V(4).Info("TLS Secret does not contain tls.crt key") + return true + } + + // Decode base64 if necessary (K8s secrets store data as base64-encoded strings) + var tlsCrtBytes []byte + switch v := tlsCrtRaw.(type) { + case string: + decoded, err := base64.StdEncoding.DecodeString(v) + if err != nil { + log.V(4).Info("Failed to decode tls.crt base64", "error", err.Error()) + return true + } + tlsCrtBytes = decoded + case []byte: + tlsCrtBytes = v + default: + log.V(4).Info("tls.crt is not a string or byte slice", "type", fmt.Sprintf("%T", v)) + return true + } + + // Parse PEM certificate chain + certs, err := parsePEMCertificateChain(tlsCrtBytes) + if err != nil || len(certs) == 0 { + log.V(4).Info("Failed to parse tls.crt as PEM encoded X.509 certificate chain", "error", err.Error()) + return true + } + + // Check if the leaf certificate is a client certificate + if isClientCertificate(certs[0]) { + log.V(4).Info("TLS Secret contains a client certificate") + return false + } + + log.V(4).Info("TLS Secret does not contain a client certificate") + return true +} + +// isClientCertificate checks if the given certificate is a client certificate +// by checking if it has the ClientAuth EKU. +func isClientCertificate(cert *x509.Certificate) bool { + if cert == nil { + return false + } + // Check if the certificate has the ClientAuth EKU + for _, eku := range cert.ExtKeyUsage { + if eku == x509.ExtKeyUsageClientAuth { + return true + } + } + return false +} + +// parsePEMCertificateChain parses a PEM encoded certificate chain and returns +// a slice of x509.Certificate pointers. It returns an error if the data cannot +// be parsed as a certificate chain. +// The supplied data can contain multiple PEM blocks, the function will parse +// all of them and return a slice of certificates. +func parsePEMCertificateChain(data []byte) ([]*x509.Certificate, error) { + // Parse the PEM encoded certificate chain + var certs []*x509.Certificate + var block *pem.Block + rest := data + for { + block, rest = pem.Decode(rest) + if block == nil { + break + } + if block.Type != "CERTIFICATE" || len(block.Bytes) == 0 { + continue + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("failed to parse certificate: %w", err) + } + certs = append(certs, cert) + } + if len(certs) == 0 { + return nil, fmt.Errorf("no certificates found") + } + return certs, nil +} + // onAdd handles the informer creation events, adding the created runtime.Object // to the data gatherer's cache. The cache key is the uid of the object -func onAdd(log logr.Logger, obj interface{}, dgCache *cache.Cache) { +// The object is wrapped in a GatheredResource struct. +// If the object is already present in the cache, it gets replaced. +// The cache key is the uid of the object +// The supplied filter functions can be used to filter out objects that +// should not be added to the cache. +// If multiple filter functions are supplied, the object is filtered out +// if any of the filter functions returns true. +func onAdd(log logr.Logger, obj interface{}, dgCache *cache.Cache, filters ...cacheFilterFunction) { + for _, filter := range filters { + if filter != nil && filter(log, obj) { + return + } + } + item, ok := obj.(cacheResource) if ok { cacheObject := &api.GatheredResource{ diff --git a/pkg/datagatherer/k8s/dynamic.go b/pkg/datagatherer/k8s/dynamic.go index 7cb131b5..a67d2d71 100644 --- a/pkg/datagatherer/k8s/dynamic.go +++ b/pkg/datagatherer/k8s/dynamic.go @@ -43,6 +43,11 @@ type ConfigDynamic struct { IncludeNamespaces []string `yaml:"include-namespaces"` // FieldSelectors is a list of field selectors to use when listing this resource FieldSelectors []string `yaml:"field-selectors"` + // Filters is a list of filter functions to apply to the resources before adding them to the cache. + // Each filter function should return true if the resource should be excluded, false otherwise. + // Available filter functions: + // - ExcludeTLSSecretsWithoutClientCert: ignores all TLS secrets that do not contain client certificates + Filters []cacheFilterFunction `yaml:"filters"` } // UnmarshalYAML unmarshals the ConfigDynamic resolving GroupVersionResource. @@ -57,6 +62,7 @@ func (c *ConfigDynamic) UnmarshalYAML(unmarshal func(interface{}) error) error { ExcludeNamespaces []string `yaml:"exclude-namespaces"` IncludeNamespaces []string `yaml:"include-namespaces"` FieldSelectors []string `yaml:"field-selectors"` + Filters []string `yaml:"filters"` }{} err := unmarshal(&aux) if err != nil { @@ -71,6 +77,15 @@ func (c *ConfigDynamic) UnmarshalYAML(unmarshal func(interface{}) error) error { c.IncludeNamespaces = aux.IncludeNamespaces c.FieldSelectors = aux.FieldSelectors + for _, filterName := range aux.Filters { + switch filterName { + case "ExcludeTLSSecretsWithoutClientCert": + c.Filters = append(c.Filters, excludeTLSSecretsWithoutClientCert) + default: + return fmt.Errorf("filters contains an unknown filter function: %s. Must be one of: ExcludeTLSSecretsWithoutClientCert", filterName) + } + } + return nil } @@ -107,6 +122,9 @@ type sharedInformerFunc func(informers.SharedInformerFactory) k8scache.SharedInd // kubernetesNativeResources map of the native kubernetes resources, linking each resource to a sharedInformerFunc for that resource. // secrets are still treated as unstructured rather than corev1.Secret, for a faster unmarshaling +// +// TODO(wallrj): What does "faster unmarshaling" mean in this context? If +// unstructured is faster then why not use it for all resources? var kubernetesNativeResources = map[schema.GroupVersionResource]sharedInformerFunc{ corev1.SchemeGroupVersion.WithResource("pods"): func(sharedFactory informers.SharedInformerFactory) k8scache.SharedIndexInformer { return sharedFactory.Core().V1().Pods().Informer() @@ -144,6 +162,15 @@ var kubernetesNativeResources = map[schema.GroupVersionResource]sharedInformerFu } // NewDataGatherer constructs a new instance of the generic K8s data-gatherer for the provided +// configuration. +// +// If the GroupVersionResource is a native Kubernetes resource, the data +// gatherer will use a typed clientset and SharedInformerFactory, otherwise it +// will use a dynamic client and dynamic informer factory, for CRDs like those +// of cert-manager. +// +// Secret is a special case, it is a native resource but it will be treated as unstructured +// rather than corev1.Secret, for "faster unmarshaling". func (c *ConfigDynamic) NewDataGatherer(ctx context.Context) (datagatherer.DataGatherer, error) { if isNativeResource(c.GroupVersionResource) { clientset, err := NewClientSet(c.KubeConfigPath) @@ -218,7 +245,7 @@ func (c *ConfigDynamic) newDataGathererWithClient(ctx context.Context, cl dynami registration, err := newDataGatherer.informer.AddEventHandlerWithOptions(k8scache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { - onAdd(log, obj, dgCache) + onAdd(log, obj, dgCache, c.Filters...) }, UpdateFunc: func(oldObj, newObj interface{}) { onUpdate(log, oldObj, newObj, dgCache) diff --git a/pkg/datagatherer/k8s/dynamic_test.go b/pkg/datagatherer/k8s/dynamic_test.go index 525c8892..44406edd 100644 --- a/pkg/datagatherer/k8s/dynamic_test.go +++ b/pkg/datagatherer/k8s/dynamic_test.go @@ -1,8 +1,16 @@ package k8s import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" "encoding/json" + "encoding/pem" "fmt" + "math/big" "reflect" "regexp" "sort" @@ -24,6 +32,8 @@ import ( "k8s.io/client-go/informers" fakeclientset "k8s.io/client-go/kubernetes/fake" k8scache "k8s.io/client-go/tools/cache" + "k8s.io/klog/v2" + "k8s.io/klog/v2/ktesting" "github.com/jetstack/preflight/api" ) @@ -217,6 +227,8 @@ include-namespaces: - default field-selectors: - type!=kubernetes.io/service-account-token +filters: +- ExcludeTLSSecretsWithoutClientCert ` expectedGVR := schema.GroupVersionResource{ @@ -259,6 +271,9 @@ field-selectors: if got, want := cfg.FieldSelectors, expectedFieldSelectors; !reflect.DeepEqual(got, want) { t.Errorf("FieldSelectors does not match: got=%+v want=%+v", got, want) } + // Can't compare functions, so just check that one filter was loaded. + // See https://go.dev/ref/spec#Comparison_operators + assert.Equal(t, 1, len(cfg.Filters), "unexpected number of filters") } func TestConfigDynamicValidate(t *testing.T) { @@ -366,6 +381,10 @@ func init() { } func TestDynamicGatherer_Fetch(t *testing.T) { + clientCertificateChain := sampleCertificateChain(t, x509.ExtKeyUsageClientAuth) + nonClientCertificateChain := sampleCertificateChain(t, x509.ExtKeyUsageServerAuth) + peerCertificateChain := sampleCertificateChain(t, x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth) + // start a k8s client // init the datagatherer's informer with the client // add/delete resources watched by the data gatherer @@ -632,12 +651,64 @@ func TestDynamicGatherer_Fetch(t *testing.T) { map[string]interface{}{"prod": "true"}, )}}, }, + "ignore-tls-secrets-containing-non-client-certificates": { + // Demonstrates the ExcludeTLSSecretsWithoutClientCert filter + // function, which is used to exclude TLS secrets that do not contain + // client certificates. Client certificates are identified by having + // the "ExtKeyUsageClientAuth" Extended Key Usage (EKU) bit set. + // + // In this test case, one secret contains a client certificate and + // should be included in the gathered results, while another secret + // does not contain a client certificate and should be excluded. + // + // A selection of other edge cases are also included, such as a secret + // with a certificate that has both client and server EKU bits set, + // as well as secrets with invalid base64 and invalid PEM data. + // + // The expected result is that only the secrets with the valid client + // certificates are included in the gathered resources. + config: ConfigDynamic{ + IncludeNamespaces: []string{""}, + GroupVersionResource: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}, + Filters: []cacheFilterFunction{excludeTLSSecretsWithoutClientCert}, + }, + addObjects: []runtime.Object{ + getSecret("client", "ns1", map[string]interface{}{ + "tls.crt": clientCertificateChain, + }, true, true), + getSecret("server", "ns2", map[string]interface{}{ + "tls.crt": nonClientCertificateChain, + }, true, true), + getSecret("peer", "ns3", map[string]interface{}{ + "tls.crt": peerCertificateChain, + }, true, true), + getSecret("invalid-base64", "ns4", map[string]interface{}{ + "tls.crt": "invalid-base64", + }, true, true), + getSecret("invalid-pem", "ns5", map[string]interface{}{ + "tls.crt": "DEADBEEF", + }, true, true), + }, + expected: []*api.GatheredResource{ + { + Resource: getSecret("client", "ns1", map[string]interface{}{ + "tls.crt": clientCertificateChain, + }, true, false), + }, + { + Resource: getSecret("peer", "ns3", map[string]interface{}{ + "tls.crt": peerCertificateChain, + }, true, false), + }, + }, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { + log := ktesting.NewLogger(t, ktesting.DefaultConfig) + ctx := klog.NewContext(t.Context(), log) var wg sync.WaitGroup - ctx := t.Context() gvrToListKind := map[schema.GroupVersionResource]string{ {Group: "foobar", Version: "v1", Resource: "foos"}: "UnstructuredList", {Group: "apps", Version: "v1", Resource: "deployments"}: "UnstructuredList", @@ -1264,3 +1335,179 @@ func toRegexps(keys []string) []*regexp.Regexp { } return regexps } + +func TestExcludeTLSSecretsWithoutClientCert(t *testing.T) { + type testCase struct { + name string + secret interface{} + exclude bool + } + + tests := []testCase{ + { + name: "TLS secret with client cert", + secret: newTLSSecret("tls-secret-with-client", sampleCertificateChain(t, x509.ExtKeyUsageClientAuth)), + exclude: false, + }, + { + name: "TLS secret without client cert", + secret: newTLSSecret("tls-secret-without-client", sampleCertificateChain(t, x509.ExtKeyUsageServerAuth)), + exclude: true, + }, + { + name: "Non-unstructured", + secret: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "non-unstructured-secret", + Namespace: "default", + }, + }, + exclude: false, + }, + { + name: "Non-secret", + secret: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "cert-manager/v1", + "kind": "Certificate", + "metadata": map[string]interface{}{ + "name": "non-secret", + "namespace": "default", + }, + }, + }, + exclude: false, + }, + { + name: "Non-TLS secret", + secret: newOpaqueSecret("non-tls-secret"), + exclude: false, + }, + { + name: "TLS secret with invalid base64", + secret: newTLSSecret("tls-secret-with-invalid-cert", "invalid-base64"), + exclude: true, + }, + { + name: "TLS secret with no cert data", + secret: newTLSSecret("tls-secret-with-no-cert", nil), + exclude: true, + }, + { + name: "TLS secret with empty cert data", + secret: newTLSSecret("tls-secret-with-empty-cert", ""), + exclude: true, + }, + { + name: "TLS secret with invalid PEM", + secret: newTLSSecret("tls-secret-with-invalid-pem", base64.StdEncoding.EncodeToString([]byte("invalid-pem"))), + exclude: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + log := ktesting.NewLogger(t, ktesting.DefaultConfig) + excluded := excludeTLSSecretsWithoutClientCert(log, tc.secret) + assert.Equal(t, tc.exclude, excluded, "case: %s", tc.name) + }) + } +} + +// newTLSSecret creates a Kubernetes TLS secret with the given name and certificate data. +// If crt is nil, the secret will not contain a "tls.crt" entry. +func newTLSSecret(name string, crt interface{}) *unstructured.Unstructured { + data := map[string]interface{}{"tls.key": "dummy-key"} + if crt != nil { + data["tls.crt"] = crt + } + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": name, + "namespace": "default", + }, + "type": "kubernetes.io/tls", + "data": data, + }, + } +} + +// newOpaqueSecret creates a Kubernetes Opaque secret with the given name. +func newOpaqueSecret(name string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": name, + "namespace": "default", + }, + "type": "Opaque", + "data": map[string]interface{}{ + "key": "value", + }, + }, + } +} + +// sampleCertificateChain returns a PEM encoded sample certificate chain for testing purposes. +// The leaf certificate is signed by a self-signed CA certificate. +// Uses an eliptic curve key for the CA and leaf certificates for speed. +// The returned string is base64 encoded to match how TLS certificates +// are typically provided in Kubernetes secrets. +func sampleCertificateChain(t testing.TB, usages ...x509.ExtKeyUsage) string { + t.Helper() + + caPrivKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + caTemplate := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + Organization: []string{"Test CA"}, + CommonName: "Test CA", + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + ExtKeyUsage: []x509.ExtKeyUsage{}, + BasicConstraintsValid: true, + IsCA: true, + } + + caCertDER, err := x509.CreateCertificate(rand.Reader, &caTemplate, &caTemplate, &caPrivKey.PublicKey, caPrivKey) + require.NoError(t, err) + + caCertPEM := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: caCertDER, + }) + + clientPrivKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + clientTemplate := x509.Certificate{ + SerialNumber: big.NewInt(2), + Subject: pkix.Name{ + Organization: []string{"Test Organization"}, + CommonName: "example.com", + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: usages, + } + + clientCertDER, err := x509.CreateCertificate(rand.Reader, &clientTemplate, &caTemplate, &clientPrivKey.PublicKey, caPrivKey) + require.NoError(t, err) + + clientCertPEM := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: clientCertDER, + }) + + return base64.StdEncoding.EncodeToString(append(clientCertPEM, caCertPEM...)) +}