Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check for libcrypto using EVP_CIPHER_CTX_new #1174

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryancdotorg
Copy link

I build static versions of tcpdump for my own convenience, and I disable DES in OpenSSL when doing so. This causes tcpdump to fail to detect libcrypto.

Currently, I have a script that applies this patch - looking for AES_cbc_encrypt rather than DES_cbc_encrypt - before building.

Given that AES is now over a quarter of a century old, and is a required algorithm in TLS, I think we can assume that if libcrypto exsists, it will support AES.

DES, on the other hand, seems very reasonable to disable when building OpenSSL, and I expect that some Linux distributions will begin doing this by default in the next few years.

@infrastation
Copy link
Member

This is exactly where #1111 started.

@ryancdotorg
Copy link
Author

ryancdotorg commented Apr 10, 2024

From having a quick look at #1111 I have changed this to use EVP_CIPHER_CTX_new.

Could the maintainers please let me know if they're willing to accept a patch to bundle an MD5 implementation to fall back to if libcrypto isn't available to address concerns raised in #1111?

I would be happy to provide HMAC-MD5 that can reuse key material across multiple calls (somewhat faster) as well.

@ryancdotorg ryancdotorg changed the title check for libcrypto using AES_cbc_encrypt check for libcrypto using EVP_CIPHER_CTX_new Apr 10, 2024
@ryancdotorg
Copy link
Author

Actually, there's a public domain implementation of MD5 that's compatible with the OpenSSL function prototypes here

https://openwall.info/wiki/people/solar/software/public-domain-source-code/md5

which could be used.

@ryancdotorg
Copy link
Author

This needs more changes, please do not merge.

@fxlb fxlb marked this pull request as draft April 10, 2024 11:41
@infrastation
Copy link
Member

With long-term maintenance in mind, a useful way to reason about it seems to be "if a user wants to use a particular old/new cipher/mode, it is their responsibility to provide a library version that supports it, and it is tcpdump code responsibility to detect and to use the required library functions, but not to reimplement any functionality that the library does not have".

@ryancdotorg
Copy link
Author

That seems sensible. Separate checks for MD5 and disabling that code when not available is straightforward.

So, then:

  • I will switch this back to doing a gate on AES_cbc_encrypt.

In order to future-proof, in anticipation of not being able to rely on the long term availability if MD5_Init/MD5_Update/MD5_Final in libcrypto:

  • I will add checks for MD5 availability to the configure/cmake scripts.
  • I will change the ifdefs for code depending on MD5 availability to use HAVE_MD5_INIT.

guyharris added a commit to guyharris/tcpdump that referenced this pull request Jul 10, 2024
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 the-tcpdump-group#1174.

This means that we don't need to check wehether 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.
@guyharris
Copy link
Member

Given that we use neither DES_cbc_encrypt() nor AES_cbc_encrypt(), we shouldn't check for either one.

We currently use EVP_CIPHER_CTX_new() if it's present, and use calloc() to allocate a EVP_CIPHER_CTX if it's not.

We do use EVP_CIPHER_CTX_block_size() unconditionally, so, in order to check whether we can build with libcrypto, we can just check for that. See pull request #1197.

guyharris added a commit to guyharris/tcpdump that referenced this pull request Jul 10, 2024
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 the-tcpdump-group#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.
guyharris added a commit that referenced this pull request Jul 10, 2024
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.
guyharris added a commit that referenced this pull request Jul 12, 2024
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.

(cherry picked from commit 749988b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants