4 ms·

Hey, fantastic work! Really happy to see some amateur crypto development out there. I started as an amateur also, now I work in crypto product dev. Definitel

Hey, fantastic work! Really happy to see some amateur crypto development out there. I started as an amateur also, now I work in crypto product dev. Definitely impressed if this is your first crypto work. I really like the simplicity, reminds me of OpenSSH. I gotta agree with some of the naysayers though - this needs more time to brew. Even if it's perfect, there still more proof to show. My suggestions: * A long-running interoperability test between your lib and another. Test vectors don't catch everything. Run it for a week, check results and publish the results. * Fuzzing. You need to to catch false results, random crashes, stack overflows, etc. Fuzz every input, do it for random lengths (both underflow and overflow). * Maybe port it and test on Win, Mac, ARM, MIPS. I find a lot of bugs shake out during ports. * Timing. You can't prevent all side-channel attacks, but bits going out are easy attack vectors. Use a high-precision timing mechanism to catch any unusual spikes. Capture a few hours of points for each functional path, and publish the results. (crypto_aead_unlock has two return paths, it might be better to maintain timing). * Personal preferences: Rather than using int returns, use bool. Zero your ctx vars, e.g. a_ctx ctx = {0,}; I spend more time in testing products than development, honestly. Publish as much relevant data as you can.

loup-vaillant 9y ago
> Definitely impressed if this is your first crypto work. It is. :-) > A long-running interoperability test between your lib and another. Already have one for all primitives. Just tweak the parameters to make it run for a long time. The test code is in test/sodium.c and test/donna.c > Fuzzing. You need to to catch false results, random crashes, stack overflows, etc. Fuzz every input, do it for random lengths (both underflow and overflow). I'm not sure how to go about it. I already check every input size, from zero to several times the size of the internal block, in my comparison tests. Of course, all inputs are correct. Incorrect inputs result in undefined behaviour, checking for that is out of scope. (That's why most functions return void.) > Maybe port it and test on Win, Mac, ARM, MIPS. I find a lot of bugs shake out during ports. Yeah, I only tried GCC and Clang on my Intel Ubuntu machine. But I did use Valgrind, ASan, MSan, UBSan, and the TIS-Interpreter. I also compiled under various standards: C99, C11, C++98, C++11, C++14, C++17. > Timing. […] Use a high-precision timing mechanism to catch any unusual spikes Isn't that a bit overkill? I'd rather manually review the source code for the absence of secret dependant branches or secret dependent array indices. I'm not implementing AES or RSA, the primitive I have chosen were specifically designed to easily avoid timing attacks. > crypto_aead_unlock has two return paths, it might be better to maintain timing. No. There are 2 cases to consider: failure, and success. Each path runs in constant time, so timing can only distinguish success from failure… which is revealed anyway by the return code. > Rather than using int returns, use bool. I'm afraid bool is a tiny bit less portable than int. I also went with the flow about error codes, which are traditionally int. > Zero your ctx vars, e.g. a_ctx ctx = {0,}; I'm not sure how to do it portably, with zero dependency.

loup-vaillant 9y ago

> Definitely impressed if this is your first crypto work. It is. :-) > A long-running interoperability test between your lib and another. Already have one for all primitives. Just tweak the parameters to make it run for a long time. The test code is in test/sodium.c and test/donna.c > Fuzzing. You need to to catch false results, random crashes, stack overflows, etc. Fuzz every input, do it for random lengths (both underflow and overflow). I'm not sure how to go about it. I already check every input size, from zero to several times the size of the internal block, in my comparison tests. Of course, all inputs are correct. Incorrect inputs result in undefined behaviour, checking for that is out of scope. (That's why most functions return void.) > Maybe port it and test on Win, Mac, ARM, MIPS. I find a lot of bugs shake out during ports. Yeah, I only tried GCC and Clang on my Intel Ubuntu machine. But I did use Valgrind, ASan, MSan, UBSan, and the TIS-Interpreter. I also compiled under various standards: C99, C11, C++98, C++11, C++14, C++17. > Timing. […] Use a high-precision timing mechanism to catch any unusual spikes Isn't that a bit overkill? I'd rather manually review the source code for the absence of secret dependant branches or secret dependent array indices. I'm not implementing AES or RSA, the primitive I have chosen were specifically designed to easily avoid timing attacks. > crypto_aead_unlock has two return paths, it might be better to maintain timing. No. There are 2 cases to consider: failure, and success. Each path runs in constant time, so timing can only distinguish success from failure… which is revealed anyway by the return code. > Rather than using int returns, use bool. I'm afraid bool is a tiny bit less portable than int. I also went with the flow about error codes, which are traditionally int. > Zero your ctx vars, e.g. a_ctx ctx = {0,}; I'm not sure how to do it portably, with zero dependency.