mbedtls files changed and cmake instruction added to support MbedTLS … by seyednasermoravej · Pull Request #782 · cisco/libsrtp

seyednasermoravej · 2025-12-25T12:10:21Z

Hi Cisco team, and Merry Christmas 🎄

This PR is a draft for the migration to Mbed TLS v4.0.
For easier review and comparison, I’ve kept both the legacy code and the new implementation side by side.

There are still some unnecessary fields in a few structures, and in some places the implementation can be improved (for example, using status instead of errCode). I intentionally kept the changes as minimal as possible for now; we can clean up and refine these parts in follow-up steps once the migration direction is agreed upon.

Feedback and suggestions are very welcome.

seyednasermoravej · 2026-01-07T17:25:46Z

How can I check CI before push my code?
I'm facing GCM mode error in the CI while it passed on my side.
My command to build the code is:
cmake -S . -B build -DCRYPTO_LIBRARY=mbedtls -DMBEDTLS_INCLUDE_DIRS=/opt/mbedtls-4/include -DMBEDTLS_LIBRARY="/opt/mbedtls-4/lib/libmbedtls.a;/opt/mbedtls-4/lib/libmbedcrypto.a" && cmake --build build
assuming MbedTLS V4.0 is installed in /opt/mbedtls-4 directory.
I have tested rtpw application while the sender is output from mbedtls4.0 and the receiver is from previous mbedtls and vice versa.

pabuhler · 2026-01-07T21:15:30Z

I normally just try to replicate the environment, or else do a lot of pushing to a PR and then clean up and squash before creating the final PR.

Just looking quickly I would guess you need to update the CI scripts to use MbedTLS 4 or ?
If you tell me how to install the correct version of mbedtls on ubuntu I could try to build and run locally and look at CI script.

seyednasermoravej · 2026-01-08T16:29:50Z

I normally just try to replicate the environment, or else do a lot of pushing to a PR and then clean up and squash before creating the final PR.

Just looking quickly I would guess you need to update the CI scripts to use MbedTLS 4 or ? If you tell me how to install the correct version of mbedtls on ubuntu I could try to build and run locally and look at CI script.

I tested this scripts on Ubuntu 24.04 and Ubuntu 22.04. Maybe you face python errors because of absence of some packages. Please install them via pip. If you face no python error, you could create a .sh file and source it:

cd ~ git clone https://github.com/Mbed-TLS/mbedtls.git cd mbedtls git checkout mbedtls-4.0.0 git submodule update --init --recursive cmake -S . -B build cmake --build build cd ~/mbedtls/build sudo cmake --install . --prefix=/opt/mbedtls-4 cd ~ git clone https://github.com/seyednasermoravej/libsrtp.git cd libsrtp git checkout migration-to-psa-crypto cmake -S . -B build -DCRYPTO_LIBRARY=mbedtls -DMBEDTLS_INCLUDE_DIRS=/opt/mbedtls-4/include -DMBEDTLS_LIBRARY=/opt/mbedtls-4/lib/libmbedtls.a -DMBEDX509_LIBRARY=/opt/mbedtls-4/lib/libmbedx509.a -DMBEDCRYPTO_LIBRARY=/opt/mbedtls-4/lib/libmbedcrypto.a cmake --build build cp test/words.txt build cd build ./rtpw -s -k c1eec3717da76195bb878578790af71c4ee9f859e197a414a78d5abc7451 -e 128 -a 0.0.0.0 9999
That is the sender, you could run the receiver using this command:
cd ~/libsrtp/build ./rtpw -r -k c1eec3717da76195bb878578790af71c4ee9f859e197a414a78d5abc7451 -e 128 -a 0.0.0.0 9999

pabuhler · 2026-01-10T20:14:40Z

@seyednasermoravej I made some change to get the CI to pass, hope it was OK to push to this branch. You can do what you want with them.

I would suggest rebasing and removing the commits which are not related to getting mbtls working.

seyednasermoravej · 2026-01-28T04:46:39Z

@seyednasermoravej I made some change to get the CI to pass, hope it was OK to push to this branch. You can do what you want with them.

I would suggest rebasing and removing the commits which are not related to getting mbtls working.

Apologies for my delayed response. Is there anything in the code that could be improved?

pabuhler · 2026-01-28T14:27:27Z

@seyednasermoravej I am not sure you need the change to the readme as when this is merged then it should just build as before.
Other than that I think you should make it "ready for review" and see if any one else has comments.

…V4.0

The aes interface in libsrtp expects to be able to call encrypt multiple times in a progressive manor. A call to set the IV should reset for a new set of operations. This interface is not so oblivious and there is currently no indication of the last call. This should really be improved.

Pass dst_len as it is the actual buffer size. Remove unused code, full_tag & ciphertext_len.

seyednasermoravej · 2026-01-28T20:59:45Z

@seyednasermoravej I am not sure you need the change to the readme as when this is merged then it should just build as before. Other than that I think you should make it "ready for review" and see if any one else has comments.

I removed the "README.md" changes from the branch "migration-to-psa-crypto"

pabuhler

Some small clean up and some unanswered questions.
Please ensure you understand the PSA API properly, this review focused mostly on the code and not the usage of the PSA API.

pabuhler · 2026-02-04T20:02:08Z

crypto/cipher/aes_icm_mbedtls.c

    }
-
-    *dst_len = src_len;
+    total_len += out_len;


total_len and out_len are probably not needed, could just pass dst_len psa_cipher_update, this does not make much difference but cleans the code up, at least total_len could be removed

crypto/include/aes_gcm.h

pabuhler · 2026-02-04T20:06:04Z

crypto/include/aes_gcm.h


-#ifdef MBEDTLS
+#ifdef PSA
+// #ifdef MBEDTLS


what should I continue with for naming?
MBEDTLS or PSA?

I think MBEDTLS might make more sense but I do not mind.

pabuhler · 2026-02-04T20:08:13Z

crypto/hash/hmac_mbedtls.c

+        return status;
    }

+    /*Note: I don't know when to distroy a key*/


is this a problem ?

pabuhler · 2026-02-04T20:09:06Z

crypto/cipher/aes_icm_mbedtls.c

 *
 * Parameters:
- *	c	Crypto context
+ *	c	Crypto contextsrtp_aes_icm_mbedtls_encrypt


crypto/include/aes_gcm.h

seyednasermoravej · 2026-02-05T05:16:33Z

Some small clean up and some unanswered questions. Please ensure you understand the PSA API properly, this review focused mostly on the code and not the usage of the PSA API.

I'd added some comments to make the code easier for review. like:
/* There is no need for mbedtls_md_free function as it done automatically by psa_mac_sign_finish function */ // mbedtls_md_free(hmac_ctx);
or
/* Please note that the two funcitons mbedtls_md_setup and mbedtls_md_hmac_starts are combined into a single function psa_mac_sign_setup Legacy: Call mbedtls_md_setup to select the hash algorithm, with hmac=1. Then call mbedtls_md_hmac_starts to set the key. PSA API: Call psa_mac_sign_setup to specify the algorithm and the key. See “MAC key management” for how to obtain a key identifier. */

should I remove these comments too?

pabuhler · 2026-02-05T07:16:18Z

Some small clean up and some unanswered questions. Please ensure you understand the PSA API properly, this review focused mostly on the code and not the usage of the PSA API.

I'd added some comments to make the code easier for review. like: /* There is no need for mbedtls_md_free function as it done automatically by psa_mac_sign_finish function */ // mbedtls_md_free(hmac_ctx); or /* Please note that the two funcitons mbedtls_md_setup and mbedtls_md_hmac_starts are combined into a single function psa_mac_sign_setup Legacy: Call mbedtls_md_setup to select the hash algorithm, with hmac=1. Then call mbedtls_md_hmac_starts to set the key. PSA API: Call psa_mac_sign_setup to specify the algorithm and the key. See “MAC key management” for how to obtain a key identifier. */
should I remove these comments too?

It's abit up to you, comments are fine if they are concise or explain something that otherwise does not make a lot of sense. No need to comment on "normal" behavior of API usage.

seyednasermoravej force-pushed the migration-to-psa-crypto branch 3 times, most recently from 1791e08 to 40c43bf Compare January 7, 2026 15:50

seyednasermoravej force-pushed the migration-to-psa-crypto branch from e9c82bb to 6c4dadb Compare January 28, 2026 04:35

seyednasermoravej and others added 4 commits January 29, 2026 00:19

mbedtls files changed and cmake instruction added to support MbedTLS …

5dcb624

…V4.0

Use MbedTLS 4 on Ubuntu

c157358

Fix mbedtls aes gcm

2bf6953

Pass dst_len as it is the actual buffer size. Remove unused code, full_tag & ciphertext_len.

seyednasermoravej force-pushed the migration-to-psa-crypto branch from 6c4dadb to 2bf6953 Compare January 28, 2026 20:57

seyednasermoravej marked this pull request as ready for review January 28, 2026 20:58

pabuhler requested changes Feb 4, 2026

View reviewed changes

Conversation

seyednasermoravej commented Dec 25, 2025

Uh oh!

seyednasermoravej commented Jan 7, 2026

Uh oh!

pabuhler commented Jan 7, 2026

Uh oh!

seyednasermoravej commented Jan 8, 2026

Uh oh!

pabuhler commented Jan 10, 2026

Uh oh!

seyednasermoravej commented Jan 28, 2026

Uh oh!

pabuhler commented Jan 28, 2026

Uh oh!

seyednasermoravej commented Jan 28, 2026

Uh oh!

pabuhler left a comment

Choose a reason for hiding this comment

Uh oh!

pabuhler Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pabuhler Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

seyednasermoravej Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

pabuhler Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

pabuhler Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

pabuhler Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seyednasermoravej commented Feb 5, 2026

Uh oh!

pabuhler commented Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants