8000 hmac,md5,sha: add mbedtls backend by cspiel1 · Pull Request #871 · baresip/re · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

hmac,md5,sha: add mbedtls backend #871

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jul 12, 2023
Merged

hmac,md5,sha: add mbedtls backend #871

merged 4 commits into from
Jul 12, 2023

Conversation

cspiel1
Copy link
Collaborator
@cspiel1 cspiel1 commented Jul 7, 2023 •
edited
Loading

We are evaluating re/baresip on Cortex M7/M4 with Zephyr.

First step is to build re without TLS support. But at least the hash functions needs an MbedTLS backend.

Build with:

cd build
cmake -DUSE_OPENSSL=no ..
make

@cspiel1 cspiel1 marked this pull request as ready for review July 7, 2023 11:06
@cspiel1 cspiel1 mentioned this pull request Jul 7, 2023
Merged
@sreimers
Copy link
Member
sreimers commented Jul 7, 2023

You have to include a FindMbedTLS.cmake file (its not included with default cmake):

CMake Warning at cmake/re-config.cmake:9 (find_package):
  By not providing "FindMbedTLS.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "MbedTLS", but
  CMake did not find one.

  Could not find a package configuration file provided by "MbedTLS" with any
  of the following names:

    MbedTLSConfig.cmake
    mbedtls-config.cmake

  Add the installation prefix of "MbedTLS" to CMAKE_PREFIX_PATH or set
  "MbedTLS_DIR" to a directory containing one of the above files.  If
  "MbedTLS" provides a separate development package or SDK, be sure it has
  been installed.
Call Stack (most recent call first):
  CMakeLists.txt:96 (find_package)

@sreimers
Copy link
Member
sreimers commented Jul 7, 2023

We had similar request here (not merged):

#494

alfredh
alfredh reviewed Jul 8, 2023
View reviewed changes
src/sha/wrap.c
#elif defi 8000 ned (MBEDTLS_MD_C)
int err;

err = mbedtls_sha1(d, n, md);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int err can be declared and assigned in the same operation.

@alfredh
Copy link
Contributor
alfredh commented Jul 8, 2023

https://github.com/curl/curl/blob/master/CMake/FindMbedTLS.cmake

@cspiel1
Copy link
Collaborator Author
cspiel1 commented Jul 10, 2023 •
edited
Loading

You have to include a FindMbedTLS.cmake file (its not included with default cmake):

CMake Warning at cmake/re-config.cmake:9 (find_package):
  By not providing "FindMbedTLS.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "MbedTLS", but
  CMake did not find one.

  Could not find a package configuration file provided by "MbedTLS" with any
  of the following names:

    MbedTLSConfig.cmake
    mbedtls-config.cmake

  Add the installation prefix of "MbedTLS" to CMAKE_PREFIX_PATH or set
  "MbedTLS_DIR" to a directory containing one of the above files.  If
  "MbedTLS" provides a separate development package or SDK, be sure it has
  been installed.
Call Stack (most recent call first):
  CMakeLists.txt:96 (find_package)

You mean for building baresip? I added a FindMBEDTLS.cmake and put it to the install files of re. But baresip still has this warning

$ ls /usr/local/lib/cmake/re
FindMBEDTLS.cmake  re-config.cmak

~/src/baresip/build (main)
$ cmake -DUSE_OPENSSL=no ..
CMake Warning at /home/cspiel/src/re/cmake/re-config.cmake:9 (find_package):
  By not providing "FindMBEDTLS.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "MBEDTLS", but
  CMake did not find one.

What is missing?

@sreimers
Copy link
Member

What is missing?

re-config.cmake is for shared options, if its only libre related, find_package has to be placed in CMakeLists.txt.

@cspiel1
Copy link
Collaborator Author
cspiel1 commented Jul 10, 2023

What is missing?

re-config.cmake is for shared options, if its only libre related, find_package has to be placed in CMakeLists.txt.

Ah, okay. In this case mbedTLS is a static library. Linking to libre.so/libre.a should be enough. I hope, that I am thinking right now.

@cspiel1
Copy link
Collaborator Author
cspiel1 commented Jul 10, 2023

Linking libmbedtls.a to libre.so is not possible. So we have to build libre.a only. Right?
Does this imply that we are bound to libbaresip.a and baresip built with STATIC=1?

@cspiel1
Copy link
Collaborator Author
cspiel1 commented Jul 10, 2023

Linking libmbedtls.a to libre.so is not possible. So we have to build libre.a only. Right? Does this imply that we are bound to libbaresip.a and baresip built with STATIC=1?

I think so. Thus find_package(MBEDTLS) stays in re-config.cmake and we need also a baresip PR: baresip/baresip#2646

And also a stub for re module tls. A commit follows.

@cspiel1
Copy link
Collaborator Author
cspiel1 commented Jul 10, 2023 •
edited
Loading

Pre-condition: have built and installed mbedtls. E.g. I have

/usr/local/lib/libmbedcrypto.a
/usr/local/lib/libmbedtls.a
/usr/local/lib/libmbedx509.a

Build re:

cmake .. -DUSE_OPENSSL=no

Build the app, e.g. baresip:

cmake -DUSE_OPENSSL=no -DSTATIC=yes ..

@sreimers Is this the right way now? Are the assumptions in #871 (comment) correct?

edit: There is another important question in baresip/baresip#2646 (comment) .

@sreimers
Copy link
Member

I think its better to use cmake -B build -DUSE_MBEDTLS=yes. This way a application has not to provide FindMBEDTLS.cmake if re-config is loaded and not needed:

if(USE_MBEDTLS)
  find_package(MBEDTLS)
else()
  find_package(OpenSSL "1.1.1")
endif()

@alfredh
Copy link
Contributor
alfredh commented Jul 11, 2023

Thanks for writing this patch!

The code looks fine to me, please merge to main if okay.

@cspiel1
Copy link
Collaborator Author
cspiel1 commented Jul 11, 2023

Your welcome! I think @sreimers should merge if the cmake looks good to him.

@sreimers sreimers merged commit 7a8a3a9 into baresip:main Jul 12, 2023
@sreimers
Copy link
Member

Thanks!

@cspiel1 cspiel1 deleted the mbedtls_backend branch July 13, 2023 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alfredh alfredh alfredh left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cspiel1 @sreimers @alfredh
0