On 16 Nov 2012, at 4:36 AM, Jeffrey Walton <noloa...@gmail.com> wrote:
> On Thu, Nov 15, 2012 at 10:41 AM, Jeffrey Walton <noloa...@gmail.com> wrote: >> On Thu, Nov 15, 2012 at 6:03 AM, Pravesh Rai <pravesh....@gmail.com> wrote: >>> >>> CryptGenRandom(hCryptProv, SEED_SIZE, buf); // On Windows OS >>> apr_generate_random_bytes(buf, SEED_SIZE); // On Linux OS >>> > Speaking of poor documentation….. Why are you discussing APR on the openssl list? Surely if you had a problem with the APR documentation this would be a matter for the APR lists instead? > I looked at the "header" and the "source". They are different "style > sheets" applied to the same file (I expected to see the H file, and > the C file). Neither had comments. Really? According to the source code, the header file is here: https://svn.apache.org/repos/asf/apr/apr/branches/1.4.x/include/apr_general.h The implementation is platform specific (that's the point of APR), and for unix it is here: https://svn.apache.org/repos/asf/apr/apr/branches/1.4.x/misc/unix/rand.c Both the header and source contain comments. > Confer > http://apr.apache.org/docs/apr/0.9/apr__general_8h-source.html and > http://apr.apache.org/docs/apr/0.9/group__apr__random.html. Why would you choose the obsolete v0.9 of APR as an example, when the latest version is v1.4.6? Have you read the documentation at http://apr.apache.org/ that covers this? > I'll reproduce it here without the markup: > > apr_status_t apr_generate_random_bytes( > unsigned char * buf, > int length > ) > > So, there are a few problems here. First is no documentation. Verbum > sapienti sat. APR uses doxygen as a documentation generation system: http://en.wikipedia.org/wiki/Doxygen The documentation is generated from the source headers, for example: /** * Generate random bytes. * @param buf Buffer to fill with random bytes * @param length Length of buffer in bytes */ APR_DECLARE(apr_status_t) apr_generate_random_bytes(unsigned char * buf, apr_size_t length); The phrase "generate random bytes" is woefully inadequate, so I did the right thing and raised the issue on the right mailing list, archived here: http://www.mail-archive.com/dev@apr.apache.org/msg24968.html > Second, you don't know what conditions need to be satisfied to define > APR_HAS_RANDOM (did you even know it was there?). This could be fixed > with documentation, but APR chose otherwise. If you look closer at APR, you'll notice that to build it, you run the configure script generated by a tool called autoconf. If you had occasion to care where APR_HAS_RANDOM came from, you would ensure that you understood autoconf and how it tests for system capability at compile time. It is not APR's job to re-document the autoconf tool: http://en.wikipedia.org/wiki/Autoconf > Third, you don't know what the function returns on success. Is there a > apr_succes? Or apr_true? This could be fixed with documentation, but > APR chose otherwise. The error codes are documented extensively here: http://apr.apache.org/docs/apr/1.4/group__apr__errno.html > Fourth, the API tells you a negative length is acceptable. This could > be fixed with documentation, but APR chose otherwise. Really? The API specifies a length of apr_size_t. If you read the documentation (Hint: try a google search for "site:apr.apache.org apr_size_t") you discover that apr_size_t is documented here as being equivalent to size_t: http://apr.apache.org/docs/apr/1.4/group__apr__platform.html In turn, size_t is defined as an unsigned type, such as unsigned int, depending on your platform. By reading the documentation you would have discovered that a negative length is not possible. > A negative > length makes no sense whatsoever (I know, its not limited to APR). I > would encourage you to write a few negative self-tests and submit it > to the project: send in a NULL`buf`, a zero `length`, and a negative > `length`. See how the library handles it. Since they botched the API > design, I would not be surprised if they SIGABRT on you (that's how > *not* to build a resilient system). I would suggest instead that you read the documentation. > Fifth, there is probably some internal state, but we don't know that > for sure. This could be fixed with documentation, but APR chose > otherwise. If there is state, you don't know where it came from or its > quality. Did they limit themselves to (1) Time of Day, (2) Mac > address, (3) /dev/{u}rand, (4) the kernel's hwrand, or (5) virtio > gear? Perhaps some other clever combination? Are they constantly > hedging (probably not)? If there is no state, they have already broken > you (that's how *not* to build a resilient system). Correct, and I have raised this on the d...@apr.apache.org list, just as you should have done. > This is a bit more personal taste, but I require PRNGs to be thread > safe. So Sixth, is the library thread safe? Is the call to > apr_generate_random_bytes() thread safe? I would definitely write a > multithreaded self test and try to break it. I can email you a set if > you need a canned test that spins up 64 threads (hit me off list). This should be documented, yes. Again, raised it on the APR list? As to writing an elaborate test, why not read the source as described above? If you had you would have discovered that either /dev/random, EGD, or truerand is used to generate the randomness, and you would be able to read the documentation on those to determine whether they are thread safe or not. Look for the documentation on /dev/random, EGD or truerand by using Google. > Headless servers, entropy starvation, and rollbacks are a concern in > modern environments. OpenSSL and other entropy gathers, such as EDG, > don't account for the later. Its best to take the bull by the horns > and do it yourself. At minimum, you need to call RAND_add() with > entropy external to /dev/{u}rand. > > The following may also be useful to you: > * Analysis of the Linux Random Number Generator, eprint.iacr.org/2006/086.pdf > * Cryptanalysis of the Random Number Generator of the Windows > Operating System, eprint.iacr.org/2007/419.pdf > > Most recent analysis of Linux RNG (AFAIK): > * Mining Your Ps and Qs: Detection of Widespread Weak Keys in Network > Devices, https://factorable.net/paper.html To bring this back to OpenSSL, missing documentation is a real problem. OpenSSL has chosen man pages as its preferred method of documentation, but not all API functions have man pages, and I believe that needs to be fixed. Most specifically, I believe patches submitted with new functionality that is not documented should be rejected by the project until the person making the submission finishes the job. Users however are expected to read that documentation that does exist, and understand the documentation and library within the wider context in which it is deployed. Regards, Graham --
smime.p7s
Description: S/MIME cryptographic signature
