[tarantool-patches] [PATCH 0/3] swim encryption preparation

[tarantool-patches] [PATCH 0/3] swim encryption preparation

[Vladislav Shpilevoy]

SWIM needs encryption because it transmits packets affecting cluster state and
topology, probably via public networks between datacenters. Tarantool hasn't had
normal crypto library with useful C API on board until now. OpenSSL was used,
but its API is crazy, and before this patchset it was used in Lua only, via FFI.

The patchset moves existing OpenSSL wrappers into a separate library and extends
it with pretty API. It is going to be used by SWIM.

Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-3234-swim-crypto
Issue: https://github.com/tarantool/tarantool/issues/3234

Vladislav Shpilevoy (3):
  crypto: move crypto business into a separate library
  crypto: make exported methods conform code style
  crypto: implement crypto codec API and AES 128 encryption

 extra/exports                 |  13 +-
 src/CMakeLists.txt            |   3 +-
 src/lib/CMakeLists.txt        |   1 +
 src/lib/core/diag.h           |   2 +
 src/lib/core/exception.cc     |  25 ++++
 src/lib/core/exception.h      |   7 +
 src/lib/crypto/CMakeLists.txt |   5 +
 src/lib/crypto/crypto.c       | 249 ++++++++++++++++++++++++++++++++++
 src/lib/crypto/crypto.h       | 135 ++++++++++++++++++
 src/lua/crypto.c              |  73 ----------
 src/lua/crypto.h              |  54 --------
 src/lua/crypto.lua            |  42 +++---
 src/main.cc                   |   3 +
 test/unit/CMakeLists.txt      |   3 +
 test/unit/crypto.c            | 153 +++++++++++++++++++++
 test/unit/crypto.result       |  32 +++++
 16 files changed, 642 insertions(+), 158 deletions(-)
 create mode 100644 src/lib/crypto/CMakeLists.txt
 create mode 100644 src/lib/crypto/crypto.c
 create mode 100644 src/lib/crypto/crypto.h
 delete mode 100644 src/lua/crypto.c
 delete mode 100644 src/lua/crypto.h
 create mode 100644 test/unit/crypto.c
 create mode 100644 test/unit/crypto.result

-- 
2.20.1 (Apple Git-117)

[tarantool-patches] [PATCH 1/3] crypto: move crypto business into a separate library

[Vladislav Shpilevoy]

Crypto in Tarantool core was implemented and used very poorly
uintil now. It was just a one tiny file with one-line wrappers
around OpenSSL API. Despite being small and simple, it provided a
poweful interface to the Lua land used by Lua 'crypto' public and
documented module.

Now the time comes when OpenSSL crypto features are wanted on
lower level and with richer API, in core library SWIM written
in C. This patch moves crypto wrappers into a separate library
in src/lib, and drops some methods from the header file because
they are never used from C, and needed for exporting only.

Needed for #3234
---
 src/CMakeLists.txt               |   3 +-
 src/lib/CMakeLists.txt           |   1 +
 src/lib/crypto/CMakeLists.txt    |   5 ++
 src/lib/crypto/crypto.c          | 103 +++++++++++++++++++++++++++++++
 src/{lua => lib/crypto}/crypto.h |  18 ++----
 src/lua/crypto.c                 |  73 ----------------------
 6 files changed, 114 insertions(+), 89 deletions(-)
 create mode 100644 src/lib/crypto/CMakeLists.txt
 create mode 100644 src/lib/crypto/crypto.c
 rename src/{lua => lib/crypto}/crypto.h (73%)
 delete mode 100644 src/lua/crypto.c

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 7c2395517..2cbbf0dcd 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -111,7 +111,6 @@ set (server_sources
      lua/socket.c
      lua/pickle.c
      lua/fio.c
-     lua/crypto.c
      lua/httpc.c
      lua/utf8.c
      lua/info.c
@@ -163,7 +162,7 @@ endif()
 set_source_files_compile_flags(${server_sources})
 add_library(server STATIC ${server_sources})
 target_link_libraries(server core coll http_parser bit uri uuid swim swim_udp
-                      swim_ev)
+                      swim_ev crypto)
 
 # Rule of thumb: if exporting a symbol from a static library, list the
 # library here.
diff --git a/src/lib/CMakeLists.txt b/src/lib/CMakeLists.txt
index b7179d04f..b306634e7 100644
--- a/src/lib/CMakeLists.txt
+++ b/src/lib/CMakeLists.txt
@@ -10,6 +10,7 @@ add_subdirectory(http_parser)
 add_subdirectory(core)
 add_subdirectory(uuid)
 add_subdirectory(coll)
+add_subdirectory(crypto)
 add_subdirectory(swim)
 if(ENABLE_BUNDLED_MSGPUCK)
     add_subdirectory(msgpuck EXCLUDE_FROM_ALL)
diff --git a/src/lib/crypto/CMakeLists.txt b/src/lib/crypto/CMakeLists.txt
new file mode 100644
index 000000000..7e2c6e1d3
--- /dev/null
+++ b/src/lib/crypto/CMakeLists.txt
@@ -0,0 +1,5 @@
+set(lib_sources crypto.c)
+
+set_source_files_compile_flags(${lib_sources})
+add_library(crypto STATIC ${lib_sources})
+target_link_libraries(crypto ${OPENSSL_LIBRARIES})
diff --git a/src/lib/crypto/crypto.c b/src/lib/crypto/crypto.c
new file mode 100644
index 000000000..28dbc71dd
--- /dev/null
+++ b/src/lib/crypto/crypto.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include "crypto.h"
+#include <openssl/crypto.h>
+#include <openssl/evp.h>
+#include <openssl/err.h>
+#include <openssl/ssl.h>
+#include <openssl/hmac.h>
+
+int
+tnt_openssl_init(void)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+	OpenSSL_add_all_digests();
+	OpenSSL_add_all_ciphers();
+	ERR_load_crypto_strings();
+#else
+	OPENSSL_init_crypto(0, NULL);
+	OPENSSL_init_ssl(0, NULL);
+#endif
+	return 0;
+}
+
+int tnt_EVP_CIPHER_key_length(const EVP_CIPHER *cipher)
+{
+	return EVP_CIPHER_key_length(cipher);
+}
+
+int tnt_EVP_CIPHER_iv_length(const EVP_CIPHER *cipher)
+{
+	return EVP_CIPHER_iv_length(cipher);
+}
+
+EVP_MD_CTX *tnt_EVP_MD_CTX_new(void)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+	return EVP_MD_CTX_create();
+#else
+	return EVP_MD_CTX_new();
+#endif
+};
+
+void tnt_EVP_MD_CTX_free(EVP_MD_CTX *ctx)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+	return EVP_MD_CTX_destroy(ctx);
+#else
+	return EVP_MD_CTX_free(ctx);
+#endif
+}
+
+HMAC_CTX *tnt_HMAC_CTX_new(void)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+	HMAC_CTX *ctx = (HMAC_CTX *)OPENSSL_malloc(sizeof(HMAC_CTX));
+	if(!ctx){
+		return NULL;
+	}
+	HMAC_CTX_init(ctx);
+	return ctx;
+#else
+	return HMAC_CTX_new();
+#endif
+
+}
+
+void tnt_HMAC_CTX_free(HMAC_CTX *ctx)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+	HMAC_cleanup(ctx); /* Remove key from memory */
+	OPENSSL_free(ctx);
+#else
+	HMAC_CTX_free(ctx);
+#endif
+}
diff --git a/src/lua/crypto.h b/src/lib/crypto/crypto.h
similarity index 73%
rename from src/lua/crypto.h
rename to src/lib/crypto/crypto.h
index 9808db1e5..9026db322 100644
--- a/src/lua/crypto.h
+++ b/src/lib/crypto/crypto.h
@@ -1,7 +1,7 @@
-#ifndef INCLUDES_TARANTOOL_LUA_CRYPTO_H
-#define INCLUDES_TARANTOOL_LUA_CRYPTO_H
+#ifndef TARANTOOL_LIB_CRYPTO_H_INCLUDED
+#define TARANTOOL_LIB_CRYPTO_H_INCLUDED
 /*
- * Copyright 2010-2015, Tarantool AUTHORS, please see AUTHORS file.
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
  *
  * Redistribution and use in source and binary forms, with or
  * without modification, are permitted provided that the following
@@ -31,24 +31,14 @@
  * SUCH DAMAGE.
  */
 
-#include <openssl/evp.h>
-#include <openssl/hmac.h>
-
 #if defined(__cplusplus)
 extern "C" {
 #endif
 
-int tnt_EVP_CIPHER_key_length(const EVP_CIPHER *cipher);
-int tnt_EVP_CIPHER_iv_length(const EVP_CIPHER *cipher);
 int tnt_openssl_init();
-EVP_MD_CTX *tnt_EVP_MD_CTX_new(void);
-void tnt_EVP_MD_CTX_free(EVP_MD_CTX *ctx);
-
-HMAC_CTX *tnt_HMAC_CTX_new(void);
-void tnt_HMAC_CTX_free(HMAC_CTX *ctx);
 
 #if defined(__cplusplus)
 }
 #endif
 
-#endif /* INCLUDES_TARANTOOL_LUA_CRYPTO_H */
+#endif /* TARANTOOL_LIB_CRYPTO_H_INCLUDED */
diff --git a/src/lua/crypto.c b/src/lua/crypto.c
deleted file mode 100644
index 80adaca78..000000000
--- a/src/lua/crypto.c
+++ /dev/null
@@ -1,73 +0,0 @@
-#include <openssl/crypto.h>
-#include <openssl/evp.h>
-#include <openssl/err.h>
-#include <openssl/ssl.h>
-
-/* Helper function for openssl init */
-int tnt_openssl_init()
-{
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
-	OpenSSL_add_all_digests();
-	OpenSSL_add_all_ciphers();
-	ERR_load_crypto_strings();
-#else
-	OPENSSL_init_crypto(0, NULL);
-	OPENSSL_init_ssl(0, NULL);
-#endif
-	return 0;
-}
-
-/* Helper functions for tarantool crypto api */
-
-int tnt_EVP_CIPHER_key_length(const EVP_CIPHER *cipher)
-{
-	return EVP_CIPHER_key_length(cipher);
-}
-
-int tnt_EVP_CIPHER_iv_length(const EVP_CIPHER *cipher)
-{
-	return EVP_CIPHER_iv_length(cipher);
-}
-
-EVP_MD_CTX *tnt_EVP_MD_CTX_new(void)
-{
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
-	return EVP_MD_CTX_create();
-#else
-	return EVP_MD_CTX_new();
-#endif
-};
-
-void tnt_EVP_MD_CTX_free(EVP_MD_CTX *ctx)
-{
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
-	return EVP_MD_CTX_destroy(ctx);
-#else
-	return EVP_MD_CTX_free(ctx);
-#endif
-}
-
-HMAC_CTX *tnt_HMAC_CTX_new(void)
-{
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
-	HMAC_CTX *ctx = (HMAC_CTX *)OPENSSL_malloc(sizeof(HMAC_CTX));
-	if(!ctx){
-		return NULL;
-	}
-	HMAC_CTX_init(ctx);
-	return ctx;
-#else
-	return HMAC_CTX_new();
-#endif
-
-}
-
-void tnt_HMAC_CTX_free(HMAC_CTX *ctx)
-{
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
-	HMAC_cleanup(ctx); /* Remove key from memory */
-	OPENSSL_free(ctx);
-#else
-	HMAC_CTX_free(ctx);
-#endif
-}
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] [PATCH 2/3] crypto: make exported methods conform code style

[Vladislav Shpilevoy]

Tarantool has a strict rule for naming methods of libraries - use
the library name as a prefix. For crypto lib methods it should be
'crypto_', not 'tnt_'.
---
 extra/exports           | 13 ++++++-------
 src/lib/crypto/crypto.c | 31 +++++++++++++++++++++---------
 src/lib/crypto/crypto.h |  6 +++++-
 src/lua/crypto.lua      | 42 ++++++++++++++++++++---------------------
 src/main.cc             |  3 +++
 5 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/extra/exports b/extra/exports
index 21fa9bf00..6fabc4d92 100644
--- a/extra/exports
+++ b/extra/exports
@@ -77,13 +77,12 @@ space_run_triggers
 space_bsize
 box_schema_version
 
-tnt_openssl_init
-tnt_EVP_CIPHER_key_length
-tnt_EVP_CIPHER_iv_length
-tnt_EVP_MD_CTX_new
-tnt_EVP_MD_CTX_free
-tnt_HMAC_CTX_new
-tnt_HMAC_CTX_free
+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
 
 # Module API
 
diff --git a/src/lib/crypto/crypto.c b/src/lib/crypto/crypto.c
index 28dbc71dd..4b192ba2d 100644
--- a/src/lib/crypto/crypto.c
+++ b/src/lib/crypto/crypto.c
@@ -35,8 +35,8 @@
 #include <openssl/ssl.h>
 #include <openssl/hmac.h>
 
-int
-tnt_openssl_init(void)
+void
+crypto_init(void)
 {
 #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
 	OpenSSL_add_all_digests();
@@ -46,20 +46,30 @@ tnt_openssl_init(void)
 	OPENSSL_init_crypto(0, NULL);
 	OPENSSL_init_ssl(0, NULL);
 #endif
-	return 0;
 }
 
-int tnt_EVP_CIPHER_key_length(const EVP_CIPHER *cipher)
+void
+crypto_free(void)
+{
+#ifdef OPENSSL_cleanup
+	OPENSSL_cleanup();
+#endif
+}
+
+int
+crypto_EVP_CIPHER_key_length(const EVP_CIPHER *cipher)
 {
 	return EVP_CIPHER_key_length(cipher);
 }
 
-int tnt_EVP_CIPHER_iv_length(const EVP_CIPHER *cipher)
+int
+crypto_EVP_CIPHER_iv_length(const EVP_CIPHER *cipher)
 {
 	return EVP_CIPHER_iv_length(cipher);
 }
 
-EVP_MD_CTX *tnt_EVP_MD_CTX_new(void)
+EVP_MD_CTX *
+crypto_EVP_MD_CTX_new(void)
 {
 #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
 	return EVP_MD_CTX_create();
@@ -68,7 +78,8 @@ EVP_MD_CTX *tnt_EVP_MD_CTX_new(void)
 #endif
 };
 
-void tnt_EVP_MD_CTX_free(EVP_MD_CTX *ctx)
+void
+crypto_EVP_MD_CTX_free(EVP_MD_CTX *ctx)
 {
 #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
 	return EVP_MD_CTX_destroy(ctx);
@@ -77,7 +88,8 @@ void tnt_EVP_MD_CTX_free(EVP_MD_CTX *ctx)
 #endif
 }
 
-HMAC_CTX *tnt_HMAC_CTX_new(void)
+HMAC_CTX *
+crypto_HMAC_CTX_new(void)
 {
 #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
 	HMAC_CTX *ctx = (HMAC_CTX *)OPENSSL_malloc(sizeof(HMAC_CTX));
@@ -92,7 +104,8 @@ HMAC_CTX *tnt_HMAC_CTX_new(void)
 
 }
 
-void tnt_HMAC_CTX_free(HMAC_CTX *ctx)
+void
+crypto_HMAC_CTX_free(HMAC_CTX *ctx)
 {
 #if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
 	HMAC_cleanup(ctx); /* Remove key from memory */
diff --git a/src/lib/crypto/crypto.h b/src/lib/crypto/crypto.h
index 9026db322..bc807e1a2 100644
--- a/src/lib/crypto/crypto.h
+++ b/src/lib/crypto/crypto.h
@@ -35,7 +35,11 @@
 extern "C" {
 #endif
 
-int tnt_openssl_init();
+void
+crypto_init(void);
+
+void
+crypto_free(void);
 
 #if defined(__cplusplus)
 }
diff --git a/src/lua/crypto.lua b/src/lua/crypto.lua
index e76370517..30504b1de 100644
--- a/src/lua/crypto.lua
+++ b/src/lua/crypto.lua
@@ -4,7 +4,6 @@ local ffi = require('ffi')
 local buffer = require('buffer')
 
 ffi.cdef[[
-    int tnt_openssl_init(void);
     /* from openssl/err.h */
     unsigned long ERR_get_error(void);
     char *ERR_error_string(unsigned long e, char *buf);
@@ -14,16 +13,16 @@ ffi.cdef[[
 
     typedef struct {} EVP_MD_CTX;
     typedef struct {} EVP_MD;
-    EVP_MD_CTX *tnt_EVP_MD_CTX_new(void);
-    void tnt_EVP_MD_CTX_free(EVP_MD_CTX *ctx);
+    EVP_MD_CTX *crypto_EVP_MD_CTX_new(void);
+    void crypto_EVP_MD_CTX_free(EVP_MD_CTX *ctx);
     int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl);
     int EVP_DigestUpdate(EVP_MD_CTX *ctx, const void *d, size_t cnt);
     int EVP_DigestFinal_ex(EVP_MD_CTX *ctx, unsigned char *md, unsigned int *s);
     const EVP_MD *EVP_get_digestbyname(const char *name);
 
     typedef struct {} HMAC_CTX;
-    HMAC_CTX *tnt_HMAC_CTX_new(void);
-    void tnt_HMAC_CTX_free(HMAC_CTX *ctx);
+    HMAC_CTX *crypto_HMAC_CTX_new(void);
+    void crypto_HMAC_CTX_free(HMAC_CTX *ctx);
     int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
                  const EVP_MD *md, ENGINE *impl);
     int HMAC_Update(HMAC_CTX *ctx, const unsigned char *data, size_t len);
@@ -40,17 +39,14 @@ ffi.cdef[[
     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 EVP_CIPHER_CTX_cleanup(EVP_CIPHER_CTX *ctx);
 
-    int tnt_EVP_CIPHER_iv_length(const EVP_CIPHER *cipher);
-    int tnt_EVP_CIPHER_key_length(const EVP_CIPHER *cipher);
+    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);
 ]]
 
-ffi.C.tnt_openssl_init();
-
 local function openssl_err_str()
   return ffi.string(ffi.C.ERR_error_string(ffi.C.ERR_get_error(), nil))
 end
@@ -70,11 +66,11 @@ end
 local digest_mt = {}
 
 local function digest_gc(ctx)
-    ffi.C.tnt_EVP_MD_CTX_free(ctx)
+    ffi.C.crypto_EVP_MD_CTX_free(ctx)
 end
 
 local function digest_new(digest)
-    local ctx = ffi.C.tnt_EVP_MD_CTX_new()
+    local ctx = ffi.C.crypto_EVP_MD_CTX_new()
     if ctx == nil then
         return error('Can\'t create digest ctx: ' .. openssl_err_str())
     end
@@ -121,7 +117,7 @@ local function digest_final(self)
 end
 
 local function digest_free(self)
-    ffi.C.tnt_EVP_MD_CTX_free(self.ctx)
+    ffi.C.crypto_EVP_MD_CTX_free(self.ctx)
     ffi.gc(self.ctx, nil)
     self.ctx = nil
     self.initialized = false
@@ -141,14 +137,14 @@ local hmacs = digests
 local hmac_mt = {}
 
 local function hmac_gc(ctx)
-    ffi.C.tnt_HMAC_CTX_free(ctx)
+    ffi.C.crypto_HMAC_CTX_free(ctx)
 end
 
 local function hmac_new(digest, key)
     if key == nil then
         return error('Key should be specified for HMAC operations')
     end
-    local ctx = ffi.C.tnt_HMAC_CTX_new()
+    local ctx = ffi.C.crypto_HMAC_CTX_new()
     if ctx == nil then
         return error('Can\'t create HMAC ctx: ' .. openssl_err_str())
     end
@@ -195,7 +191,7 @@ local function hmac_final(self)
 end
 
 local function hmac_free(self)
-    ffi.C.tnt_HMAC_CTX_free(self.ctx)
+    ffi.C.crypto_HMAC_CTX_free(self.ctx)
     ffi.gc(self.ctx, nil)
     self.ctx = nil
     self.initialized = false
@@ -234,13 +230,15 @@ 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)')
+    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
-    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)')
+    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()
     if ctx == nil then
diff --git a/src/main.cc b/src/main.cc
index 5ded83223..606c64c13 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -75,6 +75,7 @@
 #include "box/lua/init.h" /* box_lua_init() */
 #include "box/session.h"
 #include "systemd.h"
+#include "crypto/crypto.h"
 
 static pid_t master_pid = getpid();
 static struct pidfh *pid_file_handle;
@@ -621,6 +622,7 @@ tarantool_free(void)
 	memory_free();
 	random_free();
 #endif
+	crypto_free();
 	coll_free();
 	systemd_free();
 	say_logger_free();
@@ -780,6 +782,7 @@ main(int argc, char **argv)
 	signal_init();
 	cbus_init();
 	coll_init();
+	crypto_init();
 	tarantool_lua_init(tarantool_bin, main_argc, main_argv);
 
 	start_time = ev_monotonic_time();
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] [PATCH 3/3] crypto: implement crypto codec API and AES 128 encryption

[Vladislav Shpilevoy]

OpenSSL API is quite complex and hard to follow, additionally it
is very unstable. Encoding/decoding via OpenSSL methods usually
consists of multiple calls of a lot of functions. This patch
wraps OpenSSL API with one more easy to use and conforming
Tarantool code style in scope of crypto library.

The API provides struct crypto_codec which encapsulates all the
encryption logic and exposes quite simple API like this:

    crypto_codec_encode/decode(in, in_size, out, out_size)

A caller can create a needed codec via crypto_codec_new, which
now supports only AES 128 algorithm. The reason behind such a
choice is simple - it is needed in SWIM now.

The API is flexible in terms of extending and adding new
algorithms in future.

Needed for #3234
---
 src/lib/core/diag.h           |   2 +
 src/lib/core/exception.cc     |  25 ++++++
 src/lib/core/exception.h      |   7 ++
 src/lib/crypto/CMakeLists.txt |   2 +-
 src/lib/crypto/crypto.c       | 133 +++++++++++++++++++++++++++++
 src/lib/crypto/crypto.h       |  87 +++++++++++++++++++
 test/unit/CMakeLists.txt      |   3 +
 test/unit/crypto.c            | 153 ++++++++++++++++++++++++++++++++++
 test/unit/crypto.result       |  32 +++++++
 9 files changed, 443 insertions(+), 1 deletion(-)
 create mode 100644 test/unit/crypto.c
 create mode 100644 test/unit/crypto.result

diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h
index 78f0cdbdf..fd3831e66 100644
--- a/src/lib/core/diag.h
+++ b/src/lib/core/diag.h
@@ -251,6 +251,8 @@ struct error *
 BuildCollationError(const char *file, unsigned line, const char *format, ...);
 struct error *
 BuildSwimError(const char *file, unsigned line, const char *format, ...);
+struct error *
+BuildCryptoError(const char *file, unsigned line, const char *format, ...);
 
 struct index_def;
 
diff --git a/src/lib/core/exception.cc b/src/lib/core/exception.cc
index 6124c70d0..a6999af43 100644
--- a/src/lib/core/exception.cc
+++ b/src/lib/core/exception.cc
@@ -280,6 +280,19 @@ SwimError::SwimError(const char *file, unsigned line, const char *format, ...)
 	va_end(ap);
 }
 
+const struct type_info type_CryptoError =
+	make_type("CryptoError", &type_Exception);
+
+CryptoError::CryptoError(const char *file, unsigned line,
+			 const char *format, ...)
+	: Exception(&type_CryptoError, file, line)
+{
+	va_list ap;
+	va_start(ap, format);
+	error_vformat_msg(this, format, ap);
+	va_end(ap);
+}
+
 #define BuildAlloc(type)				\
 	void *p = malloc(sizeof(type));			\
 	if (p == NULL)					\
@@ -371,6 +384,18 @@ BuildSwimError(const char *file, unsigned line, const char *format, ...)
 	return e;
 }
 
+struct error *
+BuildCryptoError(const char *file, unsigned line, const char *format, ...)
+{
+	BuildAlloc(CryptoError);
+	CryptoError *e =  new (p) CryptoError(file, line, "");
+	va_list ap;
+	va_start(ap, format);
+	error_vformat_msg(e, format, ap);
+	va_end(ap);
+	return e;
+}
+
 struct error *
 BuildSocketError(const char *file, unsigned line, const char *socketname,
 		 const char *format, ...)
diff --git a/src/lib/core/exception.h b/src/lib/core/exception.h
index 4f5b66f2e..a29281427 100644
--- a/src/lib/core/exception.h
+++ b/src/lib/core/exception.h
@@ -51,6 +51,7 @@ extern const struct type_info type_IllegalParams;
 extern const struct type_info type_SystemError;
 extern const struct type_info type_CollationError;
 extern const struct type_info type_SwimError;
+extern const struct type_info type_CryptoError;
 
 const char *
 exception_get_string(struct error *e, const struct method_info *method);
@@ -166,6 +167,12 @@ public:
 	virtual void raise() { throw this; }
 };
 
+class CryptoError: public Exception {
+public:
+	CryptoError(const char *file, unsigned line, const char *format, ...);
+	virtual void raise() { throw this; }
+};
+
 /**
  * Initialize the exception subsystem.
  */
diff --git a/src/lib/crypto/CMakeLists.txt b/src/lib/crypto/CMakeLists.txt
index 7e2c6e1d3..4e2e5e403 100644
--- a/src/lib/crypto/CMakeLists.txt
+++ b/src/lib/crypto/CMakeLists.txt
@@ -2,4 +2,4 @@ set(lib_sources crypto.c)
 
 set_source_files_compile_flags(${lib_sources})
 add_library(crypto STATIC ${lib_sources})
-target_link_libraries(crypto ${OPENSSL_LIBRARIES})
+target_link_libraries(crypto ${OPENSSL_LIBRARIES} core)
diff --git a/src/lib/crypto/crypto.c b/src/lib/crypto/crypto.c
index 4b192ba2d..35e53e238 100644
--- a/src/lib/crypto/crypto.c
+++ b/src/lib/crypto/crypto.c
@@ -29,12 +29,145 @@
  * SUCH DAMAGE.
  */
 #include "crypto.h"
+#include "diag.h"
+#include "exception.h"
+#include "core/random.h"
 #include <openssl/crypto.h>
 #include <openssl/evp.h>
 #include <openssl/err.h>
 #include <openssl/ssl.h>
 #include <openssl/hmac.h>
 
+/** Set a diag error with the latest OpenSSL error message. */
+static inline void
+diag_set_OpenSSL(void)
+{
+	diag_set(CryptoError, "OpenSSL error: %s",
+		 ERR_error_string(ERR_get_error(), NULL));
+}
+
+/**
+ * OpenSSL codec. It contains only key, initial vector, and a
+ * constant cipher type, because OpenSSL does not allow
+ * long-living EVP_CIPHER_CTX contexts.
+ */
+struct crypto_codec {
+	/**
+	 * Secret key, usually is unchanged among multiple data
+	 * packets. Only AES 128 is supported now, so the buffer
+	 * is of 16 byte size.
+	 */
+	unsigned char key[CRYPTO_AES128_KEY_SIZE];
+	/**
+	 * Initial vector, and in fact a public key. It should be
+	 * regenerated randomly for each data packet.
+	 */
+	unsigned char iv[CRYPTO_AES_IV_SIZE];
+	/** Cipher type. Depends on algorithm and key size. */
+	const EVP_CIPHER *type;
+};
+
+struct crypto_codec *
+crypto_codec_new(enum crypto_algo algo, const char *key)
+{
+	if (algo != CRYPTO_AES128) {
+		diag_set(CryptoError, "Only AES 128 is supported");
+		return NULL;
+	}
+	struct crypto_codec *c = (struct crypto_codec *) malloc(sizeof(*c));
+	if (c == NULL) {
+		diag_set(OutOfMemory, sizeof(*c), "malloc", "c");
+		return NULL;
+	}
+	memcmp(c->key, key, sizeof(c->key));
+	memset(c->iv, 0, sizeof(c->iv));
+	c->type = EVP_aes_128_cbc();
+	return c;
+}
+
+const char *
+crypto_codec_gen_iv(struct crypto_codec *c)
+{
+	random_bytes((char *) c->iv, sizeof(c->iv));
+	return (char *) c->iv;
+}
+
+int
+crypto_codec_encode(struct crypto_codec *c, const char *in, int in_size,
+		    char *out, int out_size)
+{
+	const unsigned char *uin = (const unsigned char *) in;
+	unsigned char *uout = (unsigned char *) out;
+	/*
+	 * Note, that even if in_size is already multiple of block
+	 * size, additional block is still needed. Result is
+	 * always bigger than input.
+	 */
+	int len, result, need = in_size + CRYPTO_AES_BLOCK_SIZE -
+				(in_size % CRYPTO_AES_BLOCK_SIZE);
+	if (need > out_size)
+		return need;
+
+	EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();
+	if (ctx == NULL)
+		goto error;
+	if (EVP_EncryptInit_ex(ctx, c->type, NULL, c->key, c->iv) != 1)
+		goto error;
+	if (EVP_EncryptUpdate(ctx, uout, &len, uin, in_size) != 1)
+		goto error;
+	result = len;
+	assert(result <= need);
+	if (EVP_EncryptFinal_ex(ctx, uout + result, &len) != 1)
+		goto error;
+	result += len;
+	assert(result == need);
+	EVP_CIPHER_CTX_free(ctx);
+	return result;
+
+error:
+	diag_set_OpenSSL();
+	if (ctx != NULL)
+		EVP_CIPHER_CTX_free(ctx);
+	return -1;
+}
+
+int
+crypto_codec_decode(struct crypto_codec *c, const char *iv,
+		    const char *in, int in_size, char *out, int out_size)
+{
+	if (in_size > out_size)
+		return in_size;
+	const unsigned char *uin = (const unsigned char *) in;
+	const unsigned char *uiv = (const unsigned char *) iv;
+	unsigned char *uout = (unsigned char *) out;
+	int len, result;
+
+	EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();
+	if (ctx == NULL)
+		goto error;
+	if (EVP_DecryptInit_ex(ctx, c->type, NULL, c->key, uiv) != 1)
+		goto error;
+	if (EVP_DecryptUpdate(ctx, uout, &len, uin, in_size) != 1)
+		goto error;
+	result = len;
+	if (EVP_DecryptFinal_ex(ctx, uout + result, &len) != 1)
+		goto error;
+	EVP_CIPHER_CTX_free(ctx);
+	return result + len;
+
+error:
+	diag_set_OpenSSL();
+	if (ctx != NULL)
+		EVP_CIPHER_CTX_free(ctx);
+	return -1;
+}
+
+void
+crypto_codec_delete(struct crypto_codec *c)
+{
+	free(c);
+}
+
 void
 crypto_init(void)
 {
diff --git a/src/lib/crypto/crypto.h b/src/lib/crypto/crypto.h
index bc807e1a2..2c803dbf8 100644
--- a/src/lib/crypto/crypto.h
+++ b/src/lib/crypto/crypto.h
@@ -35,6 +35,93 @@
 extern "C" {
 #endif
 
+enum crypto_algo {
+	/**
+	 * AES - Advanced Encryption Standard. It is a symmetric
+	 * key block cipher algorithm. AES encodes data in blocks
+	 * of 128 bits, for what it uses a key and an initial
+	 * vector. The key is a secret information which needs to
+	 * be shared among communicating nodes. Key size can be
+	 * 128, 192 and 256 bits. Initial vector is an additional
+	 * entropy factor to prevent an attack when an attacker
+	 * somehow learns a purpose or content of one data packet
+	 * and immediately learns purpose of all the same looking
+	 * packets. Initial vector should be generated randomly
+	 * for each data packet, nonetheless it is not private and
+	 * can be sent without encryption.
+	 */
+	CRYPTO_AES128,
+};
+
+enum {
+	/**
+	 * Block size of AES operation. Encrypted data size is
+	 * always divisible by this value.
+	 */
+	CRYPTO_AES_BLOCK_SIZE = 16,
+	/**
+	 * Size of AES initial vector. It does not depend on key
+	 * size.
+	 */
+	CRYPTO_AES_IV_SIZE = 16,
+	/**
+	 * Obviously, 128 bits = 16 bytes always. It is just
+	 * syntax sugar so as not to hardcode 16 everywhere.
+	 */
+	CRYPTO_AES128_KEY_SIZE = 16,
+};
+
+struct crypto_codec;
+
+/**
+ * Create a new codec working with a specified @a algo algorithm
+ * and a secret @a key. Size of @a key depends on the chosen
+ * algorithm.
+ */
+struct crypto_codec *
+crypto_codec_new(enum crypto_algo algo, const char *key);
+
+/** Generate a new initial vector and return it. */
+const char *
+crypto_codec_gen_iv(struct crypto_codec *c);
+
+/**
+ * Encode plain data by codec @a c.
+ * @param c Codec.
+ * @param in Plain input data.
+ * @param in_size Byte size of @a in.
+ * @param out Output buffer.
+ * @param out_size Byte size of @a out.
+ *
+ * @retval -1 Error. Diag is set.
+ * @return Number of written bytes. If > @a out_size then nothing
+ *         is written, needed number of bytes is returned.
+ */
+int
+crypto_codec_encode(struct crypto_codec *c, const char *in, int in_size,
+		    char *out, int out_size);
+
+/**
+ * Decode cipher by codec @a c.
+ * @param c Codec.
+ * @param iv Initial vector used to encode @a in.
+ * @param in Cipher to decode.
+ * @param in_size Byte size of @a in.
+ * @param out Output buffer.
+ * @param out_size Byte size of @a out.
+ *
+ * @retval -1 Error. Diag is set.
+ * @return Number of written bytes. If > @a out_size then nothing
+ *         is written, needed number of bytes is returned.
+ */
+int
+crypto_codec_decode(struct crypto_codec *c, const char *iv,
+		    const char *in, int in_size, char *out, int out_size);
+
+/** Delete codec. */
+void
+crypto_codec_delete(struct crypto_codec *c);
+
 void
 crypto_init(void);
 
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index bb88b9c9b..368ceb649 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -213,6 +213,9 @@ target_link_libraries(checkpoint_schedule.test m unit)
 add_executable(sio.test sio.c)
 target_link_libraries(sio.test unit core)
 
+add_executable(crypto.test crypto.c)
+target_link_libraries(crypto.test crypto unit)
+
 add_executable(swim.test swim.c swim_test_transport.c swim_test_ev.c
                swim_test_utils.c ${PROJECT_SOURCE_DIR}/src/version.c)
 target_link_libraries(swim.test unit swim)
diff --git a/test/unit/crypto.c b/test/unit/crypto.c
new file mode 100644
index 000000000..c490e4f12
--- /dev/null
+++ b/test/unit/crypto.c
@@ -0,0 +1,153 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include "crypto/crypto.h"
+#include "core/random.h"
+#include "unit.h"
+#include "trivia/util.h"
+#include "memory.h"
+#include "fiber.h"
+
+static void
+test_aes128_codec(void)
+{
+	header();
+	plan(20);
+
+	char key[CRYPTO_AES128_KEY_SIZE];
+	random_bytes(key, sizeof(key));
+	struct crypto_codec *c = crypto_codec_new(-1, key);
+	is(c, NULL, "crypto does not support non-128 AES");
+	c = crypto_codec_new(CRYPTO_AES128, key);
+	isnt(c, NULL, "128 is supported");
+
+	int rc = crypto_codec_encode(c, NULL, 10, NULL, 0);
+	is(rc, 16, "encode returns needed number of bytes");
+	rc = crypto_codec_encode(c, NULL, 10, NULL, 15);
+	is(rc, 16, "encode does not write anything when too small "\
+	   "buffer");
+	rc = crypto_codec_encode(c, NULL, 0, NULL, 0);
+	is(rc, 16, "encode does not allow 0 sized buffer");
+	rc = crypto_codec_encode(c, NULL, 30, NULL, 0);
+	is(rc, 32, "encode rounds up by block size");
+	rc = crypto_codec_encode(c, NULL, 32, NULL, 0);
+	is(rc, 48, "encode requires additional block when buffer "\
+	   "size is multiple of block size");
+
+	const char *plain = "plain text";
+	int plain_size = strlen(plain) + 1;
+	char buffer1[128], buffer2[128];
+	int buffer_size = sizeof(buffer1);
+	const char *iv = crypto_codec_gen_iv(c);
+
+	rc = crypto_codec_encode(c, plain, plain_size, buffer1, buffer_size);
+	is(rc, 16, "encode works when buffer is big enough");
+	rc = crypto_codec_encode(c, plain, plain_size, buffer2, buffer_size);
+	is(rc, 16, "encode returns the same on second call");
+	is(memcmp(buffer1, buffer2, rc), 0, "encrypted data is the same");
+	isnt(memcmp(buffer1, plain, plain_size), 0,
+	     "and it is not just copied from the plain text");
+
+	rc = crypto_codec_decode(c, iv, NULL, 16, NULL, 0);
+	is(rc, 16, "decode also checks length and returns needed number "\
+	   "of bytes");
+	rc = crypto_codec_decode(c, iv, buffer1, 16, buffer2, buffer_size);
+	is(rc, plain_size, "decode returns correct number of bytes");
+	is(memcmp(buffer2, plain, plain_size), 0, "and correctly decodes data");
+
+	rc = crypto_codec_decode(c, "false iv not meaning anything",
+				 buffer1, 16, buffer2, buffer_size);
+	is(rc, -1, "decode can fail with wrong IV");
+	ok(! diag_is_empty(diag_get()), "diag error is set");
+
+	const char *iv2 = crypto_codec_gen_iv(c);
+	rc = crypto_codec_encode(c, plain, plain_size, buffer2, buffer_size);
+	is(rc, 16, "encode with different IV and the same number of written "\
+	   "bytes returned")
+	isnt(memcmp(buffer2, buffer1, rc), 0,
+	     "the encoded data looks different");
+	rc = crypto_codec_decode(c, iv2, buffer2, 16, buffer1, buffer_size);
+	is(rc, plain_size, "decode works with correct but another IV");
+	is(memcmp(buffer1, plain, plain_size), 0, "data is the same");
+
+	crypto_codec_delete(c);
+
+	check_plan();
+	footer();
+}
+
+static void
+test_aes128_stress(void)
+{
+	header();
+	plan(1);
+	char key[CRYPTO_AES128_KEY_SIZE];
+	random_bytes(key, sizeof(key));
+	struct crypto_codec *c = crypto_codec_new(CRYPTO_AES128, key);
+
+	char plain[515], cipher[1024], result[1024];
+	int rc, size = 10;
+	for (int size = 10; size < (int) sizeof(plain); size += 10) {
+		random_bytes(plain, size);
+		const char *iv = crypto_codec_gen_iv(c);
+		rc = crypto_codec_encode(c, plain, size,
+					 cipher, sizeof(cipher));
+		rc = crypto_codec_decode(c, iv, cipher, rc,
+					 result, sizeof(result));
+		fail_if(memcmp(result, plain, rc) != 0);
+	}
+	ok(true, "try encode/decode on a variety of sizes, keys, and ivs");
+
+	check_plan();
+	crypto_codec_delete(c);
+	footer();
+}
+
+int
+main(void)
+{
+	header();
+	plan(2);
+	random_init();
+	crypto_init();
+	memory_init();
+	fiber_init(fiber_c_invoke);
+
+	test_aes128_codec();
+	test_aes128_stress();
+
+	fiber_free();
+	memory_free();
+	crypto_free();
+	random_free();
+	int rc = check_plan();
+	footer();
+	return rc;
+}
diff --git a/test/unit/crypto.result b/test/unit/crypto.result
new file mode 100644
index 000000000..a3f479be8
--- /dev/null
+++ b/test/unit/crypto.result
@@ -0,0 +1,32 @@
+	*** main ***
+1..2
+	*** test_aes128_codec ***
+    1..20
+    ok 1 - crypto does not support non-128 AES
+    ok 2 - 128 is supported
+    ok 3 - encode returns needed number of bytes
+    ok 4 - encode does not write anything when too small buffer
+    ok 5 - encode does not allow 0 sized buffer
+    ok 6 - encode rounds up by block size
+    ok 7 - encode requires additional block when buffer size is multiple of block size
+    ok 8 - encode works when buffer is big enough
+    ok 9 - encode returns the same on second call
+    ok 10 - encrypted data is the same
+    ok 11 - and it is not just copied from the plain text
+    ok 12 - decode also checks length and returns needed number of bytes
+    ok 13 - decode returns correct number of bytes
+    ok 14 - and correctly decodes data
+    ok 15 - decode can fail with wrong IV
+    ok 16 - diag error is set
+    ok 17 - encode with different IV and the same number of written bytes returned
+    ok 18 - the encoded data looks different
+    ok 19 - decode works with correct but another IV
+    ok 20 - data is the same
+ok 1 - subtests
+	*** test_aes128_codec: done ***
+	*** test_aes128_stress ***
+    1..1
+    ok 1 - try encode/decode on a variety of sizes, keys, and ivs
+ok 2 - subtests
+	*** test_aes128_stress: done ***
+	*** main: done ***
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 3/3] crypto: implement crypto codec API and AES 128 encryption

[Vladislav Shpilevoy]

Found a typo. The fix is below and force pushed
on the branch:

====================================================================================
diff --git a/src/lib/crypto/crypto.c b/src/lib/crypto/crypto.c
index 35e53e238..557c92916 100644
--- a/src/lib/crypto/crypto.c
+++ b/src/lib/crypto/crypto.c
@@ -79,7 +79,7 @@ crypto_codec_new(enum crypto_algo algo, const char *key)
 		diag_set(OutOfMemory, sizeof(*c), "malloc", "c");
 		return NULL;
 	}
-	memcmp(c->key, key, sizeof(c->key));
+	memcpy(c->key, key, sizeof(c->key));
 	memset(c->iv, 0, sizeof(c->iv));
 	c->type = EVP_aes_128_cbc();
 	return c;
diff --git a/test/unit/crypto.c b/test/unit/crypto.c
index 4d51e21a2..1ea843664 100644
--- a/test/unit/crypto.c
+++ b/test/unit/crypto.c
@@ -39,7 +39,7 @@ static void
 test_aes128_codec(void)
 {
 	header();
-	plan(20);
+	plan(22);
 
 	char key[CRYPTO_AES128_KEY_SIZE];
 	random_bytes(key, sizeof(key));
@@ -99,6 +99,15 @@ test_aes128_codec(void)
 	is(rc, plain_size, "decode works with correct but another IV");
 	is(memcmp(buffer1, plain, plain_size), 0, "data is the same");
 
+	struct crypto_codec *c2 = crypto_codec_new(CRYPTO_AES128, key);
+	rc = crypto_codec_encode(c, plain, plain_size, buffer1, buffer_size);
+	memset(buffer2, 0, rc);
+	rc = crypto_codec_decode(c2, iv2, buffer1, rc, buffer2, buffer_size);
+	is(rc, plain_size, "encode with one codec, but decode with another "\
+	   "codec and the same key");
+	is(memcmp(plain, buffer2, plain_size), 0, "data is the same");
+
+	crypto_codec_delete(c2);
 	crypto_codec_delete(c);
 
 	check_plan();
diff --git a/test/unit/crypto.result b/test/unit/crypto.result
index a3f479be8..735105914 100644
--- a/test/unit/crypto.result
+++ b/test/unit/crypto.result
@@ -1,7 +1,7 @@
 	*** main ***
 1..2
 	*** test_aes128_codec ***
-    1..20
+    1..22
     ok 1 - crypto does not support non-128 AES
     ok 2 - 128 is supported
     ok 3 - encode returns needed number of bytes
@@ -22,6 +22,8 @@
     ok 18 - the encoded data looks different
     ok 19 - decode works with correct but another IV
     ok 20 - data is the same
+    ok 21 - encode with one codec, but decode with another codec and the same key
+    ok 22 - data is the same
 ok 1 - subtests
 	*** test_aes128_codec: done ***
 	*** test_aes128_stress ***
====================================================================================

[tarantool-patches] Re: [PATCH 3/3] crypto: implement crypto codec API and AES 128 encryption

[Vladislav Shpilevoy]

I've decided that the patch is too raw and reworked it a bit.
I force pushed my changed resent the whole patchset in a new
thread as V2.

Main change is addition of a second encryption algorithm - None.
It is needed because SWIM has encryption API like

    swim_set_codec(enum crypto_algo algo, const char *key);

And I need a way how to disable encryption after its enabling.
It can be done now via

    swim_set_codec(CRYPTO_NONE, NULLL);


Secondly, I renamed 'encode/decode' to 'encrypt/decrypt' in
all methods and tests.

Thirdly, I removed hardcoded block sizes from encrypt() method.

[tarantool-patches] Re: [PATCH 3/3] crypto: implement crypto codec API and AES 128 encryption

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/29 14:09]:
> I've decided that the patch is too raw and reworked it a bit.
> I force pushed my changed resent the whole patchset in a new
> thread as V2.
> 
> Main change is addition of a second encryption algorithm - None.
> It is needed because SWIM has encryption API like
> 
>     swim_set_codec(enum crypto_algo algo, const char *key);
> 
> And I need a way how to disable encryption after its enabling.
> It can be done now via
> 
>     swim_set_codec(CRYPTO_NONE, NULLL);
> 
> 
> Secondly, I renamed 'encode/decode' to 'encrypt/decrypt' in
> all methods and tests.
> 
> Thirdly, I removed hardcoded block sizes from encrypt() method.

All of the above is good stuff.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32