iOS SecRandomCopyBytes ret checking suggestion

[weipin]

Hi,

When browsing the repos and looking at the iOS part (ios.rs),
I happen to notice that the ret checking code seems suspicious.

Given the doc on SecRandomCopyBytes, the ret is set to errSecSuccess on success or some other value on failure.

Return Value: A result code set to errSecSuccess or some other value on failure.

And the value of errSecSuccess seems to be 0. Here is the related source code I located in a quick internet search:

https://github.com/apple-oss-distributions/Security/blob/Security-59754.140.13/base/SecBase.h#L323

The doc doesn't mention that -1 is the only possible failure code. And unlikely by looking through the codes in SecBase.h.

Wouldn't it be better to have the code written like this (also aligning with the sample code in the API doc):

let ret = unsafe { SecRandomCopyBytes(null(), dest.len(), dest.as_mut_ptr()) };
if ret == 0 {
    Ok(())
} else {
    Err(Error::IOS_SEC_RANDOM)
}

Here is the current code:

let ret = unsafe { SecRandomCopyBytes(null(), dest.len(), dest.as_mut_ptr()) };
if ret == -1 {
    Err(Error::IOS_SEC_RANDOM)
} else {
    Ok(())
}

It seems possible that when SecRandomCopyBytes fails and returns some non -1 error code, getrandom_inner will return Ok with dest buffer in unknown state. Did I miss anything here?

[josephlr]

@weipin thanks for reporting this. You're exactly correct, we were not handling errors correctly here. This should be fixed by #244.