- [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.
Chop any trailing '/' char, be more careful about handling incoming large HTTP
responses, print the HTTP request and response in --verbose --verbose mode.
Applied patch from Jonathan Schulz to ensure that the fwknop client reads all
data from a remote webserver when resolving the client IP address in -R mode.
Jonathan indicated that some webservers would transfer HTTP headers and data
separately, and a single recv() would therefore fail to get the necessary IP
information.
This commit fixes memory leaks like the following in the fwknop client:
HEAP SUMMARY:
in use at exit: 300 bytes in 11 blocks
total heap usage: 100 allocs, 89 frees, 16,583 bytes allocated
16 bytes in 1 blocks are indirectly lost in loss record 1 of 11
at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5146C59: __nss_lookup_function (nsswitch.c:456)
by 0x5C3D63E: ???
by 0x50FF3FC: getpwuid_r@@GLIBC_2.2.5 (getXXbyYY_r.c:256)
by 0x508938E: cuserid (cuserid.c:37)
by 0x4E3983A: fko_set_username (fko_user.c:65)
by 0x4E38D5C: fko_new (fko_funcs.c:84)
by 0x10A824: main (fwknop.c:75)
16 bytes in 1 blocks are indirectly lost in loss record 2 of 11
at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5146C59: __nss_lookup_function (nsswitch.c:456)
by 0x5C3D658: ???
by 0x50FF3FC: getpwuid_r@@GLIBC_2.2.5 (getXXbyYY_r.c:256)
by 0x508938E: cuserid (cuserid.c:37)
by 0x4E3983A: fko_set_username (fko_user.c:65)
by 0x4E38D5C: fko_new (fko_funcs.c:84)
by 0x10A824: main (fwknop.c:75)
16 bytes in 1 blocks are indirectly lost in loss record 3 of 11
at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5146C59: __nss_lookup_function (nsswitch.c:456)
by 0x5C3D672: ???
by 0x50FF3FC: getpwuid_r@@GLIBC_2.2.5 (getXXbyYY_r.c:256)
by 0x508938E: cuserid (cuserid.c:37)
by 0x4E3983A: fko_set_username (fko_user.c:65)
by 0x4E38D5C: fko_new (fko_funcs.c:84)
by 0x10A824: main (fwknop.c:75)
16 bytes in 1 blocks are indirectly lost in loss record 4 of 11
at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5146C59: __nss_lookup_function (nsswitch.c:456)
by 0x5C3D68C: ???
by 0x50FF3FC: getpwuid_r@@GLIBC_2.2.5 (getXXbyYY_r.c:256)
by 0x508938E: cuserid (cuserid.c:37)
by 0x4E3983A: fko_set_username (fko_user.c:65)
by 0x4E38D5C: fko_new (fko_funcs.c:84)
by 0x10A824: main (fwknop.c:75)
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.
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.
This commit adds a new configuration variable "FORCE_NAT" to the access.conf
file:
For any valid SPA packet, force the requested connection to be NAT'd
through to the specified (usually internal) IP and port value. This is
useful if there are multiple internal systems running a service such as
SSHD, and you want to give transparent access to only one internal system
for each stanza in the access.conf file. This way, multiple external
users can each directly access only one internal system per SPA key.
This commit also implements a few minor code cleanups.
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.
This change fixes the following compiler warning that was seen with many of
the source files in server/
fwknopd_common.h:223: warning: ‘config_map’ defined but not used
This commit fixes several compiler warnings like the following (now that -Wall
is the default):
config_init.h:68: warning: ‘cmd_opts’ defined but not used
In the save_args() function the args_str_len variable was being used before
being initialized as reported via the splint static code analysis tool. Here
is the splint output that found this bug:
client/fwknop.c:650:13: Variable args_str_len used before definition
An rvalue is used that may not be initialized to a value on some execution
path. (Use -usedef to inhibit warning)
is not defined in the fwknopd.conf file or the access.conf file, then this default will take
over.
git-svn-id: file:///home/mbr/svn/fwknop/trunk@252 510a4753-2344-4c79-9c09-4d669213fbeb
