It's described in
https://github.com/openssl/openssl/issues/11715#issuecomment-2164442688
as being one of several routines that "are internal macros and not
really suitable for the Brave New Provider World", and the bug in
question complains that they're not documented.
Instead, just map "3des" to "des-ede3-cbc" ourselves.
This should fix#1175.
Grab the stuff from libpcap's configure script that looks for libssl
(and libcrypto) and adapt it to look for libcrypto.
his includes some macros to check using pkg-config (and other macros,
such as macros to save and restore CFLAGS, LIBS, and LDFLAGS; any
resemblance between their names and the cmake_push_check_state() and
cmake_pop_check_state() commands is *entirely* coincidental :-)).
Instead of checking for DES_cbc_encrypt(), which we don't use, to
determine whether the libcrypto we found is usable, check for
EVP_CIPHER_CTX_block_size(), which we *do* use. (We also check whether
the openssl/evp.h header exists; if it doesn't, we might have found the
libcrypto that Apple bundles with macOS, for which they do *NOT* provide
the header in newer versions of Xcode.) See also #1174.
This means that we don't need to check whether we have openssl/evp.h at
compile time - now, if we don't, we don't even set HAVE_LIBCRYPTO, so
there's no need to check HAVE_OPENSSL_EVP_H.
We already report a warning if the secret is too long; do so if there's
an invalid hex digit in the secret.
See
https://github.com/the-tcpdump-group/tcpdump/pull/1185#issuecomment-2119211162
While we're at it, put the length of the string, in hex-digit pairs,
into a size_t rather than an unsigned int, just in case (it's unlikely
that it will overflow a 32-bit unsigned integer, but...).
Some compilers might not understand the declaration of the ndo_error
member of an netdissect_options as pointing to a function that never
returns; add a /* NOTREACHED */ comment in case they recognize that as a
hint.
Builds using Autotools or CMake generate config.h, thus remove the
'#ifdef HAVE_CONFIG_H'/'#endif'.
Remove also the 'add_definitions(-DHAVE_CONFIG_H)' in CMakeLists.txt.
Found with clang, CFLAGS=-fsanitize=memory.
Fix GitHub issues #848 and #849.
The problem is that for some unknown reason the pt buffer is not
initialized after EVP_DecryptUpdate() call, no error, in:
print-esp.c:260: if (!EVP_DecryptUpdate(ctx, pt, &len, ct, ctlen)) {
Always call ndo->ndo_error with a memory-allocation error if they fail.
Add WARN_UNUSED_RESULT for compilers that support it, and use it for
those routines, so that any future code that doesn't check for failure
gets a warning.
Have nd_push_buffer() take a snapshot length, not a snapshot end, as
its last argument.
Replace nd_push_snapend() and nd_change_snapend() with nd_push_snaplen()
and nd_change_snaplen(), both of which take a pointer into the packet
buffer and snapshot length relative to that pointer as arguments. Have
those routines check the snapshot length to make sure it's not bigger
than the number of bytes in the packet past the pointer, and silently
ignore the requst if it is.
Using a length rather than a pointer avoids the possibility of the
calculation of the snapshot end overflowing and resulting in a snapshot
end *before* the point in the buffer.
Add a test for this, with a capture file containing an IPv6 packet with
an extremely large "jumbo" packet size.
Revert the "Make sure we don't set the snapend before the beginning of
the packet." changes, as they no longer apply with this change (which
also makes sure we don't set the snapend before the beginning of the
packet).
It's explicitly cheating in some places; just cast away the warnings.
Add a Clang version testing macro to compiler-tests.h.
Move the diagnostic control #defines from netdissect-stdinc.h to
diag-control.h, libpcap-style, so that they can be used by code that
doesn't use (or need) netdissect-stdinc.h. This also means that we can
limit the inclusion of diag-control.h, and the definition of those
Do them more libpcap-style, with separate DIAG_OFF/DIAG_ON pairs for
particular issues, rather than having DIAG_OFF() and DIAG_ON() macros
that take -W option names as arguments; that way, if we need to define
them for compilers that don't have the GCC/Clang syntax for those
pragmas, e.g. MSVC, we can do so.
Rename esp_print_decodesecret() to esp_decodesecret_print() and
esp_print_decrypt_buffer_by_ikev2() to esp_decrypt_buffer_by_ikev2_print()
with _print suffix like in most similar cases.
At least as I read RFC 5996 section 3.14 and RFC 4303 section 2.4, if
the cipher has a block size of which the ciphertext's size must be a
multiple, the payload must be padded to make that happen, so the
ciphertext length must be a multiple of the block size. Instead of
allocating a buffer, copying the ciphertext to it, and padding it to the
block size, fail if its size isn't a multiple of the block size.
(Note also that the old padding code added a block's worth of padding to
the end of a ciphertext block that *was* a multiple of the cipher block
size; this might have caused problems.)
Don't use the undocumented EVP_Cipher(); the lack of documentation means
a lack of information about whatever requirements it might impose. Use
EVP_DecryptUpdate() instead.
Before calling it, use EVP_CIPHER_CTX_set_padding() to say "don't do
your own padding, this block is a multiple of the cipher block size".
Instead of using EVP_CipherInit() or EVP_CipherInit_ex(), use
EVP_DecryptInit() or EVP_DecryptInit_ex(). as we're always doing
decryption and never doing encryption - the extra parameter to
EVP_CipherInit() and EVP_CipherInit_ex() is always 0.
This may address GitHub issue #814.
It may also make it a bit easier to have the code use Common Crypto on
macOS (rather than requiring that OpenSSL be installed - macOS ships
with an OpenSSL shared library for binary compatibility with older
releases, but doesn't ship with the headers, because Apple wants you
using their crypto code) and use Cryptography API: Next Generation on
Windows (Vista/Server 2008 and later) (rather than requiring a Windows
build of OpenSSL).
(Hopefully this will all work with LibreSSL.)
If you split a string by overwriting the separator character with a NUL,
and you want to compare the first part with various values, use a
pointer to the beginning of the string, not to the NUL you just dropped
in.
This is a bug found while testing some cleanups for GitHub issue #814;
it's necessary for the cleanups to work.
Some of them are locale-dependent, and all of them run the risk of
failing if you hand them a char with the 8th bit set.
Move our replacements to a new netdissect-ctype.h file, and, for the
ones that check for particular character types, add _ASCII to the name,
to indicate that only ASCII characters pass the check. Do the same for
the ones that map between cases, to indicate that they only map ASCII
letters.
For isspace(), explicitly check for the characters we care about, to
make it clearer what we're doing.
If we have an Ethernet packet where the last 2 octets of the header are
a length rather than an Ethernet type, and it's less than the remaining
length of the packet, shorten the length and captured length, update the
snapshot end.
Turn the buffer stack into a "packet information" stack, so that, if we
*do* update the snapshot end, we push the old end onto the stack, and
pop it off as soon as we're done dissecting the Ethernet packet, in case
there's more data in the packet after the Ethernet packet.
Use the stack when we use the IPv4 and IPv6 length fields as well.
If a dissector has to process its input - decryption, decompression,
etc. - rather than dissect the raw input, it should push the processed
input onto the buffer stack. As soon as the dissection is done, the
stack should be popped, to free the buffer into which the processing was
done, and restore the "pointer to packet data" and "pointer to end of
packet data" members of the netdissect_options structure, so the code
can go back to dissecting the original data.
The stack will get everything popped off it when dissection is done.
Use this mechanism in the ESP decryption code rather than scribbling on
top of the input packet data.
"ivoff" is a pointer to the IV, not the offset of the IV; call it ivptr.
Have a variable that points to the beginning of the ciphertext, and use
that.
Fix the check that makes sure the authentication data/integrity check
value length isn't too big - it needs to make sure that it doesn't go
before the beginning of the ciphertext, i.e. doesn't overlap with the
IV.
Don't bother with a variable pointing to the secret, just pass
sa->secret.
Fix the check that makes sure the padding length isn't too big - make
sure it, plus 2 for the padding length and next header bytes, isn't
bigger than the ciphertext length.
Update a test to reflect the stricter length checks.
Just check whether the pointer to it is within the available packet data
- and, if it's not, report truncation.
While we're at it:
Make the initialization vector length unsigned; it's either zero or a
positive number.
Rename a variable used for the ciphertext len to ctlen.
If we can't decrypt the payload, we can't dissect it, so don't try -
just give up immediately.
While we're at it:
If EVP_CIPHER_CTX_new() fails, it means a memory allocation failed;
treat that as such.
Use some of the arguments we're passed rather than re-fetching them from
the IP header.
Add some comments.
Call nd_print_trunc() for failed length sanity checks, and note that
they can fail due to the decryption being done with the wrong key.
Update one test's output; it is, I think, being decrypted with the wrong
key.
The exceptions are currently:
Some EXTRACT_ in print-juniper.c, not used on packet buffer pointer.
An EXTRACT_BE_U_3 in addrtoname.c, not always used on packet buffer
pointer.
Found with -Wunreachable-code-return clang compiler option.
The errors were:
./print-esp.c:317:10: warning: 'return' will never be executed
[-Wunreachable-code-return]
return 0;
^
./print-esp.c:552:4: warning: 'return' will never be executed
[-Wunreachable-code-return]
return;
^~~~~~
Found with -Wunreachable-code clang compiler option.
The errors were:
./print-esp.c:263:3: warning: code will never be executed
[-Wunreachable-code]
free(input_buffer);
^~~~
./print-esp.c:246:3: warning: code will never be executed
[-Wunreachable-code]
EVP_CIPHER_CTX_free(ctx);
^~~~~~~~~~~~~~~~~~~
./print-esp.c:843:5: warning: code will never be executed
[-Wunreachable-code]
free(input_buffer);
^~~~
./print-esp.c:826:5: warning: code will never be executed
[-Wunreachable-code]
EVP_CIPHER_CTX_free(ctx);
^~~~~~~~~~~~~~~~~~~
This can prevent bizarre failures if, for example, you've done a
configuration in the top-level source directory, leaving behind one
config.h file, and then do an out-of-tree build in another directory,
with different configuration options. This way, we always pick up the
same config.h, in the build directory.
