Fix s390x AES OFB/CFB cipher implementation on updating IV#16291
Fix s390x AES OFB/CFB cipher implementation on updating IV#16291ifranzki wants to merge 2 commits intoopenssl:masterfrom
Conversation
Copy the current IV from the cipher context into the kmo/kmf param before the operation, and copy the modified IV back to the context afterwards. Without this, an application that obtains the running IV from the context would still get the original IV, but not the updated one. This implementation in e_aes.c now matches the code in cipher_aes_hw_s390x.inc that is used for the provider implementation. Signed-off-by: Ingo Franzki <[email protected]>
|
Please add @p-steuer @juergenchrist as reviewer. |
|
PR for OpenSSL 1.1.1: #16292 |
|
When fixing the provider we could not figure out how to use these implementations. So we just did not fix them. But in principle I agree with you about fixing them. Either that or removing them if they really are no longer needed. |
In theory then can be reached by using the |
t8m
added
branch: master
|
Looks like the test I added fails because it does not find the cipher it uses in amny cases. Could it be that aes-128-cfb/ofb/etc. are not available in the default provider? Should I change it to aes-256 instead? |
This is not the cause, please look at other tests in the evp_extra_test.c - you have to use |
ifranzki
force-pushed
the
aes-ofb-cfb-s390x-fix
branch
from
d1fe78c to
8a37b85
Compare
Ah yes, sure. Changed. |
Ensure that an EVP_CipherUpdate operation updates the context's IV for AES CBC, CFB, OFB, and CTR. An application can get the updated IV via EVP_CIPHER_CTX_iv(). The s390x implementation of the CFB and OFB ciphers in e_aes.c did not update the IV in the context, but only within its s390x specific context data. Signed-off-by: Ingo Franzki <[email protected]>
ifranzki
force-pushed
the
aes-ofb-cfb-s390x-fix
branch
from
8a37b85 to
e1470e5
Compare
|
Can someone please approve the workflows, so that the checks can run again? |
p-steuer
added
approval: done
openssl-machine
added
approval: ready to merge
|
This pull request is ready to merge |
Copy the current IV from the cipher context into the kmo/kmf param before the operation, and copy the modified IV back to the context afterwards. Without this, an application that obtains the running IV from the context would still get the original IV, but not the updated one. This implementation in e_aes.c now matches the code in cipher_aes_hw_s390x.inc that is used for the provider implementation. Signed-off-by: Ingo Franzki <[email protected]> Reviewed-by: Patrick Steuer <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16291)
Ensure that an EVP_CipherUpdate operation updates the context's IV for AES CBC, CFB, OFB, and CTR. An application can get the updated IV via EVP_CIPHER_CTX_iv(). The s390x implementation of the CFB and OFB ciphers in e_aes.c did not update the IV in the context, but only within its s390x specific context data. Signed-off-by: Ingo Franzki <[email protected]> Reviewed-by: Patrick Steuer <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16291)
|
Merged to master. Thank you for your contribution. |
6 participants
The s390x specific implementation in
crypto/evp/e_aes.cdoes not update the cipher context's IV field during an operation for AES OFB and CFB.An application that uses
EVP_CIPHER_CTX_iv()to get the updated IV would thus always get the original IV value on s390x. Other platforms are not affected as far as I can tell.Please note that the provider based implementation in
providers/implementations/ciphers/cipher_aes_hw_s390x.incdoes it correctly already. The fix ine_aes.caligns the code to the code incipher_aes_hw_s390x.inc.Because the provider based implementation is correct, an application using OpenSSL 3.0 will not see the error. However, the same error is in the OpenSSL 1.1.1 branch, and with OpenSSL 1.1.1 the error can be seen by an application (on s390x). I would suggest to fix this anyway in the master branch, since the code in
e_aes.cis wrong, too.I have added a testcase in
test/evp_extra_test.cto reproduce this error. Of course, it won't fail with OpenSSL 3.0, because the code goes the provider path. I don't know how the code ine_aes.ccould be tested in OpenSSL 3.0....I will submit another PR for the same fix in the OpenSSL 1.1.1 branch.