Hi Yaron,<br><br>Some comments inline:<br><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
 * The documentation of "generate" says that randfunc returns X random<br>
   bytes. I think this should be bits.<br></blockquote><div><br>I am not 100% sure, but <span class="k">a common idiom in pycrypto is:<br><br>"<br>if</span> <span class="n">randfunc</span> <span class="ow">is</span> <span class="bp">None</span><span class="p">:<br>
</span><span class="n">     </span><span class="p"></span><span class="n">randfunc</span> <span class="o">=</span> <span class="n">Random</span><span class="o">.</span><span class="n">new</span><span class="p">()</span><span class="o">.</span><span class="n">read</span><br>
"<br><br>so randfunc(N) returns N bytes of full entropy.<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
 * The "generate" function is way too conservative. We construct p as<br>
   2*q+1, where both p and q are prime. This makes p a classic "safe<br>
   prime". It also makes two of the checks redundant: g cannot divide<br>
   p-1, because only 2 and q divide it. g cannot be 2, and most likely<br>
   will not be q during the lifetime of the universe. I believe that<br>
   similarly, g**-1 cannot divide p-1, but my algebra skills are too<br>
   rusty to prove it.<br></blockquote><div><br>I contributed to that part with a patch. My intention was actually to list in the loop<br>as many criteria as possible that a generator safe for both Elgamal encryption and Elgamal signatures (because .generate() does not know how the key will be used) must fulfill.<br>
<br>It's true they are redundant in practice, but I think it's good to leave a track behind with the general conditions that one must check, regardless of how the domain parameters are computed.<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

 * For the same reasons, there is no need for the loop when<br>
   constructing K (the secret parameter), e.g. on line #342. You just<br>
   need to ensure that it is an odd number, otherwise its GCD with p-1<br>
   would be 2. So choose a random t, 2 < t < q-1, and let K=2*t+1. No<br>
   need for a loop or for the GCD calculation.<br></blockquote><div><br>In the _sign() method I see only a loop to ensure that residues remains in the range 0..p-1. The loop does not contribute to GDC computation.<br> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
 * An important check is missing: the message M needs to be less than<br>
   p, both when signing and certainly when encrypting it.<br></blockquote><div><br>True. Note that M must not the message when signing with PyCrypto's Elgamal. It must be really be the cryptographic hash of the message.<br>
</div></div>