Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH 1/1] crypto: fix assertion on cipher reinitialization
@ 2019-05-17 11:24 Vladislav Shpilevoy
  2019-05-20 13:53 ` [tarantool-patches] " Георгий Кириченко
  2019-05-21 15:25 ` Vladislav Shpilevoy
  0 siblings, 2 replies; 3+ messages in thread
From: Vladislav Shpilevoy @ 2019-05-17 11:24 UTC (permalink / raw)
  To: tarantool-patches; +Cc: georgy

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)

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-21 15:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-17 11:24 [tarantool-patches] [PATCH 1/1] crypto: fix assertion on cipher reinitialization Vladislav Shpilevoy
2019-05-20 13:53 ` [tarantool-patches] " Георгий Кириченко
2019-05-21 15:25 ` Vladislav Shpilevoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox