-
Notifications
You must be signed in to change notification settings - Fork 24
Convert RSA envelope encryption to JWE #767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,13 @@ | ||
| // Package envelope provides types and interfaces for envelope encryption. | ||
| // | ||
| // Envelope encryption combines asymmetric and symmetric cryptography to | ||
| // efficiently encrypt data. The EncryptedData type holds the result, and | ||
| // the Encryptor interface defines the encryption operation. | ||
| // efficiently encrypt data. The Encryptor interface defines the encryption | ||
| // operation, returning data in JWE (JSON Web Encryption) format as defined | ||
| // in RFC 7516. | ||
| // | ||
| // Implementations are available in subpackages: | ||
| // | ||
| // - internal/envelope/rsa: RSA-OAEP + AES-256-GCM | ||
| // - internal/envelope/rsa: RSA-OAEP-256 + AES-256-GCM using JWE | ||
| // | ||
| // See subpackage documentation for usage examples. | ||
| package envelope |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,11 @@ | ||
| // Package rsa implements RSA envelope encryption, conforming to the interface in the envelope package. | ||
| // It uses RSA-OAEP with SHA-256 for key encryption, and AES-256-GCM for data encryption. | ||
| // Package rsa implements RSA envelope encryption using JWE (JSON Web Encryption) format. | ||
| // It conforms to the interface in the envelope package. | ||
| // | ||
| // The implementation uses: | ||
| // - RSA-OAEP-256 (RSA-OAEP with SHA-256) for key encryption | ||
| // - AES-256-GCM (A256GCM) for content encryption | ||
| // - JWE Compact Serialization format as defined in RFC 7516 | ||
| // | ||
| // The output is a JWE string with 5 base64url-encoded parts separated by dots: | ||
| // header.encryptedKey.iv.ciphertext.tag | ||
| package rsa |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,41 +1,37 @@ | ||
| package rsa | ||
|
|
||
| import ( | ||
| "crypto/aes" | ||
| "crypto/cipher" | ||
| "crypto/rand" | ||
| "crypto/rsa" | ||
| "crypto/sha256" | ||
| "fmt" | ||
|
|
||
| "github.com/lestrrat-go/jwx/v3/jwa" | ||
| "github.com/lestrrat-go/jwx/v3/jwe" | ||
|
|
||
| "github.com/jetstack/preflight/internal/envelope" | ||
| ) | ||
|
|
||
| const ( | ||
| // aesKeySize is the size of the AES-256 key in bytes; aes.NewCipher generates cipher.Block based | ||
| // on the size of key passed in, and 32 bytes corresponds to a 256-bit AES key | ||
| aesKeySize = 32 | ||
|
|
||
| // minRSAKeySize is the minimum RSA key size in bits; we'd expect that keys will be larger but 2048 is a sane floor | ||
| // to enforce to ensure that a weak key can't accidentally be used | ||
| minRSAKeySize = 2048 | ||
|
|
||
| // keyAlgorithmIdentifier is set in EncryptedData to identify the key wrapping algorithm used in this package | ||
| keyAlgorithmIdentifier = "RSA-OAEP-SHA256" | ||
| // EncryptionType is the type identifier for RSA JWE encryption | ||
| EncryptionType = "JWE-RSA" | ||
| ) | ||
|
|
||
| // Compile-time check that Encryptor implements envelope.Encryptor | ||
| var _ envelope.Encryptor = (*Encryptor)(nil) | ||
|
|
||
| // Encryptor provides envelope encryption using RSA for key wrapping | ||
| // and AES-256-GCM for data encryption. | ||
| // Encryptor provides envelope encryption using RSA-OAEP-256 for key wrapping | ||
| // and AES-256-GCM for data encryption, outputting JWE Compact Serialization format. | ||
| type Encryptor struct { | ||
| keyID string | ||
| rsaPublicKey *rsa.PublicKey | ||
| keyID string | ||
| publicKey *rsa.PublicKey | ||
| } | ||
|
|
||
| // NewEncryptor creates a new Encryptor with the provided RSA public key. | ||
| // The RSA key must be at least minRSAKeySize bits | ||
| // The RSA key must be at least minRSAKeySize bits. | ||
| // The encryptor will use RSA-OAEP-256 for key encryption and A256GCM for content encryption. | ||
| func NewEncryptor(keyID string, publicKey *rsa.PublicKey) (*Encryptor, error) { | ||
| if publicKey == nil { | ||
| return nil, fmt.Errorf("RSA public key cannot be nil") | ||
|
|
@@ -52,77 +48,39 @@ func NewEncryptor(keyID string, publicKey *rsa.PublicKey) (*Encryptor, error) { | |
| } | ||
|
|
||
| return &Encryptor{ | ||
| keyID: keyID, | ||
| rsaPublicKey: publicKey, | ||
| keyID: keyID, | ||
| publicKey: publicKey, | ||
| }, nil | ||
| } | ||
|
|
||
| // Encrypt performs envelope encryption on the provided data. | ||
| // It generates a random AES-256 key, encrypts the data with AES-256-GCM, | ||
| // then encrypts the AES key with RSA-OAEP-SHA256. | ||
| // It returns an EncryptedData struct containing JWE Compact Serialization format and type metadata. | ||
| // The JWE uses RSA-OAEP-256 for key encryption and A256GCM for content encryption. | ||
| func (e *Encryptor) Encrypt(data []byte) (*envelope.EncryptedData, error) { | ||
| if len(data) == 0 { | ||
| return nil, fmt.Errorf("data to encrypt cannot be empty") | ||
| } | ||
|
|
||
| aesKey := make([]byte, aesKeySize) | ||
| if _, err := rand.Read(aesKey); err != nil { | ||
| return nil, fmt.Errorf("failed to generate AES key: %w", err) | ||
| } | ||
|
|
||
| // zero the key from memory before the function returns | ||
| // TODO: in go1.26+, consider using secret.Do in this function | ||
| defer func() { | ||
| for i := range aesKey { | ||
| aesKey[i] = 0 | ||
| } | ||
| }() | ||
|
Comment on lines
-75
to
-79
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: we could generate the key outside of jwe.Encrypt and pass it in using This is what go1.26's secret.Do will be for. For now, having the jwe library generate the key means we never have to touch it in our code in plaintext. |
||
|
|
||
| block, err := aes.NewCipher(aesKey) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create AES cipher: %w", err) | ||
| } | ||
|
|
||
| gcm, err := cipher.NewGCM(block) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create GCM cipher: %w", err) | ||
| } | ||
|
|
||
| encryptedData := &envelope.EncryptedData{ | ||
| KeyID: e.keyID, | ||
| KeyAlgorithm: keyAlgorithmIdentifier, | ||
| EncryptedKey: nil, | ||
| EncryptedData: nil, | ||
| Nonce: make([]byte, gcm.NonceSize()), | ||
| } | ||
|
|
||
| // Generate a random nonce for AES-GCM. | ||
| // Security: Nonces must never be re-used for a given key. Since we generate a new AES key for each encryption, | ||
| // the risk of nonce reuse is not a concern here. | ||
| if _, err := rand.Read(encryptedData.Nonce); err != nil { | ||
| return nil, fmt.Errorf("failed to generate nonce: %w", err) | ||
| // Create headers with the key ID | ||
| headers := jwe.NewHeaders() | ||
| if err := headers.Set("kid", e.keyID); err != nil { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Worth adding a test case checking that the kid is correctly set in the JWE header? maybe not a big deal |
||
| return nil, fmt.Errorf("failed to set key ID header: %w", err) | ||
| } | ||
|
|
||
| // Seal encrypts and authenticates the data. This could include additional authenticated data, | ||
| // but we don't make use of that here. | ||
| // First nil: allocate new slice for output. | ||
| // Last nil: no additional authenticated data (AAD) needed. | ||
|
|
||
| encryptedData.EncryptedData = gcm.Seal(nil, encryptedData.Nonce, data, nil) | ||
|
|
||
| // Encrypt AES key with RSA-OAEP-SHA256. The nil parameter means no additional | ||
| // context data is mixed into the hash; this could be used to disambiguate different uses of the same key, | ||
| // but we only have one use for the key here. | ||
| encryptedData.EncryptedKey, err = rsa.EncryptOAEP( | ||
| sha256.New(), | ||
| rand.Reader, | ||
| e.rsaPublicKey, | ||
| aesKey, | ||
| nil, | ||
| // Encrypt using RSA-OAEP-256 for key algorithm and A256GCM for content encryption | ||
| // TODO: in go1.26+, consider using secret.Do to wrap this call, since it will generate an AES key | ||
| encrypted, err := jwe.Encrypt( | ||
| data, | ||
| jwe.WithKey(jwa.RSA_OAEP_256(), e.publicKey, jwe.WithPerRecipientHeaders(headers)), | ||
| jwe.WithContentEncryption(jwa.A256GCM()), | ||
| jwe.WithCompact(), | ||
|
Comment on lines
+72
to
+76
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From https://github.com/jetstack/jetstack-secure/pull/767/changes#r2753707291:
That makes sense. Is the memory zeroing an important concern? I've looked at the https://github.com/lestrrat-go/jwx lib and wasn't able to find mentions to zeroing, seems like it does, https://github.com/lestrrat-go/jwx/blob/1f715710fa563331b213e2dc81565e9f5e36ab2e/jwk/rsa.go#L105 suggests that it does but haven't dug.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly I think the incoming secret.Do shows that zeroing is mostly a bit of theater ATM. We need language level support (like secret.Do) to be confident that we're doing zeroing, or it's just a bit too error prone and there's too much scope for things to get copied and not zeroed. It's probably a solvable problem but I don't think it's worthwhile to spend that time! |
||
| ) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to encrypt AES key with RSA: %w", err) | ||
| return nil, fmt.Errorf("failed to encrypt data: %w", err) | ||
| } | ||
|
|
||
| return encryptedData, nil | ||
| return &envelope.EncryptedData{ | ||
| Data: encrypted, | ||
| Type: EncryptionType, | ||
| }, nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a single-person project (99% of commits come from them), but I guess I'm OK with the risk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a fair comment - I liked that there were plenty of external contributors even though they weren't contributing lots of code. I don't feel scared by it, personally!