cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.19k stars 469 forks source link

Enables the use of wolfSSL for crypto primitives #692

Closed SparkiDev closed 2 months ago

SparkiDev commented 3 months ago

To configure: ./configure --enable-wolfssl --with-wolfssl-dir=

Add implementations of SRTP KDF, HMAC, AES-GCM and AES-CTR using wolfSSL.

SparkiDev commented 3 months ago

Can I get a review of this code?

pabuhler commented 3 months ago

Can I get a review of this code?

Yes it is on my list of things todo in the next week, sorry if that seams like a long time. If possible can you add support for building with cmake as it is our main builld system these days and having a CI build for at least one build system and on at least one platform would be very helpful. see .github/workflows/cmake.yml or .github/workflows/autotools.yml

JonathanLennox commented 3 months ago

Wrong things will happen if you enable two or more of --enable-openssl, --enable-wolfssl, and --enable-nss, and I think we have nothing that guards against this. This isn't a new problem with wolfssl (it was already a problem with just openssl and nss), but should we have something to protect against this?

Or possibly even change the configure option syntax to --enable-crypto=[openssl,wolfssl,nss] with the 3.0 release, since that's where we're putting other breaking changes.

pabuhler commented 3 months ago

Wrong things will happen if you enable two or more of --enable-openssl, --enable-wolfssl, and --enable-nss, and I think we have nothing that guards against this. This isn't a new problem with wolfssl (it was already a problem with just openssl and nss), but should we have something to protect against this?

Or possibly even change the configure option syntax to --enable-crypto=[openssl,wolfssl,nss] with the 3.0 release, since that's where we're putting other breaking changes.

It think there is some protection in the cmake build files at least, but I agree at this point when there are now ~5 options the config should be changed to a switch as you suggested

SparkiDev commented 2 months ago

Changed CI testing to always use GitHub to clone wolfssl using https.

pabuhler commented 2 months ago

@SparkiDev , Hi I will look in to the failing Meson build, seams to also fail in the main branch at the moment. As to the other failures, maybe try to install "brew install automake" on macos and building wolfssl as a static library .... They are are just quick thoughts I have looked to deeply into the failures..

SparkiDev commented 2 months ago

Changes: Installing autoconf,, automake and libtool on make to build wolfSSL from source. Invloking ldconfig on Ubuntu so that wolfSSL library is seen.

SparkiDev commented 2 months ago

Attempted to fix cmake on macOS by exporting include path /usr/local/include

SparkiDev commented 2 months ago

Changed WOLFSSL_INCLUDE_DIRS -> WOLFSSL_INCLUDE_DIR in FindwolfSSL.cmake to match CMakeLists.txt.

SparkiDev commented 2 months ago

Changed when the Aes object is allocated to minimize memory usage.

SparkiDev commented 2 months ago

Improved memory usage around AES-GCM.

pabuhler commented 2 months ago

@SparkiDev , it is merged but now the ci builds fail to build wolfssl ... https://github.com/cisco/libsrtp/actions/runs/8861716469/job/24333886680 Can you take a look? As a side note should probably not be build from tip of main branch of wolfssl, mayne use v5.7.0-stable tag ? Thanks

SparkiDev commented 2 months ago

https://github.com/cisco/libsrtp/pull/704 should fix this.