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 663212E2A8 for ; Tue, 7 May 2019 16:54:04 -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 XVI-B3z5U5aL for ; Tue, 7 May 2019 16:54:04 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 E753A2E273 for ; Tue, 7 May 2019 16:54:03 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] [PATCH v3 4/4] crypto: use crypto library in crypto.lua Date: Tue, 7 May 2019 23:53:59 +0300 Message-Id: <470c7d23d93014f4c03bb7f1085a33cc8e249097.1557262174.git.v.shpilevoy@tarantool.org> In-Reply-To: References: 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: kostja@tarantool.org, georgy@tarantool.org crypto.lua is a public module using OpenSSL directly. But now lib/crypto encapsulates OpenSSL with additional checks and similar but more conforming API. It allows to replace OpenSSL cipher in crypto.lua with lib/crypto methods. --- extra/exports | 7 +- src/lib/crypto/crypto.c | 12 -- src/lua/crypto.lua | 241 +++++++++++++++++++++------------------ test/app/crypto.result | 114 +++++++++++++----- test/app/crypto.test.lua | 32 +++++- 5 files changed, 249 insertions(+), 157 deletions(-) diff --git a/extra/exports b/extra/exports index 562f8a164..269672fa4 100644 --- a/extra/exports +++ b/extra/exports @@ -77,12 +77,15 @@ space_run_triggers space_bsize box_schema_version -crypto_EVP_CIPHER_key_length -crypto_EVP_CIPHER_iv_length crypto_EVP_MD_CTX_new crypto_EVP_MD_CTX_free crypto_HMAC_CTX_new crypto_HMAC_CTX_free +crypto_stream_new +crypto_stream_begin +crypto_stream_append +crypto_stream_commit +crypto_stream_delete # Module API diff --git a/src/lib/crypto/crypto.c b/src/lib/crypto/crypto.c index c1c8c88ea..9b9d89b0b 100644 --- a/src/lib/crypto/crypto.c +++ b/src/lib/crypto/crypto.c @@ -343,18 +343,6 @@ crypto_free(void) #endif } -int -crypto_EVP_CIPHER_key_length(const EVP_CIPHER *cipher) -{ - return EVP_CIPHER_key_length(cipher); -} - -int -crypto_EVP_CIPHER_iv_length(const EVP_CIPHER *cipher) -{ - return EVP_CIPHER_iv_length(cipher); -} - EVP_MD_CTX * crypto_EVP_MD_CTX_new(void) { diff --git a/src/lua/crypto.lua b/src/lua/crypto.lua index 30504b1de..88f562b45 100644 --- a/src/lua/crypto.lua +++ b/src/lua/crypto.lua @@ -28,23 +28,45 @@ ffi.cdef[[ int HMAC_Update(HMAC_CTX *ctx, const unsigned char *data, size_t len); int HMAC_Final(HMAC_CTX *ctx, unsigned char *md, unsigned int *len); - typedef struct {} EVP_CIPHER_CTX; - typedef struct {} EVP_CIPHER; - EVP_CIPHER_CTX *EVP_CIPHER_CTX_new(); - void EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *ctx); - - int EVP_CipherInit_ex(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, - ENGINE *impl, const unsigned char *key, - const unsigned char *iv, int enc); - int EVP_CipherUpdate(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl, - const unsigned char *in, int inl); - int EVP_CipherFinal_ex(EVP_CIPHER_CTX *ctx, unsigned char *out, int *outl); - - int crypto_EVP_CIPHER_iv_length(const EVP_CIPHER *cipher); - int crypto_EVP_CIPHER_key_length(const EVP_CIPHER *cipher); - - int EVP_CIPHER_block_size(const EVP_CIPHER *cipher); - const EVP_CIPHER *EVP_get_cipherbyname(const char *name); + enum crypto_algo { + CRYPTO_ALGO_NONE, + CRYPTO_ALGO_AES128, + CRYPTO_ALGO_AES192, + CRYPTO_ALGO_AES256, + CRYPTO_ALGO_DES, + }; + + enum crypto_mode { + CRYPTO_MODE_ECB, + CRYPTO_MODE_CBC, + CRYPTO_MODE_CFB, + CRYPTO_MODE_OFB, + }; + + enum crypto_direction { + CRYPTO_DIR_DECRYPT = 0, + CRYPTO_DIR_ENCRYPT = 1, + }; + + struct crypto_stream; + + struct crypto_stream * + crypto_stream_new(enum crypto_algo algo, enum crypto_mode mode, + enum crypto_direction dir); + + int + crypto_stream_begin(struct crypto_stream *s, const char *key, int key_size, + const char *iv, int iv_size); + + int + crypto_stream_append(struct crypto_stream *s, const char *in, int in_size, + char *out, int out_size); + + int + crypto_stream_commit(struct crypto_stream *s, char *out, int out_size); + + void + crypto_stream_delete(struct crypto_stream *s); ]] local function openssl_err_str() @@ -206,110 +228,90 @@ hmac_mt = { } } -local ciphers = {} -for algo, algo_name in pairs({des = 'DES', aes128 = 'AES-128', - aes192 = 'AES-192', aes256 = 'AES-256'}) do - local algo_api = {} - for mode, mode_name in pairs({cfb = 'CFB', ofb = 'OFB', - cbc = 'CBC', ecb = 'ECB'}) do - local cipher = - ffi.C.EVP_get_cipherbyname(algo_name .. '-' .. mode_name) - if cipher ~= nil then - algo_api[mode] = cipher - end - end - if algo_api ~= {} then - ciphers[algo] = algo_api - end -end +local crypto_stream_mt = {} -local cipher_mt = {} - -local function cipher_gc(ctx) - ffi.C.EVP_CIPHER_CTX_free(ctx) +local function crypto_stream_gc(ctx) + ffi.C.crypto_stream_delete(ctx) end -local function cipher_new(cipher, key, iv, direction) - local needed_len = ffi.C.crypto_EVP_CIPHER_key_length(cipher) - if key == nil or key:len() ~= needed_len then - return error('Key length should be equal to cipher key length ('.. - tostring(needed_len)..' bytes)') - end - needed_len = ffi.C.crypto_EVP_CIPHER_iv_length(cipher) - if iv == nil or iv:len() ~= needed_len then - return error('Initial vector length should be equal to cipher iv '.. - 'length ('..tostring(needed_len)..' bytes)') - end - local ctx = ffi.C.EVP_CIPHER_CTX_new() +local function crypto_stream_new(algo, mode, key, iv, direction) + local ctx = ffi.C.crypto_stream_new(algo, mode, direction) if ctx == nil then - return error('Can\'t create cipher ctx: ' .. openssl_err_str()) + box.error() end - ffi.gc(ctx, cipher_gc) + ffi.gc(ctx, crypto_stream_gc) local self = setmetatable({ ctx = ctx, - cipher = cipher, - block_size = ffi.C.EVP_CIPHER_block_size(cipher), - direction = direction, buf = buffer.ibuf(), - initialized = false, - outl = ffi.new('int[1]') - }, cipher_mt) + is_initialized = false, + }, crypto_stream_mt) self:init(key, iv) return self end -local function cipher_init(self, key, iv) - if self.ctx == nil then +local function crypto_stream_begin(self, key, iv) + local ctx = self.ctx + if not ctx 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()) + self.key = key or self.key + self.iv = iv or self.iv + if self.key and self.iv then + if ffi.C.crypto_stream_begin(ctx, self.key, self.key:len(), + self.iv, self.iv:len()) ~= 0 then + box.error() + end + self.is_initialized = true end - self.initialized = true end -local function cipher_update(self, input) - if not self.initialized then +local function crypto_stream_append(self, input) + if not self.is_initialized then return error('Cipher not initialized') end if type(input) ~= 'string' then error("Usage: cipher:update(string)") end - local wpos = self.buf:reserve(input:len() + self.block_size - 1) - if ffi.C.EVP_CipherUpdate(self.ctx, wpos, self.outl, input, input:len()) ~= 1 then - return error('Can\'t update cipher:' .. openssl_err_str()) + local append = ffi.C.crypto_stream_append + local out_size = append(self.ctx, input, input:len(), nil, 0) + local wpos = self.buf:reserve(out_size) + out_size = append(self.ctx, input, input:len(), wpos, out_size) + if out_size < 0 then + box.error() end - return ffi.string(wpos, self.outl[0]) + return ffi.string(wpos, out_size) end -local function cipher_final(self) - if not self.initialized then +local function crypto_stream_commit(self) + if not self.is_initialized then return error('Cipher not initialized') end - self.initialized = false - local wpos = self.buf:reserve(self.block_size - 1) - if ffi.C.EVP_CipherFinal_ex(self.ctx, wpos, self.outl) ~= 1 then - return error('Can\'t finalize cipher:' .. openssl_err_str()) + local commit = ffi.C.crypto_stream_commit + local out_size = commit(self.ctx, nil, 0) + local wpos = self.buf:reserve(out_size) + out_size = commit(self.ctx, wpos, out_size) + if out_size < 0 then + box.error() end - self.initialized = false - return ffi.string(wpos, self.outl[0]) + self.is_initialized = false + return ffi.string(wpos, out_size) end -local function cipher_free(self) - ffi.C.EVP_CIPHER_CTX_free(self.ctx) +local function crypto_stream_free(self) ffi.gc(self.ctx, nil) + crypto_stream_gc(self.ctx) self.ctx = nil - self.initialized = false - self.buf:reset() + self.key = nil + self.iv = nil + self.is_initialized = false end -cipher_mt = { +crypto_stream_mt = { __index = { - init = cipher_init, - update = cipher_update, - result = cipher_final, - free = cipher_free + init = crypto_stream_begin, + update = crypto_stream_append, + result = crypto_stream_commit, + free = crypto_stream_free } } @@ -365,23 +367,51 @@ hmac_api = setmetatable(hmac_api, return error('HMAC method "' .. digest .. '" is not supported') end }) -local function cipher_mode_error(self, mode) - error('Cipher mode ' .. mode .. ' is not supported') -end +local crypto_algos = { + aes128 = ffi.C.CRYPTO_ALGO_AES128, + aes192 = ffi.C.CRYPTO_ALGO_AES192, + aes256 = ffi.C.CRYPTO_ALGO_AES256, + des = ffi.C.CRYPTO_ALGO_DES +} +local crypto_modes = { + ecb = ffi.C.CRYPTO_MODE_ECB, + cbc = ffi.C.CRYPTO_MODE_CBC, + cfb = ffi.C.CRYPTO_MODE_CFB, + ofb = ffi.C.CRYPTO_MODE_OFB +} +local crypto_dirs = { + encrypt = ffi.C.CRYPTO_DIR_ENCRYPT, + decrypt = ffi.C.CRYPTO_DIR_DECRYPT +} -local cipher_api = {} -for class, subclass in pairs(ciphers) do - local class_api = {} - for subclass, cipher in pairs(subclass) do - class_api[subclass] = {} - for direction, param in pairs({encrypt = 1, decrypt = 0}) do - class_api[subclass][direction] = setmetatable({ - new = function (key, iv) - return cipher_new(cipher, key, iv, param) +local algo_api_mt = { + __index = function(self, mode) + error('Cipher mode ' .. mode .. ' is not supported') + end +} +local crypto_api_mt = { + __index = function(self, cipher) + return error('Cipher method "' .. cipher .. '" is not supported') + end +} + +local crypto_api = setmetatable({}, crypto_api_mt) +for algo_name, algo_value in pairs(crypto_algos) do + local algo_api = setmetatable({}, algo_api_mt) + crypto_api[algo_name] = algo_api + for mode_name, mode_value in pairs(crypto_modes) do + local mode_api = {} + algo_api[mode_name] = mode_api + for dir_name, dir_value in pairs(crypto_dirs) do + mode_api[dir_name] = setmetatable({ + new = function(key, iv) + return crypto_stream_new(algo_value, mode_value, key, iv, + dir_value) end }, { - __call = function (self, str, key, iv) - local ctx = cipher_new(cipher, key, iv, param) + __call = function(self, str, key, iv) + local ctx = crypto_stream_new(algo_value, mode_value, key, + iv, dir_value) local res = ctx:update(str) res = res .. ctx:result() ctx:free() @@ -390,19 +420,10 @@ for class, subclass in pairs(ciphers) do }) end end - class_api = setmetatable(class_api, {__index = cipher_mode_error}) - if class_api ~= {} then - cipher_api[class] = class_api - end end -cipher_api = setmetatable(cipher_api, - {__index = function(self, cipher) - return error('Cipher method "' .. cipher .. '" is not supported') - end }) - return { digest = digest_api, hmac = hmac_api, - cipher = cipher_api, + cipher = crypto_api, } diff --git a/test/app/crypto.result b/test/app/crypto.result index 2d939b6fc..7bfb4d198 100644 --- a/test/app/crypto.result +++ b/test/app/crypto.result @@ -32,62 +32,102 @@ ciph.decrypt(enc, pass, iv) --- - test ... ---Failing scenaries +-- Failing scenarios. 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') --- -- error: 'builtin/crypto.lua:"]: Key length should be equal to cipher key length - (16 bytes)' +- error: key size expected 16, got 6 ... 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') --- -- error: 'builtin/crypto.lua:"]: Initial vector length should be equal to cipher - iv length (16 bytes)' +- error: IV size expected 16, got 2 ... 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') --- -- error: 'builtin/crypto.lua:"]: Key length should be equal to cipher key length - (32 bytes)' +- error: key size expected 32, got 6 ... crypto.cipher.aes256.cbc.decrypt('a', '12345678876543211234567887654321') --- -- 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.aes256.cbc.decrypt('12', '12345678876543211234567887654321', '12') --- -- error: 'builtin/crypto.lua:"]: Initial vector length should be equal to cipher - iv length (16 bytes)' +- error: IV size expected 16, got 2 ... -crypto.cipher.aes192.cbc.encrypt.new() +crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678', '12345') --- -- error: Key length should be equal to cipher key length (24 bytes) +- error: IV size expected 16, got 5 ... -crypto.cipher.aes192.cbc.encrypt.new('123321') +-- Set key after codec creation. +c = crypto.cipher.aes128.cbc.encrypt.new() --- -- error: Key length should be equal to cipher key length (24 bytes) ... -crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678') +key = '1234567812345678' --- -- error: Initial vector length should be equal to cipher iv length (16 bytes) ... -crypto.cipher.aes192.cbc.decrypt.new('123456788765432112345678', '12345') +iv = key +--- +... +c:init(key) +--- +... +c:update('plain') +--- +- error: Cipher not initialized +... +c:result() +--- +- error: Cipher not initialized +... +c:init(nil, iv) +--- +... +cipher = c:update('plain ') +--- +... +cipher = cipher..c:update('next plain') +--- +... +cipher = cipher..c:result() +--- +... +crypto.cipher.aes128.cbc.decrypt(cipher, key, iv) +--- +- plain next plain +... +-- Reuse. +key2 = '8765432187654321' --- -- error: Initial vector length should be equal to cipher iv length (16 bytes) +... +iv2 = key2 +--- +... +c:init(key2, iv2) +--- +... +cipher = c:update('new plain ') +--- +... +cipher = cipher..c:update('next new plain') +--- +... +cipher = cipher..c:result() +--- +... +crypto.cipher.aes128.cbc.decrypt(cipher, key2, iv2) +--- +- new plain next new plain ... crypto.cipher.aes100.efb --- @@ -103,6 +143,22 @@ crypto.digest.nodigest - error: '[string "return crypto.digest.nodigest "]:1: Digest method "nodigest" is not supported' ... +-- Check that GC really drops unused codecs and streams, and +-- nothing crashes. +weak = setmetatable({obj = c}, {__mode = 'v'}) +--- +... +c = nil +--- +... +collectgarbage('collect') +--- +- 0 +... +weak.obj +--- +- null +... bad_pass = '8765432112345678' --- ... @@ -111,13 +167,13 @@ bad_iv = '123456abcdefghij' ... ciph.decrypt(enc, bad_pass, iv) --- -- error: 'builtin/crypto.lua:"]: Can''t finalize cipher:error:06065064:digital envelope - routines:EVP_DecryptFinal_ex:bad decrypt' +- error: 'OpenSSL error: error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad + decrypt' ... ciph.decrypt(enc, pass, bad_iv) --- -- error: 'builtin/crypto.lua:"]: Can''t finalize cipher:error:06065064:digital envelope - routines:EVP_DecryptFinal_ex:bad decrypt' +- error: 'OpenSSL error: error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad + decrypt' ... test_run:cmd("clear filter") --- diff --git a/test/app/crypto.test.lua b/test/app/crypto.test.lua index 94f594bcc..16d8ce2a7 100644 --- a/test/app/crypto.test.lua +++ b/test/app/crypto.test.lua @@ -12,7 +12,7 @@ enc ciph.decrypt(enc, pass, iv) ---Failing scenaries +-- Failing scenarios. crypto.cipher.aes128.cbc.encrypt('a') crypto.cipher.aes128.cbc.encrypt('a', '123456', '435') crypto.cipher.aes128.cbc.encrypt('a', '1234567887654321') @@ -23,16 +23,40 @@ 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') +-- Set key after codec creation. +c = crypto.cipher.aes128.cbc.encrypt.new() +key = '1234567812345678' +iv = key +c:init(key) +c:update('plain') +c:result() +c:init(nil, iv) +cipher = c:update('plain ') +cipher = cipher..c:update('next plain') +cipher = cipher..c:result() +crypto.cipher.aes128.cbc.decrypt(cipher, key, iv) +-- Reuse. +key2 = '8765432187654321' +iv2 = key2 +c:init(key2, iv2) +cipher = c:update('new plain ') +cipher = cipher..c:update('next new plain') +cipher = cipher..c:result() +crypto.cipher.aes128.cbc.decrypt(cipher, key2, iv2) + crypto.cipher.aes100.efb crypto.cipher.aes256.nomode crypto.digest.nodigest +-- Check that GC really drops unused codecs and streams, and +-- nothing crashes. +weak = setmetatable({obj = c}, {__mode = 'v'}) +c = nil +collectgarbage('collect') +weak.obj bad_pass = '8765432112345678' bad_iv = '123456abcdefghij' -- 2.20.1 (Apple Git-117)