Lots of places in the code were already using {0} to initialize stack char
arrays, but memset() was being used as well. This commit removes all
unnecessary memset() calls against char arrays that are already initialized
via {0} (which sets all members to zero for such arrays).
Bug fix to ensure to release memory when invalid access stanza dates are set
and fwknopd has to exit. This leak was caught with the test suite in
--enable-valgrind mode based on the following output:
==31947== 568 bytes in 1 blocks are still reachable in loss record 1 of 1
==31947== at 0x4C2CD7B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31947== by 0x52EE42A: __fopen_internal (iofopen.c:73)
==31947== by 0x1116A2: parse_access_file (access.c:909)
==31947== by 0x10BAD5: main (fwknopd.c:194)
This is a fairly significant commit that lays the groundwork for getting
selectable HMAC modes working for both the client and server. One libfko API
change was required so that the hmac_type is passed into fko_new_with_data().
This allows the server to set the hmac_type via access.conf stanzas. The
effort in this commit will be extended to allow HMAC MD5, SHA1, and SHA512
also function properly.
This commit completes the conversion to the strtol() wrapper function in order
to remove all atoi() calls. In addition, variable max values are enforced
using more broadly defined RCHK_* values.
This commit replaces a few additional atoi() calls with the strtol() wrapper
function, and also fixes a bug where access SOURCE IP/mask combinations would
not be accepted when the string length was a long as something like
'123.123.123.123/255.255.255.255'.
[libfko] Added the ability to maintain backwards compatibility with the
now deprecated "zero padding" strategy in AES mode that was a hold over
from the old perl fwknop implementation. This enables the backwards
compatiblity tests to continue to pass in the test suite.
Fernando Arnaboldi from IOActive found several conditions in
which the server did not properly throw out maliciously constructed
variables in the access.conf file. This has been fixed along with new
fuzzing tests in the test suite.
- [client+server] Fernando Arnaboldi from IOActive found that strict
filesystem permissions for various fwknop files are not verified. Added
warnings whenever permissions are not strict enough, and ensured that
files created by the fwknop client and server are only set to user
read/write.
- [client] Fernando Arnaboldi from IOActive found a local buffer overflow
in --last processing with a maliciously constructed ~/.fwknop.run file.
This has been fixed with proper validation of .fwknop.run arguments.
- [server] Fernando Arnaboldi from IOActive found several DoS/code
execution vulnerabilities for malicious fwknop clients that manage to
get past the authentication stage (so a such a client must be in
possession of a valid access.conf encryption key). These vulnerbilities
manifested themselves in the handling of malformed access requests, and
both the fwknopd server code along with libfko now perform stronger input
validation of access request data. These vulnerabilities affect
pre-2.0.3 fwknop releases.
- [test suite] Added a new fuzzing capability to ensure proper server-side
input validation. Fuzzing data is constructed with modified fwknop
client code that is designed to emulate malicious behavior.
For GPG mode, added a new access.conf variable "GPG_ALLOW_NO_PW" to make it
possible to leverage a server-side GPG key pair that has no associated
password. This comes in handy when a system requires the user to leverage
gpg-agent / pinentry which can present a problem in automated environments as
required by the fwknopd server. Now, it might seem like a problem to remove
the passphrase from a GPG key pair, but it's important to note that simply
doing this is little worse than storing the passphrase in the clear on disk
anyway in the access.conf file. Further, this link help provides additional
detail:
http://www.gnupg.org/faq/GnuPG-FAQ.html#how-can-i-use-gnupg-in-an-automated-environment
Now that encryptions keys and hmac keys may be acquired from /dev/random with
--key-gen (and base64 encoded), they may contain NULL bytes. This emphasizes
the need to not leverage code that assumes C-style strings when making use of
key information.
Added --key-gen to allow KEY_BASE64 and HMAC_KEY_BASE64 keys to be created from
reading random data from /dev/random. These keys can be placed within server
access.conf files and corresponding client .fwknoprc files for SPA
communications. The HMAC key is not used yet with this commit, but that is
coming.
fwknopd access stanzas can have both Rijndael and GnuPG keys, so this
commit fixes a bug where any gpg info would force only gpg decryption
attempts even if a Rijndael key is provided in the stanza.
This commit causes fwknopd to exit whenever an invalid SOURCE entry is seen
such as ":ANY". Previous to this commit, valgrind threw the following errors
with ":ANY" as an access.conf SOURCE entry:
Invalid read of size 8
at 0x117695: free_acc_source_list (access.c:512)
by 0x1177E3: free_acc_stanza_data (access.c:564)
by 0x117C67: free_acc_stanzas (access.c:654)
by 0x10E32E: free_configs (config_init.c:106)
by 0x10D085: main (fwknopd.c:376)
Address 0x5a80658 is 8 bytes inside a block of size 16 free'd
at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x116AE0: add_source_mask (access.c:255)
by 0x116D57: expand_acc_source (access.c:303)
by 0x117A82: expand_acc_ent_lists (access.c:620)
by 0x119570: parse_access_file (access.c:1043)
by 0x10C77E: main (fwknopd.c:193)
Invalid free() / delete / delete[] / realloc()
at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x1176A8: free_acc_source_list (access.c:514)
by 0x1177E3: free_acc_stanza_data (access.c:564)
by 0x117C67: free_acc_stanzas (access.c:654)
by 0x10E32E: free_configs (config_init.c:106)
by 0x10D085: main (fwknopd.c:376)
Address 0x5a80650 is 0 bytes inside a block of size 16 free'd
at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x116AE0: add_source_mask (access.c:255)
by 0x116D57: expand_acc_source (access.c:303)
by 0x117A82: expand_acc_ent_lists (access.c:620)
by 0x119570: parse_access_file (access.c:1043)
by 0x10C77E: main (fwknopd.c:193)
HEAP SUMMARY:
in use at exit: 8 bytes in 1 blocks
total heap usage: 1,659 allocs, 1,659 frees, 238,310 bytes allocated
