Two bugs are fixed with this commit: verify permissions/ownership on the
fwknop.pid file only if it exists, and ensure to ru-run stat() on any directory
component if we're creating a directory.
- [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.
Updated the 'make install' step to not overwrite any existing config files in
/etc/fwknop/ and instead install new copies from the source tree at
/etc/fwknop/fwknopd.conf.inst and /etc/fwknop/access.conf.inst
Implemented a new check to ensure that the iptables 'comment' match exists to
ensure the proper environment for fwknopd operations. This check is controlled
by the new ENABLE_IPT_COMMENT_CHECK variable, and was suggested by Hank
Leininger.
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
Added FLUSH_IPFW_AT_INIT and FLUSH_IPFW_AT_EXIT for ipfw firewalls to emulate
the corresponding functionality that is implemented for iptables firewalls.
Bug fix for ipfw firewalls to ensure that if the ipfw expire set is zero, then
do not disable this set whenever the FLUSH_IPFW* variables are enabled.
These changes were suggested by Jonathan Schulz.
Ensure that an attacker cannot force a replay attack by intercepting an
SPA packet and the replaying it with the base64 version of "Salted__"
(for Rindael) or the "hQ" prefix (for GnuPG). This is an important fix.
The following comment was added into the fwknopd code:
/* Ignore any SPA packets that contain the Rijndael or GnuPG prefixes
* since an attacker might have tacked them on to a previously seen
* SPA packet in an attempt to get past the replay check. And, we're
* no worse off since a legitimate SPA packet that happens to include
* a prefix after the outer one is stripped off won't decrypt properly
* anyway because libfko would not add a new one.
*/
Conflicts:
lib/cipher_funcs.h
This commit fixes the following memory leak found with valgrind:
44 bytes in 1 blocks are definitely lost in loss record 2 of 2
at 0x482BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
by 0x490EA50: strdup (strdup.c:43)
by 0x10CD69: incoming_spa (incoming_spa.c:162)
by 0x10E000: process_packet (process_packet.c:200)
by 0x4862E63: ??? (in /usr/lib/i386-linux-gnu/libpcap.so.1.1.1)
by 0x4865667: pcap_dispatch (in /usr/lib/i386-linux-gnu/libpcap.so.1.1.1)
by 0x10DABF: pcap_capture (pcap_capture.c:226)
by 0x10A798: main (fwknopd.c:299)
Ensure that an attacker cannot force a replay attack by intercepting an
SPA packet and the replaying it with the base64 version of "Salted__"
(for Rindael) or the "hQ" prefix (for GnuPG). This is an important fix.
The following comment was added into the fwknopd code:
/* Ignore any SPA packets that contain the Rijndael or GnuPG prefixes
* since an attacker might have tacked them on to a previously seen
* SPA packet in an attempt to get past the replay check. And, we're
* no worse off since a legitimate SPA packet that happens to include
* a prefix after the outer one is stripped off won't decrypt properly
* anyway because libfko would not add a new one.
*/
This commit fixes the following memory leak found with valgrind:
44 bytes in 1 blocks are definitely lost in loss record 2 of 2
at 0x482BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
by 0x490EA50: strdup (strdup.c:43)
by 0x10CD69: incoming_spa (incoming_spa.c:162)
by 0x10E000: process_packet (process_packet.c:200)
by 0x4862E63: ??? (in /usr/lib/i386-linux-gnu/libpcap.so.1.1.1)
by 0x4865667: pcap_dispatch (in /usr/lib/i386-linux-gnu/libpcap.so.1.1.1)
by 0x10DABF: pcap_capture (pcap_capture.c:226)
by 0x10A798: main (fwknopd.c:299)
[server] Updated PCAP_LOOP_SLEEP default to 1/10th of a second (in
microseconds). This was supposed to be the default anyway, but C
Anthony Risinger reported a bug where fwknopd was consuming more
resources than necessary, and the cause was PCAP_LOOP_SLEEP set by
default to 1/100th of a second - this has been fixed.
When using the --nat-local argument on the fwknop client command line, the
fwknopd server needs to add an INPUT ACCEPT rule for the requested access
since the incoming connection is destined for a local socket. Added test
suite support to test --nat-local access.
[test suite] Minor bug fix to ensure that all file_find_regex() calls return
true if all regex's are matched and false if any regex does not match data in
the specified file.
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.
This change ensures that we only cache replay digests for those SPA packets
that actually decrypt. Not doing this would have allowed an attacker to
potentially fill up digest cache space with digests for garbage packets.
This commit fixes a bug where the same encryption key used for two stanzas in
the access.conf file would result in access requests that matched the second
stanza to always be treated as a replay attack. This has been fixed for
the fwknop-2.0.1 release, and was reported by Andy Rowland. Now the fwknopd
server computes the SHA256 digest of raw incoming payload data before
decryption, and compares this against all previous hashes. Previous to this
commit, fwknopd would add a new hash to the replay digest list right after
the first access.conf stanza match, so when SPA packet data matched the
second access.conf stanza a matching replay digest would already be there.
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
This is a significant update to allow AES encryption modes to be selected on a
per-key basis. For now, only ECB and CBC (recommended) modes are supported.
The default is ECB modes in order to maintain backwards compatibility with the
older perl version of fwknop and the Crypt::CBC CPAN module. This will likely
be changed to use CBC mode by default because of its better security
properties.
In the access.conf file on the server side, there is a new configuration
variable "ENCRYPTION_MODE" that controls the mode for the corresponding AES
key. On the client side, a new command line argument "--encryption-mode"
controls how the client encrypts SPA packets.
Michael Rash
Damien Stuart