Integration of Mbed TLS for lwm2mclient example
As mentioned in Issue #623 I can provide a sample implementation with mbedtls. Please remark this implementation was done quick and dirty with some files I had around. It was not implemented to be merged on master! I used the networklayer (mbedtls_net_context) shipped with mbedtls. Currently only PSK mode is supported. I tested it with the leshan server.
This implementation is not secure and I am sure it does not compile with tinydtls anymore.
I hope it works without greater problems. Otherwise feel free to ask me anything about it.
Thank you for providing this!
Thanks for doing this work. This is a great start.
I tried the code and it works nicely. I have a few change suggestions and I will create a PR.
In terms of scope, you have currently focused on adding support for Mbed TLS to the LwM2M Client example. The LwM2M-Boostrap Server example and the LwM2M Server example do not support DTLS (neither TinyDTLS nor Mbed TLS). I suspect that this is the intention; it is certainly good for me. Just wanted to verify whether I understood it correctly.
I created a PR with some minor changes, see https://github.com/LukasKarel/wakaama/pull/3
@sbernard31 from a legal process point of view we need to do something special to include an mbedtls submodule?
Looking at Eclipse IP policy. I guess we can consider a git submodule as a "Prerequisites" ? :thinking:
Looking at Due Diligence for Prerequisites :
- :heavy_check_mark: The licence matches as I understand that Apache v2.0 is used for mbedTLS
- :question: I'm not sure if the nature of the license can be considered as verified by a trusted source. I found ClearlyDefined score is not OK but there is a CQ in IPZilla about mbedTLS v2.24.0 (this is a workswith not a prereq)
I guess we need some help ... I will try to get help from EMO IP Team.
I'm not sure but I guess we will need to clarify which version of mbedTLS will be used.
In terms of scope, you have currently focused on adding support for Mbed TLS to the LwM2M Client example. The LwM2M-Boostrap Server example and the LwM2M Server example do not support DTLS (neither TinyDTLS nor Mbed TLS). I suspect that this is the intention; it is certainly good for me. Just wanted to verify whether I understood it correctly.
Maybe I could also add an implementation for the LwM2M Server example. I tried it once, but I am not sure if I have the files anymore. I did only implement the client example as it is the only example working with DTLS.
I guess we can consider a git submodule as a "Prerequisites" ?
+1
Including third party content as a submodule certainly sounds like a prerequisite relationship.
The challenge with submodules is that the notion of version is harder to keep track of than content consumed in some sort of distribution format from a software repository. The IP approval process is version specific, and it's not clear to me whether or not the version of the ClearlyDefined record to which you've linked matches the version of the particular branch/tag/commit of the repository that you're including as a submodule. So, it's not clear whether or not that ClearlyDefined record even applies to the content that you're referencing.
Again, Git submodules are a bit of a nightmare from an intellectual property management point of view.
Regardless, the path forward here is to engage with the IP Team by creating a CQ for the third party content like you would have done anyway citing the particular tag/commit that you're including as a submodule. Then attach a snapshot of the repository at that particular tag/commit.
From here, we need to treat the submodule like any other third party dependency. The submodule pointer can just be updated to a commit/tag that can be described as a "service release" of the vetted content. Any update to a commit/tag that that can be described as a major/minor revision requires re-engage with the IP Team for formal vetting.
FWIW, since the content that you're including provides cryptography, we need a CQ to track that anyway (we have obligations to various governments to report the export of cryptography). Be sure to indicate that the content "includes cryptography" when you create the CQ.
Does this make sense?
Does this make sense?
Yes. @waynebeaton thx a lot :pray: !
So I think team should chose the version and use the corresponding commit id in this PR. Then a committer must created the CQ following : https://www.eclipse.org/projects/handbook/#pmi-commands-cq at https://projects.eclipse.org/projects/iot.wakaama. (I can provide help/support if needed)
IMHO it would make sense to use the latest version of Mbed TLS, which is version 3.0.0: https://github.com/ARMmbed/mbedtls/releases/tag/v3.0.0
@LukasKarel Are you going to take the steps suggested by @sbernard31 ? Do you agree that using the latest Mbed TLS version is good? (My main reason for including the latest version is to have the latest bugfixes included and to also support the most recent feature list, including the CID functionality and the PSA Crypto API support.)
@LukasKarel Are you going to take the steps suggested by @sbernard31 ? Do you agree that using the latest Mbed TLS version is good? (My main reason for including the latest version is to have the latest bugfixes included and to also support the most recent feature list, including the CID functionality and the PSA Crypto API support.)
I agree with you. Its the best idea to use the latest version. And I can take the steps to get the permission to use mbedtls in wakaama. But I will refactor some source code and try to create a better interface in the examples to support different connection types.
Regarding supporting other CoAP libraries and other tranports than CoAP: I am wondering whether this would be scope of a different PR since the use of Mbed TLS (instead of TinyDTLS), as explored with this PR, happens at a very different layer and should be fairly independent.
@hannestschofenig As I am not a committer I cant create a CQ, I guess. I did not find anything according to @sbernard31 links. Is there anyone on the team who will do it?
@hannestschofenig To be able to merge the changes with your commit in the history you have to sign the ECA of eclipse. Otherwise I have to squash the commits and you will not be part of the contribution.
@wakaama team: I have refactored some code. Please review it.
@LukasKarel Thanks for the work and sorry for the late answer.
The last update contains a lot of cosmetic changes, which makes it hard to review the patch. Please do not let clang-format run on existing code (or do this in a separate PR - see discussion in #574).
Can you please remove them from the commit so that the commit/PR contains only functional changes? In case this causes a lot of work for you, I am willing to give this a shot myself.
@LukasKarel Thanks for the work and sorry for the late answer.
The last update contains a lot of cosmetic changes, which makes it hard to review the patch. Please do not let clang-format run on existing code (or do this in a separate PR - see discussion in #574).
Can you please remove them from the commit so that the commit/PR contains only functional changes? In case this causes a lot of work for you, I am willing to give this a shot myself.
It will be some work, but maybe I have some spare time over the weekend to do it!
@LukasKarel Thanks for the work and sorry for the late answer.
The last update contains a lot of cosmetic changes, which makes it hard to review the patch. Please do not let clang-format run on existing code (or do this in a separate PR - see discussion in #574).
Can you please remove them from the commit so that the commit/PR contains only functional changes? In case this causes a lot of work for you, I am willing to give this a shot myself.
Should be changed now. I hope I did it correctly.
Surely looks much better, easier to understand now. @mlasch and/or me intend to give feedback in the coming 1-2 weeks (it is quite a big PR and I am not yet sure how to handle it).
@rettichschnidi and me just discussed this a bit. We can see some nice improvements over the old code. Thanks! The existing code base is still missing some important test coverage, for new code we think that 1) it should be ensured that the code builds (in CI) and 2) that there is at least some kind of testing. Regarding 1, we realized that DTLS binaries are not yet getting built in CI. We want to improve this first. For 2, the cheapest smoke test we could think of is ensuring that a connection to leshan.eclipseprojects.io (or better, a local instance) can be established. We will try to come up with a PR for this issue too.
Questions/Notes:
- Do you intend to push this PR in a state to get it merged into master (referring to your first comment)?
- There are still some (purely) cosmetic changes e.g. in lwm2mclient.c
I will work on the comments of @rettichschnidi tomorrow.
@rettichschnidi and me just discussed this a bit. We can see some nice improvements over the old code. Thanks! The existing code base is still missing some important test coverage, for new code we think that 1) it should be ensured that the code builds (in CI) and 2) that there is at least some kind of testing. Regarding 1, we realized that DTLS binaries are not yet getting built in CI. We want to improve this first. For 2, the cheapest smoke test we could think of is ensuring that a connection to leshan.eclipseprojects.io (or better, a local instance) can be established. We will try to come up with a PR for this issue too.
Questions/Notes:
- Do you intend to push this PR in a state to get it merged into master (referring to your first comment)?
- There are still some (purely) cosmetic changes e.g. in lwm2mclient.c
- I can do that. That first comment was only because I wanted some feedback of @hannestschofenig and some contributers before completing implementation. And i wasnt sure if I have enough time. But this is not a problem anymore.
- I will look at this also tomorrow.
Is there anything I can do, to help you with the CI?
@rettichschnidi I refactored some CMake code. Maybe this works for you.
@mlash I think I removed all cosmetic changes now.
@LukasKarel Will spend time on this on Friday(s). Edit: Did not find the time to do it today, sorry.
Hi @LukasKarel. Thank you for your contribution!
I'm building with mbedtls but I faced the following error during build on Linux:
/usr/bin/ld: CMakeFiles/lwm2mclient.dir/home/juanan/git/wakaama/examples/shared/mbedtlsconnection.c.o: undefined reference to symbol 'mbedtls_entropy_func' /usr/bin/ld: /lib/x86_64-linux-gnu/libmbedcrypto.so.3: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status make[2]: *** [CMakeFiles/lwm2mclient.dir/build.make:609: lwm2mclient] Error 1 make[1]: *** [CMakeFiles/Makefile2:76: CMakeFiles/lwm2mclient.dir/all] Error 2 make: *** [Makefile:84: all] Error 2
Environment: gcc (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0 cmake version 3.16.3
Thank you
Hi @LukasKarel. Thank you for your contribution!
I'm building with mbedtls but I faced the following error during build on Linux:
/usr/bin/ld: CMakeFiles/lwm2mclient.dir/home/juanan/git/wakaama/examples/shared/mbedtlsconnection.c.o: undefined reference to symbol 'mbedtls_entropy_func' /usr/bin/ld: /lib/x86_64-linux-gnu/libmbedcrypto.so.3: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status make[2]: *** [CMakeFiles/lwm2mclient.dir/build.make:609: lwm2mclient] Error 1 make[1]: *** [CMakeFiles/Makefile2:76: CMakeFiles/lwm2mclient.dir/all] Error 2 make: *** [Makefile:84: all] Error 2
Environment: gcc (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0 cmake version 3.16.3
Thank you
Maybe I have some time on the weekend to setup a virtual machine and I will try to reproduce.
Hi @LukasKarel. Thank you for your contribution!
I'm building with mbedtls but I faced the following error during build on Linux:
/usr/bin/ld: CMakeFiles/lwm2mclient.dir/home/juanan/git/wakaama/examples/shared/mbedtlsconnection.c.o: undefined reference to symbol 'mbedtls_entropy_func' /usr/bin/ld: /lib/x86_64-linux-gnu/libmbedcrypto.so.3: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status make[2]: *** [CMakeFiles/lwm2mclient.dir/build.make:609: lwm2mclient] Error 1 make[1]: *** [CMakeFiles/Makefile2:76: CMakeFiles/lwm2mclient.dir/all] Error 2 make: *** [Makefile:84: all] Error 2
Environment: gcc (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0 cmake version 3.16.3
Thank you
I cant reproduce. I used Ubuntu 20.04.3 LTS and same gcc/cmake versions. I researched a bit:
- Could you tell me which glibc version do you have installed? run
ldd --version - Are you sure you haven't modified the code? Maybe try to add find_package(Threads) in the root CMakeLists.txt file after the project command, and change examples/client/CMakeLists.txt to
target_link_libraries(${PROJECT_NAME}
PRIVATE
mbedtls
Threads::Threads
)
instead of
target_link_libraries(${PROJECT_NAME}
PRIVATE
mbedtls
)
- Do you use the correct version of mbedtls? Or do you have a another version of mbedtls installed? as your linker reports
/lib/x86_64-linux-gnu/libmbedcrypto.so.3which seems not to be the library, which is built with wakaama.
Hi @LukasKarel. Thank you for your contribution! I'm building with mbedtls but I faced the following error during build on Linux: /usr/bin/ld: CMakeFiles/lwm2mclient.dir/home/juanan/git/wakaama/examples/shared/mbedtlsconnection.c.o: undefined reference to symbol 'mbedtls_entropy_func' /usr/bin/ld: /lib/x86_64-linux-gnu/libmbedcrypto.so.3: error adding symbols: DSO missing from command line collect2: error: ld returned 1 exit status make[2]: *** [CMakeFiles/lwm2mclient.dir/build.make:609: lwm2mclient] Error 1 make[1]: *** [CMakeFiles/Makefile2:76: CMakeFiles/lwm2mclient.dir/all] Error 2 make: *** [Makefile:84: all] Error 2 Environment: gcc (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0 cmake version 3.16.3 Thank you
I cant reproduce. I used Ubuntu 20.04.3 LTS and same gcc/cmake versions. I researched a bit:
- Could you tell me which glibc version do you have installed? run
ldd --version- Are you sure you haven't modified the code? Maybe try to add find_package(Threads) in the root CMakeLists.txt file after the project command, and change examples/client/CMakeLists.txt to
target_link_libraries(${PROJECT_NAME} PRIVATE mbedtls Threads::Threads )instead of
target_link_libraries(${PROJECT_NAME} PRIVATE mbedtls )
- Do you use the correct version of mbedtls? Or do you have a another version of mbedtls installed? as your linker reports
/lib/x86_64-linux-gnu/libmbedcrypto.so.3which seems not to be the library, which is built with wakaama.
Hi @LukasKarel . I couldn't answer you before, sorry. Thank you for your response.
- My glibc version is: ldd (Ubuntu GLIBC 2.31-0ubuntu9.2) 2.31
- I haven't modified the code. I'm sure. I tried to add find_package(Threads) in the root CMakeLists.txt file after the project command, and change examples/client/CMakeLists.txt, but it gave me this error
-- Configuring done CMake Error at CMakeLists.txt:40 (add_executable): Target "lwm2mclient" links to target "Threads::Threads" but the target was not found. Perhaps a find_package() call is missing for an IMPORTED target, or an ALIAS target is missing?
-- Generating done CMake Generate step failed. Build files cannot be regenerated correctly. make: *** [Makefile:1226: cmake_check_build_system] Error 1
- I believe that the error is in the use the correct version of mbedtls. I think I'm doing this wrong. My steps are:
- Install libmbedtls with commad: sudo apt install libmbedtls-dev.
- Download your wakaama project and change to branch draft/mbedtls.
- I follow the steps for client execution: - apt install build-essential clang-format clang-format-10 clang-tools-10 cmake gcovr git libcunit1-dev ninja-build python3-pip - pip3 install gitlint - git submodule init - git submodule update - Create a build directory and change to that. - cmake -DDTLS_MBEDTLS=1 [wakaama directory]/examples/client - make --> The error appears here: /usr/bin/ld: CMakeFiles/lwm2mclient.dir/home/juanan/git/wakaama/examples/shared/mbedtlsconnection.c.o: undefined reference to symbol 'mbedtls_entropy_func' /usr/bin/ld: /lib/x86_64-linux-gnu/libmbedcrypto.so.3: error al añadir símbolos: DSO missing from command line collect2: error: ld returned 1 exit status
What steps am I doing wrong?
Thank you very much for your help
@juananldt okay I think I have a solution. You do not have to install libmbedtls-dev. The correct version of mbedtls is checked out as submodule and is built from source. Submodules must be checked out (git submodule init & git submodule update). I have updated the README.md. The build steps have changed. Use following steps for building.
cmake -DDTLS_MBEDTLS=1 [wakaama directory]
make lwm2mclient
As you installed ninja-build, you could also use
cmake -DDTLS_MBEDTLS=1 -GNinja [wakaama directory]
ninja lwm2mclient
which will speed up building.
Please let me know if this worked.
@juananldt okay I think I have a solution. You do not have to install libmbedtls-dev. The correct version of mbedtls is checked out as submodule and is built from source. Submodules must be checked out (git submodule init & git submodule update). I have updated the README.md. The build steps have changed. Use following steps for building.
cmake -DDTLS_MBEDTLS=1 [wakaama directory] make lwm2mclientAs you installed ninja-build, you could also use
cmake -DDTLS_MBEDTLS=1 -GNinja [wakaama directory] ninja lwm2mclientwhich will speed up building.
Please let me know if this worked.
Hi @LukasKarel!!!!!
Yes, it's working now. Great job!!!!
Thank you very much!!!!
Sorry for my tardiness. Will try (once again/still) to spend time on this on Friday(s).
@hannestschofenig As I am not a committer I cant create a CQ, I guess. I did not find anything according to @sbernard31 links. Is there anyone on the team who will do it?
I just created a CQ for Mbed TLS 3.1.0: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=23927