On Thu, Aug 09, 2012 at 11:10:40AM -0700, Justin Pettit wrote:
> On Aug 9, 2012, at 10:39 AM, Ben Pfaff <[email protected]> wrote:
> > + if (!RAND_status()) {
>
> I assume SSL_library_init() seeds the PRNG? I looked briefly, but
> couldn't see definitively. I have a weak gag reflex, so I didn't
> want to download the code and look for myself.
When I tested the patch, RAND_status() returned 1 for me. I decided
that this showed empirically that SSL_library_init() (or perhaps
RAND_status() itself) seeds the PRNG.
> > + /* We occasionally see OpenSSL fail to seed its random number
> > generator
> > + * in heavily loaded hypervisors. I suspect the following
> > scenario:
> > + *
> > + * 1. OpenSSL calls read() to get 32 bytes from /dev/urandom.
> > + * 2. The kernel generates 10 bytes of randomness and copies it
> > out.
> > + * 3. A signal arrives (perhaps SIGALRM).
> > + * 4. The kernel interrupts the system call to service the signal.
> > + * 5. Userspace gets 10 bytes of entropy.
> > + * 6. OpenSSL doesn't read again to get the final 22 bytes.
> > Therefore
> > + * OpenSSL doesn't have enough entropy to consider itself
> > + * initialized.
> > + *
> > + * The only part I'm not entirely sure about is #6, because the
> > OpenSSL
> > + * code is so hard to read. */
>
> I think this last sentence makes sense to have in the commit
> message, but it seems confusing in code. If the code lasts the test
> of time, then you were almost certainly correct in your hypothesis.
> If not, then this code will likely be reverted.
My perspective is different. This code will hardly ever run, and if
#6 is not right, then it may *never* run. If that's the case, we may
never have a reason to re-examine this code. So I wanted a little
extra rationale.
> > + if (!retval) {
> > + RAND_seed(seed, sizeof seed);
> > + } else {
> > + VLOG_ERR("failed to obtain entropy (%s)",
> > + ovs_retval_to_string(retval));
> > + }
>
> Should we return an error if we failed to get enough entropy?
Sure. I folded this in:
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index c97e83c..db7b68e 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -923,12 +923,13 @@ do_ssl_init(void)
VLOG_WARN("OpenSSL random seeding failed, reseeding ourselves");
retval = get_entropy(seed, sizeof seed);
- if (!retval) {
- RAND_seed(seed, sizeof seed);
- } else {
+ if (retval) {
VLOG_ERR("failed to obtain entropy (%s)",
ovs_retval_to_string(retval));
+ return retval > 0 ? retval : ENOPROTOOPT;
}
+
+ RAND_seed(seed, sizeof seed);
}
/* New OpenSSL changed TLSv1_method() to return a "const" pointer, so the
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
