Hi,
Thanks for reviewing!
On 23-02-18 10:17, Antonio Quartulli wrote:
> On 07/02/18 20:22, Steffan Karger wrote:
>> - mbedtls_sha256(cert->tbs.p, cert->tbs.len, sha256_hash, false);
>> + if (0 != md_full(sha256_kt, cert->tbs.p, cert->tbs.len,
>> sha256_hash))
>
> Why not using mbedtls_sha256_ret() since we are already in
> mbedtls-specific code here?
> Any advantage in using a wrapper for another wrapper? :-P
>
> (mbedtls_sha256_ret() is also the suggested replacement for
> mbedtls_sha256())
>
>
> Moreover, SHA256 is statically selected, therefore using
> mbedtls_sha256_ret() would also avoid the md_kt_t local variable.
This was the only place in the code where we took a (direct) dependency
on functions from sha256.h. By using our internal wrapper, I could also do:
@@ -60,7 +60,6 @@
#include <mbedtls/oid.h>
#include <mbedtls/pem.h>
-#include <mbedtls/sha256.h>
Since reducing dependencies usually reduces maintenance burden, I prefer
this solution.
>> + {
>> + msg(M_WARN, "WARNING: failed to personalise random");
>> + }
>> +
>
> Since we now have a reason for the failure, may it make sense to print a
> proper description based on the return value? (even though I think
> mbedtls_sha256_ret() can't really return something different from 0)
md_full returns true/false, but you're right we can improve error
reporting. Are you satisfied if I add an mbed_ok() wrapper *inside*
md_full() to print the internal mbed error? I think that should be
sufficient information.
-Steffan
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
