Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USE_SODIUM in internal.c is never set #16

Open
jpf91 opened this issue Mar 30, 2017 · 6 comments
Open

USE_SODIUM in internal.c is never set #16

jpf91 opened this issue Mar 30, 2017 · 6 comments

Comments

@jpf91
Copy link

jpf91 commented Mar 30, 2017

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
Copy link
Author

jpf91 commented Mar 30, 2017

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
Copy link

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

@benma
Copy link

benma commented Jan 21, 2019

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

@danielschonfeld
Copy link

danielschonfeld commented Jul 31, 2019

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
Copy link

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants