From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id E5B662F1CA for ; Fri, 17 May 2019 07:24:29 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eoTTaAa8B-vh for ; Fri, 17 May 2019 07:24:29 -0400 (EDT) Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 030052F1C8 for ; Fri, 17 May 2019 07:24:28 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] [PATCH 1/1] crypto: fix assertion on cipher reinitialization Date: Fri, 17 May 2019 14:24:20 +0300 Message-Id: <73be47571ecdce078ad0a9d155653034c338c161.1558092222.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: georgy@tarantool.org 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:"]: Key length should be equal to cipher key length - (16 bytes)' +- error: 'builtin/crypto.lua:"]: 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:"]: Initial vector length should be equal to cipher - iv length (16 bytes)' +- error: 'builtin/crypto.lua:"]: 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:"]: Key length should be equal to cipher key length - (32 bytes)' +- error: 'builtin/crypto.lua:"]: 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:"]: Cipher not initialized' +... +crypto.cipher.aes256.cbc.decrypt('12', '12345678876543211234567887654321', '12') +--- - error: 'builtin/crypto.lua:"]: 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:"]: Key length should be equal to cipher key length + (24 bytes)' +... +crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678', '12345') --- - error: 'builtin/crypto.lua:"]: 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)