Add ability to send encrypted secrets to disco backend #770
Conversation
SgtCoDFish
force-pushed
the
sendsecrets
branch
from
841cc3f to
dbdca9c
Compare
SgtCoDFish
left a comment
SgtCoDFish
left a comment
There was a problem hiding this comment.
Self review of notes and things to look at
| - name: ARK_SEND_SECRETS | ||
| value: {{ .Values.config.sendSecrets | default "false" | quote }} |
There was a problem hiding this comment.
note: this defaults to false for now (while the feature is in development) but the planned default for this is "true" (i.e. this will be an opt-out feature, not opt-in)
There was a problem hiding this comment.
just my 2c. Although the name of the flag refers to secrets, even now secrets are being sent, but only their metadata. Perhaps we should clarify that this concerns the private data of the secrets.
| slices.SortStableFunc(list, func(a, b *api.GatheredResource) int { | ||
| aNamer, ok := a.Resource.(namer) | ||
| if !ok { | ||
| panic("got unexpected resource type") | ||
| } | ||
|
|
||
| bNamer, ok := b.Resource.(namer) | ||
| if !ok { | ||
| panic("got unexpected resource type") | ||
| } | ||
|
|
||
| return strings.Compare(aNamer.GetName(), bNamer.GetName()) | ||
| }) |
There was a problem hiding this comment.
self-review: This is unrelated to the other changes in this PR, but I couldn't bear to leave the old implementation!
SgtCoDFish
force-pushed
the
sendsecrets
branch
6 times, most recently
from
c3ab66c to
0143f61
Compare
For now, this uses a hardcoded RSA key for which I threw away the private key, since we don't have the ability to pull JWKs yet This also includes a few test tweaks to help make this easier, and an example folder which produces an output.json showing how this can work Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>
SgtCoDFish
force-pushed
the
sendsecrets
branch
from
0143f61 to
fd22a37
Compare
| excludeAnnotsKeys []string | ||
| excludeLabelKeys []string | ||
| addObjects []runtime.Object | ||
| addObjects []*unstructured.Unstructured |
There was a problem hiding this comment.
note: Changing this to unstructured helps with logic later. Working with runtime.Object sucks and would make it painful to check the _encryptedData field we set.
| addObjs := make([]runtime.Object, len(tc.addObjects)) | ||
| for i, obj := range tc.addObjects { | ||
| addObjs[i] = obj | ||
| } |
There was a problem hiding this comment.
note: Unfortunately, the change to making tc.addObjects a slice of *unstructured.Unstructured forces this so we can create cl - but I still think it's worth it because it makes testing the encryption logic simpler!
| // Verify JWE can be parsed | ||
| _, err = jwe.Parse(jweBytes) | ||
| require.NoError(t, err, "data should be a valid JWE") |
There was a problem hiding this comment.
note: this is slightly redundant since we decrypt just below, but having an explicit check makes the error a little clearer if this fails!
| - name: ARK_SEND_SECRETS | ||
| value: {{ .Values.config.sendSecrets | default "false" | quote }} |
There was a problem hiding this comment.
just my 2c. Although the name of the flag refers to secrets, even now secrets are being sent, but only their metadata. Perhaps we should clarify that this concerns the private data of the secrets.
|
|
||
| if encryptSecrets == "true" { | ||
| // TODO(@SgtCoDFish): this will fetch a key from JWKS endpoint when that endpoint is available | ||
| key, keyID, err := rsa.LoadHardcodedPublicKey() |
There was a problem hiding this comment.
Help me understand something. Does this mean that the public keys will only be loaded once at startup? Will there be an option to refresh them while the agent is still running?
There was a problem hiding this comment.
That won't be the case when JWKS is actually available - we'll fetch regularly. I was intending to fetch before every upload attempt.
I just did a one-off here because there's only one key that never changes!
wallrj-cyberark
left a comment
wallrj-cyberark
left a comment
There was a problem hiding this comment.
I ran the test script on my laptop and it worked. You've explained that the embedded public key is a temporary measure, but I think it would be nicer to instead load the public key from a file, because I wanted to try using my own key pair and decrypting the resulting output.json.
I did try creating my own key pair using step CLI, and pasting that public key into the code. But I wasn't able to decrypt the content in the output.json file. I'm probably missing something.
Here's what I tried
$ kind create cluster
$ kubectl create secret generic test-1 --from-literal=password=TOPSECRET
$ step crypto keypair pub.pem key.pem --no-password --insecure --kty RSA
# pasted content of `pub.pem` into `keys.go`
$ ./test.sh
=== Encrypted Secrets Example Test ===
Testing agent with Kubernetes secrets
✓ Connected to Kubernetes cluster
Context: kind-kind
Found 2 secrets in cluster
Running disco agent with encrypted secrets enabled...
Command: go run ../.. agent --agent-config-file config.yaml --one-shot --output-path output.json
I0205 19:35:13.405794 68893 run.go:59] "Starting" logger="Run" version="development" commit=""
I0205 19:35:13.406752 68893 run.go:117] "Healthz endpoints enabled" logger="Run.APIServer" addr=":8081" path="/healthz"
I0205 19:35:13.406839 68893 run.go:121] "Readyz endpoints enabled" logger="Run.APIServer" addr=":8081" path="/readyz"
I0205 19:35:13.406902 68893 run.go:290] "Pod event recorder disabled" logger="Run" reason="The agent does not appear to be running in a Kubernetes cluster." detail="When running in a Kubernetes cluster the following environment variables must be set: POD_NAME, POD_NODE, POD_UID, POD_NAMESPACE"
I0205 19:35:13.517566 68893 client_file.go:35] "Data saved to local file" outputPath="output.json"
I0205 19:35:13.517690 68893 run.go:440] "Data sent successfully" logger="postData"
✓ Agent completed successfully
✓ Output file created: output.json
$ jq -r '.[0].data.items[0].resource._encryptedData.data | @base64d' < output.json | step crypto jwe decrypt --key key.pem
error decrypting data: go-jose/go-jose: error in cryptographic primitive
Merge as-is if you prefer and ping me for reviews on follow up PRs.
3 participants
For now, this uses a hardcoded RSA key for which I threw away the private key, since we don't have the ability to pull JWKs yet. Once that's available, we'll be able to pull an RSA public key from the backend and use that for encryption.
This also includes a few test tweaks to help make this easier, and an example folder which produces an output.json showing how this can work.
Below is a single secret with encrypted data included, taken from a run of the example. The
_encryptedDatafield is currently a[]byteand so is automatically base64 encoded by Go's JSON marshaler - but the underlying JWE format is a string made up of concatenated base64 strings, so base64 encoding is slightly redundant here given we already have a JSON-compatible string.If we were to use a non-JWE format later (say HPKE with a custom encoding) then presumably that would need to be base64 encoded, so here I'm proposing a convention where the encrypted data field is always base64 encoded.
{ "resource": { "_encryptedData": { "data": "ZXlKaGJHY2lPaUpTVTBFdFQwRkZVQzB5TlRZaUxDSmxibU1pT2lKQk1qVTJSME5OSWl3aWEybGtJam9pUVRNNU56azRSVFl0T0VORk55MDBSVFpGTFRsRFJqWXRNalJCTTBNNU1qTkNNMEUzSW4wLmJQdkpfTTMyUkRqcUxNd21tU1o4UGFuUVRsYnNDbjR4RXpJZUFQWWNTalZBOEphUjFaSm9oNk1oQ3BiZUp0MVpzV0IwRXp4V2E2TE5rSDE3N1o0RmE3U2ZxYnJxaFRhQUFPekx4cHpWYk1hc1VxdVFLQmQyMHV6d3hmaXMtYU9TdWJ3eWM2MlhjTDc3SFJXSTBvVEgzLWdxcHM5X1RQRDNWY0g2R3pVeWpBa2JwcFpMUjFJeUlUVmcwNWFySGJYUHA1SnJlOWExTGQ4dkpIdjZaY1NwbDdsNTNwV3lSSGFrSDNBY2pfdFpIZlh2R3h1RC13STNLMV9ZaGVxc0JUYml6cERLMXhlUFE1emZkekFnY0pTOUhvRmdYeTVKNFhkaHBVbm1mOGYwdk9HZ2k0b2dYZUVFSWVDOXJyRnh4bEFvcDc5Q3YwbXBTRG1GWDNmT1lJalctdy5ralp2TzFhcWRyNUpJVGZFLncwcGNwN21sdTRieXlsVFY5SFFsdXlRX3ozVFI4VF9oNVY0V2RfeWtQSmtnS3c4dW02OGRMNV8yVE1ZRlVYTklVWkRkNU5vWmVuLUNEcEZ4VmtHM0xJcFlFeUpTUHVHY24yRXNsUzRBYjBCWWtCVDhBNkRSZTRvUnk3bHJRQ3RVQmxkSGU1b3pQR040VkVoMXVBajN1d2hLaXZ5Y29OTDl4N0g4Q0Zyc0swVGFMRm4tSWVqQnJNSFBQX1BaYzF6WW5Ucncza25uZmxGRENybi1nWGFJSERjeGsyUndTZVc3SmZqV19FRjFQYVJFM2R5RnpBakE3T2w2Zmk1QnRiWUZEWGJVQXVOclpqMFhTeVFUZ0NnelhBN1hHRmJfeGxRSkJsaldHR2xnZjJURS1DSElQcGtiN2tVemZDUEVkbmpIbWFYLTdPQ09LbVVndHZHNXpJSHJSQlBYVFp5bHduSmRYMkx0YXFKSHdyb2tNWVFKTUtuRGo0X1Z4dXJHcUJFdlpHVXBST2RzRWswMzlJXzRJMjZTeTZFNHN5MjhfLWZHS1hZcmMtT09wNFFqYTZZR2lsOWFBdEN0MVFOcUN4SDFzdU9IZUQ2WkpSbHgwajBXQ3VSc1lSdjQ1YzNHUzFaOHAtbkhfMGkzaXJZbzEtRFRHdElfYVpNbmdja3dCT05Da2JMQUFHTzdUNFdpVV9nOXZ6VldPSTc2X2dKOWlOOXdSZVlYWFd3dFhUblBqWVNlME9iSjh6NG10dDg1ZVBiS0lDd3N2MmZLZUV6Q0VIcDdrWWFMaDliYjM1Mm9ycHU5ZGZWODk3RW4wZmp5NFdrTnV1bWFtQkpVQXFtbE1EX0Y5cEFpbXc3Q2h1dzhGSzdmVGZ0NnhURVVXVXB3eDhYZkdtMk1fb1JJS0ZxZGxtdkVteTVwa1o5M3FheUhPSjJhdWZNTmlxWUd6VWNKMzNFcUZEMDhkZ1UwRk1UWGY0NDM1c2tQNllGakV3OFlqa0lldkNGZ1d3amYxa3lIMkVLeG1LNzJ0Wl9maVJmd1ZJNHdqY0tPUmZnXzVGbVpPd2xJQVM1b1RiVXZaM0UwZjdBY1RrX0pxLWY1UTZDU3VzQXhsM2NhelF1aGdhemQtTXpXRFBtcnNvVzRLcVJIWldScURPNVVROFMxYTRxVXpOUnBrT0NESlNPS2ZIWXE2UWRtbld3ZkMyd29rbUxvSU5oY3BrUzdSUWhYRFJ1enhuQU9qdUpWRktUTVdNanVyckJjelhETW5SR1EtUVN0ZXRra0RZeW1XajFZMlR3U3d2b1FPcUpoYXFPRXpOUEJTdUR4QzkxWkVPbEhPMDlqd056YnBTTDVBa1Q5YUpPUXBsWXZFYXh5dTNxdlFhNkRsLXFweUNBM2ZDRlJ1RWlsOWgycUNvMXdWYWFaTS1xR0JZOW1sSXFWUk5Uakh5WWlUV0d1MHV0VlowTFpfZUl2VTVTTWxUSFo5Qmpzd0c1Sk1CTWNBTzk5bGZaX0JGMXBtSjBqdkdCT0xmNUpoWi1KOVk2LU56U0ozVjdMSFhXbkFuWjlRMnRvT0N1Rzhrb3ZJb1FncnJ1LXl2R09oUXBkSWxRNmotcU9OYXhyYXNKZmZtd2xCVmdoZHNTam1BLV9mMjNvcDBRejFMTHZSekJGYWZCYlFmM1puV2FscHBmcHA5WHdUU2ZVZnhuMWFvVkw0NTV1SmlFaXltTUtOU0loSjFXYkF1SjV5MkU3eEMyal91NVdxWjk3cG9mZFg5UHUxMTBQS0t3Q05RTUEwX25BNjI1a1hIbjF3cDYzaU5PVkFmNThBcGxjRHFHMnZ3R1l4aGJIVlJoYzk5dDN6d3pUNjRrR3RtYktWSklZcDRtWHBuTzZhMUU2bWxxZ2hXTmJBUU9xRGZuSC10REdyU05fMS13M2UzdURmNXhVemxoN1JodlhNX3lteng3YV82aXZVTUZyVEduc0lPaE5UNC1pUUY0WDY2YTRWeW5tNXR1OXFWTDU0U1UzUWt4eTJNT1JpWDBZWXlGWXZpdTExbzRCM3dzN2lQR3RXYlBOZE5iLUtNZ3hLVGZqWUZGakh4WjFvZTFNcnZ4al94ZFdqQkRJbk12N3I1aFhNWWpNWVlqc01obnFlS052STlFOWJGYTJCWldjSml6WmlJWHphZjNxRUlPdENFa2FRRU5fdWNhUmFjR0lxM2J2VkxHVURhbnlKLV93MF9BSnlpUGg5SW5HSVBkclVZNHluWEhjQjAxdkpIcngwV21BWHJCcF9aVkZoZVc3dEdwS2FnSjZpWXNHb08ySXRCR0ZGeXM2aFNkbVpSajFJbklKV2ROSzYtajBIbm16N1BxaTdXVVVtcGtTb3I4T3dGWjl2RkdQcVJ1ZjFyUGhLOWZTOGM3WGVuTVBMcTFvLVpPZXljMzhKZUUyU2ZyZU80ZERSTjBMUTJ0TVQ0d3hfSExzLUJEX0FRSnNYa2Iybm1jdFBPenFuQjlVQjd2a2xfTzlLRHA0dTFTcG9oMlYtTnQ4Rzlsc3BpRDY3cHY4UW5Jc1BudFhaYUdQRkI0emZNR0FOMjBXa2JqSzNkX21YTldzanRidTIzWnhBaW14LW9IYW51SFplMHZLZUpwMWc1REVoOWpyYWZVV1NvTTJ3YS1lN3hsWFg4YWQ4VkQ5QnZxOVRjaXVxOUV3T2V3Vnc4QXJ2WFZFSjJmdWhKeXhUNVNzcWVQaVB0YmljQWNFYTNYTjZ0Wmp1VUV3NEpKaE9EN0x4Wk4tOHNwUnJEbTFtOFl1T1RtdDZpRC1STHNyVTBtTUdIOGFKZFBkVW05NGRDUnFGSmN2azBtS2FzUG9SYVk1ZFVjMUtheDNPV1hyMUo1alFDZ1c5X3VGbjZCRTZVbzlpcUNRZ1lzWVJrakRoVUNrb0t6YlU1ZEJTQVp4TTBGNzZyQzd0cnFadnN0bW9nLTZjMk1TaUUwVVNsamtXMzNia092SFF0QmxLcV91eWVJMk1NNjFVUGZVWmw1c1A0T1lub09xTWIzQ19XaDdtMHJJcjliS0ppVWdtcmxSNm16OXdtbk1XME1xYU5wXy1sTVdnUlpTSjlZbXhCcVc0OW80WUNEeWJKTXFDZGp0Vm04X3lESGZxdS0zR0RGS3FzXzJsdUV5aHl1N2EtYUZjSHl4MFF1a2ptTE1rZW9CRFpmQVY4c05GNmFxTUFmVVFEYjNMXzZLVjZuLTNmXzF6aFk2SW80Y0ozSmJLREZ6bE5ULTZRaXNzb0pWNGtkR1ZpQ09zZnBxTElLSjRCTnJfX0Zoa0dCSWcwYVFBRFZMck42cEFNX1ltMWJZLVF3LmV0cWxzSkNQSTNtSXd3MndyWlVpTXc=", "type": "JWE-RSA" }, "apiVersion": "v1", "data": { "ca.crt": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJaRENDQVFxZ0F3SUJBZ0lRQi9rbkhiS201amxLdHlMajRCTFhaREFLQmdncWhrak9QUVFEQWpBU01SQXcKRGdZRFZRUURFd2R5YjI5MExXTmhNQjRYRFRJMk1ESXdNekE0TkRRMU4xb1hEVEkyTURVd05EQTRORFExTjFvdwpFakVRTUE0R0ExVUVBeE1IY205dmRDMWpZVEJaTUJNR0J5cUdTTTQ5QWdFR0NDcUdTTTQ5QXdFSEEwSUFCT25NClhKZ0xrdVJRbEhZUTUzN0RZZktZWWRvTjNlWUh6dVRKajRMeWJoYTlUSlJPcjBlWVRRNmtxS3o1OEZTYTVuOVYKdVdTMUJBVWp0T3ZhZ3ZHb1VsQ2pRakJBTUE0R0ExVWREd0VCL3dRRUF3SUNwREFQQmdOVkhSTUJBZjhFQlRBRApBUUgvTUIwR0ExVWREZ1FXQkJRdmJONEc3L3lTMWdqeG1rbFM3VEQyc3FGK3REQUtCZ2dxaGtqT1BRUURBZ05JCkFEQkZBaUVBOU1VemtmbTcrSEwzejQxN1VRRHBGak9yTVJCNGQwNk5OekxHK2lUVnNzY0NJRHBRZFdEamllWFgKNHl4T3NNZk1TaVd6d1c1VnVOcnhEek9QdW5SRXBuamUKLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo=", "tls.crt": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUJrRENDQVRhZ0F3SUJBZ0lSQU0zeHNEcCtsQ0FsZFVNejlaYWt4R1F3Q2dZSUtvWkl6ajBFQXdJd0VqRVEKTUE0R0ExVUVBeE1IY205dmRDMWpZVEFlRncweU5qQXlNRE13T0RRME5UZGFGdzB5TmpBMU1EUXdPRFEwTlRkYQpNQnd4R2pBWUJnTlZCQU1URVdsdWRHVnliV1ZrYVdGMFpTMWpZUzB4TUZrd0V3WUhLb1pJemowQ0FRWUlLb1pJCnpqMERBUWNEUWdBRXU5M3dkaUNwaWJ0Ui83c2trbUVjSC8zNHpYWnR3bTNHQTgvaDhXOVlXdXo4TGF5a2xsVncKaUl3QU1nSkx5YXJvamZSTUk1SDdTNXNOTENXcXR0bUFiNk5qTUdFd0RnWURWUjBQQVFIL0JBUURBZ0trTUE4RwpBMVVkRXdFQi93UUZNQU1CQWY4d0hRWURWUjBPQkJZRUZPd3NVL0c1Y1ljRFlyVksyYWtJMzBpc0hndmFNQjhHCkExVWRJd1FZTUJhQUZDOXMzZ2J2L0pMV0NQR2FTVkx0TVBheW9YNjBNQW9HQ0NxR1NNNDlCQU1DQTBnQU1FVUMKSUVibFIxNjU2Q00yWlYzbEQ4alNsUXJIUWVsTW1FVG5sWGhhSVVhYnJuc1dBaUVBalhKZjR1YTA3blM1QXUzRApWaTZpVlZsRVc1aTlJZit4R3h1WFVEVE5JNFE9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K" }, "kind": "Secret", "metadata": { "annotations": { "cert-manager.io/alt-names": "", "cert-manager.io/certificate-name": "intermediate-ca-1", "cert-manager.io/common-name": "intermediate-ca-1", "cert-manager.io/ip-sans": "", "cert-manager.io/issuer-group": "cert-manager.io", "cert-manager.io/issuer-kind": "Issuer", "cert-manager.io/issuer-name": "root-ca-issuer", "cert-manager.io/uri-sans": "" }, "creationTimestamp": "2026-02-03T08:44:58Z", "labels": { "controller.cert-manager.io/fao": "true" }, "name": "intermediate-ca-1", "namespace": "sandbox", "resourceVersion": "12841", "uid": "b908c267-2397-428b-9fe7-0cf34a2d30af" }, "type": "kubernetes.io/tls" } }