[tarantool-patches] Re: [PATCH v2 2/3] crypto: make exported methods conform code style
Konstantin Osipov
kostja at tarantool.org
Mon Apr 29 15:23:54 MSK 2019
* Vladislav Shpilevoy <v.shpilevoy at 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
More information about the Tarantool-patches
mailing list