Skip to content
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

cipher: do not reuse cipher ctx for certain operations #146

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions aes_test.go
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
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
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
Loading