flakes / mirc_fish_10

"FiSH 10" - a blowfish encryption script for mIRC 7, compatible to previous FiSH scripts and other clients! Come visit us in #fish10 on EFNet!
https://syndicode.org/fish_10/
86 stars 16 forks source link

Long delay creating DH key pair #61

Closed maroonbells closed 3 years ago

maroonbells commented 4 years ago

When compiled under 1.1.* there is a freeze of approx half second for every call to DH1080_gen

//var %i 9 , %t $ticks | while (%i) { tokenize 32 $dll(%FiSH_dll,DH1080_gen,NOT_USED) | dec %i } | echo -a ticks $calc($ticks - %t)

Under the old 1.0.x version in v7.55 this takes 280 ticks. Under Oct.6 version in 7.57 it takes 5000 ticks. Others are reporting 2300, but that's still too long.

I assumed the delay would be in DH_generate_key() but someone added a debugging benchmark timestamp and said the delay was in DH_check()

From what I can tell, DH_check() is only verifying that the 'g' 'p' and 'q' parameters are legit, which are not changing after load, so this only needs to be called once not every key pair generated. Or, a global variable could be created to allow DH_check to be called only 1 time, and only if someone called DH1080_gen.

(Technically DH_check should be called prior to executing DH_compute_key() within DH1080_Compute but that would only happen if someone manually unloaded the dll during the DH handshake.)

Moving the call to DH_check() to the initialization would mitigate the problem, but would instead add quarter-to-half second delay during ON START event.

flakes commented 4 years ago

How often are you exchanging keys so that this is a problem? More key exchanges can even make communication less secure than more (unless you have some external means of confirming the other side's identity).

But indeed the DH_check call is not well-placed. Thanks for your input already, I'll see if I can come up with a solution that uses DH_check_params (which should be much quicker) and/or performs a lazy check on startup or on first use. I'll also double check the flow around DH_compute_key.

maroonbells commented 4 years ago

with auto key exchange, it would introduce a noticeable freeze while typing, mouse clicking, etc.

A global memory variable within the DLL could be initialized to zero, and could be flipped to 1 after running DH_Check, so checking the global var would allow the dll to do the DH_check only 1 time, or do it zero times if they never generated a key pair.

I don't think DH_check_params is a good substitute. If I'm reading the source correctly, it looks like all it does is verify:

g >= 2 g < (p-1) p is odd p > 1 p >= minimum bits 512 p <= max bits 65536

If I'm reading that correctly, DH_check_params would validate approximately half of random 'p' values since they were odd numbers, so isn't worth the effort.

I guess if the older semi-fast check isn't part of OpenSSL anymore, it either needs to keep using the slower DH_check or not bother checking. The former check was reasonably fast and still managed to do reasonable validation of 'p'. Under the old dll, it verified that 'p' was prime, as well as q=(p-1)/2 being prime, and that 'p modulo 24 was 11. What I don't know is how strong were the verify that p and q were prime.

flakes commented 4 years ago

I'm just going to fire up a background thread for initialization ON STARTUP with the appropriate synchronization primitives.

maroonbells commented 4 years ago

That sounds like a great solution that I didn't think of, and would work as long as someone didn't manually unload FiSH with "/dll -u fish_10.dll" then try to immediately call DH1080_Generate causing it to reload the dll, which there should be no legit reason for doing.

flakes commented 3 years ago

New version released today, including a fix for this issue, please test, and open a new issue if required 😝