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

OpenSSL's EVP_* interface not used correctly? #213

Open
dnnr opened this issue Mar 3, 2015 · 2 comments
Open

OpenSSL's EVP_* interface not used correctly? #213

dnnr opened this issue Mar 3, 2015 · 2 comments

Comments

@dnnr
Copy link

dnnr commented Mar 3, 2015

I'm not entirely sure on this, but it seems to me that the use of EVP_EncryptUpdate in attic's AES.encrypt is missing a corresponding call to EVP_EncryptFinal. As far as I can tell, the documentation mandates such a call. However, its absence doesn't seem to cause any issues, which might be due to the nature of selected cipher. If that's actually the case, there should probably be a comment somewhere, at least to caution anyone intending to change the cipher at some point in the future.

And another thing I was wondering: the OpenSSL docs say that EVP_EncryptUpdate should be provided with an output buffer that is at least inl + cipher_block_size - 1 long, whereas attic only uses inl as the output buffer size. Again, this might be fine in conjunction with CTR mode, but I couldn't find any definitive statement on that (on OpenSSL's part) so I'm not sure if it's okay for attic to rely on this behavior.

@ThomasWaldmann
Copy link
Contributor

That additionally required buffer space they tell sounds like space needed for padding, IF the cipher uses padding (ctr mode does not).

BUT, I think then it should be cipher_block_size more than inl (without that -1) because padding is also usually used when inl is precisely a multiple of the block size because then a full block just with padding is added (AFAIK).

Update: I verified this and found it to be really a documentation bug. Notified openssl-security contact about it.

Update: got reply from openssl-security. read docs again. it was a misunderstanding of mine, they document inl + cipher_block_size - 1 only for the EVP_EncryptUpdate call. For EVP_EncryptFinal they tell about cipher_block_size. So, when you use them both together into the same buffer (as I did), it ends up being inl + cipher_block_size. So both the docs and my assumption for the total buffer size are correct.

@ThomasWaldmann
Copy link
Contributor

Fixed + cleaned up, see PR #214.

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

2 participants