rweather / noise-c

Noise-C, a plain C implementation of the Noise protocol
MIT License
295 stars 82 forks source link

USE_SODIUM in internal.c is never set #16

Open jpf91 opened 7 years ago

jpf91 commented 7 years ago

The USE_SODIUM flag in internal.c is not set by the configure scripts when configuring with libsodium. This leads to undefined references to noise_aesgcm_new_ref on older systems (like travis-Ci ubuntu 14.04). On newer systems the symbol is still undefined, but this is somehow ignored by the linker:

nm src/protocol/libnoiseprotocol.a | grep noise_aesgcm_new_ref

U noise_aesgcm_new_ref
jpf91 commented 7 years ago

Turns out the linker error actually is unrelated to USE_SODIUM being undefined. The linker error is caused by a libsodium/no openssl configuration. It seems the _ref backend code is not build in this case but still required by internal.c.

plizonczyk commented 7 years ago

I can confirm. Compilation fails due to unknown symbol noise_aesgcm_new_ref when trying to build in libsodium and no openssl configuration.

benma commented 5 years ago

Same error for me. Anyone has an idea how to solve it?

danielschonfeld commented 4 years ago

If anyone is still interested, it seems like adding: /backend/ref/cipher-aesgcm.c /crypto/aes/rijndael-alg-fst.c /crypto/ghash/ghash.c

To the build process solves this problem, also a good idea to add a USE_SODIUM=1 in configure or alternatively to change USE_SODIUM to USE_LIBSODIUM in internal.c

david-komarek commented 4 years ago

It seems that in internal.c are #ifdefs that should be rewritten.

Please inspect following patch:

From 075023ef0459ce56341f1625ca6e15b3fa72fe62 Mon Sep 17 00:00:00 2001
From: David Komarek
Date: Wed, 6 May 2020 10:47:42 +0200
Subject: [PATCH] #ifdefed calls to reference implementation in case that
 USE_SODIUM is #defined

---
 src/protocol/internal.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/protocol/internal.c b/src/protocol/internal.c
index 28e97e6..e7eede5 100644
--- a/src/protocol/internal.c
+++ b/src/protocol/internal.c
@@ -28,7 +28,8 @@ NoiseCipherState *noise_aesgcm_new_sodium(void);
 #endif
 #if USE_OPENSSL
 NoiseCipherState *noise_aesgcm_new_openssl(void);
-#else
+#endif
+#if !USE_SODIUM && !USE_OPENSSL
 NoiseCipherState *noise_aesgcm_new_ref(void);
 #endif

@@ -47,7 +48,8 @@ NoiseCipherState *noise_aesgcm_new(void)
 #if USE_OPENSSL
     if (!state)
         state = noise_aesgcm_new_openssl();
-#else
+#endif
+#if !USE_SODIUM && !USE_OPENSSL
     if (!state)
         state = noise_aesgcm_new_ref();
 #endif
-- 
2.7.4
dereulenspiegel commented 2 years ago

I just wanted to let you know that PR #49 should fix this problem. Imho the behavior regarding the defines makes sense, since it seems to be possible that libsodium gets used, but was compiled without AES GCM support. As long as build flags of libsodium aren't accessible and interpreted it makes sense to have a fallback to the reference implementation. But I would also wish for a solution which doesn't rely on the reference implementation. But I think consequences of this should be discussed.