Replace strlcpy() with memcpy() since the source buffer is not a string.
strlcpy() caught this anyway, but memcpy() usage is probably more valid.
==29766==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x562bc2e50420 in strlcpy /home/mbr/git/fwknop.git/common/strlcpy.c:61:3
#1 0x562bc2e25362 in process_packet /home/mbr/git/fwknop.git/server/process_packet.c:225:5
#2 0x7fa6173c9d57 (/lib64/libpcap.so.1+0x1fd57)
#3 0x562bc2e2456a in pcap_capture /home/mbr/git/fwknop.git/server/pcap_capture.c:227:15
#4 0x562bc2e13ef0 in main /home/mbr/git/fwknop.git/server/fwknopd.c:296:13
#5 0x7fa61643724a in __libc_start_main /usr/src/debug/glibc-2.27-74-g68c1bf8097/csu/../csu/libc-start.c:308:16
#6 0x562bc2d9dec9 in _start (/home/mbr/git/fwknop.git/server/.libs/fwknopd+0x1dec9)
Uninitialized value was created by a heap allocation
#0 0x562bc2da6c84 in malloc (/home/mbr/git/fwknop.git/server/.libs/fwknopd+0x26c84)
#1 0x7fa6173ca996 (/lib64/libpcap.so.1+0x20996)
SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/mbr/git/fwknop.git/common/strlcpy.c:61:3 in strlcpy
Github user Scotte noticed that in his setup a 'VSS-Monitoring ethernet trailer'
was being added to incoming Ethernet frames that contained SPA packets. This
caused base64 decoding to break because the packet data length was calculated
from what libpcap returned for the frame (so these bytes became included in the
SPA payload itself). This issue was reported as #163 on github.
This commit has fwknopd calculate the SPA payload length using the length field
in the IP header so that any trailing bytes in the Ethernet frame are not
included. This solution also applies to the Ethernet Frame Check Sequence issue
(#126).
This commit fixes issue #143 on github reported by Coacher. The previous
commit introduced a build time error for non UDP server mode as seen
here:
https://paste.kde.org/pkaxwobwr
Workaround for libpcap returning a length that is 4 bytes longer than
the
packet on the wire. Observed on:
Linux beaglebone 3.8.13-bone50 #1 SMP Tue May 13 13:24:52 UTC 2014
armv7l GNU/Linux
ldd fwknopd
libfko.so.2 => /usr/local/lib/libfko.so.2 (0xb6f62000)
libpcap.so.0.8 => /usr/lib/arm-linux-gnueabihf/libpcap.so.0.8
(0xb6f20000)
libc.so.6 => /lib/arm-linux-gnueabihf/libc.so.6 (0xb6e3b000)
/lib/ld-linux-armhf.so.3 (0xb6f94000)
libgcc_s.so.1 => /lib/arm-linux-gnueabihf/libgcc_s.so.1 (0xb6e17000)
Calculate the new pkt_end from the length in the ip header.
This commit updates all authorship and copyright information to include a
standard header that references the AUTHORS and CREDITS file. This standard
header was written by the Debian legal team at the request of Franck Joncourt.
- [client] Added '-P udpraw' to allow the client to send SPA packets over
UDP with a spoofed source IP address. This is in addition to the
original 'tcpraw' and 'icmp' protocols that also support a spoofed
source IP.
- [server] Bug fix to accept SPA packets over ICMP if the fwknop client
is executed with '-P icmp' and the user has the required privileges.
This commit does several things. First, a memory leak in fwknopd has been
fixed by ensuring to free access.conf stanzas. This bug was found with the
new test suite running in --enable-valgrind mode. Here is what some of the
valgrind output looked like to find the leak:
==19217== 11 bytes in 1 blocks are indirectly lost in loss record 3 of 5
==19217== at 0x4C2815C: malloc (vg_replace_malloc.c:236)
==19217== by 0x52F6B81: strdup (strdup.c:43)
==19217== by 0x10FC8B: add_acc_string (access.c:49)
==19217== by 0x1105C8: parse_access_file (access.c:756)
==19217== by 0x10B79B: main (fwknopd.c:194)
==19217==
==19217== 16 bytes in 1 blocks are indirectly lost in loss record 4 of 5
==19217== at 0x4C27480: calloc (vg_replace_malloc.c:467)
==19217== by 0x10FEC0: add_source_mask (access.c:88)
==19217== by 0x110100: expand_acc_source (access.c:191)
==19217== by 0x1104B0: parse_access_file (access.c:500)
==19217== by 0x10B79B: main (fwknopd.c:194)
==19217==
==19217== 183 (152 direct, 31 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 5
==19217== at 0x4C27480: calloc (vg_replace_malloc.c:467)
==19217== by 0x1103E4: parse_access_file (access.c:551)
==19217== by 0x10B79B: main (fwknopd.c:194)
==19217==
==19217== LEAK SUMMARY:
==19217== definitely lost: 152 bytes in 1 blocks
==19217== indirectly lost: 31 bytes in 3 blocks
==19217== possibly lost: 0 bytes in 0 blocks
==19217== still reachable: 8 bytes in 1 blocks
==19217== suppressed: 0 bytes in 0 blocks
Second, this commit changes how fwknopd acquires packet data with
pcap_dispatch() - packets are now processed within the callback function
process_packet() that is provided to pcap_dispatch(), the global packet
counter is incremented by the return value from pcap_dispatch() (since this is
the number of packets processed per pcap loop), and there are two new
fwknopd.conf variables PCAP_DISPATCH_COUNT and PCAP_LOOP_SLEEP to control the
number of packets that pcap_dispatch() should process per loop and the number
of microseconds that fwknopd should sleep per loop respectively. Without this
change, it was fairly easy to cause fwknopd to miss packets by creating bursts
of packets that would all be processed one at time with the usleep() delay
between each. For fwknopd deployed on a busy network and with a permissive
pcap filter (i.e. something other than the default that causes fwknopd to look
at, say, TCP ACK's), this change should help.
Third, the criteria that a packet must reach before data copying into the
buffer designed for SPA processing has been tightened. A packet less than
/greater than the minimum/maximum expected sizes is ignored before data is
copied, and the base64 check is done as well.
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.
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>
