-
Notifications
You must be signed in to change notification settings - Fork 143
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
Always check soter_rand() for success #570
Merged
Merged
Commits on Dec 23, 2019
-
Warn about unused result of soter_rand()
Checking random number generation for failures is pretty imporant [1, 2] so let's make it easier for users to track such issues. Introduce a SOTER_MUST_USE macro that adds an attribute to the function which will cause a compiler warning if the return value is not used (or not explicitly ignored by casting to void). See [3] for description. Since we treat warnings as errors, CI compilation will fail like this: compile build/obj/tests/common/test_utils.c.o tests/common/test_utils.c:70:3: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result] soter_rand((uint8_t*)(&res), sizeof(size_t)); ^~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 warning generated. if anyone adds a soter_rand() call without checking the return value. [1]: https://github.com/veorq/cryptocoding#use-strong-randomness [2]: http://jbp.io/2014/01/16/openssl-rand-api [3]: http://releases.llvm.org/9.0.0/tools/clang/docs/AttributeReference.html#nodiscard-warn-unused-result
Configuration menu - View commit details
-
Copy full SHA for cdbcafa - Browse repository at this point
Copy the full SHA cdbcafaView commit details -
Add missing tests for themispp::secure_rand_t
We have this API, but it's not tested (or documented either). However, it's there and we should at least exercise it in the test suite. Turns out that it was not checking return value of soter_rand() and it's not visible if there are no tests. The API is... wacky, but that's what it is. I'm not really motivated to clean it up at the moment (and that's out of scope).
Configuration menu - View commit details
-
Copy full SHA for 747d9ab - Browse repository at this point
Copy the full SHA 747d9abView commit details -
Add missing error handling. For ThemisPP throw an exception. For tests just abort with non-zero exit code as the API does not allow for returning an error. That's actually the case for ed25519 code too [1]. [1]: https://github.com/cossacklabs/themis/blob/5f886e1fa1742beb2070da93a2036ad79030dac5/src/soter/ed25519/gen_rand_32.c#L31-L35
Configuration menu - View commit details
-
Copy full SHA for 2fd6033 - Browse repository at this point
Copy the full SHA 2fd6033View commit details -
Configuration menu - View commit details
-
Copy full SHA for fcead0c - Browse repository at this point
Copy the full SHA fcead0cView commit details -
Typo in clang-tidy configuration
Nothing interestring. I have found this typo when trying to run clang-tidy manually. It is obscured during normal runs due to "2>/dev/null" which is added there because clang-tidy likes to talk even when no errors are detected and this is treated as warning output by our build system.
Configuration menu - View commit details
-
Copy full SHA for 853c185 - Browse repository at this point
Copy the full SHA 853c185View commit details -
Reorder SOTER_MUST_USE to make Clang happy
Make sure that it goes first because when compiling for C++14 this gets expanded into __attribute__((visibility("default"))) [[nodiscard]] soter_status_t soter_rand(uint8_t* buffer, size_t length); which throws Clang off-guard with this GCC compatibility syntax for attributes. For some reason Clang thinks that [[nodiscard]] applies only to "soter_status_t" which is not allowed. Make sure that C++ attributes go before non-standard attributes.
Configuration menu - View commit details
-
Copy full SHA for 77db0d6 - Browse repository at this point
Copy the full SHA 77db0d6View commit details
Commits on Dec 24, 2019
-
Configuration menu - View commit details
-
Copy full SHA for 26572c3 - Browse repository at this point
Copy the full SHA 26572c3View commit details
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.