Skip to content

Commit 9f050a9

Browse files
author
DV
committed
added test for possible integer overflow within Aes-Cbc-Hmac
1 parent 325a4b1 commit 9f050a9

File tree

2 files changed

+111
-58
lines changed

2 files changed

+111
-58
lines changed

‎aes_cbc_hmac.go

+62-58
Original file line numberDiff line numberDiff line change
@@ -1,108 +1,112 @@
11
package jose
22

33
import (
4-
"errors"
5-
"github.com/dvsekhvalnov/jose2go/arrays"
6-
"github.com/dvsekhvalnov/jose2go/padding"
7-
"crypto/hmac"
8-
"crypto/cipher"
94
"crypto/aes"
5+
"crypto/cipher"
6+
"crypto/hmac"
7+
"errors"
108
"fmt"
9+
"github.com/dvsekhvalnov/jose2go/arrays"
10+
"github.com/dvsekhvalnov/jose2go/padding"
1111
)
1212

1313
// AES CBC with HMAC authenticated encryption algorithm implementation
14-
type AesCbcHmac struct{
14+
type AesCbcHmac struct {
1515
keySizeBits int
1616
}
1717

1818
func init() {
19-
RegisterJwe(&AesCbcHmac{keySizeBits:256})
20-
RegisterJwe(&AesCbcHmac{keySizeBits:384})
21-
RegisterJwe(&AesCbcHmac{keySizeBits:512})
19+
RegisterJwe(&AesCbcHmac{keySizeBits: 256})
20+
RegisterJwe(&AesCbcHmac{keySizeBits: 384})
21+
RegisterJwe(&AesCbcHmac{keySizeBits: 512})
2222
}
2323

2424
func (alg *AesCbcHmac) Name() string {
2525
switch alg.keySizeBits {
26-
case 256: return A128CBC_HS256
27-
case 384: return A192CBC_HS384
28-
default: return A256CBC_HS512
29-
}
26+
case 256:
27+
return A128CBC_HS256
28+
case 384:
29+
return A192CBC_HS384
30+
default:
31+
return A256CBC_HS512
32+
}
3033
}
3134

3235
func (alg *AesCbcHmac) KeySizeBits() int {
3336
return alg.keySizeBits
3437
}
3538

39+
func (alg *AesCbcHmac) SetKeySizeBits(bits int) {
40+
alg.keySizeBits = bits
41+
}
42+
3643
func (alg *AesCbcHmac) Encrypt(aad, plainText, cek []byte) (iv, cipherText, authTag []byte, err error) {
37-
38-
cekSizeBits := len(cek)<<3
44+
45+
cekSizeBits := len(cek) << 3
3946
if cekSizeBits != alg.keySizeBits {
40-
return nil,nil,nil, errors.New(fmt.Sprintf("AesCbcHmac.Encrypt(): expected key of size %v bits, but was given %v bits.",alg.keySizeBits, cekSizeBits))
41-
}
42-
43-
hmacKey := cek[0:len(cek)/2]
44-
aesKey := cek[len(cek)/2:]
45-
46-
if iv,err = arrays.Random(16);err!=nil {
47-
return nil,nil,nil,err
47+
return nil, nil, nil, errors.New(fmt.Sprintf("AesCbcHmac.Encrypt(): expected key of size %v bits, but was given %v bits.", alg.keySizeBits, cekSizeBits))
48+
}
49+
50+
hmacKey := cek[0 : len(cek)/2]
51+
aesKey := cek[len(cek)/2:]
52+
53+
if iv, err = arrays.Random(16); err != nil {
54+
return nil, nil, nil, err
4855
}
49-
56+
5057
var block cipher.Block
5158

52-
if block, err = aes.NewCipher(aesKey);err!=nil {
53-
return nil,nil,nil,err
59+
if block, err = aes.NewCipher(aesKey); err != nil {
60+
return nil, nil, nil, err
5461
}
55-
5662

57-
padded := padding.AddPkcs7(plainText,16)
63+
padded := padding.AddPkcs7(plainText, 16)
5864

59-
cipherText = make([]byte,len(padded),cap(padded))
65+
cipherText = make([]byte, len(padded), cap(padded))
6066
mode := cipher.NewCBCEncrypter(block, iv)
61-
mode.CryptBlocks(cipherText,padded)
62-
67+
mode.CryptBlocks(cipherText, padded)
68+
6369
authTag = alg.computeAuthTag(aad, iv, cipherText, hmacKey)
64-
65-
return iv,cipherText,authTag,nil
66-
}
6770

71+
return iv, cipherText, authTag, nil
72+
}
6873

6974
func (alg *AesCbcHmac) Decrypt(aad, cek, iv, cipherText, authTag []byte) (plainText []byte, err error) {
70-
71-
cekSizeBits := len(cek)<<3
72-
73-
if cekSizeBits != alg.keySizeBits {
74-
return nil, errors.New(fmt.Sprintf("AesCbcHmac.Decrypt(): expected key of size %v bits, but was given %v bits.",alg.keySizeBits, cekSizeBits))
75-
}
76-
77-
hmacKey := cek[0:len(cek)/2]
78-
aesKey := cek[len(cek)/2:]
79-
75+
76+
cekSizeBits := len(cek) << 3
77+
78+
if cekSizeBits != alg.keySizeBits {
79+
return nil, errors.New(fmt.Sprintf("AesCbcHmac.Decrypt(): expected key of size %v bits, but was given %v bits.", alg.keySizeBits, cekSizeBits))
80+
}
81+
82+
hmacKey := cek[0 : len(cek)/2]
83+
aesKey := cek[len(cek)/2:]
84+
8085
// Check MAC
81-
expectedAuthTag := alg.computeAuthTag(aad, iv, cipherText, hmacKey);
86+
expectedAuthTag := alg.computeAuthTag(aad, iv, cipherText, hmacKey)
8287

83-
if !hmac.Equal(expectedAuthTag, authTag) {
84-
return nil,errors.New("AesCbcHmac.Decrypt(): Authentication tag do not match.")
88+
if !hmac.Equal(expectedAuthTag, authTag) {
89+
return nil, errors.New("AesCbcHmac.Decrypt(): Authentication tag do not match.")
8590
}
8691

8792
var block cipher.Block
8893

89-
if block, err = aes.NewCipher(aesKey);err==nil {
94+
if block, err = aes.NewCipher(aesKey); err == nil {
9095
mode := cipher.NewCBCDecrypter(block, iv)
91-
92-
var padded []byte=make([]byte, len(cipherText), cap(cipherText))
96+
97+
var padded []byte = make([]byte, len(cipherText), cap(cipherText))
9398
mode.CryptBlocks(padded, cipherText)
94-
95-
return padding.RemovePkcs7(padded,16),nil
99+
100+
return padding.RemovePkcs7(padded, 16), nil
96101
}
97-
98-
return nil,err
102+
103+
return nil, err
99104
}
100105

101106
func (alg *AesCbcHmac) computeAuthTag(aad []byte, iv []byte, cipherText []byte, hmacKey []byte) (signature []byte) {
102-
al := arrays.UInt64ToBytes(uint64(len(aad) << 3));
107+
al := arrays.UInt64ToBytes(uint64(len(aad) << 3))
103108
hmacInput := arrays.Concat(aad, iv, cipherText, al)
104-
hmac :=calculateHmac(alg.keySizeBits, hmacInput, hmacKey);
109+
hmac := calculateHmac(alg.keySizeBits, hmacInput, hmacKey)
105110

106-
return hmac[0:len(hmac)/2];
111+
return hmac[0 : len(hmac)/2]
107112
}
108-

‎sec_test/security_vulnerabilities_test.go

+49
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"crypto/ecdsa"
55
"fmt"
66
"github.com/dvsekhvalnov/jose2go"
7+
"github.com/dvsekhvalnov/jose2go/arrays"
78
"github.com/dvsekhvalnov/jose2go/keys/ecc"
89
. "gopkg.in/check.v1"
910
"testing"
@@ -44,6 +45,54 @@ func (s *SecurityTestSuite) Test_InvalidCurve(c *C) {
4445
c.Assert(test, Equals, "")
4546
}
4647

48+
func (s *SecurityTestSuite) Test_AAD_IntegerOverflow(c *C) {
49+
//Borrowed test case from https://bitbucket.org/b_c/jose4j/commits/b79e67c13c23
50+
51+
cek := []byte{57, 188, 52, 101, 199, 208, 135, 76, 159, 67, 65, 71, 196, 136, 137, 113, 227, 232, 28, 1, 61, 157, 73, 156, 68, 103, 67, 250, 215, 162, 181, 161}
52+
53+
aad := []byte{0, 1, 2, 3, 4, 5, 6, 7}
54+
plainText := make([]byte, 536870928, 536870928)
55+
56+
//generate random plaintext
57+
for i := 0; i < len(plainText); i += 8 {
58+
bytes := arrays.UInt64ToBytes(uint64(i))
59+
plainText[i] = bytes[0]
60+
plainText[i+1] = bytes[1]
61+
plainText[i+2] = bytes[2]
62+
plainText[i+3] = bytes[3]
63+
plainText[i+4] = bytes[4]
64+
plainText[i+5] = bytes[5]
65+
plainText[i+6] = bytes[6]
66+
plainText[i+7] = bytes[7]
67+
}
68+
69+
enc := &jose.AesCbcHmac{}
70+
enc.SetKeySizeBits(256)
71+
72+
iv, cipherText, authTag, _ := enc.Encrypt(aad, plainText, cek)
73+
74+
// Now shift aad and ciphertext around so that HMAC doesn't change,
75+
// but the plaintext will change.
76+
77+
buffer := arrays.Concat(aad, iv, cipherText)
78+
79+
// Note that due to integer overflow 536870920 * 8 = 64
80+
newAadSize := 536870920
81+
82+
newAad := buffer[0:newAadSize]
83+
newIv := buffer[newAadSize : newAadSize+16]
84+
newCipherText := buffer[newAadSize+16:]
85+
86+
//decrypt shifted binary, it should fail, since content is different now
87+
test, err := enc.Decrypt(newAad, cek, newIv, newCipherText, authTag)
88+
89+
//if we reach that point HMAC check was bypassed although the decrypted data is different
90+
91+
c.Assert(err, NotNil)
92+
fmt.Printf("\nerr= %v\n", err)
93+
c.Assert(test, IsNil)
94+
}
95+
4796
func Ecc256() *ecdsa.PrivateKey {
4897
return ecc.NewPrivate([]byte{193, 227, 73, 203, 97, 236, 112, 36, 140, 232, 1, 3, 76, 56, 52, 225, 184, 142, 190, 17, 97, 203, 37, 175, 56, 116, 31, 120, 95, 207, 196, 196},
4998
[]byte{123, 201, 103, 8, 239, 128, 149, 43, 83, 248, 210, 85, 95, 231, 43, 132, 30, 208, 69, 136, 98, 139, 29, 55, 138, 89, 73, 57, 80, 14, 201, 201},

0 commit comments

Comments
 (0)