otrv4 / libotr-ng

A new implementation of OTR with support for version 4. This is a mirror of https://bugs.otr.im/otrv4/libotr-ng
Other
43 stars 9 forks source link

Missing symbols with libgcrypt 1.7.8 #102

Closed juniorz closed 6 years ago

juniorz commented 6 years ago

make test (and also make coverage-check) fail with the following error on libgcrypt 1.7.8

libtool: link: gcc -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/local/include -DOTRNG_TESTS -g -O0 -Wall -Werror -Wformat -Wno-format-extra-args -Wfatal-errors -Wbad-function-cast -Wdiv-by-zero -Wfloat-equal -Wnested-externs -Wpointer-arith -Wredundant-decls -Wlogical-op -Wformat-nonliteral -o test test-test.o ../test-auth.o ../test-client.o ../test-client_callbacks.o ../test-client_state.o ../test-dake.o ../test-data_message.o ../test-deserialize.o ../test-dh.o ../test-ed448.o ../test-fingerprint.o ../test-fragment.o ../test-instance_tag.o ../test-keys.o ../test-key_management.o ../test-list.o ../test-messaging.o ../test-mpi.o ../test-v3.o ../test-otrng.o ../test-serialize.o ../test-smp.o ../test-str.o ../test-tlv.o ../test-user_profile.o  -lglib-2.0 -L/usr/local/lib /usr/local/lib/libgoldilocks.so -lsodium -lotr
/usr/bin/ld: test-test.o: undefined reference to symbol 'gcry_mpi_new@@GCRYPT_1.6'
//lib/x86_64-linux-gnu/libgcrypt.so.20: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

Note there is no .o files from libgcrypt added during the linking phase.

The output of ./configure does not seem to find the installed version of libgcrypt (note there is no check for the minimum version 1.6.0 if the maximum version is not found):

...
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for GLIB... yes
checking for LIBGOLDILOCKS... yes
checking for LIBSODIUM... yes
checking for libotr CFLAGS... 
checking for libotr LIBS...  -lotr
checking for libotr headers version 4.x >= 4.0.0... found.
checking for otrl_message_receiving in -lotr... yes
checking for libgcrypt-config... /usr/bin/libgcrypt-config
checking for LIBGCRYPT - version >= 1.8.0... no
checking for stdint.h... (cached) yes
checking for stdlib.h... (cached) yes
checking for string.h... (cached) yes
...

As a temporary fix, I have on my local machine the following diff, and everything is working as expected with libgcrypt 1.7.8:

diff --git a/configure.ac b/configure.ac
index a8cba81..860164f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -59,12 +59,8 @@ PKG_CHECK_MODULES([GLIB], [glib-2.0 >= 2.18])
 PKG_CHECK_MODULES([LIBGOLDILOCKS], [libgoldilocks >= 0.0.1])
 PKG_CHECK_MODULES([LIBSODIUM], [libsodium >= 1.0.0])
 AM_PATH_LIBOTR(4.0.0,,AC_MSG_ERROR(libotr 4.x >= 4.0.0 is required.))
-# TODO: this seems to be not correctly working on Darwin.
-# We probably need the config script
-AM_PATH_LIBGCRYPT(1:1.8.0,,
-  [AC_DEFINE([HAVE_GCRYPT], [1], [Use GCRYPT])],
-  [AM_PATH_LIBGCRYPT(1:1.6.0,, [AC_MSG_ERROR(libgcrypt 1.6.0 or newer is required.)])]
-)
+
+AM_PATH_LIBGCRYPT(1:1.6.0,, [AC_MSG_ERROR(libgcrypt 1.6.0 or newer is required.)])

 dnl Checks for header files.
 AC_CHECK_HEADERS([stdint.h stdlib.h string.h])

./configure can find the 1.7.8 version:

checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for GLIB... yes
checking for LIBGOLDILOCKS... yes
checking for LIBSODIUM... yes
checking for libotr CFLAGS... 
checking for libotr LIBS...  -lotr
checking for libotr headers version 4.x >= 4.0.0... found.
checking for otrl_message_receiving in -lotr... yes
checking for libgcrypt-config... /usr/bin/libgcrypt-config
checking for LIBGCRYPT - version >= 1.6.0... yes (1.7.8)
checking LIBGCRYPT API version... okay
checking for stdint.h... (cached) yes
checking for stdlib.h... (cached) yes
checking for string.h... (cached) yes

And the linker is informed to use -lgcrypt

libtool: link: gcc -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/local/include -DOTRNG_TESTS -g -O0 -Wall -Werror -Wformat -Wno-format-extra-args -Wfatal-errors -Wbad-function-cast -Wdiv-by-zero -Wfloat-equal -Wnested-externs -Wpointer-arith -Wredundant-decls -Wlogical-op -Wformat-nonliteral -o test test-test.o ../test-auth.o ../test-client.o ../test-client_callbacks.o ../test-client_state.o ../test-dake.o ../test-data_message.o ../test-deserialize.o ../test-dh.o ../test-ed448.o ../test-fingerprint.o ../test-fragment.o ../test-instance_tag.o ../test-keys.o ../test-key_management.o ../test-list.o ../test-messaging.o ../test-mpi.o ../test-v3.o ../test-otrng.o ../test-serialize.o ../test-smp.o ../test-str.o ../test-tlv.o ../test-user_profile.o  -lglib-2.0 -L/usr/local/lib /usr/local/lib/libgoldilocks.so -lgcrypt -lsodium -lotr

On the CI, we build libgcrypt 1.8.1 from source and the linker is informed about the appropriate .o:


libtool: link: gcc -std=gnu99 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/local/include -I/usr/local/include -I/usr/local/include -DOTRNG_TESTS -g -O0 -Wall -Werror -Wformat -Wno-format-extra-args -Wfatal-errors -Wbad-function-cast -Wdiv-by-zero -Wfloat-equal -Wnested-externs -Wpointer-arith -Wredundant-decls -Wlogical-op -Wformat-nonliteral -o test test-test.o ../test-auth.o ../test-client.o ../test-client_callbacks.o ../test-client_state.o ../test-dake.o ../test-data_message.o ../test-deserialize.o ../test-dh.o ../test-ed448.o ../test-fingerprint.o ../test-fragment.o ../test-instance_tag.o ../test-keys.o ../test-key_management.o ../test-list.o ../test-messaging.o ../test-mpi.o ../test-v3.o ../test-otrng.o ../test-serialize.o ../test-smp.o ../test-str.o ../test-tlv.o ../test-user_profile.o  -lglib-2.0 -L/usr/local/lib /usr/local/lib/libgoldilocks.so /usr/local/lib/libsodium.so /usr/local/lib/libotr.so /usr/local/lib/libgcrypt.so /usr/local/lib/libgpg-error.so -pthread
``
juniorz commented 6 years ago

I suspect AM_PATH_LIBGCRYPT may not be reentrant.

I also think we are using the configure.ac in attempt to enforce the runtime version of libgcrypt (at least 1.8.0 if available, at least 1.6.0 otherwise), which seems to be the role of the package (because different operating systems may have different libgcrypt packages available at runtime).

If we don't use any API that is only available since 1.8.0, it should be OK to compile with >= 1.6.0 (maybe even a lower version).

Note the libotr we use requires a libgcrypt >= 1.2.0 at compile time (logs from the CI):

checking for libgcrypt-config... /usr/local/bin/libgcrypt-config
checking for LIBGCRYPT - version >= 1.2.0... yes (1.8.1)
checking LIBGCRYPT API version... okay
olabini commented 6 years ago

If I remember correctly, our library builds with earlier versions if we wanted to. But the problem was that some 1.7 versions had serious security problems, that was fixed in a patch release on some places. The idea was to check for either 1.8 OR that patch release, but I'm not sure if that was actually happening or not, or if that work was finished or not. Does someone remember which security issue it was?

juniorz commented 6 years ago

@olabini this was the issue: https://github.com/otrv4/libotr-ng/issues/22

EDIT: Not the security issue, but I am adding the previous issue for reference.

juniorz commented 6 years ago

We want to make sure users, at runtime, are not vulnerable to one particular security problem. It is unsure to me if the problem affects specific versions on the 1.6.x (and 1.7.x) releases or if it affects every version before 1.8.

From #22, both Debian (1.6.3-2+deb8u2) and Fedora 25 (1.6.6-1) provide patched packages for the release 1.6.

We do not depend on 1.7 API anymore (we don't use GCRY_MD_SHA3_512) so from an API perspective requiring 1.6 at compile time should be OK. Maybe even an earlier release.

I suspect enforcing the version at runtime (using gcry_check_version in OTRNG_INIT, for example) by rejecting the problematic versions may be what we need, although I am not sure if patched packages also change the library version number.

At the moment, ./configure does not report any version problem with 1.7.8 but it fails to use it for linking.

juniorz commented 6 years ago

I suspect the referred security issue is: https://security-tracker.debian.org/tracker/CVE-2016-6313

The mixing functions in the random number generator in Libgcrypt before 1.5.6, 1.6.x before 1.6.6, and 1.7.x before 1.7.3 and GnuPG before 1.4.21 make it easier for attackers to obtain the values of 160 bits by leveraging knowledge of the previous 4640 bits.

We use both gcry_random_bytes_secure and gcry_randomize.

juniorz commented 6 years ago

At compile time we only use the MPI API and these two functions. At runtime, we want a version that does not have this security issue: 1.5.6, 1.6.6, 1.7.3 (or greater) should be fine.

olabini commented 6 years ago

OK, that makes sense. So we should be fine compiling against whatever, so we can relax the requirements there. At runtime, as long as we know what the real issue is, can we test for it at startup, and fail or warn?

juniorz commented 6 years ago

I believe so.

libgcrypt has recommendations in regard to how to deal with its initialization: https://www.gnupg.org/documentation/manuals/gcrypt/Initializing-the-library.html

It says that libraries should not initialize and configure libgcrypt without first checking if the application has not done it yet. It suggests to print a warn and initializes libgcrypt if it is not initialized.

We could simply warn if we detect a version vulnerable to CVE-2016-6313. Alternatively, we could fail if this this happen AND libgcrypt has to be initialized by libotr-ng.

I am against failing if the application has already initialized libgcrypt and accepted a vulnerable version because it seems to go against the recommendation in the doc.

claucece commented 6 years ago

@juniorz

libgcrypt has recommendations in regard to how to deal with its initialization: https://www.gnupg.org/documentation/manuals/gcrypt/Initializing-the-library.html

Yes! We sort of do that for the tests: https://github.com/otrv4/libotr-ng/blob/master/src/test/test.c#L58

Furthermore, we may also need to check for secure memory: https://github.com/otrv4/libotr-ng/blob/master/src/test/test.c#L61

I am against failing if the application has already initialized libgcrypt and accepted a vulnerable version because it seems to go against the recommendation in the doc.

I will not like to go for the vulnerable version, and tbh, even warning. But that last thing, I think, might be the only thing we could do.

olabini commented 6 years ago

I feel uncomfortable running the library with a vulnerable version. However, doing an exit() from a library is also EXTREMELY rude...

claucece commented 6 years ago

So, I have been investigating a little about this. When running over the CI with 1.7.3 only one machine fails, manly, "full-valgrind-checks". Oddly enough, this machine is exactly the same as another machine (ubuntu precise and using gcc), but that one does not fail. The only difference is that "full-valgrind-checks", configures with the --enable-valgrind flag.

When changing the script to:

AM_PATH_LIBGCRYPT(1:1.8.0,,
  [AC_DEFINE([HAVE_GCRYPT], [1], [Use GCRYPT])],
  [AM_PATH_LIBGCRYPT(1:1.7.0,, [AC_MSG_ERROR(libgcrypt 1.6.0 or newer is required.)])]
)

It still complains about 1.6: undefined reference to symbol 'gcry_mpi_new@@GCRYPT_1.6'.

Furthermore, when using 1.7.8, on other machines a lot of memory leaks regarding libgcrypt and libotr start appearing:

==11913== 16 bytes in 1 blocks are possibly lost in loss record 205 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D04D2: _gcry_malloc_secure_core (global.c:946)
==11913==    by 0x57D04D2: _gcry_xmalloc_secure (global.c:1145)
==11913==    by 0x57D0596: _gcry_xcalloc_secure (global.c:1190)
==11913==    by 0x5892259: _gcry_mpi_resize (mpiutil.c:187)
==11913==    by 0x588FFCB: mpi_fromstr (mpicoder.c:134)
==11913==    by 0x588FFCB: _gcry_mpi_scan (mpicoder.c:602)
==11913==    by 0x57CC3C8: gcry_mpi_scan (visibility.c:357)
==11913==    by 0x42F036: otrng_dh_init (dh.c:85)
==11913==    by 0x429C89: main (test.c:68)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 284 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x58920F1: _gcry_mpi_alloc_secure (mpiutil.c:105)
==11913==    by 0x588FAC1: _gcry_mpi_scan (mpicoder.c:601)
==11913==    by 0x57CC3C8: gcry_mpi_scan (visibility.c:357)
==11913==    by 0x42EFEE: otrng_dh_init (dh.c:79)
==11913==    by 0x429C89: main (test.c:68)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 285 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x58920F1: _gcry_mpi_alloc_secure (mpiutil.c:105)
==11913==    by 0x588FAC1: _gcry_mpi_scan (mpicoder.c:601)
==11913==    by 0x57CC3C8: gcry_mpi_scan (visibility.c:357)
==11913==    by 0x42F012: otrng_dh_init (dh.c:82)
==11913==    by 0x429C89: main (test.c:68)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 286 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x58920F1: _gcry_mpi_alloc_secure (mpiutil.c:105)
==11913==    by 0x588FAC1: _gcry_mpi_scan (mpicoder.c:601)
==11913==    by 0x57CC3C8: gcry_mpi_scan (visibility.c:357)
==11913==    by 0x42F036: otrng_dh_init (dh.c:85)
==11913==    by 0x429C89: main (test.c:68)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 287 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x5891F81: _gcry_mpi_alloc (mpiutil.c:84)
==11913==    by 0x42F040: otrng_dh_init (dh.c:88)
==11913==    by 0x429C89: main (test.c:68)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 288 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x58920F1: _gcry_mpi_alloc_secure (mpiutil.c:105)
==11913==    by 0x58925EC: _gcry_mpi_copy (mpiutil.c:358)
==11913==    by 0x42F4BF: otrng_dh_mpi_copy (dh.c:221)
==11913==    by 0x4325BA: otrng_key_manager_set_their_dh (key_management.c:716)
==11913==    by 0x437CE9: receive_identity_message_on_state_start (otrng.c:1612)
==11913==    by 0x437F1D: receive_identity_message (otrng.c:1672)
==11913==    by 0x4394B9: receive_decoded_message (otrng.c:2195)
==11913==    by 0x4395E4: receive_encoded_message (otrng.c:2228)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 289 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x58920F1: _gcry_mpi_alloc_secure (mpiutil.c:105)
==11913==    by 0x588FCBE: _gcry_mpi_scan (mpicoder.c:519)
==11913==    by 0x57CC3C8: gcry_mpi_scan (visibility.c:357)
==11913==    by 0x42F16A: otrng_dh_keypair_generate (dh.c:119)
==11913==    by 0x431249: otrng_key_manager_generate_ephemeral_keys (key_management.c:155)
==11913==    by 0x437D14: receive_identity_message_on_state_start (otrng.c:1615)
==11913==    by 0x437F1D: receive_identity_message (otrng.c:1672)
==11913==    by 0x4394B9: receive_decoded_message (otrng.c:2195)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 290 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x5891F81: _gcry_mpi_alloc (mpiutil.c:84)
==11913==    by 0x42F1A5: otrng_dh_keypair_generate (dh.c:127)
==11913==    by 0x431249: otrng_key_manager_generate_ephemeral_keys (key_management.c:155)
==11913==    by 0x437D14: receive_identity_message_on_state_start (otrng.c:1615)
==11913==    by 0x437F1D: receive_identity_message (otrng.c:1672)
==11913==    by 0x4394B9: receive_decoded_message (otrng.c:2195)
==11913==    by 0x4395E4: receive_encoded_message (otrng.c:2228)
==11913==    by 0x43978B: receive_message_v4_only (otrng.c:2278)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 291 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x58920F1: _gcry_mpi_alloc_secure (mpiutil.c:105)
==11913==    by 0x58925EC: _gcry_mpi_copy (mpiutil.c:358)
==11913==    by 0x42F4BF: otrng_dh_mpi_copy (dh.c:221)
==11913==    by 0x4325BA: otrng_key_manager_set_their_dh (key_management.c:716)
==11913==    by 0x438470: receive_auth_r (otrng.c:1787)
==11913==    by 0x4394DA: receive_decoded_message (otrng.c:2197)
==11913==    by 0x4395E4: receive_encoded_message (otrng.c:2228)
==11913==    by 0x43978B: receive_message_v4_only (otrng.c:2278)
==11913== 
==11913== 32 bytes in 1 blocks are possibly lost in loss record 292 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D03CC: _gcry_malloc (global.c:936)
==11913==    by 0x57D03CC: _gcry_xmalloc (global.c:1110)
==11913==    by 0x5891F81: _gcry_mpi_alloc (mpiutil.c:84)
==11913==    by 0x42F1A5: otrng_dh_keypair_generate (dh.c:127)
==11913==    by 0x431249: otrng_key_manager_generate_ephemeral_keys (key_management.c:155)
==11913==    by 0x434CC4: start_dake (otrng.c:554)
==11913==    by 0x434EBF: receive_query_message (otrng.c:608)
==11913==    by 0x439772: receive_message_v4_only (otrng.c:2275)
==11913==    by 0x439891: otrng_receive_message (otrng.c:2308)
==11913==    by 0x4039AE: do_dake_fixture (test_fixtures.h:192)
==11913== 
==11913== 88 bytes in 1 blocks are possibly lost in loss record 412 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D04D2: _gcry_malloc_secure_core (global.c:946)
==11913==    by 0x57D04D2: _gcry_xmalloc_secure (global.c:1145)
==11913==    by 0x5891F4A: _gcry_mpi_alloc_limb_space (mpiutil.c:123)
==11913==    by 0x5892106: _gcry_mpi_alloc_secure (mpiutil.c:106)
==11913==    by 0x588FCBE: _gcry_mpi_scan (mpicoder.c:519)
==11913==    by 0x57CC3C8: gcry_mpi_scan (visibility.c:357)
==11913==    by 0x42F16A: otrng_dh_keypair_generate (dh.c:119)
==11913==    by 0x431249: otrng_key_manager_generate_ephemeral_keys (key_management.c:155)
==11913==    by 0x437D14: receive_identity_message_on_state_start (otrng.c:1615)
==11913==    by 0x437F1D: receive_identity_message (otrng.c:1672)
==11913== 
==11913== 392 bytes in 1 blocks are possibly lost in loss record 427 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D04D2: _gcry_malloc_secure_core (global.c:946)
==11913==    by 0x57D04D2: _gcry_xmalloc_secure (global.c:1145)
==11913==    by 0x57D0596: _gcry_xcalloc_secure (global.c:1190)
==11913==    by 0x5892259: _gcry_mpi_resize (mpiutil.c:187)
==11913==    by 0x588FFCB: mpi_fromstr (mpicoder.c:134)
==11913==    by 0x588FFCB: _gcry_mpi_scan (mpicoder.c:602)
==11913==    by 0x57CC3C8: gcry_mpi_scan (visibility.c:357)
==11913==    by 0x42EFEE: otrng_dh_init (dh.c:79)
==11913==    by 0x429C89: main (test.c:68)
==11913== 
==11913== 392 bytes in 1 blocks are possibly lost in loss record 428 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D04D2: _gcry_malloc_secure_core (global.c:946)
==11913==    by 0x57D04D2: _gcry_xmalloc_secure (global.c:1145)
==11913==    by 0x57D0596: _gcry_xcalloc_secure (global.c:1190)
==11913==    by 0x5892259: _gcry_mpi_resize (mpiutil.c:187)
==11913==    by 0x588FFCB: mpi_fromstr (mpicoder.c:134)
==11913==    by 0x588FFCB: _gcry_mpi_scan (mpicoder.c:602)
==11913==    by 0x57CC3C8: gcry_mpi_scan (visibility.c:357)
==11913==    by 0x42F012: otrng_dh_init (dh.c:82)
==11913==    by 0x429C89: main (test.c:68)
==11913== 
==11913== 392 bytes in 1 blocks are possibly lost in loss record 429 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D04D2: _gcry_malloc_secure_core (global.c:946)
==11913==    by 0x57D04D2: _gcry_xmalloc_secure (global.c:1145)
==11913==    by 0x5891F4A: _gcry_mpi_alloc_limb_space (mpiutil.c:123)
==11913==    by 0x5892106: _gcry_mpi_alloc_secure (mpiutil.c:106)
==11913==    by 0x58925EC: _gcry_mpi_copy (mpiutil.c:358)
==11913==    by 0x42F4BF: otrng_dh_mpi_copy (dh.c:221)
==11913==    by 0x4325BA: otrng_key_manager_set_their_dh (key_management.c:716)
==11913==    by 0x437CE9: receive_identity_message_on_state_start (otrng.c:1612)
==11913==    by 0x437F1D: receive_identity_message (otrng.c:1672)
==11913==    by 0x4394B9: receive_decoded_message (otrng.c:2195)
==11913== 
==11913== 392 bytes in 1 blocks are possibly lost in loss record 430 of 447
==11913==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2BC9: otrl_mem_malloc (mem.c:60)
==11913==    by 0x57CEE89: do_malloc (global.c:912)
==11913==    by 0x57D04D2: _gcry_malloc_secure_core (global.c:946)
==11913==    by 0x57D04D2: _gcry_xmalloc_secure (global.c:1145)
==11913==    by 0x5891F4A: _gcry_mpi_alloc_limb_space (mpiutil.c:123)
==11913==    by 0x5892106: _gcry_mpi_alloc_secure (mpiutil.c:106)
==11913==    by 0x58925EC: _gcry_mpi_copy (mpiutil.c:358)
==11913==    by 0x42F4BF: otrng_dh_mpi_copy (dh.c:221)
==11913==    by 0x4325BA: otrng_key_manager_set_their_dh (key_management.c:716)
==11913==    by 0x438470: receive_auth_r (otrng.c:1787)
==11913==    by 0x4394DA: receive_decoded_message (otrng.c:2197)
==11913==    by 0x4395E4: receive_encoded_message (otrng.c:2228)
==11913== 
==11913== 400 bytes in 1 blocks are possibly lost in loss record 431 of 447
==11913==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2CB7: otrl_mem_realloc (mem.c:136)
==11913==    by 0x57D020F: _gcry_realloc_core (global.c:998)
==11913==    by 0x57D0454: _gcry_xrealloc (global.c:1127)
==11913==    by 0x5892217: _gcry_mpi_resize (mpiutil.c:179)
==11913==    by 0x588BDD9: _gcry_mpi_sub_ui (mpi-add.c:178)
==11913==    by 0x42F065: otrng_dh_init (dh.c:89)
==11913==    by 0x429C89: main (test.c:68)
==11913== 
==11913== 776 bytes in 1 blocks are possibly lost in loss record 439 of 447
==11913==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2CB7: otrl_mem_realloc (mem.c:136)
==11913==    by 0x57D020F: _gcry_realloc_core (global.c:998)
==11913==    by 0x57D0454: _gcry_xrealloc (global.c:1127)
==11913==    by 0x5892217: _gcry_mpi_resize (mpiutil.c:179)
==11913==    by 0x588E3F7: _gcry_mpi_powm (mpi-pow.c:540)
==11913==    by 0x42F1D3: otrng_dh_keypair_generate (dh.c:128)
==11913==    by 0x431249: otrng_key_manager_generate_ephemeral_keys (key_management.c:155)
==11913==    by 0x434CC4: start_dake (otrng.c:554)
==11913==    by 0x434EBF: receive_query_message (otrng.c:608)
==11913==    by 0x439772: receive_message_v4_only (otrng.c:2275)
==11913==    by 0x439891: otrng_receive_message (otrng.c:2308)
==11913== 
==11913== 776 bytes in 1 blocks are possibly lost in loss record 440 of 447
==11913==    at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11913==    by 0x55B2CB7: otrl_mem_realloc (mem.c:136)
==11913==    by 0x57D020F: _gcry_realloc_core (global.c:998)
==11913==    by 0x57D0454: _gcry_xrealloc (global.c:1127)
==11913==    by 0x5892217: _gcry_mpi_resize (mpiutil.c:179)
==11913==    by 0x588E3F7: _gcry_mpi_powm (mpi-pow.c:540)
==11913==    by 0x42F1D3: otrng_dh_keypair_generate (dh.c:128)
==11913==    by 0x431249: otrng_key_manager_generate_ephemeral_keys (key_management.c:155)
==11913==    by 0x437D14: receive_identity_message_on_state_start (otrng.c:1615)
==11913==    by 0x437F1D: receive_identity_message (otrng.c:1672)
==11913==    by 0x4394B9: receive_decoded_message (otrng.c:2195)
==11913==    by 0x4395E4: receive_encoded_message (otrng.c:2228)
==11913== 

There is this bug that seems similar: https://bugzilla.redhat.com/show_bug.cgi?id=625459, and it explains that is probably a problem with the order of libraries we are passing while linking. This article explains further: http://fedoraproject.org/wiki/UnderstandingDSOLinkChange

Other people suggest that this can be a problem with circular dependencies.

Changing to AM_PATH_LIBGCRYPT(1:1.6.0,, [AC_MSG_ERROR(libgcrypt 1.7.8 or newer is required.)]) fixed the problem, but a lot of memory leaks on several machines start popping out.

I'm afraid that this is an actual linkage problem and changing to AM_PATH_LIBGCRYPT(1:1.6.0,, [AC_MSG_ERROR(libgcrypt 1.7.8 or newer is required.)]) just masks it, due to the oddity of how it happens.

So @juniorz what is your os and compiler?

Any thoughts @juniorz @olabini ?

Anyhow, will look further into this.

claucece commented 6 years ago

Update: running with ./configure --enable-valgrind LIBS="-lgcrypt" also fixes it.

olabini commented 6 years ago

Uhm. That's weird. - incidentally, by using the LIBS="-lgcrypt" you are just forcing the compiler to also link against whatever libgcrypt is on your ldconfig - instead of using the one configure decides based on versions and package information.

This sounds like the kind of fun problem I know @claucece loves to investigate...

No, but honestly, I have no useful ideas right now. It's confusing.

juniorz commented 6 years ago

@claucece, from the logs compiler is gcc.

$ gcc --version
gcc (Ubuntu 7.2.0-8ubuntu3.2) 7.2.0
...

$ cat /etc/os-release 
NAME="Ubuntu"
VERSION="17.10 (Artful Aardvark)"
...
juniorz commented 6 years ago

I feel uncomfortable running the library with a vulnerable version. However, doing an exit() from a library is also EXTREMELY rude...

libotr has a otrl_init function which simply returns an error if initialization fails, but the macro OTRL_INIT invokes this function and aborts if the initialization failed.

We could follow the same approach. It allows people to opt-out of the rudeness if they invoke the function and deal with the error appropriately.

claucece commented 6 years ago

This sounds like the kind of fun problem I know @claucece loves to investigate...

hahhaha maybe ;)

from the logs compiler is gcc.

My bad, of course ;)