Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

write/show/using keys not always agree with enforce_max_key_length #74

Open
maroonbells opened this issue Feb 15, 2021 · 1 comment
Open

Comments

@maroonbells
Copy link
Contributor

In addition to those using enforce_max_key_length setting=0, this issue affects users of CBC mode, regardless whether or not they've enabled the enforce_max_key_length setting.

  • FiSH_WriteKey10 enforces enforce_max_key_length mode only against ECB mode, and only as a flag of whether or not to allow a key longer than length 56.
  • FiSH_WriteKey10 does not actually enforce any maximum length against CBC keys, and when enforce_max_key_length mode is disabled, it doesn't enforce any max length against ECB keys either.
  • When FiSH encrypts or decrypts a message, it silently truncates the key to the length required by a correct interpretation of the enforce_max_key_length rules, possibly without the user being aware of this.
  • FiSH_GetKey10 returns whatever key is stored in the 'key' item inside blow.ini, regardless whether it's invalid or not, or whether that's going to be the actual string used for encrypting/decrypting.

If the intention of enforce_max_key_length is to be something that is only for enabling compatibility with older systems which used ECB-only encryption in order to support having ECB-only key lengths 57-72, that's not obvious from the documentation, nor from the DLL's behavior.

The DLL allows you to create CBC keys of length 57-72 regardless whether enforce_max_key_length is disabled or not. The only effect is whether FiSH_WriteKey10 allows creating ECB keys longer than length 56. A Blowfish key longer than 72 is invalid, but the DLL never forbids creating a CBC key longer than 72, and only forbids doing the same thing for an ECB key when it's also enforcing the not-longer-than-56 limit against ECB keys.

However, when the DLL is encrypting and decrypting messages, it silently applies the max 56 limit to CBC messages regardless of which enforce_max_key_length setting, and silently uses that setting against stored ECB keys to decide whether to truncate at length 56 or 72.

FiSH_GetKey10 is not completely helpful to provide guidance to the user as to the actual key that's being used for encrypting your messages. It simply reports whatever key is stored inside blow.ini, even if it's longer than 72, and even if the key will be silently truncated to length 56. In addition, FiSH_GetKey10 reports the key as a text string, and since mIRC v7's always translate any ANSI text input into a UTF8 string before placing it into a %variable or displaying on screen, there's no way to tell what the actual key is if there are codepoints above 127.

These next examples assume the command is being pasted into the editbox of a channel, in order to have $network and $chan fill the correct parameters.

This next command disables enforcement of the length-56 limit, changing the max limit from 56 to 72 bytes, and shows confirmation that the DLL sees the 56-max mode being disabled:

//dll %FiSH_dll INI_SetBool enforce_max_key_length 0 | echo -a enforce_max_key_length $dll(%FiSH_dll,INI_GetBool,enforce_max_key_length)

Now use this command to write a length 72 CBC key and then get the DLL to tell you what the new key is:

//echo -a $dll(%FiSH_dll,FiSH_WriteKey10,decode_utf8 $network $$chan cbc: $+ $left($str(1234567890,8),56) $+ $left(abcdefghijklmnopqrstuvwxyz,16)) | echo -a $dll(%FiSH_dll,FiSH_GetKey10,$$network $$chan)

result:
cbc:12345678901234567890123456789012345678901234567890123456abcdefghijklmnop

Even though it displays the key as being cbc: followed by a length 72 string, the actual key being used to encrypt the message is the length 72 string silently chopped to 56 bytes, excluding these alphabet letters.

You can confirm this by having 2 different clients each disabling the enforce_max_key_length mode, then each setting their key to a string where the 1st 56 bytes are identical but the next 16 bytes are different, or if one client is using a 56-byte CBC string while the other client uses a 72-byte CBC string where the final 16 bytes are not the same as the 1st 16 bytes. These clients will continue being able to read each other's messages even though FiSH_GetKey10 reports they are using different key strings. That's because the DLL never encrypts a CBC message using a key longer than length 56, even when it's using the enforce_max_key_length=0 mode that's supposed to allow it to create keys as long as 72 bytes.

//echo -a $dll(%FiSH_dll,FiSH_WriteKey10,decode_utf8 $network $$chan $left($str(1234567890,8),56) $+ $left(abcdefghijklmnopqrstuvwxyz,16)) | echo -a $dll(%FiSH_dll,FiSH_GetKey10,$$network $$chan)

result:
12345678901234567890123456789012345678901234567890123456abcdefghijklmnop

However, if you enable differing ECB keys of length 72 by having both clients repeat the above commands slightly altered by removing the cbc: $+ prefix and keeping the 2 clients having identical strings in the first 56 bytes but differing strings within the 16 byte range of the 57th through 72nd bytes, the clients will not be able to read each other's ECB keys while the enforce-56 setting is disabled, because the DLL correctly uses ECB key lengths 57-72 while that setting is set to 0.

In the above examples, FiSH_GetKey10 was reporting whatever key that FiSH_WriteKey10 permitted to be written to blow.ini, instead of reporting whatever key would actually be used. However it will also report invalid lengths longer than 72, which were written to disk. If you edit the above command by changing the '16' into '26', FiSH_WriteKey10 will write the full 56+26=82 length string to disk, and FiSH_GetKey10 will return that entire invalid key, instead of returning the portion it will actually be using, whether silently truncating it to 56 or 72.

//echo -a $dll(%FiSH_dll,FiSH_WriteKey10,decode_utf8 $network $$chan cbc: $+ $left($str(1234567890,8),56) $+ $left(abcdefghijklmnopqrstuvwxyz,26)) | echo -a $dll(%FiSH_dll,FiSH_GetKey10,$$network $$chan)

result:
cbc:12345678901234567890123456789012345678901234567890123456abcdefghijklmnopqrstuvwxyz

If you reset the enforce_max_key_length setting from 0 back to 1, FiSH_WriteKey10 continues to allow you to write strings longer than 72 when writing CBC keys to disk, and it instead enforces the 56 limit only against writing ECB keys to disk.

--

I realize there could be scripting to block the input string from being too long, but it would not be completely accurate without a lot of complex verification steps, which would be even more complex when trying to figure out what kind of ANSI text the DLL would be creating with the decode_utf8 option if there are codepoints above 127, as I reported in the other issue report.

Also, because mIRC alway UTF8-encodes the DLL's output string before it's given to a script or displayed in a window, there's no way to tell from the string returned by FiSH_GetKey10 whether the existing password containing codepoints above 127 is an ANSI string created by decode_utf8 - or if it's a UTF8 string created by raw_bytes. i.e. the 56 and 72 lengths apply to bytes not characters.

Even with using using raw_bytes, the script would need to defend against the possibility that someone tries to use a key containing leading/trailing/consecutive spaces.

--

A summary of the fixes needed:

FiSH_WriteKey10 needs to enforce the 72 as a max length regardless of the enforce_max_key_length setting.

Either the docs need to make it clear that the length 57-72 passwords are available only for legacy compatibility with clients who supported only ECB mode, or the enforce_max_key_length setting should be enforced/relaxed by FiSH_WriteKey10 against CBC keys too, preventing it from writing a key that will be mis-reported by FiSH_GetKey10, and preventing the encrypted message using a key shorter than the string returned by FiSH_GetKey10.

FiSH_GetKey10 should report the actual key being used in channel, as enforced by the current setting for enforce_max_key_length. That means that if a length 72 key is created while enforce_max_key_length=0, and it's later set to =1, the DLL should have FiSH_GetKey10 report the shorter length. Rather than silently using a key that's different than previously written to disk, there could be a popup alert similar to the one when the DLL isn't able to load properly due to SSL settings. Since the actual encrypted output is hidden from the user, it shouldn't have the DLL just output no message to the server while displaying text in the channel as if everything were perfectly fine.

It would be great if there was something similar to FiSH_GetKey10 which reported the key as a hex or mime string (hex would be better), as this would make it easier to discern between a UTF8 key containing codepoint 233 as the 2 bytes 0xc3 0xa9 or the same display string as ANSI text using the single byte 0xe9.

An alternative could be something similar to the current .ini setting cbc=1 which indicates that the encoded key should be used in CBC mode when encrypting the message. This would be something like raw_bytes=X where X would be the boolean 0|1 value which indicates the key was created using the raw_bytes method, or not. I'm guessing the default in its absence would be that the key was created in raw_bytes=1 mode, considering the existing bug in how decode_utf8 handles all codepoints above 255 and some codepoints in the 128-255 range.

@maroonbells
Copy link
Contributor Author

A continuation of the issue of using a different key than written to disk is a rare edge case where the password itself begins with a case-insensitive version of the string "cbc:".

The key is correctly written to disk, but is incorrectly reported by FiSH_GetKey10 and is incorrectly used when encrypting the message. I only discovered this from testing a script that would create random text strings to be used as passwords. I already knew that the current system prevents an ECB key from beginning with the 'cbc:' string, but I wanted to see what would happen if the string following the cbc: also happened to begin with "cbc:".

Because the syntax assumes that the input string beginning with cbc: always means that's a flag that the following string should be used in CBC mode, the ECB key cannot begin with case-insensitive CBC:, and the first 4 letters of the actual key are stripped from the display by FiSH_GetKey10, and is also excluded from being part of the actual key used to encrypt the message.

This table shows 3 strings for each row:

#1: the input string given to FiSH_WriteKey10
#2: the string actually written to disk by FiSH_WriteKey10
#2: the string displayed by FiSH_GetKey10
#3: the string actually used as the encryption key in CBC mode:

#1			#2		#3		#4
CBC:test		test		cbc:test	test
cbc:CbC:test		CbC:test	cbc:test	test
cbc:CbC:Cbc:test	CbC:Cbc:test	cbc:Cbc:test	Cbc:test
cbc:CbC:		cbc:		cbc:		NOT ENCRYPTED

A work-around could be to check if the input string begins with case-insensitive "cbc:cbc:", then it would insert a bogus cbc: in front of it in order to have the correct key actually being used. However that would not solve the issue where FiSH_GetKey10 displays the key incorrectly, and it would risk running afoul of the length 56 limit should FiSH_WriteKey10 start enforcing the limit against CBC keys according to the enforce_max_key_length setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant