node-scrypt
node-scrypt copied to clipboard
barrysteyn
"Generic scrypt code is broken - please report bug!"
While running the latest 6.0.1 version compiled with npm I ran into "Generic scrypt code is broken - please report bug!" which appears to be coming from the native scrypt layer. Notably this error does not appear in the 5.4 version compiled and run in the same way. For now sticking with 5.4 solves my problem but I'm tracking this in case it helps others.
I filed a bug with scrypt directly here https://github.com/Tarsnap/scrypt/issues/36#issuecomment-194623492 however the developer is askng me specifics I don't really have the answers for. I'm wondering if one of the devs on this project can take a look, and I'll of course support how I can.
Just to provide some context here -- this error is coming from a failed self-test, which probably means that crypto_scrypt_smix.c is being compiled incorrectly (probably due to a compiler optimizing inappropriately).
The self-test doesn't exist in earlier versions, so it's possible that earlier versions are silently producing incorrect results...
@acgourley Can you please tell us your setup. What compiler were you using, what platform are running things on?
So it was built on a debian system via node-gyp as a dependency for node-scrypt@6 which used this gyp config file: https://github.com/barrysteyn/node-scrypt/blob/v6/binding.gyp
uname -a
Linux worker 3.2.0-4-amd64 #1 SMP Debian 3.2.63-2 x86_64 GNU/Linux
However I don't really know which compiler that invokes.
type gcc --version - that should give us the version. Although, at this point, it might not be of any use.
gcc --version
gcc (Debian 4.7.2-5) 4.7.2
It may also be important this was done as part of a docker build done on a debian wheezy image.
Perhaps the docker could be affecting it (maybe you have not installed a library which scrypt assumes to be present). Anyways, what I am going to do is try to change things to use Colin's reference implementation (and nothing fancy). I will try change it tonight, and then lets see if you have any success after that?
Great. Happy to test and see if it fixes it. Not a huge rush as I have a work around and this isn't a production system yet.
@acgourley On further investigation, what node-scrypt is doing on your platform is running the configure script before building your platform. My guess is that something in that script is being incorrectly set. Go to where node-scrypt is (you will need to run this inside your docker container), and find the scrypt/scrypt-1.2.0 folder. Once there, run make clean. After it has been cleaned, run ./configure, and then screen dump what is in there.
@acgourley Try cloning the branch scrypt-reference-imp - build that and see if things work. Report back to me...
There was no clean target, so I just removed the object files
root@0ade0c6211fe:/app/node_modules/scrypt/scrypt/scrypt-1.2.0# rm *.o
root@0ade0c6211fe:/app/node_modules/scrypt/scrypt/scrypt-1.2.0# ./configure
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking for style of include used by make... GNU
checking dependency style of gcc... gcc3
checking for ranlib... ranlib
checking whether to enable maintainer-specific portions of Makefiles... no
checking for clock_gettime in -lrt... yes
checking for clock_gettime... yes
checking how to run the C preprocessor... gcc -E
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking sys/sysinfo.h usability... yes
checking sys/sysinfo.h presence... yes
checking for sys/sysinfo.h... yes
checking for sysinfo... yes
checking for struct sysinfo... yes
checking for struct sysinfo.totalram... yes
checking for struct sysinfo.mem_unit... yes
checking sys/param.h usability... yes
checking sys/param.h presence... yes
checking for sys/param.h... yes
checking sys/sysctl.h usability... yes
checking sys/sysctl.h presence... yes
checking for sys/sysctl.h... yes
checking for posix_memalign... yes
checking for mmap... yes
checking for special C compiler options needed for large files... no
checking for _FILE_OFFSET_BITS value needed for large files... no
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating config.h
config.status: config.h is unchanged
config.status: executing depfiles commands
There should be a clean target. Can you make sure you have version 6.01. Not that that would make a difference, but please make sure you have the latest release.
@acgourley I have confirmed with Colin that this is not a fix. Using the reference implementation will be a security risk as you give an attacker a huge advantage: The attacker will be able to run scrypt MUCH faster than you will with the reference implementation.
There is a way forward though: We can repro what is happening very easily since you are using Docker. Are you prepared to give me an image (or a docker build file so I can build the image) that you are running the module on? I am then able to very easily repro and Colin will be able to help as well.
I may need about a month time to put this together as I need to remove sensitive IP and will be traveling soon. If there is a faster way to to send over information let me know. Thanks for your work here.
On Tue, Mar 15, 2016 at 4:22 PM, Barry Steyn [email protected] wrote:
@acgourley https://github.com/acgourley I have confirmed with Colin that this is not a fix. Using the reference implementation will be a security risk as you give an attacker a huge advantage: The attacker will be able to run scrypt MUCH faster than you will with the reference implementation.
There is a way forward though: We can repro what is happening very easily since you are using Docker. Are you prepared to give me an image (or a docker build file so I can build the image) that you are running the module on? I am then able to very easily repro and Colin will be able to help as well.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/barrysteyn/node-scrypt/issues/123#issuecomment-197066265
This might be a faster way forward - I tried simply running the tests on the v6 branch using the on the same build server which made the failing docker container and it failed with the same problem.
$ uname -a
Linux worker 3.2.0-4-amd64 #1 SMP Debian 3.2.63-2 x86_64 GNU/Linux
$ gcc --version
gcc (Debian 4.7.2-5) 4.7.2
$ make clean
test -z "scrypt" || rm -f scrypt
test -z "cpusupport-config.h cpusupport-config.h.tmp" || rm -f cpusupport-config.h cpusupport-config.h.tmp
test -z "libcperciva_aesni.a libscrypt_sse2.a" || rm -f libcperciva_aesni.a libscrypt_sse2.a
rm -f *.o
rm -f lib/crypto/*.o
rm -f lib/scryptenc/*.o
rm -f lib/util/*.o
rm -f libcperciva/alg/*.o
rm -f libcperciva/cpusupport/*.o
rm -f libcperciva/crypto/*.o
rm -f libcperciva/util/*.o
$ ./configure
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking for style of include used by make... GNU
checking dependency style of gcc... gcc3
checking for ranlib... ranlib
checking whether to enable maintainer-specific portions of Makefiles... no
checking for clock_gettime in -lrt... yes
checking for clock_gettime... yes
checking how to run the C preprocessor... gcc -E
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking sys/sysinfo.h usability... yes
checking sys/sysinfo.h presence... yes
checking for sys/sysinfo.h... yes
checking for sysinfo... yes
checking for struct sysinfo... yes
checking for struct sysinfo.totalram... yes
checking for struct sysinfo.mem_unit... yes
checking sys/param.h usability... yes
checking sys/param.h presence... yes
checking for sys/param.h... yes
checking sys/sysctl.h usability... yes
checking sys/sysctl.h presence... yes
checking for sys/sysctl.h... yes
checking for posix_memalign... yes
checking for mmap... yes
checking for special C compiler options needed for large files... no
checking for _FILE_OFFSET_BITS value needed for large files... no
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating config.h
config.status: config.h is unchanged
config.status: executing depfiles commands
@barrysteyn I also encountered this problem running in the centos6.5 system：
(unknown): Generic scrypt code is broken - please report bug!
events.js:182
throw er; // Unhandled 'error' event ^
$ uname -a
Linux testserver 2.6.32-642.11.1.el6.x86_64 #1 SMP Fri Nov 18 19:25:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
$ gcc --version
gcc (GCC) 4.8.2
Replace the scrypt/scrypt-1.2.0/lib/crypto/crypto_scrypt.c under the binding.gyp file as scrypt/scrypt-1.2.0/lib/crypto/crypto_scrypt-ref.c, It's fixed.