maroonbells
commented
4 years ago /* { As a work-around for the bug where a bad pub key is created 1 of every 128 times, this alias should fix the problem at the cost of a few extra ticks doing the DH handshake. Instead of asking fish_10.dll to simply generate the key pair then use what's provided, this alias tests the generated key pair to see if the public key was corrupted. Since the mime string of the safe prime begins with "+" and the public key must always be less than prime, any public key mime string beginning with "/" must be invalid. In addition to that, this alias uses an existing test key pair to test the newly generated key pair being used on both sides of the handshake. When there's a bad public key, the secrets generated on each side of the handshake will either be $null or be different from each other. If a bad public key is detected, it repeats the key generation until it's a good key. You can add the -l switch to make this alias local, if you don't want it to be accessible from outside the same script. maroon
status window shows this replacing an average of 2 bad keys: /timer 256 1 Bugfix_of_FiSH_DH1080_Generate
Simply add this alias to the fish_10.ini script, then find the other 2 places in the script containing
$dll(%FiSH_dll,DH1080_gen,NOT_USED)
and replace that with
$Bugfix_of_FiSH_DH1080_Generate } */
alias Bugfix_of_FiSH_DH1080_Generate {
var %test_priv This+is+a+test+private+key+used+to+check+FiSH+DH1080+handshake+for+bad+public+key+caused+by+OpenSSL+zeropad+bug++If+public+key+is+bad+then+script+should+generate+another+pair//////
var %test_pub 92NDRLzvLlsCQtHP4zxZDKyDONnppkJjIoe5UMxuPyiYEw9iYOq7bMqQnLhXQvxX4DD5wHIZMbEM1RV9CQNJc4eYl/P6iKdMxBSDX4EQg0s4MRBOPKjRIH52PIPJu3iaGae/pIszm4/iWTUpxxovI7AKkOBWGpXjtmLtMyrXt0jh0rwVCDNw
:generate_again
tokenize 32 $dll(%FiSH_dll,DH1080_gen,NOT_USED) | if (/* iswm $2) { echo -s fail | goto pubfail }
if ($2 == $null) { echo -s fail! DH1080_gen returns null key | return }
var %secret1 $dll(%FiSH_dll,DH1080_comp, $1 %test_pub)
var %secret2 $dll(%FiSH_dll,DH1080_comp, %test_priv $2)
if ((%secret1 !=== %secret2) || (%secret1 == $null)) {
:pubfail
echo -s $chr(3) $+ 0,4 Generating new DH key to replace bad pair! | goto generate_again }
return $1 $2
}
flakes
1 out of every 128.25 DH1080 handshakes fails because 1 out of every 256 outputs from $dll(%FiSH_dll,DH1080_gen,NOT_USED) returns the wrong string for the 2nd string "Public Key".
When the public key should begin with 1 or more 0x00 bytes, the mime public key string has the correct length, but it skips the leading 0x00 bytes and adds the needed number of garbage bytes at the tail end. Because the public key appears to be pseudo-random, it has a 1/256th chance to begin with 0x00, which triggers this bug.
The handshake fails if either nick generates one of these bogus public keys. The odds that 1 nick does not generate a bad pubkey is 255/256, and the odds that neither of 2 nicks generates the bad pubkey is (255/256)^2, so the odds of either nick in the handshake having a bad key is
//echo -a $calc( 1/ ( 1-((255/256)^2)) ) = 1 in 128.25 handshakes
The bug is likely caused either by
OpenSSL skipping the leading 0x00's and giving a shorter binary string, which causes un-initialized data to be captured in the mime string.
The code creating the mime has the bug skipping leading zeroes.
If it's the former, there appears to be a function BN_bn2binpad that might be the one which should be called instead.
As a work-around to avoid this bug, the script would need to be adapted to keep a long-term sample private/public key pair, and after each time it generates a new private/public pair, it would need to test both sides of the DH handshake to see if they both arrive at the same secret value. If they don't, which would happen 1 out of every 256 generated key pair, it would need to discard that new key pair and keep generating new pairs until the test handshake doesn't fail. A very small percent of the time, around 1 key per 16k, a bad public key can be detected quickly if the mime string begins with "/", which should not be possible when the "p" prime as a mime string begins with "+".
This work-around would be a pain, because it requires each side of the handshake to perform 2 additional compute of the secret against the long-term test key, and possibly more when it turns out that they generated a bogus public key.
As proof of this bug, someone running the FiSH script can add these 2 aliases, and run the command: /find_bad_pubkey It steals most of the focus of your client, so you can either use the F9 client to halt the demo, or remove the comment from the line that shows the Yes/No prompt after each bad key pair found.
alias F9 { timerfind_bad_pubkey off } alias find_bad_pubkey { if (find_bad_pubkey !iswm $ctimer) { :try_again tokenize 32 $dll(%FiSH_dll,DH1080_gen,NOT_USED) $dll(%FiSH_dll,DH1080_gen,NOT_USED) var %secret_B^a $dll(%FiSH_dll,DH1080_comp, $1 $4) var %secret_A^b $dll(%FiSH_dll,DH1080_comp, $3 $2) if (%secret_B^a != %secret_A^b) goto try_again .timerfind_bad_pubkey 1 1 find_bad_pubkey $1 $2 0 0 echo -s new private/public pairs will be compared against valid pair: $1 $2 return } var %dh.tested $3 , %dh.failed $4 , %private_a $1 , %public_A $2 var %i 256 , %j 0 | while (%i) { tokenize 32 $dll(%FiSH_dll,DH1080_gen,NOT_USED) | inc %dh.tested var %secret_B^a $dll(%FiSH_dll,DH1080_comp, %private_a $2) var %secret_A^b $dll(%FiSH_dll,DH1080_comp, $1 %public_A) if (%secret_B^a != %secret_A^b) { inc %dh.failed | inc %j | break } dec %i } if (%j) { echo -s fail! %i echo 3 -s private_a %private_a echo 3 -s publicA %public_A echo 4 -s private_b $1 echo 4 -s publicB $2 echo 3 -s secret_B^a %secret_B^a echo 3 -s secret_A^b %secret_A^b echo -s fail rate: 1 in $calc(%dh.tested / %dh.failed) ( %dh.failed of %dh.tested ) $asctime ; beep 5 | var %a $input(keep going?,yn) | if (!%a) { .timer $+ $ctimer off | return } write bad_fish_pubkey_pair prv $1 write bad_fish_pubkey_pair pub $2 } .timerfind_bad_pubkey $+ $rand(1,99) 1 5 find_bad_pubkey %private_a %public_A %dh.tested %dh.failed }