Skip to content

Commit 56dbef5

Browse files
feat(client): minimize snapshot by excluding non-clientauth TLS secrets
- Add minimizeSnapshot to filter out TLS secrets lacking client certificates - Improve privacy and reduce snapshot size for CyberArk uploads - Implement isExcludableSecret and isExcludableTLSSecret helpers - Add unit tests for minimization and exclusion logic Signed-off-by: Richard Wall <richard.wall@cyberark.com>
1 parent 7b58625 commit 56dbef5

File tree

2 files changed

+440
-0
lines changed

2 files changed

+440
-0
lines changed

pkg/client/client_cyberark.go

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,23 @@ package client
22

33
import (
44
"context"
5+
"crypto/x509"
6+
"encoding/base64"
7+
"encoding/pem"
58
"fmt"
69
"net/http"
710

11+
"github.com/go-logr/logr"
12+
corev1 "k8s.io/api/core/v1"
13+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
814
"k8s.io/apimachinery/pkg/runtime"
915
"k8s.io/apimachinery/pkg/util/sets"
16+
"k8s.io/klog/v2"
1017

1118
"github.com/jetstack/preflight/api"
1219
"github.com/jetstack/preflight/internal/cyberark"
1320
"github.com/jetstack/preflight/internal/cyberark/dataupload"
21+
"github.com/jetstack/preflight/pkg/logs"
1422
"github.com/jetstack/preflight/pkg/version"
1523
)
1624

@@ -40,14 +48,20 @@ func NewCyberArk(httpClient *http.Client) (*CyberArkClient, error) {
4048

4149
// PostDataReadingsWithOptions uploads data readings to CyberArk.
4250
// It converts the supplied data readings into a snapshot format expected by CyberArk.
51+
// It then minimizes the snapshot to avoid uploading unnecessary data.
4352
// It initializes a data upload client with the configured HTTP client and credentials,
4453
// then uploads a snapshot.
4554
// The supplied Options are not used by this publisher.
4655
func (o *CyberArkClient) PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, _ Options) error {
56+
log := klog.FromContext(ctx)
4757
var snapshot dataupload.Snapshot
4858
if err := convertDataReadings(defaultExtractorFunctions, readings, &snapshot); err != nil {
4959
return fmt.Errorf("while converting data readings: %s", err)
5060
}
61+
62+
// Minimize the snapshot to reduce size and improve privacy
63+
minimizeSnapshot(log.V(logs.Debug), &snapshot)
64+
5165
snapshot.AgentVersion = version.PreflightVersion
5266

5367
cfg, err := o.configLoader()
@@ -190,3 +204,158 @@ func convertDataReadings(
190204
}
191205
return nil
192206
}
207+
208+
// minimizeSnapshot reduces the size of the snapshot by removing unnecessary data.
209+
//
210+
// This reduces the bandwidth used when uploading the snapshot to CyberArk,
211+
// it reduces the storage used by CyberArk to store the snapshot, and
212+
// it provides better privacy for the cluster being scanned; only the necessary
213+
// data is included in the snapshot.
214+
//
215+
// This is a best-effort attempt to minimize the snapshot size. Errors during
216+
// minimization are logged but do not prevent the snapshot from being uploaded.
217+
//
218+
// It performs the following minimization steps:
219+
//
220+
// 1. Removal of non-clientauth TLS secrets: It filters out TLS secrets that do
221+
// not contain a client certificate. This is done to avoid uploading large
222+
// TLS secrets that are not relevant for the CyberArk Discovery and Context
223+
// service.
224+
func minimizeSnapshot(log logr.Logger, snapshot *dataupload.Snapshot) {
225+
originalSecretCount := len(snapshot.Secrets)
226+
filteredSecrets := make([]runtime.Object, 0, originalSecretCount)
227+
for _, secret := range snapshot.Secrets {
228+
if isExcludableSecret(log, secret) {
229+
continue
230+
}
231+
filteredSecrets = append(filteredSecrets, secret)
232+
}
233+
snapshot.Secrets = filteredSecrets
234+
log.Info("Minimized snapshot", "originalSecretCount", originalSecretCount, "filteredSecretCount", len(snapshot.Secrets))
235+
}
236+
237+
// isExcludableSecret filters out TLS secrets that are definitely of no interest
238+
// to CyberArk's Discovery and Context service, specifically TLS secrets that do
239+
// not contain a client certificate.
240+
//
241+
// The Secret is kept if there is any doubt or if there is a problem decoding
242+
// its contents.
243+
//
244+
// Secrets are obtained by a DynamicClient, so they have type
245+
// *unstructured.Unstructured.
246+
func isExcludableSecret(log logr.Logger, obj runtime.Object) bool {
247+
// Fast path: type assertion and kind/type checks
248+
unstructuredObj, ok := obj.(*unstructured.Unstructured)
249+
if !ok {
250+
log.Info("Object is not a Unstructured", "type", fmt.Sprintf("%T", obj))
251+
return false
252+
}
253+
if unstructuredObj.GetKind() != "Secret" || unstructuredObj.GetAPIVersion() != "v1" {
254+
return false
255+
}
256+
257+
log = log.WithValues("namespace", unstructuredObj.GetNamespace(), "name", unstructuredObj.GetName())
258+
dataMap, found, err := unstructured.NestedMap(unstructuredObj.Object, "data")
259+
if err != nil || !found {
260+
log.Info("Secret data missing or not a map")
261+
return false
262+
}
263+
264+
secretType, found, err := unstructured.NestedString(unstructuredObj.Object, "type")
265+
if err != nil || !found {
266+
log.Info("Secret object has no type")
267+
return false
268+
}
269+
270+
if corev1.SecretType(secretType) != corev1.SecretTypeTLS {
271+
log.Info("Secret of this type are never excluded", "type", secretType)
272+
return false
273+
}
274+
275+
return isExcludableTLSSecret(log, dataMap)
276+
}
277+
278+
// isExcludableTLSSecret checks if a TLS Secret contains a client certificate.
279+
// It returns true if the Secret is a TLS Secret and its tls.crt does not
280+
// contain a client certificate.
281+
func isExcludableTLSSecret(log logr.Logger, dataMap map[string]interface{}) bool {
282+
tlsCrtRaw, found := dataMap[corev1.TLSCertKey]
283+
if !found {
284+
log.Info("TLS Secret does not contain tls.crt key")
285+
return true
286+
}
287+
288+
// Decode base64 if necessary (K8s secrets store data as base64-encoded strings)
289+
var tlsCrtBytes []byte
290+
switch v := tlsCrtRaw.(type) {
291+
case string:
292+
decoded, err := base64.StdEncoding.DecodeString(v)
293+
if err != nil {
294+
log.Info("Failed to decode tls.crt base64", "error", err.Error())
295+
return true
296+
}
297+
tlsCrtBytes = decoded
298+
case []byte:
299+
tlsCrtBytes = v
300+
default:
301+
log.Info("tls.crt is not a string or byte slice", "type", fmt.Sprintf("%T", v))
302+
return true
303+
}
304+
305+
// Parse PEM certificate chain
306+
hasClientCert := searchPEM(tlsCrtBytes, func(block *pem.Block) bool {
307+
if block.Type != "CERTIFICATE" || len(block.Bytes) == 0 {
308+
return false
309+
}
310+
cert, err := x509.ParseCertificate(block.Bytes)
311+
if err != nil {
312+
log.Info("Failed to parse PEM block as X.509 certificate", "error", err.Error())
313+
return false
314+
}
315+
// Check if the certificate has the ClientAuth EKU
316+
return isClientCertificate(cert)
317+
})
318+
return !hasClientCert
319+
}
320+
321+
// searchPEM parses the given PEM data and applies the visitor function to each
322+
// PEM block found. If the visitor function returns true for any block, the search
323+
// stops and searchPEM returns true. If no blocks cause the visitor to return true,
324+
// searchPEM returns false.
325+
func searchPEM(data []byte, visitor func(*pem.Block) bool) bool {
326+
if visitor == nil {
327+
return false
328+
}
329+
// Parse the PEM encoded certificate chain
330+
var block *pem.Block
331+
rest := data
332+
for {
333+
block, rest = pem.Decode(rest)
334+
if block == nil {
335+
break
336+
}
337+
if visitor(block) {
338+
return true
339+
}
340+
}
341+
return false
342+
}
343+
344+
// isClientCertificate checks if the given certificate is a client certificate
345+
// by checking if it has the ClientAuth EKU.
346+
func isClientCertificate(cert *x509.Certificate) bool {
347+
if cert == nil {
348+
return false
349+
}
350+
// Skip CA certificates
351+
if cert.IsCA {
352+
return false
353+
}
354+
// Check if the certificate has the ClientAuth EKU
355+
for _, eku := range cert.ExtKeyUsage {
356+
if eku == x509.ExtKeyUsageClientAuth {
357+
return true
358+
}
359+
}
360+
return false
361+
}

0 commit comments

Comments
 (0)