This commit fixes an issue on ipfw firewalls where fwknopd would always require
seeing ipfw 'Dynamic' rules associated with newly added connections. But, such
connections may never be established for various reasons. Previous to this
commit the following warning was frequently generated by fwknopd:
Unexpected error: did not find 'Dynamic rules' string in list output.
This commit fixes an issues on systems running the ipfw firewall where the
'set' where fwknopd puts new access rules was attempted to be deleted without
first checking to see whether it exists. The following errors would be
generated (now fixed):
ipfw: rule 16777217: setsockopt(IP_FW_DEL): Invalid argument
Error 17664 from cmd:'/sbin/ipfw delete set 1':
Fatal: Errors detected during ipfw rules initialization.
Bug fix to ensure that the digest.cache file gets created at fwknopd init time
so fwknopd does not throw the following error:
Error opening digest cache file. Incoming digests will not be remembered.
If gpgme is installed on FreeBSD systems it appears that
-I/usr/local/include/gpgme must be added to the include path, but this change
only adds the path if gpgme is installed and going to be used.
gcc on FreeBSD generates the following errors when the -Wl,-z,relro -Wl,-z,now
flags are used:
gcc: -z: linker input file unused because linking not done
gcc: relro: linker input file unused because linking not done
gcc: -z: linker input file unused because linking not done
gcc: now: linker input file unused because linking not done
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
The ./configure script would generate the following error for the attempted
use of the --without-stackprotector (and other related options like
--without-pie):
configure: WARNING: unrecognized options: --without-stackprotect
Upon the receipt of a valid SPA packet, a check is done to make sure that
a jump rule from the appropriate built-in iptables chains exists to the
fwknop chains. Such rules could have been deleted by other manipulations
of the iptables policy, so it is important to ensure they exist. Running
in foreground (-f) mode, here is an illustration of the jump rule being
added after it got deleted:
SPA Packet from IP: 127.0.0.1 received.
Added jump rule from chain: INPUT to chain: FWKNOP_INPUT
Added Rule to FWKNOP_INPUT for 127.0.0.1, tcp/22 expires at 1313680648
Previous to this change a check was done for base64 characters in incoming
SPA data only up to MIN_SPA_DATA_SIZE. This check may be reinstantiated for
SPA packets that are delivered over HTTP (and the packet data is embedded
within a URL that may also contain non-base64 chars), but in the meantime the
fwknopd daemon should not accept SPA packets over arbitrary ports with any
non-base64 chars.
Replay warnings now include port and protocol information. Here is an example:
SPA Packet from IP: 127.0.0.1 received.
Replay detected from source IP: 127.0.0.1
Destination proto/port: 17/62201
Original source IP: 127.0.0.1
Original dst proto/port: 17/62201
Entry created: 08/17/11 21:06:07
First replay: 08/17/11 21:06:32
Last replay: 08/17/11 21:06:45
Replay count: 7
Added various security options that can be enabled at compile time. These
options include everything that the "hardening-check" script written by Kees
Cook checks for. After this change, the hardening-check script produces the
following output against the fwknopd binary:
$ hardening-check server/.libs/fwknopd
server/.libs/fwknopd:
Position Independent Executable: yes
Stack protected: yes
Fortify Source functions: yes
Read-only relocations: yes
Immediate binding: yes
One of the compile outputs (for example) that shows the new options is:
/bin/bash ../libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I.. -g -O2 -fstack-protector-all -fPIE -pie -D_FORTIFY_SOURCE=2 -Wl,-z,relro -Wl,-z,now -MT fko_decode.lo -MD -MP -MF .deps/fko_decode.Tpo -c -o fko_decode.lo fko_decode.c
From the hardening-check man page, here is a description of each of these
options:
NAME
hardening-check - check binaries for security hardening features
SYNOPSIS
Examine a given set of ELF binaries and check for several security
hardening features, failing if they are not all found.
DESCRIPTION
This utility checks a given list of ELF binaries for several security
hardening features that can be compiled into an executable. These
features are:
Position Independent Executable
This indicates that the executable was built in such a way
(PIE) that the "text" section of the program can be relocated
in memory. To take full advantage of this feature, the
executing kernel must support text Address Space Layout
Randomization (ASLR).
Stack Protected
This indicates that the executable was compiled with the
gcc(1) option -fstack-protector. The program will be
resistant to have its stack overflowed.
Fortify Source functions
This indicates that the executable was compiled with
-D_FORTIFY_SOURCE=2 and -O2 or higher. This causes certain
unsafe glibc functions with their safer counterparts (e.g.
strncpy instead of strcpy).
Read-only relocations
This indicates that the executable was build with -Wl,-z,relro
to have ELF markings (RELRO) that ask the runtime linker to
mark any regions of the relocation table as "read-only" if
they were resolved before execution begins. This reduces the
possible areas of memory in a program that can be used by an
attacker that performs a successful memory corruption exploit.
Immediate binding
This indicates that the executable was built with -Wl,-z,now
to have ELF markings (BIND_NOW) that ask the runtime linker to
resolve all relocations before starting program execution.
When combined with RELRO above, this further reduces the
regions of memory available to memory corruption attacks.
Applied a patch sent from Jonathan Bennett to add fwknop-2.0.0rc2 support to
openwrt. One thing to note about this patch is that the +libgdbm library
dependency has been removed because fwknop now implements its own digest
tracking file without needing gdbm/ndbm on the system.
Upon fwknopd shutdown, a new function free_replay_list() is now called in order
to free heap allocated memory dedicated to SPA digest tracking. Without this
fix, valgrind reports the following (some output snipped):
valgrind --leak-check=full ./server/.libs/fwknopd -f -i lo -P "udp port 62201"
==30864== 431 (48 direct, 383 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 17
==30864== at 0x4C27480: calloc (vg_replace_malloc.c:467)
==30864== by 0x407CB7: replay_check_file_cache (replay_cache.c:461)
==30864== by 0x407B69: replay_check (replay_cache.c:413)
==30864== by 0x405813: incoming_spa (incoming_spa.c:363)
==30864== by 0x406275: pcap_capture (pcap_capture.c:223)
==30864== by 0x40317D: main (fwknopd.c:297)
Added the source port and protocol fields to valid SPA packets in the digest
cache. This can help to discover replay trends. The format of the digest
file cache is now:
<digest> <proto> <src_ip> <src_port> <dst_ip> <dst_port> <time>
At init time fwknopd will read in the digest cache file into the in-memory
linked list of digests for SPA replay detection. This commit starts on this
code, but the file format does not yet include destination IP addresses
(to be added in an upcoming commit).
When not using gdbm/ndbm support (the default now), fwknopd implements a linked
list of SPA packet digests for replay attack detection along with writing
digest data in ascii text down to disk (in the CONF_DIGEST_FILE file).
To override the value of the PCAP_FILTER variable in the fwknopd.conf
config file, a new fwknopd command line argument "--pcap-filter" was
added. This assists in various activities by making it trivial to
change how fwknopd acquires packet data without editing the fwknopd.conf
file. Here is an example:
fwknopd -i lo -f --pcap-filter "udp port 12345"
If fwknopd is compiled with --disable-file-cache to the ./configure script
then it will assume that the default filename is "digest_db.cache" for the
digest cache. If the file cache method is used (this is the default), then
"digest.cache" is the default filename. A new variable DIGEST_DB_FILE in
the fwknopd.conf file controls the digest filename if gdbm/ndbm support is
required.
This change starts on support for a simple file-based cache mechanism
for tracking SPA digests. This removes the libgdbm/libndbm dependency
by default, but it can be re-enabled with the --disable-file-cache
argument to the ./configure script.
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)
