Return value from OQS_randombytes

[matlimatli]

The current signature of OQS_Randombytes() is to return void. This might be a limiting factor, which is also noted in OQS_randombytes_openssl() with the following comment:

// because of void signature we have no other way to signal the problem
// we cannot possibly return without randomness

and the failure handling of this function is therefore to terminate the execution via exit().

In cases where we supply our own randomness algorithm via OQS_randombytes_custom_algorithm(), the same problem applies. We have no other means of handling a failure than forcefully exiting. It is not unthinkable that the higher level code would want to take care of this situation in a custom way without having to resort to using an exit handler.

Would it be feasible to change this signature to returning a status value, which can then be propagated through the higher level functions? For example, the keypair and encaps functions already return an OQS_STATUS value, which can be taken care of in the calling code. Wouldn't it make sense if OQS_ERROR was returned if an underlying randomness call failed?

[SWilson4]

I agree that the void return is less than ideal. However, I believe this is a legacy of the DRBG that NIST provided for submissions to the PQC standardization process, which returns void. As such, almost all of the code in liboqs was written with this function signature in mind.

Modifying keypair / encaps / etc to error-check randombytes would require quite a bit of maintenance on our part. We typically pull in this code from upstream sources (e.g., PQClean / pq-crystals) which would (presumably) still be using the NIST-provided randombytes API. Hence, we would have to patch the code and re-patch every time we need to pull in an upstream update.

I suppose that it might not be too difficult to change the OQS_randombytes signature to return a status code and just ignore it in keypair / encaps / etc. while allowing developers to pass an error-checked implementation to OQS_randombytes_custom_algorithm. This would also enable us to add better error handling in the future.

[SWilson4]

It would be great to hear a little more about your use case and if that latter option would be helpful for you. (And if it would be helpful enough, please feel free to submit a PR proposing the change!)

[matlimatli]

Thanks for the explanation! It makes sense (but still is a bit unfortunate).

My use cases are mainly hypothetical at this time, but I am thinking of cases where a long-running system may for example temporarily run out of good entropy. In this case I would expect the cryptographic operations to fail, so that the calling code could remedy the situation (the obvious workaround here is to let the custom randombytes function handle it, which may or may not be a good idea) or shut down the system gracefully.

I'm not sure it would be a good idea to let OQS_randombytes return a status code if it is not checked by the callers. It might be dangerous if it could return bad randomness without anyone noticing. In cases where we want to use a function with a different signature, it is probably more safe to use a wrapper, similar to OQS_randombytes_openssl.

[baentsch]

I'm not sure it would be a good idea to let OQS_randombytes return a status code if it is not checked by the callers. It might be dangerous if it could return bad randomness without anyone noticing.

Absolutely agreed: All functions based on this (in upstream-imported algorithms) must stop working if entropy is bad.

In cases where we want to use a function with a different signature, it is probably more safe to use a wrapper, similar to OQS_randombytes_openssl.

Also agreed, but maybe thinking in reverse: What about adding such (result-returning) call to the public liboqs API and gradually introducing (or patching it) it to "LTS" code, e.g., the ML-* algorithms? The current (void) call could be a wrapper to this (still just exiting on bad retval)?

[matlimatli]

What about adding such (result-returning) call to the public liboqs API and gradually introducing (or patching it) it to "LTS" code, e.g., the ML-* algorithms? The current (void) call could be a wrapper to this (still just exiting on bad retval)?

Ah, yes, I see. That sounds like a good plan!

[baentsch]

Ah, yes, I see. That sounds like a good plan!

PR welcome :-) Time permitting, of course.