RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1979100 - FIPS mode AES CBC Decrypter produces incorrect result
Summary: FIPS mode AES CBC Decrypter produces incorrect result
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: golang
Version: 8.4
Hardware: All
OS: All
unspecified
high
Target Milestone: beta
: ---
Assignee: Derek Parker
QA Contact: Edjunior Barbosa Machado
Eva-Lotte Gebhardt
URL:
Whiteboard:
Depends On:
Blocks: 1983976
TreeView+ depends on / blocked
 
Reported: 2021-07-05 01:57 UTC by Di Wu
Modified: 2023-09-18 00:28 UTC (History)
9 users (show)

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.
Clone Of:
: 1983976 (view as bug list)
Environment:
Last Closed: 2021-11-09 17:41:36 UTC
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github openssl openssl issues 16351 0 None closed Possible AES-CBC decryption regression between 1.1.1 and 3.0 2022-10-24 08:06:55 UTC
Red Hat Product Errata RHSA-2021:4156 0 None None None 2021-11-09 17:43:32 UTC

Description Di Wu 2021-07-05 01:57:08 UTC
Description of problem:
When in FIPS mode GOLANG_FIPS=1, AES CBC block decrypter produces incorrect result on all but the first call to CryptBlocks(). See test example.

```
func TestDecryptSimple(t *testing.T) {
	key := []byte{
		0x24, 0xcd, 0x8b, 0x13, 0x37, 0xc5, 0xc1, 0xb1,
		0x0, 0xbb, 0x27, 0x40, 0x4f, 0xab, 0x5f, 0x7b,
		0x2d, 0x0, 0x20, 0xf5, 0x1, 0x84, 0x4, 0xbf,
		0xe3, 0xbd, 0xa1, 0xc4, 0xbf, 0x61, 0x2f, 0xc5,
	}
	block, err := aes.NewCipher(key)
	if err != nil {
		panic(err)
	}
	iv := []byte{
		0x91, 0xc7, 0xa7, 0x54, 0x52, 0xef, 0x10, 0xdb,
		0x91, 0xa8, 0x6c, 0xf9, 0x79, 0xd5, 0xac, 0x74,
	}

	plainText := []byte{
		0x54, 0x68, 0x65, 0x72, 0x65, 0x20, 0x69, 0x73,
		0x20, 0x6f, 0x6e, 0x6c, 0x79, 0x20, 0x6f, 0x6e,
		0x65, 0x20, 0x4c, 0x6f, 0x72, 0x64, 0x20, 0x6f,
		0x66, 0x20, 0x74, 0x68, 0x65, 0x20, 0x52, 0x69,
		0x6e, 0x67, 0x2c, 0x20, 0x6f, 0x6e, 0x6c, 0x79,
		0x20, 0x6f, 0x6e, 0x65, 0x20, 0x77, 0x68, 0x6f,
		0x20, 0x63, 0x61, 0x6e, 0x20, 0x62, 0x65, 0x6e,
		0x64, 0x20, 0x69, 0x74, 0x20, 0x74, 0x6f, 0x20,
		0x68, 0x69, 0x73, 0x20, 0x77, 0x69, 0x6c, 0x6c,
		0x2e, 0x20, 0x41, 0x6e, 0x64, 0x20, 0x68, 0x65,
		0x20, 0x64, 0x6f, 0x65, 0x73, 0x20, 0x6e, 0x6f,
		0x74, 0x20, 0x73, 0x68, 0x61, 0x72, 0x65, 0x20,
		0x70, 0x6f, 0x77, 0x65, 0x72, 0x2e, 0x00, 0x00,
		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
	}
	cipherText := make([]byte, len(plainText))
	encrypter := cipher.NewCBCEncrypter(block, iv)
	encrypter.CryptBlocks(cipherText, plainText)
	fmt.Println(cipherText)

	fmt.Println("plaintext head", plainText[:16])
	fmt.Println("plaintext tail", plainText[96:])
	fmt.Println("ciphertext head", cipherText[:16])
	fmt.Println("ciphertext tail", cipherText[96:])

	expectedCipherText := []byte{
		23, 51, 192, 210, 170, 124, 30, 218,
		176, 54, 70, 132, 141, 124, 3, 152,
		47, 3, 37, 81, 187, 101, 197, 94,
		11, 38, 128, 60, 112, 20, 235, 130,
		111, 236, 176, 99, 121, 6, 221, 181,
		190, 228, 150, 177, 218, 3, 196, 0,
		5, 141, 169, 151, 3, 161, 64, 244,
		231, 237, 252, 143, 111, 37, 68, 70,
		11, 137, 220, 243, 195, 90, 182, 83,
		96, 80, 122, 14, 93, 178, 62, 159,
		25, 162, 200, 155, 21, 150, 6, 101,
		21, 234, 12, 74, 190, 213, 159, 220,
		111, 184, 94, 169, 188, 93, 38, 150,
		3, 208, 185, 201, 212, 246, 238, 181,
	}
	if bytes.Compare(expectedCipherText, cipherText) != 0 {
		t.Fail()
	} else {
		fmt.Println("ciphertext matches expected")
	}

	firstHalf := make([]byte, 64)
	secondHalf := make([]byte, 48)
	decrypter := cipher.NewCBCDecrypter(block, iv)
	// first call decrypts correctly
	decrypter.CryptBlocks(firstHalf, cipherText[:64])
	// subsequent call decrypts incorrectly
	decrypter.CryptBlocks(secondHalf, cipherText[64:])
	decrypted := make([]byte, 0)
	decrypted = append(decrypted, firstHalf...)
	decrypted = append(decrypted, secondHalf...)
	if len(decrypted) != 112 {
		t.Fail()
	}

	fmt.Println("decrypted head", decrypted[:16])
	fmt.Println("decrypted tail", decrypted[96:])

	if bytes.Compare(plainText[:64], firstHalf) != 0 {
		fmt.Printf("first half incorrect\nexpected %v, got %v\n", plainText[:64], firstHalf)
		t.Fail()
	} else {
		fmt.Println("first half decrypted correctly")
	}

	if bytes.Compare(plainText[64:], secondHalf) != 0 {
		fmt.Printf("second half incorrect\nexpected %v, got %v\n", plainText[64:], secondHalf)
		t.Fail()
	} else {
		fmt.Println("second half decrypted correctly")
	}
}
```

Version-Release number of selected component (if applicable):
go version go1.15.13 linux/amd64

How reproducible:
Run the unit test with FIPS mode on vs off, test passes when FIPS is off but fails when FIPS is on.
```
GOLANG_FIPS=0 go test -v -run TestDecryptSimple
# pass
GOLANG_FIPS=1 go test -v -run TestDecryptSimple
# fail
```

Steps to Reproduce:
1. Turn on FIPS mode with env variable GOLANG_FIPS=1 
2. Run the decryption test
3. Verify that test passes

Actual results:
```
=== RUN   TestDecryptSimple
[23 51 192 210 170 124 30 218 176 54 70 132 141 124 3 152 47 3 37 81 187 101 197 94 11 38 128 60 112 20 235 130 111 236 176 99 121 6 221 181 190 228 150 177 218 3 196 0 5 141 169 151 3 161 64 244 231 237 252 143 111 37 68 70 11 137 220 243 195 90 182 83 96 80 122 14 93 178 62 159 25 162 200 155 21 150 6 101 21 234 12 74 190 213 159 220 111 184 94 169 188 93 38 150 3 208 185 201 212 246 238 181]
plaintext head [84 104 101 114 101 32 105 115 32 111 110 108 121 32 111 110]
plaintext tail [112 111 119 101 114 46 0 0 0 0 0 0 0 0 0 0]
ciphertext head [23 51 192 210 170 124 30 218 176 54 70 132 141 124 3 152]
ciphertext tail [111 184 94 169 188 93 38 150 3 208 185 201 212 246 238 181]
ciphertext matches expected
decrypted head [84 104 101 114 101 32 105 115 32 111 110 108 121 32 111 110]
decrypted tail [32 100 111 101 115 32 110 111 116 32 115 104 97 114 101 32]
first half decrypted correctly
second half incorrect
expected [104 105 115 32 119 105 108 108 46 32 65 110 100 32 104 101 32 100 111 101 115 32 110 111 116 32 115 104 97 114 101 32 112 111 119 101 114 46 0 0 0 0 0 0 0 0 0 0], got [32 99 97 110 32 98 101 110 100 32 105 116 32 116 111 32 104 105 115 32 119 105 108 108 46 32 65 110 100 32 104 101 32 100 111 101 115 32 110 111 116 32 115 104 97 114 101 32]
--- FAIL: TestDecryptSimple (0.00s)
FAIL
```

Expected results:
```
=== RUN   TestDecryptSimple
[23 51 192 210 170 124 30 218 176 54 70 132 141 124 3 152 47 3 37 81 187 101 197 94 11 38 128 60 112 20 235 130 111 236 176 99 121 6 221 181 190 228 150 177 218 3 196 0 5 141 169 151 3 161 64 244 231 237 252 143 111 37 68 70 11 137 220 243 195 90 182 83 96 80 122 14 93 178 62 159 25 162 200 155 21 150 6 101 21 234 12 74 190 213 159 220 111 184 94 169 188 93 38 150 3 208 185 201 212 246 238 181]
plaintext head [84 104 101 114 101 32 105 115 32 111 110 108 121 32 111 110]
plaintext tail [112 111 119 101 114 46 0 0 0 0 0 0 0 0 0 0]
ciphertext head [23 51 192 210 170 124 30 218 176 54 70 132 141 124 3 152]
ciphertext tail [111 184 94 169 188 93 38 150 3 208 185 201 212 246 238 181]
ciphertext matches expected
decrypted head [84 104 101 114 101 32 105 115 32 111 110 108 121 32 111 110]
decrypted tail [112 111 119 101 114 46 0 0 0 0 0 0 0 0 0 0]
first half decrypted correctly
second half decrypted correctly
--- PASS: TestDecryptSimple (0.00s)
PASS
```

Additional info:
I did some debugging and it feels like there's a bug with the cipher context EVP_CIPHER_CTX not carrying correct chaining across each _goboringcrypto_EVP_CipherUpdate call

Comment 1 Derek Parker 2021-07-12 16:28:19 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 ***

Comment 2 Di Wu 2021-07-12 16:50:36 UTC
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.

Comment 3 Derek Parker 2021-07-12 17:06:17 UTC
Thank you for the clarification. I will reopen this bug and investigate.

Comment 14 Di Wu 2021-07-16 17:25:39 UTC
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.

Comment 15 Derek Parker 2021-07-16 18:10:39 UTC
(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.

Comment 16 Di Wu 2021-07-16 21:35:38 UTC
When will we get a new released RPM with both the encryption and decryption bug fixes?

Comment 25 Tilmann Scheller 2021-07-20 11:06:44 UTC
Di, we're planning to fix this bug in the next release.

Comment 29 Di Wu 2021-07-22 16:49:04 UTC
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.

Comment 30 Di Wu 2021-08-05 20:47:10 UTC
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

Comment 31 Eva-Lotte Gebhardt 2021-08-18 15:26:59 UTC
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.

Comment 36 Derek Parker 2021-09-10 21:52:35 UTC
(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".

Comment 40 errata-xmlrpc 2021-11-09 17:41:36 UTC
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

Comment 41 Red Hat Bugzilla 2023-09-18 00:28:03 UTC
The needinfo request[s] on this closed bug have been removed as they have been unresolved for 120 days


Note You need to log in before you can comment on or make changes to this bug.