Skip to content

Commit

Permalink
fix: OB-37389 Obfuscate secrets inside Secret previous configuration
Browse files Browse the repository at this point in the history
Secret entities not only contain secrets in data, but might also contain
an annotation called something like "previous cofiguration", which
contains the whole JSON of the previous definition of the Secret itself.
This contains also the secrets in plaintext.
This commit adds some logic to marshal such "inner" JSON, redact the
secrets through the typed object, unmarshal the object back to a JSON
string, and then overwrite the annotation with the updated string.
  • Loading branch information
obs-gh-enricogiorio committed Oct 5, 2024
1 parent b6e28d6 commit d442766
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,27 @@ type queryWithResult struct {
expResult any
}

// Functions that check the body of a logRecord post processor actions
// These "custom" check functions are a powerful way to introspect the body
// of a log record (as a raw JSON string).
// Implementations of these functions most likely start with unmarshalling
// the body string to an API object of choice
type checkBodyFunc func(t *testing.T, body string)

// Functions that check the attributes of a logRecord post processor actions
// These custom actions take as input the key-value attributes as a
// map[string]any
type checkAttributesFunc func(t *testing.T, attributes map[string]any) //nolint:unused

type k8sEventProcessorTest struct {
name string
inLogs plog.Logs
expectedResults []queryWithResult
// Actions that are only ran when testing the body of a resulting logRecord
checkBodyFunctions []checkBodyFunc
// Actions that are only ran when testing the attributes a resulting
// logRecord
checkAttributesFunctions []checkAttributesFunc //nolint:unused
}

// LogLocation is the part of the log where to check for matches. At the moment,
Expand Down Expand Up @@ -62,8 +79,14 @@ func runTest(t *testing.T, test k8sEventProcessorTest, location LogLocation) {
// map[string]any to be able to query it with jmespath.
body := logRecord.Body().AsString()
json.Unmarshal([]byte(body), &out)
for _, fn := range test.checkBodyFunctions {
fn(t, body)
}
case LogLocationAttributes:
out = logRecord.Attributes().AsRaw()
for _, fn := range test.checkAttributesFunctions {
fn(t, out)
}
}
for _, query := range test.expectedResults {
queryJmes, err := jmespath.Compile(query.path)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
package observek8sattributesprocessor

import (
"encoding/json"
"strings"

corev1 "k8s.io/api/core/v1"
)

const (
RedactedSecretValue = "REDACTED"
)

var keysMightContainSecret map[string]struct{}

func init() {
keysMightContainSecret = map[string]struct{}{
"password": {},
"token": {},
"secret": {},
}
}

type SecretRedactorBodyAction struct{}

func NewSecretRedactorBodyAction() SecretRedactorBodyAction {
Expand All @@ -16,10 +29,46 @@ func NewSecretRedactorBodyAction() SecretRedactorBodyAction {

// ---------------------------------- Secret "data" values' redaction ----------------------------------

// Redacts secrets' values
func (SecretRedactorBodyAction) Modify(secret *corev1.Secret) error {
func redactSecretKeys(secret *corev1.Secret) {
for key := range secret.Data {
secret.Data[key] = []byte(RedactedSecretValue)
}
for key := range secret.StringData {
secret.StringData[key] = RedactedSecretValue
}
}

// Redacts secrets' values
func (SecretRedactorBodyAction) Modify(secret *corev1.Secret) error {
redactSecretKeys(secret)

annotations := secret.GetAnnotations()
for key := range annotations {
// Yes, there is a secret inside the secret
if key == corev1.LastAppliedConfigAnnotation {
var innerSecet corev1.Secret
if ok := json.Unmarshal([]byte(annotations[key]), &innerSecet); ok == nil {
redactSecretKeys(&innerSecet)
// Since the inner secret was written as a JSON (escaped) string
// inside the object, we must re-marshal the innerSecret object
// now, or nothing will change
marshalledInnerSecret, err := json.Marshal(innerSecet)
if err != nil {
return err
}
annotations[key] = string(marshalledInnerSecret)
}
} else {
// Check for potential other annotations that might contain secrets
// Even though it's unlikely
for redFlagKey := range keysMightContainSecret {
if strings.Contains(key, redFlagKey) {
annotations[key] = RedactedSecretValue
break
}
}
}
}
secret.SetAnnotations(annotations)
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package observek8sattributesprocessor

import (
"encoding/base64"
"encoding/json"
"fmt"
"testing"

corev1 "k8s.io/api/core/v1"
)

func TestSecretBodyActions(t *testing.T) {
Expand All @@ -16,6 +19,26 @@ func TestSecretBodyActions(t *testing.T) {
{fmt.Sprintf("length(values(data)[?@ != '%s'])", base64.StdEncoding.EncodeToString([]byte(RedactedSecretValue))), float64(0)},
},
},
{
name: "Redact secrets' last configuration values",
inLogs: resourceLogsFromSingleJsonEvent("./testdata/secretEventPrevConfig.json"),
checkBodyFunctions: []checkBodyFunc{
func(t *testing.T, body string) {
var secret corev1.Secret
json.Unmarshal([]byte(body), &secret)
annotations := secret.GetAnnotations()
lastAppliedConfig := annotations[corev1.LastAppliedConfigAnnotation]
var innerSecret corev1.Secret
json.Unmarshal([]byte(lastAppliedConfig), &innerSecret)
for key, s := range innerSecret.Data {
// These are not encoded automatically, they will show up in the JSON as "REDACTED"
if string(s) != RedactedSecretValue {
t.Errorf("The value of secret %v inside the secret's previous configuration (within the log's annotations) is not redacted", key)
}
}
},
},
},
} {
runTest(t, testCase, LogLocationBody)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"apiVersion": "v1",
"data": {
"postgres-url": "Tm90IHNvIGVhc3kgbXIgaGFja2VyCg==",
"postgres-url-proxy": "RGFtbiwgWU9VIFJFQUxMWSBUUklFRCEgQ21vbiBub3cuLi4K"
},
"kind": "Secret",
"metadata": {
"annotations": {
"kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"data\":{\"postgres-url\":\"Tm90IHNvIGVhc3kgbXIgaGFja2VyCg==\",\"postgres-url-proxy\":\"RGFtbiwgWU9VIFJFQUxMWSBUUklFRCEgQ21vbiBub3cuLi4K\"},\"kind\":\"Secret\",\"metadata\":{\"annotations\":{},\"creationTimestamp\":\"2021-03-18T20:20:00Z\",\"name\":\"postgres-billing-credentials\",\"namespace\":\"o2\",\"resourceVersion\":\"543685028\",\"uid\":\"619570cd-c3da-43b5-a2a3-f85f68860f30\"},\"type\":\"Opaque\"}\n"
},
"creationTimestamp": "2024-08-14T20:10:30Z",
"managedFields": [
{
"apiVersion": "v1",
"fieldsType": "FieldsV1",
"fieldsV1": {
"f:data": {
".": {},
"f:postgres-url": {},
"f:postgres-url-proxy": {}
},
"f:metadata": {
"f:annotations": {
".": {},
"f:kubectl.kubernetes.io/last-applied-configuration": {}
}
},
"f:type": {}
},
"manager": "kubectl-client-side-apply",
"operation": "Update",
"time": "2024-08-14T20:10:30Z"
}
],
"name": "postgres-billing-credentials",
"namespace": "o2",
"resourceVersion": "40105",
"uid": "638c64d7-14e0-4551-bc9f-7f8e7644c8b9"
},
"type": "Opaque"
}

0 comments on commit d442766

Please sign in to comment.