[tarantool-patches] [PATCH 1/1] crypto: fix assertion on cipher reinitialization
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri May 17 14:24:20 MSK 2019
Crypto provides API to create stream objects. These streams
consume plain and return ecnrypted data. Steps:
1 c = cipher.new([key, iv])
2 c:init(key, iv)
3 c:update(input)
4 c:result()
Step 2 is optional, if key and iv are specified in new(), but if
it called without key or iv, then result() method crashes.
The commit allows to fill key and iv gradually, in several init()
calls, and remembers previous results.
Closes #4223
---
Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4223-crypto-segfault
src/lua/crypto.lua | 37 +++++++++++++++----------
test/app/crypto.result | 60 ++++++++++++++++++++++++++++++----------
test/app/crypto.test.lua | 21 ++++++++++++--
3 files changed, 86 insertions(+), 32 deletions(-)
diff --git a/src/lua/crypto.lua b/src/lua/crypto.lua
index e76370517..9c5769bc7 100644
--- a/src/lua/crypto.lua
+++ b/src/lua/crypto.lua
@@ -234,14 +234,6 @@ local function cipher_gc(ctx)
end
local function cipher_new(cipher, key, iv, direction)
- if key == nil or key:len() ~= ffi.C.tnt_EVP_CIPHER_key_length(cipher) then
- return error('Key length should be equal to cipher key length ('
- .. tostring(ffi.C.tnt_EVP_CIPHER_key_length(cipher)) .. ' bytes)')
- end
- if iv == nil or iv:len() ~= ffi.C.tnt_EVP_CIPHER_iv_length(cipher) then
- return error('Initial vector length should be equal to cipher iv length ('
- .. tostring(ffi.C.tnt_EVP_CIPHER_iv_length(cipher)) .. ' bytes)')
- end
local ctx = ffi.C.EVP_CIPHER_CTX_new()
if ctx == nil then
return error('Can\'t create cipher ctx: ' .. openssl_err_str())
@@ -264,11 +256,28 @@ local function cipher_init(self, key, iv)
if self.ctx == nil then
return error('Cipher context isn\'t usable')
end
- if ffi.C.EVP_CipherInit_ex(self.ctx, self.cipher, nil,
- key, iv, self.direction) ~= 1 then
- return error('Can\'t init cipher:' .. openssl_err_str())
+ local cipher = self.cipher
+ key = key or self.key
+ iv = iv or self.iv
+ local needed = ffi.C.tnt_EVP_CIPHER_key_length(cipher)
+ if key ~= nil and key:len() ~= needed then
+ return error('Key length should be equal to cipher key length ('..
+ tostring(needed)..' bytes)')
+ end
+ needed = ffi.C.tnt_EVP_CIPHER_iv_length(cipher)
+ if iv ~= nil and iv:len() ~= needed then
+ return error('Initial vector length should be equal to cipher iv '..
+ 'length ('..tostring(needed)..' bytes)')
+ end
+ self.key = key
+ self.iv = iv
+ if key and iv then
+ if ffi.C.EVP_CipherInit_ex(self.ctx, cipher, nil, key, iv,
+ self.direction) ~= 1 then
+ return error('Can\'t init cipher:'..openssl_err_str())
+ end
+ self.initialized = true
end
- self.initialized = true
end
local function cipher_update(self, input)
@@ -289,12 +298,10 @@ local function cipher_final(self)
if not self.initialized then
return error('Cipher not initialized')
end
- self.initialized = false
- local wpos = self.buf:reserve(self.block_size - 1)
+ local wpos = self.buf:reserve(self.block_size)
if ffi.C.EVP_CipherFinal_ex(self.ctx, wpos, self.outl) ~= 1 then
return error('Can\'t finalize cipher:' .. openssl_err_str())
end
- self.initialized = false
return ffi.string(wpos, self.outl[0])
end
diff --git a/test/app/crypto.result b/test/app/crypto.result
index 2d939b6fc..300d8d5e0 100644
--- a/test/app/crypto.result
+++ b/test/app/crypto.result
@@ -35,8 +35,7 @@ ciph.decrypt(enc, pass, iv)
--Failing scenaries
crypto.cipher.aes128.cbc.encrypt('a')
---
-- error: 'builtin/crypto.lua:<line>"]: Key length should be equal to cipher key length
- (16 bytes)'
+- error: 'builtin/crypto.lua:<line>"]: Cipher not initialized'
...
crypto.cipher.aes128.cbc.encrypt('a', '123456', '435')
---
@@ -45,8 +44,7 @@ crypto.cipher.aes128.cbc.encrypt('a', '123456', '435')
...
crypto.cipher.aes128.cbc.encrypt('a', '1234567887654321')
---
-- error: 'builtin/crypto.lua:<line>"]: Initial vector length should be equal to cipher
- iv length (16 bytes)'
+- error: 'builtin/crypto.lua:<line>"]: Cipher not initialized'
...
crypto.cipher.aes128.cbc.encrypt('a', '1234567887654321', '12')
---
@@ -55,8 +53,7 @@ crypto.cipher.aes128.cbc.encrypt('a', '1234567887654321', '12')
...
crypto.cipher.aes256.cbc.decrypt('a')
---
-- error: 'builtin/crypto.lua:<line>"]: Key length should be equal to cipher key length
- (32 bytes)'
+- error: 'builtin/crypto.lua:<line>"]: Cipher not initialized'
...
crypto.cipher.aes256.cbc.decrypt('a', '123456', '435')
---
@@ -65,29 +62,62 @@ crypto.cipher.aes256.cbc.decrypt('a', '123456', '435')
...
crypto.cipher.aes256.cbc.decrypt('a', '12345678876543211234567887654321')
---
+- error: 'builtin/crypto.lua:<line>"]: Cipher not initialized'
+...
+crypto.cipher.aes256.cbc.decrypt('12', '12345678876543211234567887654321', '12')
+---
- error: 'builtin/crypto.lua:<line>"]: Initial vector length should be equal to cipher
iv length (16 bytes)'
...
-crypto.cipher.aes256.cbc.decrypt('12', '12345678876543211234567887654321', '12')
+crypto.cipher.aes192.cbc.encrypt.new('123321')
+---
+- error: 'builtin/crypto.lua:<line>"]: Key length should be equal to cipher key length
+ (24 bytes)'
+...
+crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678', '12345')
---
- error: 'builtin/crypto.lua:<line>"]: Initial vector length should be equal to cipher
iv length (16 bytes)'
...
-crypto.cipher.aes192.cbc.encrypt.new()
+--
+-- It is allowed to fill a cipher gradually.
+--
+c = crypto.cipher.aes128.cbc.encrypt.new()
---
-- error: Key length should be equal to cipher key length (24 bytes)
...
-crypto.cipher.aes192.cbc.encrypt.new('123321')
+c:init('1234567812345678')
---
-- error: Key length should be equal to cipher key length (24 bytes)
...
-crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678')
+c:update('plain')
---
-- error: Initial vector length should be equal to cipher iv length (16 bytes)
+- error: Cipher not initialized
...
-crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678', '12345')
+c:init(nil, '1234567812345678')
+---
+...
+c:update('plain')
+---
+-
+...
+c:free()
+---
+...
+--
+-- gh-4223: cipher:init() could rewrite previously initialized
+-- key and IV with nil values.
+--
+c = crypto.cipher.aes192.cbc.encrypt.new('123456789012345678901234', '1234567890123456')
+---
+...
+c:init()
+---
+...
+c:result()
+---
+- !!binary xzcDuRoi1sml7FB9IFf8EQ==
+...
+c:free()
---
-- error: Initial vector length should be equal to cipher iv length (16 bytes)
...
crypto.cipher.aes100.efb
---
diff --git a/test/app/crypto.test.lua b/test/app/crypto.test.lua
index 94f594bcc..2632346b9 100644
--- a/test/app/crypto.test.lua
+++ b/test/app/crypto.test.lua
@@ -23,11 +23,28 @@ crypto.cipher.aes256.cbc.decrypt('a', '123456', '435')
crypto.cipher.aes256.cbc.decrypt('a', '12345678876543211234567887654321')
crypto.cipher.aes256.cbc.decrypt('12', '12345678876543211234567887654321', '12')
-crypto.cipher.aes192.cbc.encrypt.new()
crypto.cipher.aes192.cbc.encrypt.new('123321')
-crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678')
crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678', '12345')
+--
+-- It is allowed to fill a cipher gradually.
+--
+c = crypto.cipher.aes128.cbc.encrypt.new()
+c:init('1234567812345678')
+c:update('plain')
+c:init(nil, '1234567812345678')
+c:update('plain')
+c:free()
+
+--
+-- gh-4223: cipher:init() could rewrite previously initialized
+-- key and IV with nil values.
+--
+c = crypto.cipher.aes192.cbc.encrypt.new('123456789012345678901234', '1234567890123456')
+c:init()
+c:result()
+c:free()
+
crypto.cipher.aes100.efb
crypto.cipher.aes256.nomode
--
2.20.1 (Apple Git-117)
More information about the Tarantool-patches
mailing list