This commit is a follow up to Ryman's report (#85) of a potential timing attack
that could be leveraged against fwknop when strncmp() is used to compare HMAC
digests. All strncmp() calls that do similar things have been replaced with a
new constant_runtime_cmp() function that mitigates this problem.
Lots of places in the code were already using {0} to initialize stack char
arrays, but memset() was being used as well. This commit removes all
unnecessary memset() calls against char arrays that are already initialized
via {0} (which sets all members to zero for such arrays).
[libfko] Added the ability to maintain backwards compatibility with the
now deprecated "zero padding" strategy in AES mode that was a hold over
from the old perl fwknop implementation. This enables the backwards
compatiblity tests to continue to pass in the test suite.
Added --enable-openssl-checks to send all SPA packets encrypted via libfko
through the OpenSSL library to ensure that the libfko usage of AES is always
compatible with OpenSSL. This ensures that the fwknop usage of AES is properly
implemented as verified by the OpenSSL library, which is a frequently audited
high profile crypto engine. If a vulnerability is discovered in OpenSSL and a
change is made, then the --enable-openssl-checks mode will allow the test suite
to discover this in a automated fashion for fwknop.
Significant bug fix to honor the full encryption key length for
user-supplied Rijndael keys > 16 bytes long. Previous to this bug fix,
only the first 16 bytes of a key were actually used in the encryption/
decryption process even if the supplied key was longer. The result was
a weakening of expected security for users that had keys > 16 bytes,
although this is probably not too common. Note that "passphrase" is
perhaps technically a better word for "user-supplied key" in this
context since Rijndael in CBC mode derives a real encryption/decryption
key from the passphrase through a series of applications of md5 against
the passphrase and a random salt. This issue was reported by Michael T.
Dean. Closes issue #18 on github.
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.
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.
For SPA packets encrypted with Rjindael, fwknop has always used CBC mode
even though ECB mode is mentioned in a couple of places. This change makes
more transparent use of block_encrypt() and block_decrypt() to ensure that
the appropriate mode is used. The default is CBC mode, but others can be
selected as well (-M <mode> for the fwknop client, and ENCRYPTION_MODE in
access.conf for the fwknopd server).
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.
cipher_funcs.c:85: warning: ignoring return value of ‘fread’, declared with attribute warn_unused_result
A few of these were in code in the lib/ directory, and required adding a
new error code 'FKO_ERROR_FILESYSTEM_OPERATION' and associated error
string 'Read/write bytes mismatch'.
git-svn-id: file:///home/mbr/svn/fwknop/trunk@195 510a4753-2344-4c79-9c09-4d669213fbeb