search

cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.21k stars 474 forks source link

Mbedtls support #512

Closed ycyang1229 closed 3 years ago

ycyang1229 commented 3 years ago

Recently focusing on the implementation of webrtc on rtos-based embedded devices, so adding the support of mbedtls. Hope this can help someone using this lib with mbedtls. thanks.

fluffy commented 3 years ago

I'd like to add the MBed support but I want to make sure we get a few people to review this carefully.

ycyang1229 commented 3 years ago

I'd like to add the MBed support but I want to make sure we get a few people to review this carefully.

Thanks a lot. It is nice to have someone to review this work.

ycyang1229 commented 3 years ago

As far as I can tell this is ok. I have done testing and it works on Ubuntu for me. The majority of the change is the same as the openssl/nss integration. As a follow up note we should look at converting more tests to the cmake build, supporting vagrind in travis for cmake and finally move the test vectors in to a common file that can be shared between back end implementations.

Cool, if anything I can help, please let me know. big thanks.

hannestschofenig commented 3 years ago

Thanks for adding Mbed TLS support to libsrtp!

I took a look at the code, as requested by Richard. The Mbed TLS related code looks good to me. I cannot comment on the requirements for tests, documentation and coding style.

(Note that we have been working on a new API, called the PSA Crypto API, which allows hardware crypto to be used conveniently. I understand that you are using our current API instead but in the future there may be benefit to also take a look at the new API. You can find the details of the PSA Crypto API at https://armmbed.github.io/mbed-crypto/PSA_Cryptography_API_Specification.pdf and I have placed examples at https://github.com/ARMmbed/mbedtls/pull/3833)

Just for my understanding: Are you expecting this code to be used in combination with the Mbed TLS code (with MBEDTLS_SSL_DTLS_SRTP enabled) so that the key exchange is done based on the code in https://github.com/ARMmbed/mbedtls/ and then for SRTP you are using the code from this library?

ycyang1229 commented 3 years ago

Thanks for adding Mbed TLS support to libsrtp!

I took a look at the code, as requested by Richard. The Mbed TLS related code looks good to me. I cannot comment on the requirements for tests, documentation and coding style.

(Note that we have been working on a new API, called the PSA Crypto API, which allows hardware crypto to be used conveniently. I understand that you are using our current API instead but in the future there may be benefit to also take a look at the new API. You can find the details of the PSA Crypto API at https://armmbed.github.io/mbed-crypto/PSA_Cryptography_API_Specification.pdf and I have placed examples at ARMmbed/mbedtls#3833)

Just for my understanding: Are you expecting this code to be used in combination with the Mbed TLS code (with MBEDTLS_SSL_DTLS_SRTP enabled) so that the key exchange is done based on the code in https://github.com/ARMmbed/mbedtls/ and then for SRTP you are using the code from this library?

Sure, I would love to take a look, and it is my plesaure. Yes, I took this pr(https://github.com/ARMmbed/mbedtls/pull/3235/files) in the beginning. Big thanks.

pabuhler commented 3 years ago

@ycyang1229 the failed build is related to some meson issues in travis that started yesterday, so I think you just need to ignore that for now.

ycyang1229 commented 3 years ago

@ycyang1229 the failed build is related to some meson issues in travis that started yesterday, so I think you just need to ignore that for now.

Thanks for immediate response. I just tried to see why. :D