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

["Георгий Кириченко" <georgy@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 9477 bytes --]

LGTM
On Tuesday, May 7, 2019 11:53:56 PM MSK Vladislav Shpilevoy wrote:
> 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
> -}


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]