Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Trailing bytes don't appear to work correctly #1

Open
vlionix opened this Issue · 8 comments

3 participants

vlion Nathan Froyd Camille Troillard
vlion

When I have a message with bytes that are not multiples of the cipher's block length, the trailing bytes are not encrypted.

I understand that this is an aspect of block ciphers, but it would be real nice if an error was thrown indicating "Hey, your block length needs to be a multiple of n".

Nathan Froyd
Owner

I agree that this is a problem; I've had several people complain about this as a bona fide bug. I think throwing an error would be a poor way of handling this, though. A different API (say, one that specified the number of blocks to encrypt) would be a better solution, I think: you're preventing the user from even making an error in the first place.

OTOH, an API that only lets you encrypt in full blocks means that the user must handle buffering if a block cipher + padding is being used, which is IMHO not very friendly. Encrypting in blocks is also a bit odd for stream ciphers.

I'd rather not have an API for full block encryption and an API for byte-wise encryption. Suggestions for better API design are welcome.

vlion

I think that silently failing to encrypt is a genuine bug and should always raise a condition.

This is what's currently there:

(ironclad:make-cipher :blowfish  :mode :ecb  :key thekey))

However, we know that Blowfish is a block cipher. Ergo, if we pass in trailing data, Blowfish won't handle it, so what might be a good idea is:

(ironclad:make-cipher :blowfish :mode :ecb :key thekey :allow-trailing)

where allow-trailing suppresses the error. :allow-trailing, however, is ignored (or raises condition) when it is used with a stream cipher.

Possibly, too, make-cipher could return the values cipher, block-byte-size (where block-byte-size is determined by the cipher).

This is me ruminating on what would be useful for my code, I haven't delved into ironclad's code significantly.

vlion

Somehow github closed the issue!?

Nathan Froyd
Owner

I don't know whether you hit the "Comment and Close" button or I did. I've reopened the issue.

I think raising a condition on non-block aligned inputs for ECB and CBC mode would be reasonable. I'll look at implementing that.

Camille Troillard

Hi,
I just stumbled over this issue, and was relieved to find this information.
Ironclad is really a very nice library, but I wished I was warned that my input data was not of the proper size!
Best,
Cam

Camille Troillard

I am not totally sure if the following is related to the current issue, but it is fairly annoying as the encrypted data can not always be decrypted with OpenSSL.

I am encoding bytes with the Blowfish / ECB cipher and 128 bits keys.
Because the input needs to be padded, I adjust the length of the arrays to the next block boundary (modulo 8 bytes). So far so good.

If my input is 11 bytes, the result is 16 bytes, and all is well, I can decrypt the result using the EVP_Decrypt* functions in OpenSSL. However, I can't get an array of 128 bytes to be properly encrypted. I believe there should be no padding since 128 is a multiple of 8.

After decrypting the first 120 bytes, OpenSSL fails at finalization with:
69979:error:06065064:digital envelope routines:EVP_DecryptFinal:bad decrypt:evp_enc.c:509:

I was wondering if this could be a bug in ironclad?
It's difficult to tell, thank you in advance for your help!

Nathan Froyd
Owner
froydnj commented

@tuscland If you could open a separate issue for the OpenSSL problem, that'd be appreciated. Including an example key, plaintext, and ciphertext produced by OpenSSL would be helpful. Thanks!

Camille Troillard

Sure Nathan, I will send an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.