technion/libscrypt

libscrypt does not compile on OS X

Closed this issue · 15 comments

alex commented

I assume this is a result of the GNU vs. clang toolchain. Anyways, here is the error:

$ make
gcc -O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC   -c -o crypto_scrypt-nosse.o crypto_scrypt-nosse.c
gcc -O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC   -c -o sha256.o sha256.c
gcc -O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC   -c -o crypto_scrypt-hexconvert.o crypto_scrypt-hexconvert.c
gcc -O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC   -c -o crypto-mcf.o crypto-mcf.c
gcc -O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC   -c -o b64.o b64.c
gcc -O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC   -c -o crypto-scrypt-saltgen.o crypto-scrypt-saltgen.c
gcc -O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC   -c -o crypto_scrypt-check.o crypto_scrypt-check.c
gcc -O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC   -c -o crypto_scrypt-hash.o crypto_scrypt-hash.c
gcc  -Wl,-z,now -Wl,-z,relro -shared -Wl,-soname,libscrypt.so.0 -Wl,--version-script=libscrypt.version -o libscrypt.so.0 -lc -lm  crypto_scrypt-nosse.o sha256.o crypto_scrypt-hexconvert.o crypto-mcf.o b64.o crypto-scrypt-saltgen.o crypto_scrypt-check.o crypto_scrypt-hash.o
ld: unknown option: -z
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [library] Error 1
hynek commented

FWIW, there’s several GNU specific options used in the Makefile. Ripping them out like https://gist.github.com/hynek/86ac9d9cfb8ccedf0453 makes it compile (and pass the tests, but I’m not sure what the linker options were needed for).

Unfortunately, cross-platform compiling is still annoying, that’s why we’re still punished by auto tools. :(

acv commented

Naming convention for shared lib on OS X is name.version.dylib like libscrypt.0.dylib

The book: https://developer.apple.com/library/mac/documentation/DeveloperTools/Conceptual/DynamicLibraries/100-Articles/DynamicLibraryDesignGuidelines.html

technion commented

Hey guys,

Thanks for the report. I've tried to support as much as I can but Mac is one OS I don't have access to, so I'm relying on someone else to provide a patch. I'd really prefer not to remove all the GNU options from the Makefile given how heavily they have been tested elsewhere, but if someone could provide the right syntax so someone could "make osx" I'd be happy to merge it.

technion commented

Or, please just try a LDFLAGS="" make, which will zero out some of the potentially impacted flags, and let us know if that does the job.
To clarify, those flags are gcc's security features, and no libscrypt functionality relies on it. If clearing the flags from the make command line allows a correct run on OSX I'll document as such.

acv commented

So LDFLAGS="" doesn't cut it.

Are you opposed to using the GNU auto tools?

technion commented

I have used autoconf in other projects. The vast majority of development time, bugfixes and git commits ended up related to autoconf, and the vast majority of portability issues stem from how autoconf handles their OS, or the creation of a build environment that requires an exact version of autoconf, completely removing the purpose of the tool.

I was originally told I would need to use autoconf if libscrypt was to work on both Debian and Redhat. Sounds like people haven't heard of portable code.

If I was developing something like Apache and needed to support modules, there's a reason for that effort. In this case there is not. As discussed, libscrypt compiles and runs without issues on CentOS, Raspbian(ARM), Fedora(tested on x64 and s390 platform), FreeBSD, OpenBSD, aside from OS X the portability argument just isn't there.

If OS X compatibility is down to an LDFLAGS declaration, I don't see that as a reason to triple the development effort on this project. Why don't you feel setting LDFLAGS="" before running make cuts it? What output do you get?

acv commented

I totally should not be allowed near a compiler without coffee.

Diff:

diff --git a/Makefile b/Makefile
index 8460768..0dc0153 100644
--- a/Makefile
+++ b/Makefile
@@ -6,18 +6,19 @@ INSTALL_DATA = install

 CC=gcc
 CFLAGS=-O2 -Wall -g -D_FORTIFY_SOURCE=2 -fstack-protector -fPIC
-LDFLAGS=-Wl,-z,now -Wl,-z,relro
+CFLAGS_EXTRA=-Wl,-rpath=.
+LDFLAGS=-Wl,-z,now -Wl,-z,relro -Wl,-soname,libscrypt.so.0 -Wl,--version-script=libscrypt.version
 all: reference

 OBJS= crypto_scrypt-nosse.o sha256.o crypto_scrypt-hexconvert.o crypto-mcf.o b64.o crypto-scrypt-saltgen.o crypto_scrypt-check.o crypto_scrypt-hash.o

 library: $(OBJS) 
-       $(CC)  $(LDFLAGS) -shared -Wl,-soname,libscrypt.so.0 -Wl,--version-script=libscrypt.version -o libscrypt.so.0 -lc -lm  $(OBJS)
+       $(CC)  $(LDFLAGS) -shared -o libscript.so.0 -lc -lm  $(OBJS)
        ar rcs libscrypt.a  $(OBJS)

 reference: library main.o b64.o
        ln -s -f libscrypt.so.0 libscrypt.so
-       $(CC) -Wall -o reference main.o  b64.o -Wl,-rpath=. -L.  -lscrypt
+       $(CC) -Wall -o reference main.o  b64.o $(CFLAGS_EXTRA) -L. -lscrypt

 clean:
        rm -f *.o reference libscrypt.so* libscrypt.a endian.h

And command line:

LDFLAGS="" CFLAGS_EXTRA="" make -e

Result:

energya:libscrypt acv (git:macosx+?) $ ./reference 
TEST ONE: Direct call to reference function with password 'password' and salt 'NaCL'
TEST ONE: SUCCESSFUL

So yes, not too bad.

technion commented

Thanks for that acv. I'll just need a little time to test it out on all architectures and then I'll pull in that patch.

technion commented

I had to alter the patch to make it work with another recent commit - but otherwise - it's been pulled in. If an OSX user can confirm that the command line given by acv now compiles I will update the README as such.

hynek commented

It does compile with

make LDFLAGS= CFLAGS_EXTRA=

for me.

It would be great if you could get libscypt into homebrew ( https://github.com/Homebrew/homebrew/ ), I’m sure they’ll be happy to help you out with build-issues on OS X too because that’s what they do. :)

technion commented

Thanks Hynek. I've updated the README accordingly.
I'll be consulting with Homebrew regarding their requirements.

technion commented

A note from the Homebrew team:
We frown on authors submitting their own work

I'm happy to help but I would suggest someone else will need to make the submission.

alex commented

If you do a release I'm happy to try to package this for homebrew.

On Wed, Feb 19, 2014 at 4:27 PM, technion notifications@github.com wrote:

A note from the Homebrew team:
We frown on authors submitting their own work

I'm happy to help but I would suggest someone else will need to make the
submission.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/7#issuecomment-35569234
.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

technion commented

V1.16 has been tagged with the discussed osx patches.

hynek commented

@alex are you on it? :)