Add initial (unused) RSA envelope encryption #756

SgtCoDFish · 2026-01-19T13:31:30Z

Summary

Add RSA-based envelope encryption capabilities to secure secrets sent from disco-agent to the DisCo backend. This PR introduces the internal/envelope package, which provides envelope encryption primitives.

Motivation

The disco-agent needs to encrypt sensitive secrets before transmitting them to the DisCo backend. This PR provides the cryptographic building blocks to enable end-to-end encryption of secrets, ensuring that sensitive data is protected in transit and at rest.

Note: This encryption is specifically for disco-agent and not for venafi-kubernetes-agent although the code here is not used in either agent yet.

Changes

`internal/envelope` Package

Implements envelope encryption using RSA for key wrapping and AES-256-GCM for data encryption:

Encryptor (internal/envelope/encryptor.go:24-91):
- Generates random AES-256 key for each encryption operation
- Encrypts data with AES-256-GCM (authenticated encryption)
- Wraps AES key using RSA-OAEP-SHA256
- Minimum 2048-bit RSA key requirement for security
Key Management (internal/envelope/keys.go:14-57):
- Helpers for loading RSA public keys from PEM format
- Support for both PKIX (PUBLIC KEY) and PKCS1 (RSA PUBLIC KEY) formats
- File-based and byte-based key loading
Data Structures (internal/envelope/types.go:1-23):
- EncryptedData containing encrypted key, encrypted data, and nonce
- Clean separation of encryption components for transmission

Dependencies

No new external dependencies required - uses Go standard library crypto packages.

Technical Details

Envelope Encryption Flow

Generate random AES-256 key
Generate random 12-byte nonce
Encrypt plaintext using AES-256-GCM with the AES key → ciphertext
Encrypt AES key using RSA-OAEP-SHA256 with RSA public key → encrypted key
Return {encryptedKey, encryptedData, nonce}

Design Choices

AES-256-GCM: Industry-standard authenticated encryption
RSA-OAEP-SHA256: Secure RSA encryption padding with SHA-256
32-byte AES keys: AES-256 for strong symmetric encryption
12-byte nonces: Standard GCM nonce size.
- NB: Nonce sizes can be security critical. Reusing a nonce with the same key breaks AES-256 GCM completely.
- Due to the birthday paradox, the risk of reusing (randomly-generated) nonces can be quite high with 12-byte nonces
- But this is not an issue for us - we generate a new AES key and a new nonce for each upload
2048-bit minimum RSA key size: Industry standard. We ultimately will get a key from DisCo so we don't choose the size.

Testing

Run tests:

go test ./internal/envelope/...

Next Steps

Integration with disco-agent to use these encryption primitives when sending secrets
Key distribution mechanism (retrieving RSA public keys from DisCo endpoint)

This commit adds support for envelope encryption using an RSA key. Note: This encryption is specifically for disco-agent and not for venafi-kubernetes-agent although the code here is not used in either agent yet. Signed-off-by: Ashley Davis <[email protected]>

maelvls · 2026-01-20T15:51:14Z

I'm surprised we are relying on RSA for encryption rather than something faster and more robust (EC based like ECDH?). Is that because the DisCo backend is old or something?

I thought the proper way to do hybrid public key encryption would be to use Go 1.26's ML-KEM support? Is that because the lib is too bleeding edge? The RFC is from 2022, I imagine we would be OK using it...

Sorry about the dumb questions, I haven't looked at any of the design docs or anything related to this feature 😅

SgtCoDFish · 2026-01-20T15:58:55Z

I'm surprised we are relying on RSA for encryption rather than something faster and more robust (EC based like ECDH?). Is that because the DisCo backend is old or something?

I thought the proper way to do hybrid public key encryption would be to use Go 1.26's ML-KEM support? Is that because the lib is too bleeding edge? The RFC is from 2022, I imagine we would be OK using it...

The agent is ultimately going to have to react to whatever public key it gets from the server. I don't think the backend is old - I think RSA is just the "first thing" getting implemented. There are discussions ongoing around what this looks like longer term, but RSA is secure enough today and the amount of data being encrypted with RSA is so small that performance shouldn't be a concern.

I thought the proper way to do hybrid public key encryption would be to use Go 1.26's ML-KEM support? Is that because the lib is too bleeding edge? The RFC is from 2022, I imagine we would be OK using it...

The gold standard today would be to use HPKE, which go will support in its stdlib from 1.26. We could absolutely use that instead, but the initial decision for now is RSA.

wallrj-cyberark

Thanks @SgtCoDFish

It looks good, but I'd like a bit more documentation with references to the principles and best-practices of Envelope encryption. Perhaps a doc.go. I'm no crypto expert.

I'm happy to review followup PRs as you develop this further.

Here's the copilot review I referred to, above:

https://gist.github.com/wallrj-cyberark/93b875cd5ebc646912e1bc27eeb642e4#151702-copilotgpt-5-mini

wallrj-cyberark · 2026-01-20T15:45:22Z

internal/envelope/encryptor.go

+	if err != nil {
+		return nil, fmt.Errorf("failed to encrypt AES key with RSA: %w", err)
+	}
+


Copilot suggested zeroing the aesKey from memory, once it has been encrypted.

:) Ash is waiting for the secret.Do() to become available in order to avoid manually zeroing the memory.

wallrj-cyberark · 2026-01-20T16:11:02Z

internal/envelope/encryptor.go

+		return nil, fmt.Errorf("failed to generate nonce: %w", err)
+	}
+
+	encryptedData.EncryptedData = gcm.Seal(nil, encryptedData.Nonce, data, nil)


It'd help me, as a maintainer, to know what these nil parameters are. Maybe add a comment about why they're not needed here.
I found another library, called tink that does use supply these, but I don't really understand what I'm looking at:

https://github.com/tink-crypto/tink-go/blob/main/aead/xaesgcm/aead.go#L110-L117

wallrj-cyberark · 2026-01-20T16:15:04Z

internal/envelope/encryptor.go

+	aesKey := make([]byte, aesKeySize)
+	if _, err := rand.Read(aesKey); err != nil {
+		return nil, fmt.Errorf("failed to generate AES key: %w", err)
+	}


Is this sometimes known as DEK (Data Encryption Key)? Is that a standard term? Would it be appropriate to use that standard terminology here, with reference to some open blueprint for envelope encryption:

https://docs.cloud.google.com/kms/docs/envelope-encryption

That doc also talks about KEK Key Encryption Key, which I guess corresponds to the Encryptor.rsaPublicKey

Yeah those are standard terms and we could use them for sure. Even if I just add a comment linking the terminology up, that would improve it. I'll have a think!

wallrj-cyberark · 2026-01-20T16:17:47Z

internal/envelope/encryptor.go

+		rand.Reader,
+		e.rsaPublicKey,
+		aesKey,
+		nil,


Maybe add a comment about this nil parameter too.

wallrj-cyberark · 2026-01-20T16:25:48Z

internal/envelope/types.go

+	// Nonce is the 12-byte nonce used for AES-GCM encryption.
+	// This is intentionally plaintext.
+	Nonce []byte `json:"nonce"`
+}


This is serializable to JSON, but does the EncryptedKey, EncryptedData, and nonce need to be base64 encoded first?
Maybe add a test to demonstrate that the json serialization works.

stdlib encoding/json converts byte slices to b64 by default, so I don't think there's too much value in explicitly testing the serialization

From go doc json Marshal:

Array and slice values encode as JSON arrays, except that []byte encodes as
a base64-encoded string, and a nil slice encodes as the null JSON value.

wallrj-cyberark · 2026-01-20T16:29:17Z

internal/envelope/encryptor.go

+	// Due to the birthday paradox, the risk of reusing (randomly-generated) nonces can be quite high.
+	// This package is assumed to be used in contexts where a new key is generated for each encryption operation,
+	// so the nonce size doesn't matter.
+	nonceSize = 12


Copilot suggests

use gcm.NonceSize() rather than a package-level nonceSize constant

Ahh, good catch!

SgtCoDFish force-pushed the envelope branch 2 times, most recently from c4f2051 to ef671fe Compare January 20, 2026 14:39

SgtCoDFish force-pushed the envelope branch from ef671fe to 8f98e3a Compare January 20, 2026 14:46

SgtCoDFish changed the title ~~wip: envelope~~ Add initial (unused) RSA envelope encryption Jan 20, 2026

wallrj-cyberark requested changes Jan 20, 2026

View reviewed changes

Add initial (unused) RSA envelope encryption #756

Are you sure you want to change the base?

Add initial (unused) RSA envelope encryption #756

Conversation

SgtCoDFish commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

internal/envelope Package

Dependencies

Technical Details

Envelope Encryption Flow

Design Choices

Testing

Next Steps

Uh oh!

maelvls commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SgtCoDFish commented Jan 20, 2026

Uh oh!

wallrj-cyberark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SgtCoDFish commented Jan 19, 2026 •

edited

Loading

`internal/envelope` Package

maelvls commented Jan 20, 2026 •

edited

Loading