This reverts commit e57cfa2e23. This is done
because libfko now restricts the symbols it exports to only those functions
that should be visible when making use of the library - internal libfko
functions should not be exported.
Rearranged headers to reduce duplication and remove local header
references from fko.h.
Removed references to headers that did not need to be explicitly set.
Moved the MAX_PROTO_STR_LEN and MAX_PORT_STR_LEN definitions to the
fko_limits.h file.
Fixed bug where invalid nat_access or command messages were returning
FKO_ERROR_INVALID_SPA_ACCESS_MSG error code instead of the one
appropriate to the message type.
Fixed bad nat_access_msg test in Perl module test suite (caught by new
validation code).
Bug fix to check b64_decode() return value to ensure that
non-base64 encoded data is never used. Even though other validation
routines checked decoded results, it is important to discard invalid
data as early as possible. Note too that such invalid data would only
be provided to b64_decode() after proper decryption, so the client must
provide authentic SPA data.
Added validation of NAT access strings in the various NAT modes in libfko.
This applies to both the client and server, and test suite support was added
as well.
(Vlad Glagolev) Submitted a patch to fix command exec mode
under SPA message type validity test. Support for command exec mode was
also added to the test suite.
This commit fixes a condition in which the server did not properly validate
allow IP addresses from malicious authenticated clients. This has been fixed
with stronger allow IP validation.
- [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.
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
Enhanced the libfko decoding routine to include bounds checking on decrypted
SPA data. This includes verifying the number of fields within incoming SPA
data (colon separated) along with verifying string lengths of each field.
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.
The two memory leaks were found with the test suite running in
--enable-valgrind mode - here are the relevant error messages:
For fwknopd server GPG clean up:
==345== 9 bytes in 1 blocks are definitely lost in loss record 2 of 2
==345== at 0x4C2815C: malloc (vg_replace_malloc.c:236)
==345== by 0x52F6B81: strdup (strdup.c:43)
==345== by 0x10FA57: add_string_list_ent (access.c:308)
==345== by 0x110513: parse_access_file (access.c:387)
==345== by 0x10B5FB: main (fwknopd.c:193)
For fwknop client rc file processing:
==8045== 568 bytes in 1 blocks are still reachable in loss record 12 of 12
==8045== at 0x4C2815C: malloc (vg_replace_malloc.c:236)
==8045== by 0x50A53AA: __fopen_internal (iofopen.c:76)
==8045== by 0x10C3FF: process_rc (config_init.c:446)
==8045== by 0x10C8F6: config_init (config_init.c:671)
==8045== by 0x10AC9E: main (fwknop.c:62)
There is also a new clean_exit() function that makes it easier to ensure that
resources are deallocated upon existing.
Added the 'const' qualifier to function prototype variables where possible.
In addition, reduced some functions to file-scope with 'static' where possible.
Also made a few minor changes to remove extra whitespace, and fixed a bug
in create_fwknoprc() to ensure the new fwknoprc filehandle is closed.
