From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org Cc: georgy@tarantool.org Subject: [tarantool-patches] [PATCH 1/1] crypto: fix assertion on cipher reinitialization Date: Fri, 17 May 2019 14:24:20 +0300 [thread overview] Message-ID: <73be47571ecdce078ad0a9d155653034c338c161.1558092222.git.v.shpilevoy@tarantool.org> (raw) 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)
next reply other threads:[~2019-05-17 11:24 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-17 11:24 Vladislav Shpilevoy [this message] 2019-05-20 13:53 ` [tarantool-patches] " Георгий Кириченко 2019-05-21 15:25 ` Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=73be47571ecdce078ad0a9d155653034c338c161.1558092222.git.v.shpilevoy@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=georgy@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] [PATCH 1/1] crypto: fix assertion on cipher reinitialization' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox