Using the 'fiu-run' fault injection binary, a couple of cases were
turned up with libfko does not properly check the strdup() return value.
This commit fixes these issues, and here is an illustration of the stack
trace for one such issue:
Core was generated by `../client/.libs/fwknop -A tcp/22 -a 127.0.0.2 -D
127.0.0.1 --get-key local_spa.'.
Program terminated with signal 11, Segmentation fault.
#0 __strnlen_sse2 () at ../sysdeps/x86_64/multiarch/../strnlen.S:34
34 ../sysdeps/x86_64/multiarch/../strnlen.S: No such file or directory.
(gdb) where
#0 __strnlen_sse2 () at ../sysdeps/x86_64/multiarch/../strnlen.S:34
#1 0x00007effa38189bc in _rijndael_encrypt (enc_key_len=<optimized out>, enc_key=<optimized out>, ctx=0x7effa5945750) at fko_encryption.c:141
#2 fko_encrypt_spa_data (ctx=0x7effa5945750, enc_key=<optimized out>, enc_key_len=<optimized out>) at fko_encryption.c:605
#3 0x00007effa381a2d6 in fko_spa_data_final (ctx=0x7effa5945750, enc_key=enc_key@entry=0x7fff3ff4aa10 "fwknoptest", enc_key_len=<optimized out>, hmac_key=hmac_key@entry=0x7fff3ff4aaa0 "", hmac_key_len=0) at fko_funcs.c:489
#4 0x00007effa405f2fb in main (argc=<optimized out>, argv=<optimized out>) at fwknop.c:449
This commit changes how fko_new() deals with FKO context initialization
to not set ctx->initval back to zero (uninitialized) imediately after
calling each fko_set_... function and before checking the fko_set_... return
value. The reason for this change is that fko_destroy() checks for
context initialization via ctx->initval before calling free() against
any heap allocated context member. So, if fko_set_... returns an error,
fko_destroy() (previous to this commit) would have no opportunity to
free such members.
This bug was found with fault injection testing provided by libfiu
together with valgrind. Specifically the following test suite command
exposes the problem (from the test/ directory):
./test-fwknop.pl --enable-complete --include "fault injection.*libfko"
In the resulting output/2.test file valgrind reports the following:
==27941== LEAK SUMMARY:
==27941== definitely lost: 264 bytes in 1 blocks
==27941== indirectly lost: 28 bytes in 3 blocks
==27941== possibly lost: 0 bytes in 0 blocks
==27941== still reachable: 1,099 bytes in 12 blocks
==27941== suppressed: 0 bytes in 0 blocks
After this commit is applied, this changes to:
==7137== LEAK SUMMARY:
==7137== definitely lost: 0 bytes in 0 blocks
==7137== indirectly lost: 0 bytes in 0 blocks
==7137== possibly lost: 0 bytes in 0 blocks
==7137== still reachable: 1,099 bytes in 12 blocks
==7137== suppressed: 0 bytes in 0 blocks
Note that 'definitely lost' in valgrind output means there is a real
memory leak that needs to be fixed whereas 'still reachable' is most
likely not a real problem according to:
http://valgrind.org/docs/manual/faq.html#faq.deflost
This is a significant commit to add the ability to leverage libfko fault
injections from both the fwknop client and server command lines via a
new option '--fault-injection-tag <tag name>'. This option is used by
the test suite with the tests/fault_injection.pl tests.
This commit fixes a double free condition discovered through the new
python SPA payload fuzzer. This bug could be triggered in fwknopd with
a malicious SPA payload but only when GnuPG is used. When Rijndael is
used for SPA packet encryption, this bug cannot be triggered due to an
length/format check towards the end of _rijndael_decrypt(). It should
be noted that only a person in possession of the correct encryption and
authentication GnuPG keys could trigger this bug.
Add a new fko_set_encoded_data() function gated by #define
FUZZING_INTERFACES to allow encryption and authentication to be bypassed
for fuzzing purposes (and only fuzzing purposes). The fko-wrapper code
has been extended to process data in the
test/fko-wrapper/fuzz_spa_payloads file, which is created by the new
python fuzzer. Typical workflow is:
$ cd test/fko-wrapper
$ ../spa_fuzzer.py > fuzz_spa_payloads
$ make fuzzing
(as root):
./test-fwknop.pl --enable-profile-coverage --enable-fuzzing-interfaces --enable-all --include wrapper
[+] Starting the fwknop test suite...
args: --enable-profile-coverage --enable-fuzzing-interfaces --enable-all --include wrapper
Saved results from previous run to: output.last/
Valgrind mode enabled, will import previous coverage from:
output.last/valgrind-coverage/
[+] Total test buckets to execute: 2
[Rijndael] [fko-wrapper] multiple libfko calls (with valgrind)......pass (1)
[Rijndael] [fko-wrapper] multiple libfko calls......................pass (2)
[profile coverage] gcov profile coverage............................pass (3)
[valgrind output] [flagged functions] ..............................pass (4)
Run time: 5.85 minutes
[+] 0/0/0 OpenSSL tests passed/failed/executed
[+] 0/0/0 OpenSSL HMAC tests passed/failed/executed
[+] 4/0/4 test buckets passed/failed/executed
This commit fixes a double free condition discovered through the new
python SPA payload fuzzer. This bug could be triggered in fwknopd with
a malicious SPA payload but only when GnuPG is used. When Rijndael is
used for SPA packet encryption, this bug cannot be triggered due to an
length/format check towards the end of _rijndael_decrypt(). It should
be noted that only a person in possession of the correct encryption and
authentication GnuPG keys could trigger this bug.
Bug fix to correct a memory leak in GnuPG SPA packet handling within
the gpg_decrypt() function. Here is the specific valgrind leak record
that enabled the bug to be found (note that the new valgrind
suppressions usage was critical for finding this bug among all other
libgpgme memory leaks):
==23983== 1,044 bytes in 1 blocks are definitely lost in loss record 7 of 8
==23983== at 0x4C2C494: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==23983== by 0x4E41D3A: gpg_decrypt (fko_encryption.c:422)
==23983== by 0x4E42520: fko_decrypt_spa_data (fko_encryption.c:626)
==23983== by 0x1155B0: incoming_spa (incoming_spa.c:519)
==23983== by 0x1180A7: process_packet (process_packet.c:211)
==23983== by 0x506D857: ??? (in /usr/lib/x86_64-linux-gnu/libpcap.so.1.4.0)
==23983== by 0x117865: pcap_capture (pcap_capture.c:270)
==23983== by 0x10F937: main (fwknopd.c:353)
- [server] When GnuPG is used, the default now is to require that
incoming SPA packets are signed by a key listed in GPG_REMOTE_ID for each
access.conf stanza. In other words, the usage of GPG_REQUIRE_SIG
is no longer necessary in order to authenticate SPA packets via the
GnuPG signature. Verification of GnuPG signatures can be disabled with a
new access.conf variable GPG_DISABLE_SIG, but this is NOT a
recommended configuration.
- [client+server] Add --gpg-exe command line argument and GPG_EXE
config variable to ~/.fwknoprc and the access.conf file so that the path
to GnuPG can be changed from the default /usr/bin/gpg path.
This commit implements more rigorous SPA packet field count validation
that takes into account expected field counts for each SPA message type.
Two new libfko error codes have been added in support of this, and the
corresponding changes made in the perl and python modules.
Allow usernames that are compatible with Microsoft guidelines as defined
here:
http://technet.microsoft.com/en-us/library/bb726984.aspx
This allows for greater compatibility between fwknop clients on Windows
(for example that may be deployed with Cygwin) and fwknopd on other
systems. This change was suggested by Gerry Reno, and tracked by Github
issue #114.
This commit updates all authorship and copyright information to include a
standard header that references the AUTHORS and CREDITS file. This standard
header was written by the Debian legal team at the request of Franck Joncourt.
Integer lengths that are negative are never valid. This commit also
extends the fuzzing capabilities of the test/fko-wrapper code to
validate libfko calls with negative length arguments, and one crash
scenario with a negative length for the encryption key was found (and
fixed) this way.
