Skip to content

Commit b9ff5ef

Browse files
feat(k8s): add filter for ignoring TLS secrets without client certificates
- Introduce IgnoreTLSSecretsContainingNonClientCertificates filter - Update dynamic gatherer config to support resource filters via YAML - Apply filters when adding resources to cache to exclude non-client TLS secrets Signed-off-by: Richard Wall <richard.wall@cyberark.com>
1 parent 7b58625 commit b9ff5ef

File tree

5 files changed

+201
-3
lines changed

5 files changed

+201
-3
lines changed

deploy/charts/cyberark-disco-agent/templates/configmap.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ data:
3030
- type!=kubernetes.io/dockerconfigjson
3131
- type!=bootstrap.kubernetes.io/token
3232
- type!=helm.sh/release.v1
33+
filters:
34+
- IgnoreTLSSecretsContainingNonClientCertificates
3335
- kind: k8s-dynamic
3436
name: ark/serviceaccounts
3537
config:

examples/machinehub.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ data-gatherers:
2828
- type!=kubernetes.io/dockerconfigjson
2929
- type!=bootstrap.kubernetes.io/token
3030
- type!=helm.sh/release.v1
31+
filters:
32+
- IgnoreTLSSecretsContainingNonClientCertificates
3133

3234
# Gather Kubernetes service accounts
3335
- name: ark/serviceaccounts

pkg/datagatherer/k8s/cache.go

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
package k8s
22

33
import (
4+
"crypto/x509"
5+
"encoding/pem"
46
"fmt"
57
"time"
68

79
"github.com/go-logr/logr"
810
"github.com/pmylund/go-cache"
11+
corev1 "k8s.io/api/core/v1"
12+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
13+
"k8s.io/apimachinery/pkg/runtime"
914
"k8s.io/apimachinery/pkg/types"
1015

1116
"github.com/jetstack/preflight/api"
@@ -39,9 +44,141 @@ func logCacheUpdateFailure(log logr.Logger, obj interface{}, operation string) {
3944
log.Error(err, "Cache update failure", "operation", operation)
4045
}
4146

47+
// cacheFilterFunction is a function that takes an object and returns true if the
48+
// object should not be added to the cache, false otherwise.
49+
// This can be used to filter out objects that are not relevant for the data gatherer.
50+
type cacheFilterFunction func(logr.Logger, interface{}) bool
51+
52+
// IgnoreTLSSecretsContainingNonClientCertificates filters out all TLS secrets that do not
53+
// contain a client certificate in the `tls.crt` key.
54+
// Secrets are obtained by a DynamicClient, so they have type
55+
// *unstructured.Unstructured.
56+
func IgnoreTLSSecretsContainingNonClientCertificates(log logr.Logger, obj interface{}) bool {
57+
_, ok := obj.(cacheResource)
58+
if !ok {
59+
log.V(4).Info("Object is not a cacheResource", "type", fmt.Sprintf("%T", obj))
60+
return false
61+
}
62+
// Check if the object is a Unstructured
63+
unstructuredObj, ok := obj.(*unstructured.Unstructured)
64+
if !ok {
65+
log.V(4).Info("Object is not a Unstructured", "type", fmt.Sprintf("%T", obj))
66+
return false
67+
}
68+
69+
// Check if the object is a Secret
70+
if unstructuredObj.GetKind() != "Secret" {
71+
log.V(4).Info("Object is not a Secret", "kind", unstructuredObj.GetKind())
72+
return false
73+
}
74+
if unstructuredObj.GetAPIVersion() != "v1" {
75+
log.V(4).Info("Object is not a v1 Secret", "apiVersion", unstructuredObj.GetAPIVersion())
76+
return false
77+
}
78+
secretType, found, err := unstructured.NestedString(unstructuredObj.Object, "type")
79+
if err != nil || !found || secretType != string(corev1.SecretTypeTLS) {
80+
log.V(4).Info("Object is not a TLS Secret", "type", secretType)
81+
return false
82+
}
83+
84+
// Get the Secret data
85+
var secret corev1.Secret
86+
err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredObj.Object, &secret)
87+
if err != nil {
88+
log.V(4).Info("failed to convert Unstructured to Secret", "error", err.Error())
89+
return true
90+
}
91+
92+
// Check if the secret contains the `tls.crt` key
93+
data, found := secret.Data[corev1.TLSCertKey]
94+
if !found || len(data) == 0 {
95+
log.V(4).Info("TLS Secret does not contain tls.crt key")
96+
return true
97+
}
98+
99+
// Try to parse the data as a PEM encoded X.509 certificate chain
100+
certs, err := parsePEMCertificateChain(data)
101+
if err != nil {
102+
log.V(4).Info("Failed to parse tls.crt as PEM encoded X.509 certificate chain", "error", err.Error())
103+
return true
104+
}
105+
106+
if len(certs) == 0 {
107+
log.V(4).Info("No certificates found in tls.crt")
108+
return true
109+
}
110+
111+
// Check if the leaf certificate is a client certificate
112+
if isClientCertificate(certs[0]) {
113+
log.V(4).Info("TLS Secret contains a client certificate", "name", secret.Name, "namespace", secret.Namespace)
114+
return false
115+
}
116+
117+
log.V(4).Info("TLS Secret does not contain a client certificate", "name", secret.Name, "namespace", secret.Namespace)
118+
return true
119+
}
120+
121+
// isClientCertificate checks if the given certificate is a client certificate
122+
// by checking if it has the ClientAuth EKU.
123+
func isClientCertificate(cert *x509.Certificate) bool {
124+
if cert == nil {
125+
return false
126+
}
127+
// Check if the certificate has the ClientAuth EKU
128+
for _, eku := range cert.ExtKeyUsage {
129+
if eku == x509.ExtKeyUsageClientAuth {
130+
return true
131+
}
132+
}
133+
return false
134+
}
135+
136+
// parsePEMCertificateChain parses a PEM encoded certificate chain and returns
137+
// a slice of x509.Certificate pointers. It returns an error if the data cannot
138+
// be parsed as a certificate chain.
139+
// The supplied data can contain multiple PEM blocks, the function will parse
140+
// all of them and return a slice of certificates.
141+
func parsePEMCertificateChain(data []byte) ([]*x509.Certificate, error) {
142+
// Parse the PEM encoded certificate chain
143+
var certs []*x509.Certificate
144+
var block *pem.Block
145+
rest := data
146+
for {
147+
block, rest = pem.Decode(rest)
148+
if block == nil {
149+
break
150+
}
151+
if block.Type != "CERTIFICATE" || len(block.Bytes) == 0 {
152+
continue
153+
}
154+
cert, err := x509.ParseCertificate(block.Bytes)
155+
if err != nil {
156+
return nil, fmt.Errorf("failed to parse certificate: %w", err)
157+
}
158+
certs = append(certs, cert)
159+
}
160+
if len(certs) == 0 {
161+
return nil, fmt.Errorf("no certificates found")
162+
}
163+
return certs, nil
164+
}
165+
42166
// onAdd handles the informer creation events, adding the created runtime.Object
43167
// to the data gatherer's cache. The cache key is the uid of the object
44-
func onAdd(log logr.Logger, obj interface{}, dgCache *cache.Cache) {
168+
// The object is wrapped in a GatheredResource struct.
169+
// If the object is already present in the cache, it gets replaced.
170+
// The cache key is the uid of the object
171+
// The supplied filter functions can be used to filter out objects that
172+
// should not be added to the cache.
173+
// If multiple filter functions are supplied, the object is filtered out
174+
// if any of the filter functions returns true.
175+
func onAdd(log logr.Logger, obj interface{}, dgCache *cache.Cache, filters ...cacheFilterFunction) {
176+
for _, filter := range filters {
177+
if filter != nil && filter(log, obj) {
178+
return
179+
}
180+
}
181+
45182
item, ok := obj.(cacheResource)
46183
if ok {
47184
cacheObject := &api.GatheredResource{

pkg/datagatherer/k8s/dynamic.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ type ConfigDynamic struct {
4343
IncludeNamespaces []string `yaml:"include-namespaces"`
4444
// FieldSelectors is a list of field selectors to use when listing this resource
4545
FieldSelectors []string `yaml:"field-selectors"`
46+
// Filters is a list of filter functions to apply to the resources before adding them to the cache.
47+
// Each filter function should return true if the resource should be excluded, false otherwise.
48+
// Available filter functions:
49+
// - IgnoreTLSSecretsContainingNonClientCertificates: ignores all TLS
50+
// secrets that do not contain client certificates
51+
Filters []cacheFilterFunction `yaml:"filters"`
4652
}
4753

4854
// UnmarshalYAML unmarshals the ConfigDynamic resolving GroupVersionResource.
@@ -57,6 +63,7 @@ func (c *ConfigDynamic) UnmarshalYAML(unmarshal func(interface{}) error) error {
5763
ExcludeNamespaces []string `yaml:"exclude-namespaces"`
5864
IncludeNamespaces []string `yaml:"include-namespaces"`
5965
FieldSelectors []string `yaml:"field-selectors"`
66+
Filters []string `yaml:"filters"`
6067
}{}
6168
err := unmarshal(&aux)
6269
if err != nil {
@@ -71,6 +78,15 @@ func (c *ConfigDynamic) UnmarshalYAML(unmarshal func(interface{}) error) error {
7178
c.IncludeNamespaces = aux.IncludeNamespaces
7279
c.FieldSelectors = aux.FieldSelectors
7380

81+
for _, filterName := range aux.Filters {
82+
switch filterName {
83+
case "IgnoreTLSSecretsContainingNonClientCertificates":
84+
c.Filters = append(c.Filters, IgnoreTLSSecretsContainingNonClientCertificates)
85+
default:
86+
return fmt.Errorf("filters contains an unknown filter function: %s. Must be one of: IgnoreTLSSecretsContainingNonClientCertificates", filterName)
87+
}
88+
}
89+
7490
return nil
7591
}
7692

@@ -107,6 +123,9 @@ type sharedInformerFunc func(informers.SharedInformerFactory) k8scache.SharedInd
107123

108124
// kubernetesNativeResources map of the native kubernetes resources, linking each resource to a sharedInformerFunc for that resource.
109125
// secrets are still treated as unstructured rather than corev1.Secret, for a faster unmarshaling
126+
//
127+
// TODO(wallrj): What does "faster unmarshaling" mean in this context? Is it
128+
// actually faster? If so, how much faster? Is it worth the loss of type safety?
110129
var kubernetesNativeResources = map[schema.GroupVersionResource]sharedInformerFunc{
111130
corev1.SchemeGroupVersion.WithResource("pods"): func(sharedFactory informers.SharedInformerFactory) k8scache.SharedIndexInformer {
112131
return sharedFactory.Core().V1().Pods().Informer()
@@ -144,6 +163,16 @@ var kubernetesNativeResources = map[schema.GroupVersionResource]sharedInformerFu
144163
}
145164

146165
// NewDataGatherer constructs a new instance of the generic K8s data-gatherer for the provided
166+
// configuration. Returns an error if the configuration is invalid or
167+
// the data-gatherer cannot be constructed.
168+
// If the GroupVersionResource is a native Kubernetes resource, the data gatherer will use
169+
// a typed clientset and SharedInformerFactory, otherwise it will use a dynamic client and
170+
// dynamic informer factory.
171+
// Secret is a special case, it is a native resource but it will be treated as unstructured
172+
// rather than corev1.Secret, for a faster unmarshaling.
173+
//
174+
// TODO(wallrj): What does "faster unmarshaling" mean in this context? Is it
175+
// actually faster? If so, how much faster? Is it worth the loss of type safety?
147176
func (c *ConfigDynamic) NewDataGatherer(ctx context.Context) (datagatherer.DataGatherer, error) {
148177
if isNativeResource(c.GroupVersionResource) {
149178
clientset, err := NewClientSet(c.KubeConfigPath)
@@ -167,6 +196,7 @@ func (c *ConfigDynamic) newDataGathererWithClient(ctx context.Context, cl dynami
167196
if err := c.validate(); err != nil {
168197
return nil, err
169198
}
199+
170200
// init shared informer for selected namespaces
171201
fieldSelector := generateExcludedNamespacesFieldSelector(c.ExcludeNamespaces)
172202

@@ -218,7 +248,7 @@ func (c *ConfigDynamic) newDataGathererWithClient(ctx context.Context, cl dynami
218248

219249
registration, err := newDataGatherer.informer.AddEventHandlerWithOptions(k8scache.ResourceEventHandlerFuncs{
220250
AddFunc: func(obj interface{}) {
221-
onAdd(log, obj, dgCache)
251+
onAdd(log, obj, dgCache, c.Filters...)
222252
},
223253
UpdateFunc: func(oldObj, newObj interface{}) {
224254
onUpdate(log, oldObj, newObj, dgCache)

pkg/datagatherer/k8s/dynamic_test.go

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
"k8s.io/client-go/informers"
2525
fakeclientset "k8s.io/client-go/kubernetes/fake"
2626
k8scache "k8s.io/client-go/tools/cache"
27+
"k8s.io/klog/v2"
28+
"k8s.io/klog/v2/ktesting"
2729

2830
"github.com/jetstack/preflight/api"
2931
)
@@ -217,6 +219,8 @@ include-namespaces:
217219
- default
218220
field-selectors:
219221
- type!=kubernetes.io/service-account-token
222+
filters:
223+
- IgnoreTLSSecretsContainingNonClientCertificates
220224
`
221225

222226
expectedGVR := schema.GroupVersionResource{
@@ -259,6 +263,9 @@ field-selectors:
259263
if got, want := cfg.FieldSelectors, expectedFieldSelectors; !reflect.DeepEqual(got, want) {
260264
t.Errorf("FieldSelectors does not match: got=%+v want=%+v", got, want)
261265
}
266+
// Can't compare functions, so just check that one filter was loaded.
267+
// See https://go.dev/ref/spec#Comparison_operators
268+
assert.Equal(t, 1, len(cfg.Filters), "unexpected number of filters")
262269
}
263270

264271
func TestConfigDynamicValidate(t *testing.T) {
@@ -588,6 +595,25 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
588595
},
589596
},
590597
},
598+
"Secret of type kubernetes.io/tls should be filterable": {
599+
config: ConfigDynamic{
600+
IncludeNamespaces: []string{""},
601+
GroupVersionResource: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"},
602+
Filters: []cacheFilterFunction{IgnoreTLSSecretsContainingNonClientCertificates},
603+
},
604+
addObjects: []runtime.Object{
605+
getSecret("testsecret", "testns1", map[string]interface{}{
606+
"tls.key": "secretValue",
607+
"tls.crt": "value",
608+
"ca.crt": "value",
609+
}, true, true),
610+
getSecret("anothertestsecret", "testns2", map[string]interface{}{
611+
"example.key": "secretValue",
612+
"example.crt": "value",
613+
}, true, true),
614+
},
615+
expected: []*api.GatheredResource{},
616+
},
591617
"excluded annotations are removed for unstructured-based gatherers such as secrets": {
592618
config: ConfigDynamic{GroupVersionResource: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}},
593619

@@ -636,8 +662,9 @@ func TestDynamicGatherer_Fetch(t *testing.T) {
636662

637663
for name, tc := range tests {
638664
t.Run(name, func(t *testing.T) {
665+
log := ktesting.NewLogger(t, ktesting.DefaultConfig)
666+
ctx := klog.NewContext(t.Context(), log)
639667
var wg sync.WaitGroup
640-
ctx := t.Context()
641668
gvrToListKind := map[schema.GroupVersionResource]string{
642669
{Group: "foobar", Version: "v1", Resource: "foos"}: "UnstructuredList",
643670
{Group: "apps", Version: "v1", Resource: "deployments"}: "UnstructuredList",

0 commit comments

Comments
 (0)