On Jun 11, 2013, Blair Zajac wrote: > On 6/10/13 8:12 PM, Michael Rash wrote: > > Hello all, > > > > fwknop-2.5-pre2 is available: > > > > http://www.cipherdyne.org/fwknop/download/fwknop-2.5-pre2.tar.gz > > > > sha1: ea607734d3062c855419b8bb8700b12abd262080 > > > > Since -pre1, this release has a bunch of documentation updates, a few bug > > fixes > > found by Coverity (fwknop now has a Coverity defect score of zero), a > > comprehensive > > client command line -> rc file stanza saving capability (thanks to Franck), > > and more. > > On a 10.7 64-bit Intel Mac OS X system, getting these warnings from clang: > > libtool: compile: /usr/bin/clang -DHAVE_CONFIG_H -I. -I.. > -I/opt/local-10.7-2012-08/include -I ../common > -I/opt/local-10.7-2012-08/include > -pipe -O2 -arch x86_64 -Wall -Wformat -Wformat-securit > y -fstack-protector-all -fstack-protector -D_FORTIFY_SOURCE=2 -MT md5.lo -MD > -MP > -MF .deps/md5.Tpo -c md5.c -fno-common -DPIC -o .libs/md5.o > md5.c:51:6: warning: Undetermined or unsupported Byte Order... We will try > LITTLE_ENDIAN [-W#warnings] > #warning Undetermined or unsupported Byte Order... We will try > LITTLE_ENDIAN > ^ > 1 warning generated. > > > > /bin/sh ../libtool --tag=CC --mode=compile /usr/bin/clang -DHAVE_CONFIG_H > -I. > -I.. -I/opt/local-10.7-2012-08/include -I ../common > -I/opt/local-10.7-2012-08/include -pipe -O2 -arch x86_64 -Wall -Wformat > -Wformat-security -fstack-protector-all -fstack-protector -fPIE > -D_FORTIFY_SOURCE=2 -MT sha1.lo -MD -MP -MF .deps/sha1.Tpo -c -o sha1.lo > sha1.c > libtool: compile: /usr/bin/clang -DHAVE_CONFIG_H -I. -I.. > -I/opt/local-10.7-2012-08/include -I ../common > -I/opt/local-10.7-2012-08/include > -pipe -O2 -arch x86_64 -Wall -Wformat -Wformat-security -fstack-protector-all > -fstack-protector -D_FORTIFY_SOURCE=2 -MT sha1.lo -MD -MP -MF .deps/sha1.Tpo > -c > sha1.c -fno-common -DPIC -o .libs/sha1.o > sha1.c:104:11: warning: shift count >= width of type [-Wshift-count-overflow] > T >>= 32; > ^ ~~ > 1 warning generated.
The first warnings two led me to revisit the BYTEORDER strategy a bit. First, on my Ubuntu 13.04 system running on an AMD x86_64 processor, __BYTE_ORDER is derived from __LITTLE_ENDIAN or __BIG_ENDIAN, and these have values 1234 and 4321 respectively according to the /usr/include/endian.h file. So, there are no values like 12345678 around that I can see. Also, it looks like lib/sha2.c doesn't depend on the longer values even though lib/sha1.c does. So, what is needed is an additional verification that the longer values are valid, and I believe the test suite can help with this through its usage of OpenSSL to compare HMAC SHA1 and HMAC SHA256 values that it creates vs. those created by fwknop lib/sha1.c and lib/sha2.c. Note also that lib/sha2.c has its own code for resolving the byte order and doesn't depend on BYTEORDER (it uses BYTE_ORDER) and so the BYTEORDER patch to lib/fko_common.h does not affect its operation (this by itself may be something that should be changed too at some point to be cleaner, but this is a tangent). Here are two invocations of the test suite that should highlight any differences between the hashing algorithms vs. OpenSSL: ./test-fwknop.pl --enable-all --include "Rijndael.*HMAC.*complete.*SHA1.*22" --test-limit 1 ./test-fwknop.pl --enable-all --include "Rijndael.*HMAC.*complete.*SHA256.*22" --test-limit 1 I'm guessing that on your PPC system the first test will fail because of an HMAC error (with 2.5-pre2), and the second one will succeed. If so, the output at the end of the first test will have the following at the end: [+] 1/0/1 OpenSSL tests passed/failed/executed [+] 0/1/1 OpenSSL HMAC tests passed/failed/executed [+] 1/1/2 test buckets passed/failed/executed You can confirm this with (I hard-coded BYTEORDER 12345678 to produce this result): # grep mismatch output/1.test [-] OpenSSL HMAC mismatch (openssl): 'uPhvEts6f/WWY1YRu38BEYLfI5A' != (fwknop): '7J+V5M3MwOYn4RoarznMhHqLDJs' Now, the above assumes that the test suite is working properly to begin with on your system, and the previous output you sent makes me suspect that you may need to adjust your local ipfw policy to accept traffic over the loopback interface since fwknopd didn't appear to receive any SPA traffic. On another note, I'm going to add a new feature to the test suite to add cross-OS tests. The current 'backwards compatibility' tests already lay the groundwork for this - just need to collect SPA packets from as many different systems as possible. > /usr/bin/clang -DHAVE_CONFIG_H -I. -I.. -I ../lib -I ../common > -I/opt/local-10.7-2012-08/include -pipe -O2 -arch x86_64 -Wall -Wformat > -Wformat-security -fstack-protector-all -fstack-protector -fPIE > -D_FORTIFY_SOURCE=2 -MT fwknop-fwknop.o -MD -MP -MF .deps/fwknop-fwknop.Tpo > -c > -o fwknop-fwknop.o `test -f 'fwknop.c' || echo './'`fwknop.c > fwknop.c:952:61: warning: size argument in 'strlcpy' call appears to be size > of > the source; expected the size of the destination [-Wstrlcpy-strlcat-size] > strlcpy(argv_new[argc_new], arg_tmp, strlen(arg_tmp)+1); > ~~~~~~~^~~~~~~~~~ > > > > /usr/bin/clang -DHAVE_CONFIG_H -I. -I.. -I ../lib -I ../common > -I/opt/local-10.7-2012-08/include -pipe -O2 -arch x86_64 -Wall -Wformat > -Wformat-security -fstack-protector-all -fstack-protector -fPIE > -D_FORTIFY_SOURCE=2 -MT fwknop-config_init.o -MD -MP -MF > .deps/fwknop-config_init.Tpo -c -o fwknop-config_init.o `test -f > 'config_init.c' > || echo './'`config_init.c > config_init.c:1987:62: warning: size argument in 'strlcpy' call appears to be > size of the source; expected the size of the destination > [-Wstrlcpy-strlcat-size] > strlcpy(options->resolve_url, optarg, strlen(optarg)+1); > ~~~~~~~^~~~~~~~~ > 1 warning generated. Both of the above strlcpy() warnings are false positives. The destination buffers are allocated at runtime using the lengths provided by the last argument to strlcpy(). Thanks for sending the warnings though - it is always nice to have additional code reviews. --Mike > Blair ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev _______________________________________________ Fwknop-discuss mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/fwknop-discuss
