This commit changes how fko_new() deals with FKO context initialization
to not set ctx->initval back to zero (uninitialized) imediately after
calling each fko_set_... function and before checking the fko_set_... return
value. The reason for this change is that fko_destroy() checks for
context initialization via ctx->initval before calling free() against
any heap allocated context member. So, if fko_set_... returns an error,
fko_destroy() (previous to this commit) would have no opportunity to
free such members.
This bug was found with fault injection testing provided by libfiu
together with valgrind. Specifically the following test suite command
exposes the problem (from the test/ directory):
./test-fwknop.pl --enable-complete --include "fault injection.*libfko"
In the resulting output/2.test file valgrind reports the following:
==27941== LEAK SUMMARY:
==27941== definitely lost: 264 bytes in 1 blocks
==27941== indirectly lost: 28 bytes in 3 blocks
==27941== possibly lost: 0 bytes in 0 blocks
==27941== still reachable: 1,099 bytes in 12 blocks
==27941== suppressed: 0 bytes in 0 blocks
After this commit is applied, this changes to:
==7137== LEAK SUMMARY:
==7137== definitely lost: 0 bytes in 0 blocks
==7137== indirectly lost: 0 bytes in 0 blocks
==7137== possibly lost: 0 bytes in 0 blocks
==7137== still reachable: 1,099 bytes in 12 blocks
==7137== suppressed: 0 bytes in 0 blocks
Note that 'definitely lost' in valgrind output means there is a real
memory leak that needs to be fixed whereas 'still reachable' is most
likely not a real problem according to:
http://valgrind.org/docs/manual/faq.html#faq.deflost
This is a significant commit to add the ability to leverage libfko fault
injections from both the fwknop client and server command lines via a
new option '--fault-injection-tag <tag name>'. This option is used by
the test suite with the tests/fault_injection.pl tests.
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.
Integer lengths that are negative are never valid. This commit also
extends the fuzzing capabilities of the test/fko-wrapper code to
validate libfko calls with negative length arguments, and one crash
scenario with a negative length for the encryption key was found (and
fixed) this way.
This is a fairly significant commit that lays the groundwork for getting
selectable HMAC modes working for both the client and server. One libfko API
change was required so that the hmac_type is passed into fko_new_with_data().
This allows the server to set the hmac_type via access.conf stanzas. The
effort in this commit will be extended to allow HMAC MD5, SHA1, and SHA512
also function properly.
This commit fixes the following memory leak found with the test suite running
in valgrind mode:
HEAP SUMMARY:
in use at exit: 217 bytes in 3 blocks
total heap usage: 27 allocs, 24 frees, 5,260 bytes allocated
44 bytes in 1 blocks are definitely lost in loss record 1 of 3
at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x50CB861: strndup (strndup.c:46)
by 0x4E3A4D4: fko_verify_hmac (fko_hmac.c:54)
by 0x4E394DD: fko_new_with_data (fko_funcs.c:220)
by 0x10B3A7: main (fwknop.c:408)
44 bytes in 1 blocks are definitely lost in loss record 2 of 3
at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x50CB801: strdup (strdup.c:43)
by 0x4E3A3FC: fko_calculate_hmac (fko_hmac.c:162)
by 0x4E3A552: fko_verify_hmac (fko_hmac.c:86)
by 0x4E394DD: fko_new_with_data (fko_funcs.c:220)
by 0x10B3A7: main (fwknop.c:408)
129 bytes in 1 blocks are definitely lost in loss record 3 of 3
at 0x4C2B7B2: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4E36A03: add_salted_str (cipher_funcs.c:298)
by 0x4E3A587: fko_verify_hmac (fko_hmac.c:75)
by 0x4E394DD: fko_new_with_data (fko_funcs.c:220)
by 0x10B3A7: main (fwknop.c:408)
LEAK SUMMARY:
definitely lost: 217 bytes in 3 blocks
indirectly lost: 0 bytes in 0 blocks
possibly lost: 0 bytes in 0 blocks
still reachable: 0 bytes in 0 blocks
suppressed: 0 bytes in 0 blocks
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 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.
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.
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.
