Hi,
On 12/08/17 17:53, Steffan Karger wrote:
> Reduces code duplication (and prepares for tls-crypt-v2, which needs the
> same functionality at more places).
>
> Because tls_crypt_kt() is a static function we now need to include
> tls_crypt.c from the tests, rather than tls_crypt.h.
>
> Signed-off-by: Steffan Karger <steffan.kar...@fox-it.com>
> ---
> v2: improve error handling
>
> src/openvpn/tls_crypt.c | 40
> ++++++++++++++++++++-----------
> tests/unit_tests/openvpn/Makefile.am | 3 +--
> tests/unit_tests/openvpn/test_tls_crypt.c | 20 ++++------------
> 3 files changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
> index e13bb4e..403060d 100644
> --- a/src/openvpn/tls_crypt.c
> +++ b/src/openvpn/tls_crypt.c
> @@ -35,35 +35,47 @@
>
> #include "tls_crypt.h"
>
> -int
> -tls_crypt_buf_overhead(void)
> -{
> - return packet_id_size(true) + TLS_CRYPT_TAG_SIZE + TLS_CRYPT_BLOCK_SIZE;
> -}
> -
> -void
> -tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
> - const char *key_inline, bool tls_server)
> +static struct key_type
> +tls_crypt_kt(void)
> {
> - const int key_direction = tls_server ?
> - KEY_DIRECTION_NORMAL : KEY_DIRECTION_INVERSE;
> -
> struct key_type kt;
> kt.cipher = cipher_kt_get("AES-256-CTR");
> kt.digest = md_kt_get("SHA256");
>
> if (!kt.cipher)
> {
> - msg(M_FATAL, "ERROR: --tls-crypt requires AES-256-CTR support.");
> + msg(M_WARN, "ERROR: --tls-crypt requires AES-256-CTR support.");
> + return (struct key_type) { 0 };
> }
> if (!kt.digest)
> {
> - msg(M_FATAL, "ERROR: --tls-crypt requires HMAC-SHA-256 support.");
> + msg(M_WARN, "ERROR: --tls-crypt requires HMAC-SHA-256 support.");
> + return (struct key_type) { 0 };
> }
>
> kt.cipher_length = cipher_kt_key_size(kt.cipher);
> kt.hmac_length = md_kt_size(kt.digest);
>
> + return kt;
> +}
> +
> +int
> +tls_crypt_buf_overhead(void)
> +{
> + return packet_id_size(true) + TLS_CRYPT_TAG_SIZE + TLS_CRYPT_BLOCK_SIZE;
> +}
> +
> +void
> +tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
> + const char *key_inline, bool tls_server)
> +{
> + const int key_direction = tls_server ?
> + KEY_DIRECTION_NORMAL : KEY_DIRECTION_INVERSE;
> + struct key_type kt = tls_crypt_kt();
> + if (!kt.cipher || !kt.digest)
> + {
> + msg (M_FATAL, "ERROR: --tls-crypt not supported");
there should be no space between 'msg' and '('.
I hope who will commit this patch will also be able to remove it.
> + }
> crypto_read_openvpn_key(&kt, key, key_file, key_inline, key_direction,
> "Control Channel Encryption", "tls-crypt");
> }
> diff --git a/tests/unit_tests/openvpn/Makefile.am
> b/tests/unit_tests/openvpn/Makefile.am
> index 3bd382c..7b44f42 100644
> --- a/tests/unit_tests/openvpn/Makefile.am
> +++ b/tests/unit_tests/openvpn/Makefile.am
> @@ -54,5 +54,4 @@ tls_crypt_testdriver_SOURCES = test_tls_crypt.c mock_msg.c \
> $(openvpn_srcdir)/crypto_openssl.c \
> $(openvpn_srcdir)/otime.c \
> $(openvpn_srcdir)/packet_id.c \
> - $(openvpn_srcdir)/platform.c \
> - $(openvpn_srcdir)/tls_crypt.c
> + $(openvpn_srcdir)/platform.c
> diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c
> b/tests/unit_tests/openvpn/test_tls_crypt.c
> index 9b82035..0a6a08f 100644
> --- a/tests/unit_tests/openvpn/test_tls_crypt.c
> +++ b/tests/unit_tests/openvpn/test_tls_crypt.c
> @@ -39,7 +39,7 @@
> #include <setjmp.h>
> #include <cmocka.h>
>
> -#include "tls_crypt.h"
> +#include "tls_crypt.c"
>
> #include "mock_msg.h"
>
> @@ -60,23 +60,13 @@ setup(void **state) {
> struct test_context *ctx = calloc(1, sizeof(*ctx));
> *state = ctx;
>
> - ctx->kt.cipher = cipher_kt_get("AES-256-CTR");
> - ctx->kt.digest = md_kt_get("SHA256");
> - if (!ctx->kt.cipher)
> - {
> - printf("No AES-256-CTR support, skipping test.\n");
> - return 0;
> - }
> - if (!ctx->kt.digest)
> + struct key key = { 0 };
> +
> + ctx->kt = tls_crypt_kt();
> + if (!ctx->kt.cipher || !ctx->kt.digest)
> {
> - printf("No HMAC-SHA256 support, skipping test.\n");
> return 0;
> }
> - ctx->kt.cipher_length = cipher_kt_key_size(ctx->kt.cipher);
> - ctx->kt.hmac_length = md_kt_size(ctx->kt.digest);
> -
> - struct key key = { 0 };
> -
> init_key_ctx(&ctx->co.key_ctx_bi.encrypt, &key, &ctx->kt, true, "TEST");
> init_key_ctx(&ctx->co.key_ctx_bi.decrypt, &key, &ctx->kt, false, "TEST");
>
>
Much better error handling.
Even if I don't like the "return .. { 0 }" from a style perspective, the
patch is fine and does what it says. ACK.
Cheers,
--
Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
