Skip to content
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

Platform specific support for random number generation #313

Closed
jitin17 opened this issue Mar 15, 2019 · 14 comments
Closed

Platform specific support for random number generation #313

jitin17 opened this issue Mar 15, 2019 · 14 comments

Comments

@jitin17
Copy link

jitin17 commented Mar 15, 2019

Currently, I am adding support for tinydtls in libcoap in ESP_IDF. I don't see platform specific layer to add support for random number generation, as ESP_IDF uses an internal API to do so.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Mar 15, 2019

@jitin17 How are you planning to add in tinydtls?
My plan was to make tinydtls a component within esp-idf and then change the file name coap-notls.[co] to the respective coap_coap_tinydtls.[co] in component.mk and CMakeLists.txt in components/coap/. Then the client / server need to have a couple of additional functions to set up a PSK / Identity/User.

In answer to your question, an updated prng.h in tindtls/port/include should be all that is needed to handle what you are trying to do.

@jitin17
Copy link
Author

jitin17 commented Mar 16, 2019

@mrdeep1 I suppose tinydtls is already a part of libcoap as a submodule, thus I believe there is no need to pull in tinydtls in ESP-IDF.

The problem I am facing is in dtls_new_context function in ext/tinydtls/dtls.c file. If flag WITH_POSIX is enabled then dtls_new_context uses the following to generate random number:

FILE *urandom = fopen("/dev/urandom", "r");
fread(buf, 1, sizeof(buf), urandom) != sizeof(buf)

This doesn't work in ESP-IDF, which provides esp_fill_random() API to do so.

If my understanding is correct, then to solve issues like these we need platform-dependent sub tree illustrated in this PR: #6.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Mar 16, 2019

Ah - I had not appreciated that the git clone --recursive also pulled in ext/tinydtls with the updated libcoap-4.2.0 branch. So, no need to do that separately. As tinydtls is a separate product from libcoap, I have not control over that, apart from creating a PR.

It may be that we just have to add a modified version of dtls.c to coap/port directory. I will take a look at this.

Certainly platform dependent trees may be the way to go - there are to many #ifdefs in the code as it is.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Mar 19, 2019

@jitin17 For TinyDTLS, this is a work in progress to create some more platform independent code.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Mar 21, 2019

@jitin17 The latest develop version of libcoap (641b5c1) has an updated version of tinydtls (9a3496119688047b6d8b79826f13425397d92e10) as a submodule which fixes this.

@jitin17
Copy link
Author

jitin17 commented Mar 27, 2019

This is great. But I think there is a need to include esp_system.h header in ext/tinydtls/platform-specific/dtls_prng_espidf.c.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Mar 27, 2019

You are correct. I discovered that when I went back to build things that were using all the latest code.

I am shortly (probably tomorrow morning if all goes well) about to push a PR request for https://github.com/mrdeep1/esp-idf/pull/new/libcoap-tinydtls which has DTLS enabled within libcoap. If you could test this code out, it would be great as I do not have a ESP-IDF test environment. You will need to make the URI for the coap_client something like coaps://127.0.0.1/Espressif to make it do DTLS.
[I have corrected dtls_prng_espidf.c as port/dtls_prng.c to make this build]

@jitin17
Copy link
Author

jitin17 commented Apr 8, 2019

@mrdeep1 Hey, any updates on raising a PR in esp-idf?

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Apr 8, 2019

@jitin17 When testing code (thanks @leenowell as I do not have a esp-idf test environment), it highlighted a couple of issues - the primary one being the excessive use of stack - in particular when logging is enabled - and there is a constrained stack with esp-idf. There was a secondary issue where "WARN dtls_send_to_peer: cannot find local interface" was getting reported.

The constrained stack and handling the consequences of fixing "WARN dtls_send_to_peer: cannot find local interface" have been fixed in tinydtls, and the code committed, but am waiting for libcoap to have the code version updated for the submodule tinydtls to get in the new changes.

There is a code fix for constrained stack in the libcoap code, but that has not been accepted yet.

When the git submodule tinydtls code version is updated and the constrained stack PR is accepted, then it makes sense to get this fixed code into esp-idf.

@obgm
Copy link
Owner

obgm commented Apr 8, 2019

Looking at the constrained stack PR right now, will do the tinydtls submodule update immediately...

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Apr 23, 2019

Code fix has been pushed to espressif/esp-idf#3345 . git clone -b libcoap-tinydtls --recursive https://github.com/mrdeep1/esp-idf.git if you want to try it out.

@jitin17 jitin17 closed this as completed May 15, 2019
@sajadghorbani
Copy link

sajadghorbani commented Jun 2, 2019

Code fix has been pushed to espressif/esp-idf#3345 . git clone -b libcoap-tinydtls --recursive https://github.com/mrdeep1/esp-idf.git if you want to try it out.

I tried to test your code, but unfortunately, I did not succeed.
out.log

@mrdeep1
Copy link
Collaborator

mrdeep1 commented Jun 3, 2019

@ghorbanis Can you please try again with a clean clone?
I found a minor potential issue.

@sajadghorbani
Copy link

@mrdeep1 Yes, It worked fine.
Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants