[tarantool-patches] [PATCH v3 4/4] crypto: use crypto library in crypto.lua

[Vladislav Shpilevoy <v.shpilevoy@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:<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')
 ---
-- error: 'builtin/crypto.lua:<line>"]: 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:<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')
 ---
-- error: 'builtin/crypto.lua:<line>"]: 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:<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')
 ---
-- error: 'builtin/crypto.lua:<line>"]: 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:<line>"]: Initial vector length should be equal to cipher
-    iv length (16 bytes)'
+- 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)'
+- 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:<line>"]: 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:<line>"]: 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)