net/gcoap: Improve DTLS integration #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
_read in sock_dtls callback uses memcpy to copy decrypted message from (uint8_t *) data to sock->buf. The memory areas overlaps when the payload is large (>21 bytes). This fix uses memmove instead of memcpy which is safe for overlapping memory areas.
|
One more thing. There is an issue when attempting to send a message to the wrong port. The implementation hangs somewhere behind |
|
Nice job on the _tl_sock approach! I haven't tested this myself but I'll merge it in first because there are a lot of changes in the credential manager (tlsman replaced with credman) and sock_dtls since I've last looked at this branch. The changes here still applies and will be rebased on the new code later. |
|
Sounds good. Hopefully the approach will be acceptable for the main RIOT repository. |
pokgak
pushed a commit
that referenced
this pull request
Jul 19, 2019
The evtimer_msg test is expanded to also delete entries. Furthermore the messages that are printed should now show numbers that are very close (if not equal). Something like this: At 740 ms received msg 0: "RIOT-OS#2 supposed to be 740" At 1081 ms received msg 1: "#0 supposed to be 1081" At 1581 ms received msg 2: "#1 supposed to be 1581" At 4035 ms received msg 3: "RIOT-OS#3 supposed to be 4035" The function evtimer_print is also called to show the intermediate status of evtimer entries.
pokgak
pushed a commit
that referenced
this pull request
Sep 12, 2019
The test randomly fails on `native` due to timers being not accurate but
it cannot be otherwise. So better disable it than raising fake errors.
main(): This is RIOT! (Version: buildtest)
Testing generic evtimer
This should list 2 items
ev #1 offset=1000
ev RIOT-OS#2 offset=500
This should list 4 items
ev #1 offset=659
ev RIOT-OS#2 offset=341
ev RIOT-OS#3 offset=500
ev RIOT-OS#4 offset=2454
Are the reception times of all 4 msgs close to the supposed values?
At 662 ms received msg 0: "RIOT-OS#2 supposed to be 659"
At 1009 ms received msg 1: "#0 supposed to be 1000"
At 1511 ms received msg 2: "#1 supposed to be 1500"
Traceback (most recent call last):
File "/tmp/dwq.0.3125418833043728/ef3af88c4b3615788b164464a437df5c/tests/evtimer_msg/tests/01-run.py", line 33, in <module>
sys.exit(run(testfunc))
File "/tmp/dwq.0.3125418833043728/ef3af88c4b3615788b164464a437df5c/dist/pythonlibs/testrunner/__init__.py", line 29, in run
testfunc(child)
File "/tmp/dwq.0.3125418833043728/ef3af88c4b3615788b164464a437df5c/tests/evtimer_msg/tests/01-run.py", line 26, in testfunc
assert(actual in range(expected - ACCEPTED_ERROR, expected + ACCEPTED_ERROR))
AssertionError
pokgak
pushed a commit
that referenced
this pull request
Apr 4, 2020
The ROM size is encoded in the part number of the Atmel SAM chips. RAM size is not encoded directly, so get it by parsing the chip's vendor file. The file remains in the page cache for the compiler to use, so the overhead should be minimal: on master: Benchmark #1: make BOARD=samr21-xpro -j Time (mean ± σ): 527.9 ms ± 4.9 ms [User: 503.1 ms, System: 69.6 ms] Range (min … max): 519.7 ms … 537.2 ms 10 runs with this patch: Benchmark #1: make BOARD=samr21-xpro -j Time (mean ± σ): 535.6 ms ± 4.0 ms [User: 507.6 ms, System: 75.1 ms] Range (min … max): 530.6 ms … 542.0 ms 10 runs
pokgak
pushed a commit
that referenced
this pull request
Sep 1, 2020
Coverty scan found this: > CID 298279 (#1 of 1): Out-of-bounds read (OVERRUN) > 21. overrun-local: Overrunning array of 16 bytes at byte offset 64 by dereferencing pointer The original intention was probably to advance the destination pointer by 4 bytes, not 4 * the destination type size.
pokgak
pushed a commit
that referenced
this pull request
Sep 1, 2020
Coverty scan found this: > CID 298295 (#1 of 1): Operands don't affect result (CONSTANT_EXPRESSION_RESULT) result_independent_of_operands: > (ipv6_hdr_get_fl(ipv6_hdr) & 255) >> 8 is 0 regardless of the values of its operands. Looking at the code, this appears to be a copy & paste error from the previous line.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
Ideas for better code integration with DTLS. These changes remove 9 #ifdefs from gcoap.c.
I like this idea of having an abstract top level transport definition/object. This will help with other transports like TCP.
Testing procedure
Works on native between two gcoap terminals, both in DTLS and non-DTLS mode.
Issues/PRs references
N/A