<div>Hi Daniel,</div>
<div> </div>
<div>Thanks a lot for the patch, yes it is working as expected.</div>
<div> </div>
<div># virsh -c qemu://localhost/system version<br>Compiled against library: libvirt 0.10.2<br>Using library: libvirt 0.10.2<br>Using API: QEMU 0.10.2<br>Running hypervisor: QEMU 0.14.1</div>
<div> </div>
<div>But can you please clarify my below query.</div>
<div> </div>
<div>Do we really need the red coloured lines as well ? </div>
<div> </div>
<div>Our patch :</div>
<div> </div>
<div>In function virInitialize()</div>
<div> </div>
<div><font color="#ff0000">+ #ifdef WITH_GNUTLS<br></font>- gcry_control(GCRYCTL_SET_THREAD_CBS, &virTLSThreadImpl);<br>- gcry_check_version(NULL);<br>+ if (gcry_control(GCRYCTL_ANY_INITIALIZATION_P) == 0) {<br>
+ gcry_control(GCRYCTL_SET_THREAD_CBS, &virTLSThreadImpl);<br>+ gcry_check_version(NULL);<br>+<br>+ gcry_control(GCRYCTL_DISABLE_SECMEM, NULL, 0);<br>+ gcry_control(GCRYCTL_INITIALIZATION_FINISHED, NULL, 0);<br>
+ gcry_control(GCRYCTL_ENABLE_QUICK_RANDOM, 0);<br>+ }<br><font color="#ff0000">+ #endif</font></div>
<div> </div>
<div> Thanks and Regards</div>
<div> Shree Duth Awasthi.</div>
<div> <br><br> </div>
<div class="gmail_quote">On Fri, Apr 12, 2013 at 9:31 PM, Eric Blake <span dir="ltr"><<a href="mailto:eblake@redhat.com" target="_blank">eblake@redhat.com</a>></span> wrote:<br>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote">
<div class="im">On 04/12/2013 10:27 AM, Daniel P. Berrange wrote:<br>> From: "Daniel P. Berrange" <<a href="mailto:berrange@redhat.com">berrange@redhat.com</a>><br>><br>> If libvirt makes any gcry_control() calls, then this<br>
> prevents gnutls for doing any initialization. As such<br>> we must take care to do full initialization of libcrypt<br>> on a par with what gnutls would have done. In particular<br>> we must disable "sec mem" for cases where the user does<br>
> not have mlock() permission. We also skip our init of<br>> libgcrypt if something else (ie the app using libvirt)<br>> has beaten us to it.<br><br></div>Wow, libgcrypt is just horrible with regards to its usability. At any<br>
rate, I read<br><br><a href="http://www.gnupg.org/documentation/manuals/gcrypt/Multi_002dThreading.html#Multi_002dThreading" target="_blank">http://www.gnupg.org/documentation/manuals/gcrypt/Multi_002dThreading.html#Multi_002dThreading</a><br>
<br><a href="http://www.gnupg.org/documentation/manuals/gcrypt/Controlling-the-library.html#index-gcry_005fcontrol-6" target="_blank">http://www.gnupg.org/documentation/manuals/gcrypt/Controlling-the-library.html#index-gcry_005fcontrol-6</a><br>
<br>and it seems to match with you conversion to a conditional initialization.<br><br>Is it worth pointing to<br><a href="https://bugzilla.redhat.com/show_bug.cgi?id=951630" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=951630</a> as your rationale?<br>
<div class="im"><br>><br>> Signed-off-by: Daniel P. Berrange <<a href="mailto:berrange@redhat.com">berrange@redhat.com</a>><br>> ---<br>> src/libvirt.c | 10 ++++++++--<br>> 1 file changed, 8 insertions(+), 2 deletions(-)<br>
><br>> diff --git a/src/libvirt.c b/src/libvirt.c<br>> index c5221f5..7c0a873 100644<br>> --- a/src/libvirt.c<br>> +++ b/src/libvirt.c<br>> @@ -409,8 +409,14 @@ virGlobalInit(void)<br>> goto error;<br>
><br>> #ifdef WITH_GNUTLS<br>> - gcry_control(GCRYCTL_SET_THREAD_CBS, &virTLSThreadImpl);<br>> - gcry_check_version(NULL);<br>> + if (gcry_control(GCRYCTL_ANY_INITIALIZATION_P) == 0) {<br>> + gcry_control(GCRYCTL_SET_THREAD_CBS, &virTLSThreadImpl);<br>
> + gcry_check_version(NULL);<br><br></div>Up to here makes sense.<br>
<div class="im"><br>> +<br>> + gcry_control(GCRYCTL_DISABLE_SECMEM, NULL, 0);<br><br></div>Makes sense; the manual said most applications don't need secure memory,<br>and that FIPS mode ignores this (so we aren't breaking FIPS systems by<br>
weakening anything they depend on).<br><br>> + gcry_control(GCRYCTL_INITIALIZATION_FINISHED, NULL, 0);<br><br>Makes sense - we check if anyone has called gcry_check_version<br>(ANY_INITIALIZATION_P above), and if not then we finish full<br>
initialization ourselves. But is there a possible race where one thread<br>could have started basic initialization, and then two threads are<br>competing to finish initialization?<br><br>> + gcry_control(GCRYCTL_ENABLE_QUICK_RANDOM, 0);<br>
<br>The gcrypt manual discourages this one unless you have consulted with a<br>crypto expert; are we violating the assumptions of any top layer<br>application that links with us? Furthermore, the manual states that<br>ENABLE_QUICK_RANDOM can only be used at initialization time, but we just<br>
declared that initialization was finished.<br><br>> + }<br>> #endif<br>><br>> virLogSetFromEnv();<br>><br><br>I'm not a gcrypt expert, so assuming you can explain the things I<br>questioned above, then I don't mind this patch going in as-is. But it<br>
doesn't make me feel any better about gcrypt when it is this hard to set<br>it up correctly for use in a multi-threaded library; and it doesn't help<br>when their manual describes the setup but doesn't offer any sample code<br>
to copy from. Crypto library authors have sure made life tough on everyone.<br><span class="HOEnZb"><font color="#888888"><br>--<br>Eric Blake eblake redhat com +1-919-301-3266<br>Libvirt virtualization library <a href="http://libvirt.org/" target="_blank">http://libvirt.org</a><br>
<br></font></span></blockquote></div><br>