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

Correct me if I'm wrong. As we need to set IV, we are using multipart symmetric encryption but we are doing the encryption just one time (we called psa_cipher_update once). That means in our case the total_len is equal to out_len. But the variable dst_len is the size of dst place holder. That's why I'm not sure if out_len is equal to dst_len or not. But for sure we can remove the total_len variable.

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.

crypto/hash/hmac_mbedtls.c

crypto/cipher/aes_icm_mbedtls.c

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.

crypto/cipher/aes_icm_mbedtls.c

seyednasermoravej · 2026-02-13T07:20:50Z

As the picture imply, there is no newline at the end of file! How can run CI myself? @pabuhler

pabuhler · 2026-02-13T07:28:12Z

the error is that there is no new line
"error: no newline at end of file"
I normally run the format.sh script locally it will fix up the files, it requires clang-format-14.
Worst case I can run that script and push the changes.

seyednasermoravej · 2026-02-13T07:34:58Z

the error is that there is no new line "error: no newline at end of file"

The opposite of Zephyr RTOS rule :)

seyednasermoravej · 2026-02-13T07:43:35Z

I normally run the format.sh script locally it will fix up the files, it requires clang-format-14. Worst case I can run that script and push the changes.

I did that but nothing happens

pabuhler · 2026-02-13T08:30:07Z

You are right, need clang-format 17.
Thanks for updating.

seyednasermoravej · 2026-02-13T08:36:29Z

You are right, need clang-format 17. Thanks for updating.

I think I have it. At least I didn't face any not found error when I try it in my terminal.
You're welcome 🤗

pabuhler

Look basically fine and test pass. So I am Ok with merging this, then hopefully it will get some run time.

pabuhler · 2026-02-15T20:37:07Z

crypto/cipher/aes_icm_mbedtls.c

    uint32_t key_size_in_bits = (c->key_size << 3);
-    int errcode = 0;
+
+    psa_status_t status = psa_crypto_init();


This seams a bit random, but ok.

Do you prefer psa_status_t status = PSA_SUCCESS; and then status = psa_crypto_init();?

Everywhere `psa_status_t status = PSA_SUCCESS;` but in only one place `psa_status_t status = psa_crypto_init();`, I changed the code to make it the same everywhere.

I was more asking about te placement of the call to psa_crypto_init(), but I guess this will ensure it is always called.

Ah ok, I had two choices to place psa_crypto_init();, in the srtp_aes_icm_mbedtls_context_init or srtp_aes_icm_mbedtls_alloc. I think it should be placed in the init function.

seyednasermoravej · 2026-02-17T10:23:54Z

After your approval, how long does it take to be merged in the main branch?
@pabuhler

pabuhler · 2026-02-17T10:30:40Z

After your approval, how long does it take to be merged in the main branch? @pabuhler

some time in the next few days, just in case there was any other interest.

seyednasermoravej · 2026-02-17T10:48:10Z

I don't know why the code formatting check failed. There is no explanation message for that.

Signed-off-by: Sayed Naser Moravej <seyednasermoravej@gmail.com>

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

seyednasermoravej requested a review from pabuhler February 12, 2026 17:36

seyednasermoravej commented Feb 12, 2026

View reviewed changes

crypto/cipher/aes_icm_mbedtls.c Show resolved Hide resolved

seyednasermoravej force-pushed the migration-to-psa-crypto branch 3 times, most recently from 3bd6d22 to 2b5679f Compare February 13, 2026 04:26

seyednasermoravej force-pushed the migration-to-psa-crypto branch from 2b5679f to 23cfeeb Compare February 13, 2026 07:40

pabuhler approved these changes Feb 15, 2026

View reviewed changes

seyednasermoravej force-pushed the migration-to-psa-crypto branch from 23cfeeb to 4944c2e Compare February 16, 2026 08:05

seyednasermoravej requested a review from pabuhler February 17, 2026 10:24

some cleaning, adding psa_destryoy and abort functions.

260cc0b

Signed-off-by: Sayed Naser Moravej <seyednasermoravej@gmail.com>

seyednasermoravej force-pushed the migration-to-psa-crypto branch from 492f631 to 260cc0b Compare February 17, 2026 11:35

pabuhler merged commit 49e0c07 into cisco:main Feb 19, 2026
45 checks passed

pabuhler mentioned this pull request Feb 19, 2026

Upgrade to Mbed TLS PSA API #517

Closed

Comments

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!

Choose a reason for hiding this comment

Uh oh!

seyednasermoravej Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seyednasermoravej commented Feb 5, 2026

Uh oh!

pabuhler commented Feb 5, 2026

Uh oh!

Uh oh!

seyednasermoravej commented Feb 13, 2026

Uh oh!

pabuhler commented Feb 13, 2026

Uh oh!

seyednasermoravej commented Feb 13, 2026

Uh oh!

seyednasermoravej commented Feb 13, 2026

Uh oh!

pabuhler commented Feb 13, 2026

Uh oh!

seyednasermoravej commented Feb 13, 2026

Uh oh!

pabuhler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seyednasermoravej commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pabuhler commented Feb 17, 2026

Uh oh!

seyednasermoravej commented Feb 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

seyednasermoravej Feb 12, 2026 •

edited

Loading

seyednasermoravej commented Feb 17, 2026 •

edited

Loading