Skip to content

Commit ef50cec

Browse files
[VC-43403] Minimize snapshot by excluding non-clientauth TLS secrets
This introduces minimizeSnapshot to filter out TLS secrets without client certificates before uploading snapshots to CyberArk. The goal is to improve privacy, reduce bandwidth/storage, and ensure only relevant secrets are sent. - 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 <[email protected]>
1 parent 7b58625 commit ef50cec

File tree

2 files changed

+444
-0
lines changed

2 files changed

+444
-0
lines changed

pkg/client/client_cyberark.go

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

0 commit comments

Comments
 (0)