Skip to content

Commit

Permalink
cipher: do not reuse cipher ctx for certain operations (golang-fips#146)
Browse files Browse the repository at this point in the history 
Fixes golang-fips/go#187

Co-authored-by: Derek Parker <deparker@redhat.com>
(cherry picked from commit 61a53ab)
  • Loading branch information
@derekparker @xnox
derekparker authored and xnox committed Sep 2, 2024
1 parent fde4397 commit fb9c9e3
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 56 deletions.
12 changes: 2 additions & 10 deletions aes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,14 +421,6 @@ func TestDecryptInvariantReusableNonce(t *testing.T) {
testDecrypt(t, true)
}

func Test_aesCipher_finalize(t *testing.T) {
// Test that aesCipher.finalize does not panic if neither Encrypt nor Decrypt have been called.
// This test is important because aesCipher.finalize contains logic that is normally not exercided while testing.
// We can't used NewAESCipher here because the returned object will be automatically finalized by the GC
// in case test execution takes long enough, and it can't be finalized twice.
openssl.EVPCipherFinalize()
}

func TestCipherEncryptDecrypt(t *testing.T) {
key := []byte{0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6, 0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c}
pt := []byte{0x32, 0x43, 0xf6, 0xa8, 0x88, 0x5a, 0x30, 0x8d, 0x31, 0x31, 0x98, 0xa2, 0xe0, 0x37, 0x07, 0x34}
Expand Down Expand Up @@ -541,7 +533,7 @@ func BenchmarkAESGCM_Open(b *testing.B) {
b.ReportAllocs()
b.SetBytes(int64(len(buf)))

var key = make([]byte, keySize)
key := make([]byte, keySize)
var nonce [12]byte
var ad [13]byte
c, _ := openssl.NewAESCipher(key)
Expand All @@ -564,7 +556,7 @@ func BenchmarkAESGCM_Seal(b *testing.B) {
b.ReportAllocs()
b.SetBytes(int64(len(buf)))

var key = make([]byte, keySize)
key := make([]byte, keySize)
var nonce [12]byte
var ad [13]byte
c, _ := openssl.NewAESCipher(key)
Expand Down
71 changes: 29 additions & 42 deletions cipher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package openssl

// #include "goopenssl.h"
import "C"

import (
"crypto/cipher"
"encoding/binary"
Expand Down Expand Up @@ -145,8 +146,6 @@ func loadCipher(k cipherKind, mode cipherMode) (cipher C.GO_EVP_CIPHER_PTR) {

type evpCipher struct {
key []byte
enc_ctx C.GO_EVP_CIPHER_CTX_PTR
dec_ctx C.GO_EVP_CIPHER_CTX_PTR
kind cipherKind
blockSize int
}
Expand All @@ -159,19 +158,9 @@ func newEVPCipher(key []byte, kind cipherKind) (*evpCipher, error) {
c := &evpCipher{key: make([]byte, len(key)), kind: kind}
copy(c.key, key)
c.blockSize = int(C.go_openssl_EVP_CIPHER_get_block_size(cipher))
runtime.SetFinalizer(c, (*evpCipher).finalize)
return c, nil
}

func (c *evpCipher) finalize() {
if c.enc_ctx != nil {
C.go_openssl_EVP_CIPHER_CTX_free(c.enc_ctx)
}
if c.dec_ctx != nil {
C.go_openssl_EVP_CIPHER_CTX_free(c.dec_ctx)
}
}

func (c *evpCipher) encrypt(dst, src []byte) error {
if len(src) < c.blockSize {
return errors.New("input not full block")
Expand All @@ -184,15 +173,13 @@ func (c *evpCipher) encrypt(dst, src []byte) error {
if inexactOverlap(dst[:c.blockSize], src[:c.blockSize]) {
return errors.New("invalid buffer overlap")
}
if c.enc_ctx == nil {
var err error
c.enc_ctx, err = newCipherCtx(c.kind, cipherModeECB, cipherOpEncrypt, c.key, nil)
if err != nil {
return err
}
enc_ctx, err := newCipherCtx(c.kind, cipherModeECB, cipherOpEncrypt, c.key, nil)
if err != nil {
return err
}
defer C.go_openssl_EVP_CIPHER_CTX_free(enc_ctx)

if C.go_openssl_EVP_EncryptUpdate_wrapper(c.enc_ctx, base(dst), base(src), C.int(c.blockSize)) != 1 {
if C.go_openssl_EVP_EncryptUpdate_wrapper(enc_ctx, base(dst), base(src), C.int(c.blockSize)) != 1 {
return errors.New("EncryptUpdate failed")
}
runtime.KeepAlive(c)
Expand All @@ -211,18 +198,17 @@ func (c *evpCipher) decrypt(dst, src []byte) error {
if inexactOverlap(dst[:c.blockSize], src[:c.blockSize]) {
return errors.New("invalid buffer overlap")
}
if c.dec_ctx == nil {
var err error
c.dec_ctx, err = newCipherCtx(c.kind, cipherModeECB, cipherOpDecrypt, c.key, nil)
if err != nil {
return err
}
if C.go_openssl_EVP_CIPHER_CTX_set_padding(c.dec_ctx, 0) != 1 {
return errors.New("could not disable cipher padding")
}
dec_ctx, err := newCipherCtx(c.kind, cipherModeECB, cipherOpDecrypt, c.key, nil)
if err != nil {
return err
}
defer C.go_openssl_EVP_CIPHER_CTX_free(dec_ctx)

if C.go_openssl_EVP_CIPHER_CTX_set_padding(dec_ctx, 0) != 1 {
return errors.New("could not disable cipher padding")
}

C.go_openssl_EVP_DecryptUpdate_wrapper(c.dec_ctx, base(dst), base(src), C.int(c.blockSize))
C.go_openssl_EVP_DecryptUpdate_wrapper(dec_ctx, base(dst), base(src), C.int(c.blockSize))
runtime.KeepAlive(c)
return nil
}
Expand Down Expand Up @@ -321,7 +307,7 @@ const (
)

type cipherGCM struct {
ctx C.GO_EVP_CIPHER_CTX_PTR
c *evpCipher
tls cipherGCMTLS
// minNextNonce is the minimum value that the next nonce can be, enforced by
// all TLS modes.
Expand Down Expand Up @@ -379,19 +365,10 @@ func (c *evpCipher) newGCMChecked(nonceSize, tagSize int) (cipher.AEAD, error) {
}

func (c *evpCipher) newGCM(tls cipherGCMTLS) (cipher.AEAD, error) {
ctx, err := newCipherCtx(c.kind, cipherModeGCM, cipherOpNone, c.key, nil)
if err != nil {
return nil, err
}
g := &cipherGCM{ctx: ctx, tls: tls, blockSize: c.blockSize}
runtime.SetFinalizer(g, (*cipherGCM).finalize)
g := &cipherGCM{c: c, tls: tls, blockSize: c.blockSize}
return g, nil
}

func (g *cipherGCM) finalize() {
C.go_openssl_EVP_CIPHER_CTX_free(g.ctx)
}

func (g *cipherGCM) NonceSize() int {
return gcmStandardNonceSize
}
Expand Down Expand Up @@ -464,14 +441,19 @@ func (g *cipherGCM) Seal(dst, nonce, plaintext, additionalData []byte) []byte {
panic("cipher: invalid buffer overlap")
}

ctx, err := newCipherCtx(g.c.kind, cipherModeGCM, cipherOpNone, g.c.key, nil)
if err != nil {
panic(err)
}
defer C.go_openssl_EVP_CIPHER_CTX_free(ctx)
// Encrypt additional data.
// When sealing a TLS payload, OpenSSL app sets the additional data using
// 'EVP_CIPHER_CTX_ctrl(g.ctx, C.EVP_CTRL_AEAD_TLS1_AAD, C.EVP_AEAD_TLS1_AAD_LEN, base(additionalData))'.
// This makes the explicit nonce component to monotonically increase on every Seal operation without
// relying in the explicit nonce being securely set externally,
// and it also gives some interesting speed gains.
// Unfortunately we can't use it because Go expects AEAD.Seal to honor the provided nonce.
if C.go_openssl_EVP_CIPHER_CTX_seal_wrapper(g.ctx, base(out), base(nonce),
if C.go_openssl_EVP_CIPHER_CTX_seal_wrapper(ctx, base(out), base(nonce),
base(plaintext), C.int(len(plaintext)),
base(additionalData), C.int(len(additionalData))) != 1 {

Expand Down Expand Up @@ -506,8 +488,13 @@ func (g *cipherGCM) Open(dst, nonce, ciphertext, additionalData []byte) ([]byte,
panic("cipher: invalid buffer overlap")
}

ctx, err := newCipherCtx(g.c.kind, cipherModeGCM, cipherOpNone, g.c.key, nil)
if err != nil {
return nil, err
}
defer C.go_openssl_EVP_CIPHER_CTX_free(ctx)
ok := C.go_openssl_EVP_CIPHER_CTX_open_wrapper(
g.ctx, base(out), base(nonce),
ctx, base(out), base(nonce),
base(ciphertext), C.int(len(ciphertext)),
base(additionalData), C.int(len(additionalData)), base(tag))
runtime.KeepAlive(g)
Expand Down
5 changes: 1 addition & 4 deletions export_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
package openssl

var (
ErrOpen = errOpen
EVPCipherFinalize = new(evpCipher).finalize
)
var ErrOpen = errOpen

0 comments on commit fb9c9e3

Please sign in to comment.