stef / libopaque

c implementation of the OPAQUE protocol with bindings for python, php, ruby, lua, zig, java, erlang, golang, js and SASL.
GNU Lesser General Public License v3.0
69 stars 10 forks source link

Allow systems that do not support mlock to use libopaque #7

Closed creemama closed 3 years ago

creemama commented 3 years ago

I have been working on JavaScript bindings for libopaque: https://github.com/creemama/libopaque/tree/javascript-bindings/js . sodium_mlock and sodium_munlock are not fully supported when libopaque is compiled using Emscripten to create the JavaScript bindings.

I could think of two ways to handle this.

(1) The first is this commit where we introduce opaque_mlock and opaque_munlock to wrap sodium_mlock and sodium_munlock. When compiled using EMSCRIPTEN, then we disregard sodium_mlock and sodium_munlock failures.

(2) The second is this commit (https://github.com/creemama/libopaque/commit/294bd6f4b6d167333f65c68dbe304368db610d06) where we no longer check if sodium_mlock or sodium_munlock returns successfully. This seems plausible since opaque.c does not check the return value of sodium_mlock or sodium_munlock all the time.

@stef, what do you think? Is there another possibility I am not thinking of?

stef commented 3 years ago

maybe figure out what compiler symbol is defined when compiling for emscripten, and then do something like:

ifdef EMSCRIPTEN

define sodium_mlock { return 0; }

define sodium_munlock { return 0;}

endif

or maybe figure out how other projects handle this, i guess the return 0 is the worst, maybe there's partial solutions that do not mlock but at least sanitize the memory range?

creemama commented 3 years ago

maybe figure out what compiler symbol is defined when compiling for emscripten, and then do something like:

ifdef EMSCRIPTEN

define sodium_mlock { return 0; }

define sodium_munlock { return 0;}

endif

or maybe figure out how other projects handle this, i guess the return 0 is the worst, maybe there's partial solutions that do not mlock but at least sanitize the memory range?

+1 Your recommendation is less invasive than my other attempts. I introduced a -DNO_LOCK flag in this latest pull request. I think this is ready to merge with your approval.

sodium_munlock always calls sodium_memzero, so the partial solution that sanitizes the memory range is there. See https://github.com/jedisct1/libsodium/blob/1.0.18/src/libsodium/sodium/utils.c:

int
sodium_munlock(void *const addr, const size_t len)
{
    sodium_memzero(addr, len);
    ...
}

As for how other projects handle this, I did not see any JavaScript projects addressing it: https://github.com/search?&q=sodium_mlock&type=issues . Both https://github.com/jedisct1/libsodium.js/ and https://github.com/sodium-friends/sodium-universal do not implement sodium_mlock or sodium_munlock.

stef commented 3 years ago

maybe just use __EMSCRIPTEN__ as defined per https://emscripten.org/docs/compiling/Building-Projects.html#detecting-emscripten-in-preprocessor

creemama commented 3 years ago

+1 What a good recommendation. Now I don't need to specify any additional preprocessor macros when compiling.

stef commented 3 years ago

wonderful! thanks.