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

[tarantool-patches] [PATCH v2 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/crypto-lib

Changes in V2:
- Added new codec 'None';
- Renamed 'encode/decode' to 'encrypt/decrypt';
- Removed usage of constants from crypto.c.

V1: https://www.freelists.org/post/tarantool-patches/PATCH-03-swim-encryption-preparation

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       | 260 ++++++++++++++++++++++++++++++++++
 src/lib/crypto/crypto.h       | 142 +++++++++++++++++++
 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            | 191 +++++++++++++++++++++++++
 test/unit/crypto.result       |  40 ++++++
 16 files changed, 706 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 v2 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 v2 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 v2 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       | 144 +++++++++++++++++++++++++
 src/lib/crypto/crypto.h       |  94 +++++++++++++++++
 test/unit/CMakeLists.txt      |   3 +
 test/unit/crypto.c            | 191 ++++++++++++++++++++++++++++++++++
 test/unit/crypto.result       |  40 +++++++
 9 files changed, 507 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..a96963d6f 100644
--- a/src/lib/crypto/crypto.c
+++ b/src/lib/crypto/crypto.c
@@ -29,12 +29,156 @@
  * 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];
+	/** Size of block by which the algorithm operates. */
+	uint8_t block_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)
+{
+	struct crypto_codec *c = (struct crypto_codec *) malloc(sizeof(*c));
+	if (c == NULL) {
+		diag_set(OutOfMemory, sizeof(*c), "malloc", "c");
+		return NULL;
+	}
+	switch (algo) {
+	case CRYPTO_AES128:
+		memcpy(c->key, key, sizeof(c->key));
+		memset(c->iv, 0, sizeof(c->iv));
+		c->type = EVP_aes_128_cbc();
+		break;
+	case CRYPTO_NONE:
+		c->type = EVP_enc_null();
+		break;
+	default:
+		free(c);
+		diag_set(CryptoError, "only 'none' and AES 128 are supported");
+		return NULL;
+	}
+	c->block_size = EVP_CIPHER_block_size(c->type);
+	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_encrypt(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
+	 * almost always bigger than input. OpenSSL API advises to
+	 * provide buffer of one block bigger than the data to
+	 * encode.
+	 */
+	int len, result, need = in_size + c->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_decrypt(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..c679c39f7 100644
--- a/src/lib/crypto/crypto.h
+++ b/src/lib/crypto/crypto.h
@@ -35,6 +35,100 @@
 extern "C" {
 #endif
 
+enum crypto_algo {
+	/** None to disable encryption. */
+	CRYPTO_NONE,
+	/**
+	 * 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,
+};
+
+/**
+ * Values obtained from EVP_CIPHER_*() API, but the constants are
+ * needed in some places at compilation time. For example, for
+ * statically sized buffers.
+ */
+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);
+
+/**
+ * Encrypt 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_encrypt(struct crypto_codec *c, const char *in, int in_size,
+		     char *out, int out_size);
+
+/**
+ * Decrypt 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_decrypt(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..092650543
--- /dev/null
+++ b/test/unit/crypto.c
@@ -0,0 +1,191 @@
+/*
+ * 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(19);
+
+	char key[CRYPTO_AES128_KEY_SIZE];
+	random_bytes(key, sizeof(key));
+	struct crypto_codec *c = crypto_codec_new(CRYPTO_AES128, key);
+
+	int rc = crypto_codec_encrypt(c, NULL, 10, NULL, 0);
+	is(rc, 26, "encrypt returns needed number of bytes");
+	rc = crypto_codec_encrypt(c, NULL, 10, NULL, 15);
+	is(rc, 26, "encrypt does not write anything when too small "\
+	   "buffer");
+	rc = crypto_codec_encrypt(c, NULL, 0, NULL, 0);
+	is(rc, 16, "encrypt does not allow 0 sized buffer");
+	rc = crypto_codec_encrypt(c, NULL, 32, NULL, 0);
+	is(rc, 48, "encrypt 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];
+	memset(buffer1, 0, sizeof(buffer1));
+	memset(buffer2, 0, sizeof(buffer2));
+	int buffer_size = sizeof(buffer1);
+	const char *iv = crypto_codec_gen_iv(c);
+
+	rc = crypto_codec_encrypt(c, plain, plain_size, buffer1, buffer_size);
+	is(rc, 16, "encrypt works when buffer is big enough");
+	rc = crypto_codec_encrypt(c, plain, plain_size, buffer2, buffer_size);
+	is(rc, 16, "encrypt 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_decrypt(c, iv, NULL, 16, NULL, 0);
+	is(rc, 16, "decrypt also checks length and returns needed number "\
+	   "of bytes");
+	rc = crypto_codec_decrypt(c, iv, buffer1, 16, buffer2, buffer_size);
+	is(rc, plain_size, "decrypt returns correct number of bytes");
+	is(memcmp(buffer2, plain, plain_size), 0,
+	   "and correctly decrypts data");
+
+	rc = crypto_codec_decrypt(c, "false iv not meaning anything",
+				  buffer1, 16, buffer2, buffer_size);
+	is(rc, -1, "decrypt 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_encrypt(c, plain, plain_size, buffer2, buffer_size);
+	is(rc, 16, "encrypt with different IV and the same number of written "\
+	   "bytes returned")
+	isnt(memcmp(buffer2, buffer1, rc), 0,
+	     "the encrypted data looks different");
+	rc = crypto_codec_decrypt(c, iv2, buffer2, 16, buffer1, buffer_size);
+	is(rc, plain_size, "decrypt 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_encrypt(c, plain, plain_size, buffer1, buffer_size);
+	memset(buffer2, 0, rc);
+	rc = crypto_codec_decrypt(c2, iv2, buffer1, rc, buffer2, buffer_size);
+	is(rc, plain_size, "encrypt with one codec, but decrypt 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();
+	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_encrypt(c, plain, size,
+					  cipher, sizeof(cipher));
+		rc = crypto_codec_decrypt(c, iv, cipher, rc,
+					  result, sizeof(result));
+		fail_if(memcmp(result, plain, rc) != 0);
+	}
+	ok(true, "try encrypt/decrypt on a variety of sizes, keys, and ivs");
+
+	check_plan();
+	crypto_codec_delete(c);
+	footer();
+}
+
+static void
+test_none_codec(void)
+{
+	header();
+	plan(4);
+
+	struct crypto_codec *c = crypto_codec_new(CRYPTO_NONE, NULL);
+	const char *plain = "plain text";
+	int plain_size = strlen(plain) + 1;
+	char buffer1[128], buffer2[128];
+	const char *iv = crypto_codec_gen_iv(c);
+	int rc = crypto_codec_encrypt(c, plain, plain_size,
+				      buffer1, sizeof(buffer1));
+	is(rc, plain_size, "'none' codec does not require more space "\
+	   "than the data");
+	is(memcmp(buffer1, plain, plain_size), 0, "data is just copied");
+	rc = crypto_codec_decrypt(c, iv, buffer1, rc, buffer2, sizeof(buffer2));
+	is(rc, plain_size, "'decrypt' does the same");
+	is(memcmp(buffer2, plain, plain_size), 0, "data is copied back");
+
+	crypto_codec_delete(c);
+
+	check_plan();
+	footer();
+}
+
+int
+main(void)
+{
+	header();
+	plan(4);
+	random_init();
+	crypto_init();
+	memory_init();
+	fiber_init(fiber_c_invoke);
+
+	struct crypto_codec *c = crypto_codec_new(-1, "1234");
+	is(c, NULL, "crypto checks that algo argument is correct");
+	crypto_codec_delete(c);
+
+	test_aes128_codec();
+	test_aes128_stress();
+	test_none_codec();
+
+	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..de55fd2e4
--- /dev/null
+++ b/test/unit/crypto.result
@@ -0,0 +1,40 @@
+	*** main ***
+1..4
+ok 1 - crypto checks that algo argument is correct
+	*** test_aes128_codec ***
+    1..19
+    ok 1 - encrypt returns needed number of bytes
+    ok 2 - encrypt does not write anything when too small buffer
+    ok 3 - encrypt does not allow 0 sized buffer
+    ok 4 - encrypt requires additional block when buffer size is multiple of block size
+    ok 5 - encrypt works when buffer is big enough
+    ok 6 - encrypt returns the same on second call
+    ok 7 - encrypted data is the same
+    ok 8 - and it is not just copied from the plain text
+    ok 9 - decrypt also checks length and returns needed number of bytes
+    ok 10 - decrypt returns correct number of bytes
+    ok 11 - and correctly decrypts data
+    ok 12 - decrypt can fail with wrong IV
+    ok 13 - diag error is set
+    ok 14 - encrypt with different IV and the same number of written bytes returned
+    ok 15 - the encrypted data looks different
+    ok 16 - decrypt works with correct but another IV
+    ok 17 - data is the same
+    ok 18 - encrypt with one codec, but decrypt with another codec and the same key
+    ok 19 - data is the same
+ok 2 - subtests
+	*** test_aes128_codec: done ***
+	*** test_aes128_stress ***
+    1..1
+    ok 1 - try encrypt/decrypt on a variety of sizes, keys, and ivs
+ok 3 - subtests
+	*** test_aes128_stress: done ***
+	*** test_none_codec ***
+    1..4
+    ok 1 - 'none' codec does not require more space than the data
+    ok 2 - data is just copied
+    ok 3 - 'decrypt' does the same
+    ok 4 - data is copied back
+ok 4 - subtests
+	*** test_none_codec: done ***
+	*** main: done ***
-- 
2.20.1 (Apple Git-117)

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

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/29 14:09]:

Please solicit a code review from the owner of htis code, Georgy.

Thanks!
> 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)
> 

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

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

[Konstantin Osipov]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/29 14:09]:

LGTM, please ask Georgy for a second opinion.

> 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       | 144 +++++++++++++++++++++++++
>  src/lib/crypto/crypto.h       |  94 +++++++++++++++++
>  test/unit/CMakeLists.txt      |   3 +
>  test/unit/crypto.c            | 191 ++++++++++++++++++++++++++++++++++
>  test/unit/crypto.result       |  40 +++++++
>  9 files changed, 507 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..a96963d6f 100644
> --- a/src/lib/crypto/crypto.c
> +++ b/src/lib/crypto/crypto.c
> @@ -29,12 +29,156 @@
>   * 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];
> +	/** Size of block by which the algorithm operates. */
> +	uint8_t block_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)
> +{
> +	struct crypto_codec *c = (struct crypto_codec *) malloc(sizeof(*c));
> +	if (c == NULL) {
> +		diag_set(OutOfMemory, sizeof(*c), "malloc", "c");
> +		return NULL;
> +	}
> +	switch (algo) {
> +	case CRYPTO_AES128:
> +		memcpy(c->key, key, sizeof(c->key));
> +		memset(c->iv, 0, sizeof(c->iv));
> +		c->type = EVP_aes_128_cbc();
> +		break;
> +	case CRYPTO_NONE:
> +		c->type = EVP_enc_null();
> +		break;
> +	default:
> +		free(c);
> +		diag_set(CryptoError, "only 'none' and AES 128 are supported");
> +		return NULL;
> +	}
> +	c->block_size = EVP_CIPHER_block_size(c->type);
> +	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_encrypt(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
> +	 * almost always bigger than input. OpenSSL API advises to
> +	 * provide buffer of one block bigger than the data to
> +	 * encode.
> +	 */
> +	int len, result, need = in_size + c->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_decrypt(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..c679c39f7 100644
> --- a/src/lib/crypto/crypto.h
> +++ b/src/lib/crypto/crypto.h
> @@ -35,6 +35,100 @@
>  extern "C" {
>  #endif
>  
> +enum crypto_algo {
> +	/** None to disable encryption. */
> +	CRYPTO_NONE,
> +	/**
> +	 * 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,
> +};
> +
> +/**
> + * Values obtained from EVP_CIPHER_*() API, but the constants are
> + * needed in some places at compilation time. For example, for
> + * statically sized buffers.
> + */
> +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);
> +
> +/**
> + * Encrypt 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_encrypt(struct crypto_codec *c, const char *in, int in_size,
> +		     char *out, int out_size);
> +
> +/**
> + * Decrypt 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_decrypt(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..092650543
> --- /dev/null
> +++ b/test/unit/crypto.c
> @@ -0,0 +1,191 @@
> +/*
> + * 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(19);
> +
> +	char key[CRYPTO_AES128_KEY_SIZE];
> +	random_bytes(key, sizeof(key));
> +	struct crypto_codec *c = crypto_codec_new(CRYPTO_AES128, key);
> +
> +	int rc = crypto_codec_encrypt(c, NULL, 10, NULL, 0);
> +	is(rc, 26, "encrypt returns needed number of bytes");
> +	rc = crypto_codec_encrypt(c, NULL, 10, NULL, 15);
> +	is(rc, 26, "encrypt does not write anything when too small "\
> +	   "buffer");
> +	rc = crypto_codec_encrypt(c, NULL, 0, NULL, 0);
> +	is(rc, 16, "encrypt does not allow 0 sized buffer");
> +	rc = crypto_codec_encrypt(c, NULL, 32, NULL, 0);
> +	is(rc, 48, "encrypt 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];
> +	memset(buffer1, 0, sizeof(buffer1));
> +	memset(buffer2, 0, sizeof(buffer2));
> +	int buffer_size = sizeof(buffer1);
> +	const char *iv = crypto_codec_gen_iv(c);
> +
> +	rc = crypto_codec_encrypt(c, plain, plain_size, buffer1, buffer_size);
> +	is(rc, 16, "encrypt works when buffer is big enough");
> +	rc = crypto_codec_encrypt(c, plain, plain_size, buffer2, buffer_size);
> +	is(rc, 16, "encrypt 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_decrypt(c, iv, NULL, 16, NULL, 0);
> +	is(rc, 16, "decrypt also checks length and returns needed number "\
> +	   "of bytes");
> +	rc = crypto_codec_decrypt(c, iv, buffer1, 16, buffer2, buffer_size);
> +	is(rc, plain_size, "decrypt returns correct number of bytes");
> +	is(memcmp(buffer2, plain, plain_size), 0,
> +	   "and correctly decrypts data");
> +
> +	rc = crypto_codec_decrypt(c, "false iv not meaning anything",
> +				  buffer1, 16, buffer2, buffer_size);
> +	is(rc, -1, "decrypt 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_encrypt(c, plain, plain_size, buffer2, buffer_size);
> +	is(rc, 16, "encrypt with different IV and the same number of written "\
> +	   "bytes returned")
> +	isnt(memcmp(buffer2, buffer1, rc), 0,
> +	     "the encrypted data looks different");
> +	rc = crypto_codec_decrypt(c, iv2, buffer2, 16, buffer1, buffer_size);
> +	is(rc, plain_size, "decrypt 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_encrypt(c, plain, plain_size, buffer1, buffer_size);
> +	memset(buffer2, 0, rc);
> +	rc = crypto_codec_decrypt(c2, iv2, buffer1, rc, buffer2, buffer_size);
> +	is(rc, plain_size, "encrypt with one codec, but decrypt 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();
> +	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_encrypt(c, plain, size,
> +					  cipher, sizeof(cipher));
> +		rc = crypto_codec_decrypt(c, iv, cipher, rc,
> +					  result, sizeof(result));
> +		fail_if(memcmp(result, plain, rc) != 0);
> +	}
> +	ok(true, "try encrypt/decrypt on a variety of sizes, keys, and ivs");
> +
> +	check_plan();
> +	crypto_codec_delete(c);
> +	footer();
> +}
> +
> +static void
> +test_none_codec(void)
> +{
> +	header();
> +	plan(4);
> +
> +	struct crypto_codec *c = crypto_codec_new(CRYPTO_NONE, NULL);
> +	const char *plain = "plain text";
> +	int plain_size = strlen(plain) + 1;
> +	char buffer1[128], buffer2[128];
> +	const char *iv = crypto_codec_gen_iv(c);
> +	int rc = crypto_codec_encrypt(c, plain, plain_size,
> +				      buffer1, sizeof(buffer1));
> +	is(rc, plain_size, "'none' codec does not require more space "\
> +	   "than the data");
> +	is(memcmp(buffer1, plain, plain_size), 0, "data is just copied");
> +	rc = crypto_codec_decrypt(c, iv, buffer1, rc, buffer2, sizeof(buffer2));
> +	is(rc, plain_size, "'decrypt' does the same");
> +	is(memcmp(buffer2, plain, plain_size), 0, "data is copied back");
> +
> +	crypto_codec_delete(c);
> +
> +	check_plan();
> +	footer();
> +}
> +
> +int
> +main(void)
> +{
> +	header();
> +	plan(4);
> +	random_init();
> +	crypto_init();
> +	memory_init();
> +	fiber_init(fiber_c_invoke);
> +
> +	struct crypto_codec *c = crypto_codec_new(-1, "1234");
> +	is(c, NULL, "crypto checks that algo argument is correct");
> +	crypto_codec_delete(c);
> +
> +	test_aes128_codec();
> +	test_aes128_stress();
> +	test_none_codec();
> +
> +	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..de55fd2e4
> --- /dev/null
> +++ b/test/unit/crypto.result
> @@ -0,0 +1,40 @@
> +	*** main ***
> +1..4
> +ok 1 - crypto checks that algo argument is correct
> +	*** test_aes128_codec ***
> +    1..19
> +    ok 1 - encrypt returns needed number of bytes
> +    ok 2 - encrypt does not write anything when too small buffer
> +    ok 3 - encrypt does not allow 0 sized buffer
> +    ok 4 - encrypt requires additional block when buffer size is multiple of block size
> +    ok 5 - encrypt works when buffer is big enough
> +    ok 6 - encrypt returns the same on second call
> +    ok 7 - encrypted data is the same
> +    ok 8 - and it is not just copied from the plain text
> +    ok 9 - decrypt also checks length and returns needed number of bytes
> +    ok 10 - decrypt returns correct number of bytes
> +    ok 11 - and correctly decrypts data
> +    ok 12 - decrypt can fail with wrong IV
> +    ok 13 - diag error is set
> +    ok 14 - encrypt with different IV and the same number of written bytes returned
> +    ok 15 - the encrypted data looks different
> +    ok 16 - decrypt works with correct but another IV
> +    ok 17 - data is the same
> +    ok 18 - encrypt with one codec, but decrypt with another codec and the same key
> +    ok 19 - data is the same
> +ok 2 - subtests
> +	*** test_aes128_codec: done ***
> +	*** test_aes128_stress ***
> +    1..1
> +    ok 1 - try encrypt/decrypt on a variety of sizes, keys, and ivs
> +ok 3 - subtests
> +	*** test_aes128_stress: done ***
> +	*** test_none_codec ***
> +    1..4
> +    ok 1 - 'none' codec does not require more space than the data
> +    ok 2 - data is just copied
> +    ok 3 - 'decrypt' does the same
> +    ok 4 - data is copied back
> +ok 4 - subtests
> +	*** test_none_codec: done ***
> +	*** main: done ***
> -- 
> 2.20.1 (Apple Git-117)
> 

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

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

[Vladislav Shpilevoy]

Georgy, please, give a second review.

On 29/04/2019 14:07, Vladislav Shpilevoy wrote:
> 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/crypto-lib
> 
> Changes in V2:
> - Added new codec 'None';
> - Renamed 'encode/decode' to 'encrypt/decrypt';
> - Removed usage of constants from crypto.c.
> 
> V1: https://www.freelists.org/post/tarantool-patches/PATCH-03-swim-encryption-preparation
> 
> 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       | 260 ++++++++++++++++++++++++++++++++++
>  src/lib/crypto/crypto.h       | 142 +++++++++++++++++++
>  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            | 191 +++++++++++++++++++++++++
>  test/unit/crypto.result       |  40 ++++++
>  16 files changed, 706 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)
> 
>