From 3a80db953da93c741ad391ae124121459c1046b0 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Tue, 7 Sep 2021 17:47:48 +0200 Subject: stupidgcm: allow zero-length input data We used to panic in this case because it is useless. But Go stdlib supports it, so we should as well. --- internal/stupidgcm/common_test.go | 40 +++++++++++++++++++++++++++++++++------ internal/stupidgcm/openssl.go | 21 +++++++++++++------- 2 files changed, 48 insertions(+), 13 deletions(-) (limited to 'internal') diff --git a/internal/stupidgcm/common_test.go b/internal/stupidgcm/common_test.go index 519efb0..10a7ce1 100644 --- a/internal/stupidgcm/common_test.go +++ b/internal/stupidgcm/common_test.go @@ -19,6 +19,8 @@ func testCiphers(t *testing.T, our cipher.AEAD, ref cipher.AEAD) { t.Run("testCorruption_c1", func(t *testing.T) { testCorruption(t, our) }) t.Run("testCorruption_c2", func(t *testing.T) { testCorruption(t, ref) }) t.Run("testConcurrency", func(t *testing.T) { testConcurrency(t, our, ref) }) + t.Run("testOpenAllZero_our", func(t *testing.T) { testOpenAllZero(t, our) }) + t.Run("testOpenAllZero_ref", func(t *testing.T) { testOpenAllZero(t, ref) }) t.Run("testWipe", func(t *testing.T) { testWipe(t, our) }) } @@ -37,9 +39,13 @@ func testEncryptDecrypt(t *testing.T, c1 cipher.AEAD, c2 cipher.AEAD) { dst := make([]byte, 71) // 71 = arbitrary length - // Check all block sizes from 1 to 5000 - for i := 1; i < 5000; i++ { - in := make([]byte, i) + // Check all block sizes from nil to 0 to 5000 + for i := -1; i < 5000; i++ { + // stays nil at i == -1 + var in []byte + if i >= 0 { + in = make([]byte, i) + } c1out := c1.Seal(dst, iv, in, authData) c2out := c2.Seal(dst, iv, in, authData) @@ -79,8 +85,8 @@ func testConcurrency(t *testing.T, c1 cipher.AEAD, c2 cipher.AEAD) { for i := 0; i < goroutineCount; i++ { wg.Add(1) go func() { + defer wg.Done() testEncryptDecrypt(t, c1, c2) - wg.Done() }() wg.Wait() } @@ -96,8 +102,9 @@ func testInplaceSeal(t *testing.T, c1 cipher.AEAD, c2 cipher.AEAD) { iv := randBytes(c1.NonceSize()) max := 5016 - // Check all block sizes from 1 to 5000 - for i := 1; i < max-16; i++ { + + // Check all block sizes from 0 to 5000 + for i := 0; i < max-16; i++ { in := make([]byte, i) dst := make([]byte, max-i) dst = dst[:16] @@ -185,6 +192,27 @@ func testCorruption(t *testing.T, c cipher.AEAD) { } } +// testOpenAllZero tests that we do not crash for nil or any block size of zeros +func testOpenAllZero(t *testing.T, c cipher.AEAD) { + authData := make([]byte, 24) + iv := make([]byte, c.NonceSize()) + + for i := -1; i < 5000; i++ { + // stays nil at i == -1 + var cipher []byte + if i >= 0 { + cipher = make([]byte, i) + } + plain, err := c.Open(nil, iv, cipher, authData) + if err == nil { + t.Error("should have gotten error, but did not") + } + if len(plain) > 0 { + t.Errorf("should not have received data, but got %d bytes", len(plain)) + } + } +} + func testWipe(t *testing.T, c cipher.AEAD) { switch c2 := c.(type) { case *StupidGCM: diff --git a/internal/stupidgcm/openssl.go b/internal/stupidgcm/openssl.go index 82ec0a1..ae0ee5c 100644 --- a/internal/stupidgcm/openssl.go +++ b/internal/stupidgcm/openssl.go @@ -20,9 +20,6 @@ func openSSLSeal(a *stupidAEADCommon, dst, iv, in, authData []byte) []byte { if len(iv) != a.NonceSize() { log.Panicf("Only %d-byte IVs are supported, you passed %d bytes", a.NonceSize(), len(iv)) } - if len(in) == 0 { - log.Panic("Zero-length input data is not supported") - } // If the "dst" slice is large enough we can use it as our output buffer outLen := len(in) + tagLen @@ -36,7 +33,7 @@ func openSSLSeal(a *stupidAEADCommon, dst, iv, in, authData []byte) []byte { } res := int(C.openssl_aead_seal(a.openSSLEVPCipher, - (*C.uchar)(&in[0]), + slicePointerOrNull(in), C.int(len(in)), (*C.uchar)(&authData[0]), C.int(len(authData)), @@ -64,7 +61,7 @@ func openSSLOpen(a *stupidAEADCommon, dst, iv, in, authData []byte) ([]byte, err if len(iv) != a.NonceSize() { log.Panicf("Only %d-byte IVs are supported, you passed %d bytes", a.NonceSize(), len(iv)) } - if len(in) <= tagLen { + if len(in) < tagLen { return nil, fmt.Errorf("stupidChacha20poly1305: input data too short (%d bytes)", len(in)) } @@ -83,7 +80,7 @@ func openSSLOpen(a *stupidAEADCommon, dst, iv, in, authData []byte) ([]byte, err tag := in[len(in)-tagLen:] res := int(C.openssl_aead_open(a.openSSLEVPCipher, - (*C.uchar)(&ciphertext[0]), + slicePointerOrNull(ciphertext), C.int(len(ciphertext)), (*C.uchar)(&authData[0]), C.int(len(authData)), @@ -93,7 +90,7 @@ func openSSLOpen(a *stupidAEADCommon, dst, iv, in, authData []byte) ([]byte, err C.int(len(a.key)), (*C.uchar)(&iv[0]), C.int(len(iv)), - (*C.uchar)(&buf[0]), + slicePointerOrNull(buf), C.int(len(buf)))) if res < 0 { @@ -109,6 +106,16 @@ func openSSLOpen(a *stupidAEADCommon, dst, iv, in, authData []byte) ([]byte, err return append(dst, buf...), nil } +// slicePointerOrNull returns a C pointer to the beginning of the byte slice, +// or NULL if the byte slice is empty. This is useful for slices that can be +// empty, otherwise you can directly use "(*C.uchar)(&s[0])". +func slicePointerOrNull(s []byte) (ptr *C.uchar) { + if len(s) == 0 { + return + } + return (*C.uchar)(&s[0]) +} + // This functions exists to benchmark the C call overhead from Go. // See BenchmarkCCall for resuts. func noopCFunction() { -- cgit v1.2.3