Bug 1979100
Summary: | FIPS mode AES CBC Decrypter produces incorrect result | |||
---|---|---|---|---|
Product: | Red Hat Enterprise Linux 8 | Reporter: | Di Wu <di.wu> | |
Component: | golang | Assignee: | Derek Parker <deparker> | |
Status: | CLOSED ERRATA | QA Contact: | Edjunior Barbosa Machado <emachado> | |
Severity: | high | Docs Contact: | Eva-Lotte Gebhardt <egebhard> | |
Priority: | unspecified | |||
Version: | 8.4 | CC: | asm, cww, dbelyavs, dbenoit, deparker, emachado, sahana, tschelle, tstellar | |
Target Milestone: | beta | Keywords: | Bugfix, Reopened, Triaged, ZStream | |
Target Release: | --- | Flags: | pm-rhel:
mirror+
|
|
Hardware: | All | |||
OS: | All | |||
Whiteboard: | ||||
Fixed In Version: | go-toolset-rhel8-8050020210716084948.8aa62369 | Doc Type: | Bug Fix | |
Doc Text: |
.Incorrect file decryption using OpenSSL `aes-cbc` mode
The OpenSSL EVP `aes-cbc` mode did not decrypt files correctly, because it expects to handle padding while the Go CryptoBlocks interface expects full blocks. This issue has been fixed by disabling padding before executing EVP operations in OpenSSL.
|
Story Points: | --- | |
Clone Of: | ||||
: | 1983976 (view as bug list) | Environment: | ||
Last Closed: | 2021-11-09 17:41:36 UTC | Type: | Bug | |
Regression: | --- | Mount Type: | --- | |
Documentation: | --- | CRM: | ||
Verified Versions: | Category: | --- | ||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | ||
Cloudforms Team: | --- | Target Upstream Version: | ||
Embargoed: | ||||
Bug Depends On: | ||||
Bug Blocks: | 1983976 |
Description
Di Wu
2021-07-05 01:57:08 UTC
I believe this is essentially a duplicate of rhbz#1972825 just in the opposite direction (decrypt vs encrypt). Can you verify that the patch proposed in the linked bug fixes this issue as well? The patch fixes an issue where the IV was getting incorrectly re-initialized across CryptBlocks calls. *** This bug has been marked as a duplicate of bug 1972825 *** No, this is not a duplicate because I pre-applied the other fix for the encryption, and then tested this and found the failure. Try this test case on top of the build with the other fix and you'll see the failure. It is independent of the encryption bug. Thank you for the clarification. I will reopen this bug and investigate. I have a proposed fix for this, which seem to be due to OpenSSL EVP enabling padding by default in its cipher context and automatically trying to unpad results. The proposed fix is: in goopenssl.h, we need to expose the EVP_CIPHER_CTX_set_padding c function ``` 650a651 > DEFINEFUNC(int, EVP_CIPHER_CTX_set_padding, (EVP_CIPHER_CTX *ctx, int padding), (ctx, padding)) ``` and in aes.go, as part of NewCBCDecrypter, disable padding after CipherInit ``` 237a238,240 > if C.int(1) != C._goboringcrypto_EVP_CIPHER_CTX_set_padding(x.ctx, 0) { > panic("cipher: unable to disable EVP cipher ctx padding") > } ``` This will get the unit test to pass correctly. (In reply to Di Wu from comment #14) > I have a proposed fix for this, which seem to be due to OpenSSL EVP enabling > padding by default in its cipher context and automatically trying to unpad > results. > > The proposed fix is: > in goopenssl.h, we need to expose the EVP_CIPHER_CTX_set_padding c function > ``` > 650a651 > > DEFINEFUNC(int, EVP_CIPHER_CTX_set_padding, (EVP_CIPHER_CTX *ctx, int padding), (ctx, padding)) > ``` > and in aes.go, as part of NewCBCDecrypter, disable padding after CipherInit > ``` > 237a238,240 > > if C.int(1) != C._goboringcrypto_EVP_CIPHER_CTX_set_padding(x.ctx, 0) { > > panic("cipher: unable to disable EVP cipher ctx padding") > > } > ``` > > This will get the unit test to pass correctly. Thank you, sorry there is a lot of discussion on this bug that has been Red Hat internal, but we came to that same solution and have already submitted this fix: https://pagure.io/go/c/49e7e3014ebb01378d3093bbb6dc75761ae1284a?branch=go1.16-openssl-fips. When will we get a new released RPM with both the encryption and decryption bug fixes? Di, we're planning to fix this bug in the next release. Hi guys, I'm pretty certain the proposed fix also need to be applied identically to the AES ECB decryption code path. I have a test case locally that uses AES ECB and it's seeing the exact same issue (unnecessary unpadding). We need something like this please: ``` if c.dec_ctx == nil { c.dec_ctx = C._goboringcrypto_EVP_CIPHER_CTX_new() if c.dec_ctx == nil { panic("cipher: unable to create EVP cipher ctx") } k := (*C.uchar)(unsafe.Pointer(&c.key[0])) if C.int(1) != C._goboringcrypto_EVP_CipherInit_ex(c.dec_ctx, c.cipher, nil, k, nil, C.GO_AES_DECRYPT) { panic("cipher: unable to initialize EVP cipher ctx") } if C.int(1) != C._goboringcrypto_EVP_CIPHER_CTX_set_padding(c.dec_ctx, 0) { panic("cipher: unable to disable EVP cipher ctx padding") } } ``` Take the `c.dec_ctx` and also disable the padding, or else ECB mode decryption is also broken. Thanks, and please bundle this into the same release if possible so all of these decryption issues are fixed together. Hi guys, I see that there's a new RPM released, can we have the new fixed Golang rpm installed into a new version of the UBI8 Golang image? https://catalog.redhat.com/software/containers/rhel8/go-toolset/5b9c810add19c70b45cbd666 This is the RHEL8 UBI that we use. Thanks, Di Hi Derek, this is the first draft for this RN. Please let me know, if and what needs to be changed: .Incorrect file decryption using OpenSSL `EVP_aes-cbc` Currently, the `EVP_aes-cbc` interface of the OpenSSL library does not deprypt files correctly. To work around this issue, disable padding for the `aes-cbc` decryption. As a result, files are decripted correctly. (In reply to Eva-Lotte Gebhardt from comment #31) > Hi Derek, > > this is the first draft for this RN. Please let me know, if and what needs > to be changed: > > > .Incorrect file decryption using OpenSSL `EVP_aes-cbc` > > Currently, the `EVP_aes-cbc` interface of the OpenSSL library does not > deprypt files correctly. To work around this issue, disable padding for the > `aes-cbc` decryption. As a result, files are decripted correctly. I would just say: "OpenSSL aes-cbc mode decrypt expects to handle padding, but Go CryptoBlocks interface expects full blocks. This issue has been fixed by disabling padding before executing EVP operations in OpenSSL". Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (Moderate: go-toolset:rhel8 security, bug fix, and enhancement update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHSA-2021:4156 The needinfo request[s] on this closed bug have been removed as they have been unresolved for 120 days |