-
Notifications
You must be signed in to change notification settings - Fork 374
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.
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
Integration of Mbed TLS for lwm2mclient example #626
Conversation
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 LukasKarel#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. Looking at Due Diligence for Prerequisites :
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. |
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. |
+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? |
Yes. @waynebeaton thx a lot 🙏 ! So I think team should chose the version and use the corresponding commit id in this PR. |
IMHO it would make sense to use the latest version of Mbed TLS, which is version 3.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.) |
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. |
18bda34
to
724efd9
Compare
@hannestschofenig As I am not a committer I cant create a CQ, I guess. I did not find anything according to @sbernard31 links. @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: |
724efd9
to
3b62d3d
Compare
dda0612
to
3121d4b
Compare
@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! |
refactor connections for examples
3121d4b
to
c525638
Compare
Should be changed now. I hope I did it correctly. |
e30ca0c
to
4f29c3f
Compare
fixed the build process according to comments on PR
4f29c3f
to
17f53b5
Compare
@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). |
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' Environment: Thank you |
Maybe I have some time on the weekend to setup a virtual machine and I will try to reproduce. |
I cant reproduce. I used Ubuntu 20.04.3 LTS and same gcc/cmake versions.
target_link_libraries(${PROJECT_NAME}
PRIVATE
mbedtls
Threads::Threads
) instead of target_link_libraries(${PROJECT_NAME}
PRIVATE
mbedtls
)
|
Hi @LukasKarel . I couldn't answer you before, sorry. Thank you for your response.
-- Configuring done -- Generating done
What steps am I doing wrong? Thank you very much for your help |
@juananldt okay I think I have a solution. 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. |
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). |
This is a preparation for eclipse-wakaamaGH-626.
This is a preparation for eclipse-wakaamaGH-626.
This is a preparation for eclipse-wakaamaGH-626.
This is a preparation for eclipse-wakaamaGH-626.
This is a preparation for eclipse-wakaamaGH-626.
This is (also) a preparation for eclipse-wakaamaGH-626.
This is (also) a preparation for eclipse-wakaamaGH-626.
I just created a CQ for Mbed TLS 3.1.0: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=23927 |
This work is based on LukasKarel work eclipse-wakaama/wakaama#626 This commit contains a new structure for the connection layer which is more flexible and decouples layers in a better way. DTLS is not working properly. For now, messages can be sent, but packet decryption on client (maybe also on server) fails.
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.