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

EVP_CipherFinal usage? #39

Open
taktoa opened this issue Oct 25, 2018 · 4 comments
Open

EVP_CipherFinal usage? #39

taktoa opened this issue Oct 25, 2018 · 4 comments

Comments

@taktoa
Copy link

taktoa commented Oct 25, 2018

When linking HsOpenSSL against libressl-2.7.4, I get a linker warning saying

(.text+0x49fe): warning: EVP_CipherFinal is often misused, please use EVP_CipherFinal_ex and EVP_CIPHER_CTX_cleanup

Should this package be using that function, or is it nothing to worry about?

@vshabanov
Copy link
Collaborator

I'm not the original author and can't say what this error could mean. But I would gladly accept a pull request that removes this warning.

@taktoa
Copy link
Author

taktoa commented Oct 26, 2018

cc @depressed-pho

@taktoa
Copy link
Author

taktoa commented Oct 26, 2018

Seems like it's FFI bound here.

According to the libressl man page,

EVP_CIPHER_CTX_cleanup() clears all information from a cipher context and free up any allocated memory associated with it. It should be called after all operations using a cipher are complete so sensitive information does not remain in memory.

EVP_EncryptFinal(), EVP_DecryptFinal(), and EVP_CipherFinal() are identical to EVP_EncryptFinal_ex(), EVP_DecryptFinal_ex(), and EVP_CipherFinal_ex().
In previous releases of OpenSSL, they also used to clean up the ctx, but this is no longer done and EVP_CIPHER_CTX_cleanup() must be called to free any context resources.

Since we just failIf (/= 1) on the output of EVP_CipherFinal, the context is not being cleaned up correctly, which could leave sensitive information in memory (and it's not like there's a ForeignPtr finalizer doing it separately somewhere -- we don't even have an FFI binding to EVP_CIPHER_CTX_cleanup).

@vshabanov
Copy link
Collaborator

I think it worth to follow libressl advice and replace EVP_CipherFinal with EVP_CipherFinal_ex and EVP_CIPHER_CTX_cleanup but I currently have no time to implement it. If you like you could make a PR and I will release an updated version of HsOpenSSL.

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