From 1121321573fdbfc52ea3d2ebba8d01c7f6353c33 Mon Sep 17 00:00:00 2001 From: Hans Svensson Date: Thu, 8 Nov 2018 10:43:51 +0100 Subject: [PATCH 1/3] Generic hash state needs to be 64-byte aligned At least according to: https://libsodium.gitbook.io/doc/hashing/generic_hashing We noticed crashes when it was not 16-byte aligned - probably is architecture dependent. This makes the safe choice and always 64-byte align it. --- c_src/enacl_nif.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/c_src/enacl_nif.c b/c_src/enacl_nif.c index 364a47d..1497215 100644 --- a/c_src/enacl_nif.c +++ b/c_src/enacl_nif.c @@ -1511,6 +1511,13 @@ ERL_NIF_TERM enif_crypto_generichash(ErlNifEnv *env, int argc, ERL_NIF_TERM cons return enif_make_tuple2(env, ok, ret); } +static +crypto_generichash_state *align64(void *ptr){ + if((unsigned long)ptr % 64 == 0) + return ptr; + return ptr + (64 - ((unsigned long)ptr % 64)); +} + static ERL_NIF_TERM enif_crypto_generichash_init(ErlNifEnv *env, int argc, ERL_NIF_TERM const argv[]) { ErlNifBinary key; @@ -1538,14 +1545,14 @@ ERL_NIF_TERM enif_crypto_generichash_init(ErlNifEnv *env, int argc, ERL_NIF_TERM return nacl_error_tuple(env, "invalid_key_size"); } - // Create a resource for hash state - crypto_generichash_state *state = (crypto_generichash_state *)enif_alloc_resource(generichash_state_type, crypto_generichash_statebytes()); + // Create a resource for hash state (+ 60 to make room for 64-byte alignment) + void *state = enif_alloc_resource(generichash_state_type, crypto_generichash_statebytes() + 60); if( !state ) { return nacl_error_tuple(env, "alloc_failed"); } // Call the library function - if( 0 != crypto_generichash_init(state, k, key.size, hashSize) ) { + if( 0 != crypto_generichash_init(align64(state), k, key.size, hashSize) ) { return nacl_error_tuple(env, "hash_init_error"); } @@ -1569,7 +1576,7 @@ ERL_NIF_TERM enif_crypto_generichash_update(ErlNifEnv *env, int argc, ERL_NIF_TE unsigned hashSize; - crypto_generichash_state *state; + void *state; // Validate the arguments if( (argc != 3) || @@ -1586,11 +1593,10 @@ ERL_NIF_TERM enif_crypto_generichash_update(ErlNifEnv *env, int argc, ERL_NIF_TE } // Update hash state - if( 0 != crypto_generichash_update(state, message.data, message.size) ) { + if( 0 != crypto_generichash_update(align64(state), message.data, message.size) ) { return nacl_error_tuple(env, "hash_update_error"); } - // Generate return value ERL_NIF_TERM e1 = enif_make_atom(env, "hashstate"); ERL_NIF_TERM e2 = argv[0]; @@ -1606,7 +1612,7 @@ ERL_NIF_TERM enif_crypto_generichash_final(ErlNifEnv *env, int argc, ERL_NIF_TER unsigned hashSize; - crypto_generichash_state *state; + void *state; // Validate the arguments if( (argc != 2) || @@ -1626,8 +1632,8 @@ ERL_NIF_TERM enif_crypto_generichash_final(ErlNifEnv *env, int argc, ERL_NIF_TER return nacl_error_tuple(env, "alloc_failed"); } - // calculate hash - if( 0 != crypto_generichash_final(state, hash.data, hash.size) ) { + // calculate hash + if( 0 != crypto_generichash_final(align64(state), hash.data, hash.size) ) { enif_release_binary(&hash); return nacl_error_tuple(env, "hash_error"); } From 26180f42c0b3a450905d2efd8bc7fd5fd9cece75 Mon Sep 17 00:00:00 2001 From: Tino Breddin Date: Fri, 23 Nov 2018 15:16:41 +0100 Subject: [PATCH 2/3] Add win32 support Replace Make-based compilation of the nif to using rebar's port compiler. --- .gitignore | 4 +++ c_src/Makefile | 75 ----------------------------------------------- c_src/enacl_nif.c | 2 +- rebar.config | 40 +++++++++++++++++++++---- 4 files changed, 39 insertions(+), 82 deletions(-) delete mode 100644 c_src/Makefile diff --git a/.gitignore b/.gitignore index 76e43f3..a0ca3ba 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,7 @@ doc/*.png doc/*.css _build /.eqc-info +priv/enacl_nif.dll +priv/enacl_nif.exp +priv/enacl_nif.lib +c_src/enacl_nif.d diff --git a/c_src/Makefile b/c_src/Makefile deleted file mode 100644 index 6fbe66e..0000000 --- a/c_src/Makefile +++ /dev/null @@ -1,75 +0,0 @@ -# Based on c_src.mk from erlang.mk by Loïc Hoguin - -PROJECT ?= enacl_nif - -ERTS_INCLUDE_DIR ?= $(shell erl -noshell -s init stop -eval "io:format(\"~s/erts-~s/include/\", [code:root_dir(), erlang:system_info(version)]).") -ERL_INTERFACE_INCLUDE_DIR ?= $(shell erl -noshell -s init stop -eval "io:format(\"~s\", [code:lib_dir(erl_interface, include)]).") -ERL_INTERFACE_LIB_DIR ?= $(shell erl -noshell -s init stop -eval "io:format(\"~s\", [code:lib_dir(erl_interface, lib)]).") - -C_SRC_DIR = $(CURDIR) -C_SRC_OUTPUT ?= $(CURDIR)/../priv/$(PROJECT).so - -# System type and C compiler/flags. -MACHINE_SYS := $(shell uname -m) -UNAME_SYS := $(shell uname -s) -ifeq ($(UNAME_SYS), Darwin) - CC ?= cc - CFLAGS ?= -O3 -std=c99 -arch x86_64 -finline-functions -Wall -Wmissing-prototypes - CXXFLAGS ?= -O3 -arch x86_64 -finline-functions -Wall - LDFLAGS ?= -arch x86_64 -flat_namespace -undefined suppress -else ifeq ($(UNAME_SYS), FreeBSD) - CC ?= cc - CFLAGS ?= -O3 -std=c99 -finline-functions -Wall -Wmissing-prototypes -I /usr/local/include - CXXFLAGS ?= -O3 -finline-functions -Wall - LDFLAGS ?= -fPIC -L /usr/local/lib -else ifeq ($(UNAME_SYS), Linux) - CC ?= gcc - CFLAGS ?= -O3 -std=c99 -finline-functions -Wall -Wmissing-prototypes - CXXFLAGS ?= -O3 -finline-functions -Wall -else ifeq ($(UNAME_SYS), SunOS) - CC = gcc - CFLAGS ?= -m64 -I/opt/local/include -O2 -std=c99 -finline-functions -Wall -Wmissing-prototypes - CXXFALGS ?= -O2 -finline-function -Wall - LDFLAGS ?= -m64 -fPIC -L /opt/local/lib -endif - -CFLAGS += -fPIC -I $(ERTS_INCLUDE_DIR) -I $(ERL_INTERFACE_INCLUDE_DIR) -CXXFLAGS += -fPIC -I $(ERTS_INCLUDE_DIR) -I $(ERL_INTERFACE_INCLUDE_DIR) - -LDLIBS += -L $(ERL_INTERFACE_LIB_DIR) -lerl_interface -lei -lsodium -LDFLAGS += -shared - -# Verbosity. - -c_verbose_0 = @echo " C " $(?F); -c_verbose = $(c_verbose_$(V)) - -cpp_verbose_0 = @echo " CPP " $(?F); -cpp_verbose = $(cpp_verbose_$(V)) - -link_verbose_0 = @echo " LD " $(@F); -link_verbose = $(link_verbose_$(V)) - -SOURCES := $(shell find $(C_SRC_DIR) -type f \( -name "*.c" -o -name "*.C" -o -name "*.cc" -o -name "*.cpp" \)) -OBJECTS = $(addsuffix .o, $(basename $(SOURCES))) - -COMPILE_C = $(c_verbose) $(CC) $(CFLAGS) $(CPPFLAGS) -c -COMPILE_CPP = $(cpp_verbose) $(CXX) $(CXXFLAGS) $(CPPFLAGS) -c - -$(C_SRC_OUTPUT): $(OBJECTS) - $(link_verbose) $(CC) $(OBJECTS) $(LDFLAGS) $(LDLIBS) -o $(C_SRC_OUTPUT) - -%.o: %.c - $(COMPILE_C) $(OUTPUT_OPTION) $< - -%.o: %.cc - $(COMPILE_CPP) $(OUTPUT_OPTION) $< - -%.o: %.C - $(COMPILE_CPP) $(OUTPUT_OPTION) $< - -%.o: %.cpp - $(COMPILE_CPP) $(OUTPUT_OPTION) $< - -clean: - @rm -f $(C_SRC_OUTPUT) $(OBJECTS) diff --git a/c_src/enacl_nif.c b/c_src/enacl_nif.c index 1497215..2af0b0a 100644 --- a/c_src/enacl_nif.c +++ b/c_src/enacl_nif.c @@ -1515,7 +1515,7 @@ static crypto_generichash_state *align64(void *ptr){ if((unsigned long)ptr % 64 == 0) return ptr; - return ptr + (64 - ((unsigned long)ptr % 64)); + return (unsigned long)ptr + (64 - ((unsigned long)ptr % 64)); } static diff --git a/rebar.config b/rebar.config index 36767fa..36c287a 100644 --- a/rebar.config +++ b/rebar.config @@ -1,9 +1,37 @@ {erl_opts, [debug_info]}. -{pre_hooks, [{"freebsd", compile, "gmake -C c_src"}, - {"freebsd", clean, "gmake -C c_src clean"}, - {"netbsd", compile, "gmake -C c_src"}, - {"netbsd", clean, "gmake -C c_src clean"}, - {"(linux|darwin|solaris)", compile, "make -C c_src"}, - {"(linux|darwin|solaris)", clean, "make -C c_src clean"} +{plugins, [pc]}. + +{provider_hooks, [ + {pre, [ + {compile, {pc, compile}}, + {clean, {pc, clean}} + ]} +]}. + +{port_specs, [ + {"priv/enacl_nif.so", [ + "c_src/*.c" + ]} +]}. + +{port_env, [ + {"darwin", "CFLAGS", "$CFLAGS -fPIC -O3 -std=c99 -arch x86_64 -finline-functions -Wall -Wmissing-prototypes"}, + {"darwin", "CXXFLAGS", "$CXXFLAGS -fPIC -O3 -arch x86_64 -finline-functions -Wall"}, + {"darwin", "LDFLAGS", "$LDFLAGS -arch x86_64 -flat_namespace -undefined suppress -lsodium"}, + + {"linux", "CFLAGS", "$CFLAGS -fPIC -O3 -std=c99 -finline-functions -Wall -Wmissing-prototypes"}, + {"linux", "CXXFLAGS", "$CXXFLAGS -fPIC -O3 -finline-functions -Wall"}, + {"linux", "LDFLAGS", "$LDFLAGS -lsodium"}, + + {"freebsd", "CFLAGS", "$CFLAGS -fPIC -O3 -std=c99 -finline-functions -Wall -Wmissing-prototypes -I /usr/local/include"}, + {"freebsd", "CXXFLAGS", "$CXXFLAGS -fPIC -O3 -finline-functions -Wall"}, + {"freebsd", "LDFLAGS", "$LDFLAGS -fPIC -L /usr/local/lib -lsodium"}, + + {"solaris", "CFLAGS", "$CFLAGS -fPIC -m64 -I/opt/local/include -O2 -std=c99 -finline-functions -Wall -Wmissing-prototypes"}, + {"solaris", "CXXFLAGS", "$CXXFLAGS -fPIC -O2 -finline-function -Wall"}, + {"solaris", "LDFLAGS", "$LDFLAGS -m64 -fPIC -L /opt/local/lib -lsodium"}, + + {"win32", "CFLAGS", "$CFLAGS /LD /O2 /DNDEBUG"}, + {"win32", "LDFLAGS", "$LDFLAGS libsodium.dll.a"} ]}. From 868a14c25da4b5fcf0a1f4783202ad7800fe4682 Mon Sep 17 00:00:00 2001 From: Bryan Paxton Date: Sun, 26 Jul 2020 14:25:24 -0500 Subject: [PATCH 3/3] Ensure we never return 1 from sodium_init() onload sodium_init() will return 0 on success, -1 on failure, and 1 if sodium is already loaded and initialized (which is not an error). In the case where libsodium is already initialized and the system is restarted we may return 1 from onload nif function resulting in a crash. - change the call to sodium_init() to check for an error return (-1) and return -1 explicitly in this case, otherwise always return zero at the end of our onload function. --- c_src/enacl_nif.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/c_src/enacl_nif.c b/c_src/enacl_nif.c index 2af0b0a..db34ab8 100644 --- a/c_src/enacl_nif.c +++ b/c_src/enacl_nif.c @@ -36,7 +36,11 @@ int enif_crypto_load(ErlNifEnv *env, void **priv_data, ERL_NIF_TERM load_info) { return -1; } - return sodium_init(); + if (sodium_init() == -1) { + return -1; + } + + return 0; } /* Low-level functions (Hashing, String Equality, ...) */