Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.48k stars 2.59k forks source link

Please add documentation for the return value of `f_rng` used for `mbedtls_pk_parse_keyfile` #5868

Open boaks opened 2 years ago

boaks commented 2 years ago

Version 3.1.0:

pk.h, mbedtls_pk_parse_keyfile

\param f_rng RNG function, must not be \c NULL. Used for blinding.

Please add some documentation about the expected return value of f_rng or provide a link to the documentation of RNG. It's extremely hard to follow this parameter over so many call levels (at least I gave up). And it seems to cause irritating error codes (even unknowns)

returning 1:

ERR mbedtls_pk_parse_keyfile returned -0x3cff: 'PK - Key algorithm is unsupported (only RSA and EC are supported) : UNKNOWN ERROR CODE (007F)'

returning the passed in len:

ERR mbedtls_pk_parse_keyfile returned -0x3ce0: 'PK - Key algorithm is unsupported (only RSA and EC are supported) : ASN1 - Out of data when parsing an ASN1 data structure'

boaks commented 2 years ago

f_rng - where it is finally executed ...

Seems to expect a 0?

minosgalanakis commented 2 years ago

Support questions are usually better fitted and more likely to be answered in the project's Mailing List.

But to answer the question, the pseudo random generator in the test-suite is a good reference point.

The function is filling an output buffer of len sizem returning the status with zero for success , non-zero for failure. Due to different implementations across platforms, there is no standard ret_code <-> ERR_CODE mapping.

Hope that helps

I will be closing this issue, but feel free to follow up on the mailing list as discussed above.

boaks commented 2 years ago

Thanks!

gilles-peskine-arm commented 2 years ago

The typical implementation of f_rng and p_rng is mbedtls_ctr_drbg_random or mbedtls_hmac_drbg_random or mbedtls_psa_get_random with a suitable context. Following the convention in Mbed TLS, it must return 0 on success, any nonzero value (conventionally MBEDTLS_ERR_xxx) on error.

I agree that this should be documented. My preference would be to define a type name and use that throughout the API, and attach the documentation to that type.

gilles-peskine-arm commented 2 years ago

I'm reopening this issue because we do have a documentation gap here, this isn't just a support question.

boaks commented 2 years ago

we do have a documentation gap

Thanks for catching up. That was my reason to open this issue.

Developer, which are more common to mbedtls will know the mbedtls-coding-standards. Unfortunately, I'm not too common and tried to fix an issue with using it. Add some documentation will help others to deal with.

axos88 commented 2 years ago

Just lost three hours debugging this. :+1: for adding it to the docs, it was unexpected since I would have expected it to expect (ha!) the number of bytes generated.