update soter/openssl to fix compatibility with new openssl version #258

Lagovas · 2017-12-08T12:16:38Z

due to changes in openssl 1.1.0 was changed:

using openssl structs allocated in heap instead stack because now their structs are opaque in openssl (1, 2)
pass soter fields to functions like some_func(ctx->field) instead some_func(&(ctx->field)) because now they are not public and allocated in heap
create structs using openssl functions *_create and *_new instead struct variable declaration (1)
use getters/setters to access to hidden struct fields (EVP_PKEY_type/EVP_PKEY_base_id, RSA_get0_key, RSA_get0_factors, RSA_get0_crt_params, RSA_set0_key, RSA_set0_factors, RSA_set0_crt_params)

want to ask @secumod to check at least src/soter/openssl/soter_rsa_key.c file where work with rsa keys

…sts (for ide)

mark unused files to fix "ISO C forbids an empty source file" warning

turn off one nist test on ci by cflags

…-usage

fix mistake of merging test.mk

…(revert prev changes)

vixentael · 2017-12-08T12:33:55Z

src/soter/soter_error.h

@@ -63,6 +63,7 @@ typedef int soter_status_t;
 #define SOTER_DEBUG_OUT(message)
 #endif

+#define UNUSED(x) (void)(x)


vixentael · 2017-12-08T12:35:19Z

src/themis/secure_session.c

@@ -826,7 +827,7 @@ static themis_status_t secure_session_finish_client(secure_session_t *session_ct
 		return THEMIS_INVALID_PARAMETER;
 	}

-	if (memcmp(proto_message->tag, THEMIS_SESSION_PROTO_TAG, SOTER_CONTAINER_TAG_LENGTH))
+	if (memcmp(proto_message->tag, THEMIS_SESSION_PROTO_TAG, SOTER_CONTAINER_TAG_LENGTH) != 0)


we do compare public data here, right?

actually yes, it's trying to check that it's correct secure session packet or no

This reverts commit 6913bdf.

vixentael · 2017-12-08T12:40:56Z

tests/soter/soter_rand_test.c

-//	testsuite_enter_suite("soter rand: NIST STS (make take some time...)");
-//	testsuite_run_test(test_rand_with_nist);
+// always fail under ci
+#ifndef CIRICLE_TEST


vixentael · 2017-12-08T13:06:57Z

should close #208 after merge

…s/themis into lagovas/fix-openssl-usage

ignatk · 2017-12-08T13:19:34Z

src/soter/openssl/soter_asym_cipher.c

@@ -97,6 +97,7 @@ soter_status_t soter_asym_cipher_init(soter_asym_cipher_t* asym_cipher, const vo
 		return SOTER_FAIL;
 	}
 	SOTER_IF_FAIL(soter_asym_cipher_import_key(asym_cipher, key, key_length)==SOTER_SUCCESS, (EVP_PKEY_free(pkey), EVP_PKEY_CTX_free(asym_cipher->pkey_ctx)));
+    EVP_PKEY_free(pkey);


clearly breaks file indentation here

ignatk · 2017-12-08T13:21:05Z

src/soter/openssl/soter_asym_cipher.c

@@ -152,8 +150,8 @@ soter_status_t soter_asym_cipher_encrypt(soter_asym_cipher_t* asym_cipher, const
 	}

 	rsa_mod_size = RSA_size(rsa);
-
-	if (plain_data_length > (rsa_mod_size - 2 - (2 * OAEP_HASH_SIZE)))
+	int temp = (rsa_mod_size - 2 - (2 * OAEP_HASH_SIZE));


indent

let's not name stuff temp, but something more context aware, like oaep_max_payload_length

ignatk · 2017-12-08T13:30:43Z

src/soter/openssl/soter_hash.c

@@ -105,6 +111,7 @@ soter_hash_ctx_t* soter_hash_create(soter_hash_algo_t algo)
 	{
 		return NULL;
 	}
+    ctx->evp_md_ctx = NULL;


ignatk · 2017-12-08T13:30:56Z

src/soter/openssl/soter_hash.c

-	EVP_MD_CTX_cleanup(&(hash_ctx->evp_md_ctx));
+
+    EVP_MD_CTX_destroy(hash_ctx->evp_md_ctx);
+    hash_ctx->evp_md_ctx = NULL;


ignatk · 2017-12-12T19:05:01Z

src/soter/openssl/soter_asym_cipher.c

-		    EVP_PKEY_free(pkey);
-		}
+        EVP_PKEY_CTX_free(asym_cipher->pkey_ctx);
+        asym_cipher->pkey_ctx = NULL;


indentation

can you confirm that EVP_PKEY_CTX_free also frees "contained" pkey?

https://github.com/openssl/openssl/blob/master/crypto/evp/pmeth_lib.c#L339
and also checked with valgrind (he warned that there was double free when I leave EVP_PKEY_free(pkey))

ignatk · 2017-12-12T19:07:06Z

src/soter/openssl/soter_hash.c

 	{
 		return SOTER_SUCCESS;
 	}
 	else
 	{
+        soter_hash_cleanup(hash_ctx);


ignatk · 2017-12-12T19:12:34Z

src/soter/openssl/soter_sym.c

@@ -99,15 +101,16 @@ soter_sym_ctx_t* soter_sym_ctx_init(const uint32_t alg,
  ctx->alg=alg;
  uint8_t key_[SOTER_SYM_MAX_KEY_LENGTH];
  size_t key_length_=(alg&SOTER_SYM_KEY_LENGTH_MASK)/8;
-  EVP_CIPHER_CTX_init(&(ctx->evp_sym_ctx));
+  //EVP_CIPHER_CTX_init(ctx->evp_sym_ctx);
+  ctx->evp_sym_ctx = EVP_CIPHER_CTX_new();


probably should be some failure check here

ignatk · 2017-12-12T19:13:10Z

src/soter/openssl/soter_sym.c

@@ -128,12 +132,12 @@ soter_sym_ctx_t* soter_sym_aead_ctx_init(const uint32_t alg,
  ctx->alg=alg;
  uint8_t key_[SOTER_SYM_MAX_KEY_LENGTH];
  size_t key_length_=(alg&SOTER_SYM_KEY_LENGTH_MASK)/8;
-  EVP_CIPHER_CTX_init(&(ctx->evp_sym_ctx));
+  ctx->evp_sym_ctx = EVP_CIPHER_CTX_new();


failure check?

ignatk · 2017-12-12T19:15:16Z

src/soter/soter_hmac.c

@@ -46,23 +46,24 @@ soter_status_t soter_hmac_init(soter_hmac_ctx_t *hmac_ctx, soter_hash_algo_t alg
 		return SOTER_INVALID_PARAMETER;
 	}

+    hmac_ctx->hash_ctx = soter_hash_create(algo);


indentation?

ignatk · 2017-12-12T19:15:31Z

src/soter/soter_hmac.c

 		if (SOTER_SUCCESS != res)
 		{
+            soter_hash_destroy(hmac_ctx->hash_ctx);


indentation

fix incorrect indentation

Lagovas added 23 commits October 13, 2017 00:32

use pointers instead stack structs

48b2aea

fix usage of openssl hash and rsa structs

8d6cbf7

update openssl ecdsa sign usage

3ff6a58

fix test running with cmake

c17a30e

use explicit initializing digest context with new openssl api

5397ec8

CMakeLists.txt simple non-productions script to build themis/soter/te…

659f2b4

…sts (for ide)

fix secure_session tests

40ba664

mark unused files to fix "ISO C forbids an empty source file" warning

fix valgrind check

2ff5832

fix memory leaks and errors in tests

586a597

fix leak in old openssl after new changes

603c04e

install last valgrind on ci

fa5de60

turn off one nist test on ci by cflags

fix installing valgrind

c99a333

handle stack structs

45ca6d4

cache valgrind/golang fetching in ci

d2e6cc9

add valgrind dependency

4f12b69

restore installing golang

b134542

test cache on ci

48e6664

fix check cache

e279fc1

Merge remote-tracking branch 'origin/master' into lagovas/fix-openssl…

1a48c04

…-usage

revert using calloc instead malloc

ba4b01f

fix mistake of merging test.mk

revert themis_secure_session.c test changes

6913bdf

run nist test via cmakelists

a657557

use stack allocation of soter_hash instead heap in secure_comparator …

c5a8a71

…(revert prev changes)

Lagovas requested review from vixentael and ignatk December 8, 2017 12:16

vixentael reviewed Dec 8, 2017

View reviewed changes

Revert "revert themis_secure_session.c test changes"

4c84fe3

This reverts commit 6913bdf.

vixentael reviewed Dec 8, 2017

View reviewed changes

Lagovas added 3 commits December 8, 2017 15:26

run themis tests with boringssl and valgrind

3a4c001

fix memory leak in boringssl/soter_hash

14c63f2

Merge branch 'lagovas/fix-openssl-usage' of https://github.com/lagova…

dc5bbd4

…s/themis into lagovas/fix-openssl-usage

vixentael added the core Themis Core written in C, its packages label Dec 8, 2017

Lagovas added 2 commits December 11, 2017 14:43

drop openssl1.0.2 dependency for stretch

7917b98

Merge branch 'lagovas/fix-openssl-usage' of https://github.com/Lagova…

1f854b6

…s/themis into lagovas/fix-openssl-usage

ignatk reviewed Dec 12, 2017

View reviewed changes

add failure check

7a6ec33

fix incorrect indentation

vixentael merged commit 4925be9 into cossacklabs:master Dec 13, 2017

Lagovas mentioned this pull request Dec 13, 2017

Themis doesn't build on debian 9 with openssl 1.1.0f #208

Closed

Lagovas deleted the lagovas/fix-openssl-usage branch February 6, 2018 14:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update soter/openssl to fix compatibility with new openssl version #258

update soter/openssl to fix compatibility with new openssl version #258

Lagovas commented Dec 8, 2017

vixentael Dec 8, 2017

vixentael Dec 8, 2017

Lagovas Dec 8, 2017

vixentael Dec 8, 2017

vixentael commented Dec 8, 2017

ignatk Dec 8, 2017

ignatk Dec 8, 2017

ignatk Dec 8, 2017

ignatk Dec 8, 2017

ignatk Dec 12, 2017

Lagovas Dec 13, 2017

ignatk Dec 12, 2017

ignatk Dec 12, 2017

ignatk Dec 12, 2017

ignatk Dec 12, 2017

ignatk Dec 12, 2017

update soter/openssl to fix compatibility with new openssl version #258

update soter/openssl to fix compatibility with new openssl version #258

Conversation

Lagovas commented Dec 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vixentael commented Dec 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment