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

Support for OpenSSL 3.0 #599

Closed
nazar-pc opened this issue Apr 3, 2022 · 14 comments
Closed

Support for OpenSSL 3.0 #599

nazar-pc opened this issue Apr 3, 2022 · 14 comments
Milestone

Comments

@nazar-pc
Copy link
Contributor

nazar-pc commented Apr 3, 2022

In 3.0 some APIs were deprecated that results in this warnings:

[1025/1321] Compiling C object subprojects/libsrtp-2.4.2/libsrtp2.a.p/crypto_hash_hmac_ossl.c.o
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c: In function ‘srtp_hmac_alloc’:
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:106:5: warning: ‘HMAC_CTX_new’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  106 |     (*a)->state = HMAC_CTX_new();
      |     ^
In file included from ../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:54:
../../../subprojects/openssl-3.0.2/include/openssl/hmac.h:33:33: note: declared here
   33 | OSSL_DEPRECATEDIN_3_0 HMAC_CTX *HMAC_CTX_new(void);
      |                                 ^~~~~~~~~~~~
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c: In function ‘srtp_hmac_dealloc’:
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:136:5: warning: ‘HMAC_CTX_free’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  136 |     HMAC_CTX_free(hmac_ctx);
      |     ^~~~~~~~~~~~~
In file included from ../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:54:
../../../subprojects/openssl-3.0.2/include/openssl/hmac.h:35:28: note: declared here
   35 | OSSL_DEPRECATEDIN_3_0 void HMAC_CTX_free(HMAC_CTX *ctx);
      |                            ^~~~~~~~~~~~~
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c: In function ‘srtp_hmac_start’:
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:152:5: warning: ‘HMAC_Init_ex’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  152 |     if (HMAC_Init_ex(state, NULL, 0, NULL, NULL) == 0)
      |     ^~
In file included from ../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:54:
../../../subprojects/openssl-3.0.2/include/openssl/hmac.h:43:27: note: declared here
   43 | OSSL_DEPRECATEDIN_3_0 int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
      |                           ^~~~~~~~~~~~
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c: In function ‘srtp_hmac_init’:
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:164:5: warning: ‘HMAC_Init_ex’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  164 |     if (HMAC_Init_ex(state, key, key_len, EVP_sha1(), NULL) == 0)
      |     ^~
In file included from ../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:54:
../../../subprojects/openssl-3.0.2/include/openssl/hmac.h:43:27: note: declared here
   43 | OSSL_DEPRECATEDIN_3_0 int HMAC_Init_ex(HMAC_CTX *ctx, const void *key, int len,
      |                           ^~~~~~~~~~~~
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c: In function ‘srtp_hmac_update’:
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:179:5: warning: ‘HMAC_Update’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  179 |     if (HMAC_Update(state, message, msg_octets) == 0)
      |     ^~
In file included from ../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:54:
../../../subprojects/openssl-3.0.2/include/openssl/hmac.h:45:27: note: declared here
   45 | OSSL_DEPRECATEDIN_3_0 int HMAC_Update(HMAC_CTX *ctx, const unsigned char *data,
      |                           ^~~~~~~~~~~
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c: In function ‘srtp_hmac_compute’:
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:205:5: warning: ‘HMAC_Update’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  205 |     if (HMAC_Update(state, message, msg_octets) == 0)
      |     ^~
In file included from ../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:54:
../../../subprojects/openssl-3.0.2/include/openssl/hmac.h:45:27: note: declared here
   45 | OSSL_DEPRECATEDIN_3_0 int HMAC_Update(HMAC_CTX *ctx, const unsigned char *data,
      |                           ^~~~~~~~~~~
../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:208:5: warning: ‘HMAC_Final’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  208 |     if (HMAC_Final(state, hash_value, &len) == 0)
      |     ^~
In file included from ../../../subprojects/libsrtp-2.4.2/crypto/hash/hmac_ossl.c:54:
../../../subprojects/openssl-3.0.2/include/openssl/hmac.h:47:27: note: declared here
   47 | OSSL_DEPRECATEDIN_3_0 int HMAC_Final(HMAC_CTX *ctx, unsigned char *md,
      |                           ^~~~~~~~~~

Would be nice to get them fixes in upcoming releases.

@paulej
Copy link
Contributor

paulej commented Apr 13, 2022

@nazar-pc
Copy link
Contributor Author

Yes, it is not a very difficult process to refactor the code, but some needs to spend time on it

@JonathanLennox
Copy link

Note that even though these functions are deprecated, they're still fully available in OpenSSL 3.x, and the high-level APIs just call these low-level functions directly. So this change wouldn't (as far as I know) gain anything other than preventing compile-time warnings.

The old OpenSSL 1.1 APIs are also, as far as I know, still the APIs used by libressl and BoringSSL, so the old code couldn't be removed even if we wanted to drop OpenSSL 1.1 support. (The new APIs were only introduced in OpenSSL 3.x.)

@nazar-pc
Copy link
Contributor Author

  1. Having compile-time warnings is a bad practice in general, so not generating them would be nice
  2. It is possible to disable 1.1 APIs in OpenSSL 3.0 if desired; some might do that, then libsrtp will become incompatible
  3. I don't suggest to drop OpenSSL 1.1 at this point, just add OpenSSL 3.0 support explicitly to avoid build warnings

@pabuhler
Copy link
Member

We will add support for compiling cleanly against OpenSSL 3.0 either by using the OpenSSL version define or a explicit compile flag (or both). Should be ready for next release.

@pabuhler pabuhler added this to the Version 2.5 milestone Apr 26, 2022
@pabuhler
Copy link
Member

pabuhler commented May 2, 2022

#602 fails with OpenSSL 3.0.2. but passes with OpenSSL 3.0.3-dev, I will investigate why when I get time but does anyone know if EVP_MAC reuse has issues?

@pabuhler
Copy link
Member

pabuhler commented May 2, 2022

openssl/openssl#17811 :(

@pabuhler
Copy link
Member

pabuhler commented May 6, 2022

#605 is an alternative solution that simply disables the warning for now.

@kloczek
Copy link

kloczek commented Dec 3, 2022

Looks like autoconf fails on detecting openssl 3.x

checking for library containing EVP_EncryptInit... no
configure: error: in `/home/tkloczko/rpmbuild/BUILD/libsrtp-2.4.2':
configure: error: can't find openssl >= 1.0.1 crypto lib
See `config.log' for more details

I see some openssl 3.x fixes above last tag. Is it possible to make new release to fix build of the libsrtp with openssl 3.x?

@pabuhler
Copy link
Member

pabuhler commented Dec 4, 2022

Hi @kloczek am planning on a new release in next few weeks, will be sure sure to address this issue

@kloczek
Copy link

kloczek commented Dec 5, 2022

In this case |I found that it was mileading mesage. If you will look closet on the bottom openssl detection

libsrtp/configure.ac

Lines 208 to 256 in cc362ae

AC_ARG_ENABLE([openssl],
[AS_HELP_STRING([--enable-openssl], [compile in OpenSSL crypto engine])],
[], [enable_openssl=no])
AC_MSG_RESULT([$enable_openssl])
AC_MSG_CHECKING([whether to leverage NSS crypto])
AC_ARG_ENABLE([nss],
[AS_HELP_STRING([--enable-nss], [compile in NSS crypto engine])],
[], [enable_nss=no])
AC_MSG_RESULT([$enable_nss])
if test "$enable_openssl" = "yes"; then
AC_MSG_CHECKING([for user specified OpenSSL directory])
AC_ARG_WITH([openssl-dir],
[AS_HELP_STRING([--with-openssl-dir], [Location of OpenSSL installation])],
[if test "x$PKG_CONFIG" != "x" && test -f $with_openssl_dir/lib/pkgconfig/libcrypto.pc; then
if test "x$PKG_CONFIG_PATH" = "x"; then
export PKG_CONFIG_PATH="$with_openssl_dir/lib/pkgconfig"
else
export PKG_CONFIG_PATH="$with_openssl_dir/lib/pkgconfig:$PKG_CONFIG_PATH"
fi
AC_MSG_RESULT([$with_openssl_dir])
elif test -d $with_openssl_dir/lib; then
CFLAGS="$CFLAGS -I$with_openssl_dir/include"
if test "x$LDFLAGS" = "x"; then
LDFLAGS="-L$with_openssl_dir/lib"
else
LDFLAGS="$LDFLAGS -L$with_openssl_dir/lib"
fi
AC_MSG_RESULT([$with_openssl_dir])
else
AC_MSG_RESULT([invalid])
AC_MSG_FAILURE([Invalid OpenSSL location: $with_openssl_dir])
fi],
[AC_MSG_RESULT([no])])
if test "x$PKG_CONFIG" != "x"; then
PKG_CHECK_MODULES([crypto], [libcrypto >= 1.0.2i],
[CFLAGS="$CFLAGS $crypto_CFLAGS"
LIBS="$crypto_LIBS $LIBS"
openssl_cleanse_broken=no],
[PKG_CHECK_MODULES([crypto], [libcrypto >= 1.0.1],
[CFLAGS="$CFLAGS $crypto_CFLAGS"
LIBS="$crypto_LIBS $LIBS"
openssl_cleanse_broken=maybe])])
else
AC_CHECK_LIB([dl], [dlopen], [], [AC_MSG_WARN([can't find libdl])])
AC_CHECK_LIB([z], [inflate], [], [AC_MSG_WARN([can't find libz])])
fi

you can fiund libdl and libz detections.
I had no installed libz devel resources and that message was printed by incorrecvt logic in tjhat section
Looks like libz detection can be removed because nothing in source tree is usimg libz.

[tkloczko@devel-g2v libsrtp-2.4.2]$ grep zlib.h -r
[tkloczko@devel-g2v libsrtp-2.4.2]$

Whole section could be dramatically reduced if PKG_CHECK_MODULES() aclocal macro would be used.

@kloczek
Copy link

kloczek commented Dec 5, 2022

BTW looks like actual C code is not using libdl as well.

@kloczek
Copy link

kloczek commented Dec 5, 2022

Here is minimal version of the fix

--- a/configure.ac
+++ b/configure.ac
@@ -250,9 +250,6 @@
          [CFLAGS="$CFLAGS $crypto_CFLAGS"
           LIBS="$crypto_LIBS $LIBS"
           openssl_cleanse_broken=maybe])])
-   else
-     AC_CHECK_LIB([dl], [dlopen], [], [AC_MSG_WARN([can't find libdl])])
-     AC_CHECK_LIB([z], [inflate], [], [AC_MSG_WARN([can't find libz])])
    fi

    AC_SEARCH_LIBS([EVP_EncryptInit], [crypto],

@pabuhler
Copy link
Member

closing this as #602 is merged and there are now active work flows that test openssl 1.1.1 & 3.0.2 & 3.0.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants