From 40ce7865a7da4284a23000332f77b0f529e31b39 Mon Sep 17 00:00:00 2001 From: Keith Fong Date: Thu, 3 Sep 2026 13:59:07 +0800 Subject: [PATCH 1/4] feat: add AES-CBC support The G+D CII perso request for the card vending machine needs AES-CBC, so the library needs a CBC mode alongside the existing GCM wrapper. CBCCipher is a separate type from Cipher rather than a mode flag on it, because it offers a materially weaker guarantee: CBC gives confidentiality only and does not authenticate. Keeping the types apart means a caller cannot reach CBC by accident, and the obligation to authenticate the IV and ciphertext separately is documented where it will be read. PKCS#7 padding is opt-in through EncryptPadded and DecryptPadded. Padding is a choice made by the surrounding protocol rather than a property of CBC, so Encrypt and Decrypt leave block-aligned input alone. The IV is always caller-supplied and is never prefixed to or stripped from the ciphertext, for the same reason. GenerateCBCIV is named to keep it clearly distinct from the GCM nonce that Cipher generates internally. The two are not interchangeable. The key check value logic moves into shared helpers so that a key reports the same value under either mode. --- README.md | 59 +++++ VERSION.txt | 2 +- aes/aes_cbc_cipher.go | 155 +++++++++++++ aes/aes_cbc_cipher_test.go | 457 +++++++++++++++++++++++++++++++++++++ aes/aes_cbc_factory.go | 63 +++++ aes/aes_cipher.go | 23 +- aes/pkcs7.go | 65 ++++++ aes/pkcs7_test.go | 167 ++++++++++++++ 8 files changed, 986 insertions(+), 5 deletions(-) create mode 100644 aes/aes_cbc_cipher.go create mode 100644 aes/aes_cbc_cipher_test.go create mode 100644 aes/aes_cbc_factory.go create mode 100644 aes/pkcs7.go create mode 100644 aes/pkcs7_test.go diff --git a/README.md b/README.md index c569758..5f71639 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,65 @@ This repo provides a list of utility modules for common crypto operations. - encrypt & decrypt methods, the output ciphertext is prefixed with the random nonce. - verify the constructed cipher against the check value +Prefer AES-GCM. Only reach for AES-CBC below when an external protocol mandates it. + +### AES-CBC + +`CBCCipher` is provided for interoperability with external protocols that mandate +CBC. It is a separate type from the AES-GCM `Cipher` because it offers a +materially weaker guarantee. + +> **AES-CBC provides confidentiality only. It is not authenticated.** +> +> - A modified initialisation vector or ciphertext will usually still decrypt +> **without error**, producing attacker-influenced plaintext. Decryption +> succeeding is not evidence that a message is genuine. +> - Callers **must** authenticate the initialisation vector together with the +> ciphertext by separate means (for example an HMAC over both) and **must** +> verify it before decrypting. +> - A PKCS#7 padding error is **not** an integrity check. Never surface padding +> failures to an untrusted caller: doing so exposes a +> [padding oracle](https://en.wikipedia.org/wiki/Padding_oracle_attack) that +> can be used to recover plaintext. + +- factory methods to construct an AES-CBC cipher from the raw key bytes or hex text +- `Encrypt` / `Decrypt` operate on block-aligned input with no padding +- `EncryptPadded` / `DecryptPadded` apply and remove PKCS#7 padding (RFC 5652 §6.3) +- `GenerateCBCIV` returns a random 16-byte initialisation vector +- verify the constructed cipher against the check value; the value depends only + on the key, so it matches the one `Cipher` reports for the same key + +Parameters: + +| Parameter | Requirement | +| --- | --- | +| Key | 16, 24, or 32 bytes | +| Initialisation vector | Always exactly 16 bytes, for every key size | +| Input to `Encrypt` / `Decrypt` | Non-zero multiple of 16 bytes | +| Input to `EncryptPadded` | Any length, including empty | +| Padding | PKCS#7 via the `*Padded` methods, otherwise agreed by the protocol | +| Authentication | Not provided. Supplied separately by the caller | + +The initialisation vector is always supplied by the caller and is never prefixed +to, or stripped from, the ciphertext. How it is transported or derived is a +property of the surrounding protocol, so that framing is left to the caller. + +Generate a fresh initialisation vector for every message with `GenerateCBCIV`. +NIST SP 800-38A requires it to be unpredictable; reusing one under the same key +leaks whether two messages share a leading block. It need not be secret and may +travel beside the ciphertext, but it must be integrity protected along with it. + +A fixed or all-zero initialisation vector is accepted only because an external +protocol may mandate one. That is an exception to the requirement above, not an +option to pick freely, and it should be justified by the protocol specification. + +```go +cipher, err := aes.CreateCBCFromKeyString(hexKey) +iv, err := aes.GenerateCBCIV() +cipherBytes, err := cipher.EncryptPadded(plainBytes, iv) +// Now authenticate iv and cipherBytes together, and transmit the tag alongside. +``` + ### DES - factory methods to construct an DES or 3DES cipher from the raw key bytes or hex text diff --git a/VERSION.txt b/VERSION.txt index f88cf52..cd99d38 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -1.13.0 \ No newline at end of file +1.14.0 \ No newline at end of file diff --git a/aes/aes_cbc_cipher.go b/aes/aes_cbc_cipher.go new file mode 100644 index 0000000..18dca0f --- /dev/null +++ b/aes/aes_cbc_cipher.go @@ -0,0 +1,155 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aes + +import ( + "crypto/cipher" + "errors" + "fmt" +) + +// CBCCipher is a wrapper of the AES CBC cipher and stores the raw key bytes. +// +// CBC provides confidentiality only. Unlike Cipher, which wraps AES GCM, it is +// NOT authenticated: +// +// - A modified initialisation vector or ciphertext will usually still decrypt +// without error, producing attacker influenced plaintext. Decryption +// succeeding is therefore no evidence that the message is genuine. +// - Callers MUST authenticate the initialisation vector together with the +// ciphertext by separate means, for example an HMAC over both, and MUST +// verify it before decrypting. +// - Reporting padding failures back to an untrusted caller exposes a padding +// oracle, which can be used to recover plaintext. +// +// Prefer Cipher (AES GCM) unless an external protocol mandates CBC. +// +// The initialisation vector is always supplied by the caller and is never +// prefixed to, or stripped from, the ciphertext by this type. How the +// initialisation vector is transported or derived is a property of the +// surrounding protocol, so it is left to the caller. +type CBCCipher struct { + block cipher.Block + KeyBytes []byte +} + +// Encrypt encrypts plainBytes under the given initialisation vector without +// applying any padding. plainBytes must be a non-zero multiple of the AES block +// size; use EncryptPadded for arbitrary length input. iv must be exactly one AES +// block long, for every supported key size. +// +// Neither plainBytes nor iv is modified. +func (c *CBCCipher) Encrypt(plainBytes []byte, iv []byte) ([]byte, error) { + blockSize := c.block.BlockSize() + if err := validateIV(iv, blockSize); err != nil { + return nil, err + } + if len(plainBytes) == 0 { + return nil, errors.New("input is empty") + } + if len(plainBytes)%blockSize != 0 { + return nil, fmt.Errorf("input length %d is not a multiplier of block size %d", len(plainBytes), blockSize) + } + + cipherBytes := make([]byte, len(plainBytes)) + cipher.NewCBCEncrypter(c.block, cloneBytes(iv)).CryptBlocks(cipherBytes, plainBytes) + return cipherBytes, nil +} + +// Decrypt decrypts cipherBytes under the given initialisation vector without +// removing any padding. cipherBytes must be a non-zero multiple of the AES block +// size and iv must be exactly one AES block long. +// +// Decrypt does not, and cannot, detect modification of cipherBytes or iv. See +// CBCCipher for the caller's obligations. +// +// Neither cipherBytes nor iv is modified. +func (c *CBCCipher) Decrypt(cipherBytes []byte, iv []byte) ([]byte, error) { + blockSize := c.block.BlockSize() + if err := validateIV(iv, blockSize); err != nil { + return nil, err + } + if len(cipherBytes) == 0 { + return nil, errors.New("input is empty") + } + if len(cipherBytes)%blockSize != 0 { + return nil, fmt.Errorf("input length %d is not a multiplier of block size %d", len(cipherBytes), blockSize) + } + + plainBytes := make([]byte, len(cipherBytes)) + cipher.NewCBCDecrypter(c.block, cloneBytes(iv)).CryptBlocks(plainBytes, cipherBytes) + return plainBytes, nil +} + +// EncryptPadded applies PKCS#7 padding (RFC 5652 section 6.3) to plainBytes and +// encrypts the result, so plainBytes may be of any length, including empty. iv +// must be exactly one AES block long. +// +// PKCS#7 is one padding scheme among several; it is a choice made by the +// surrounding protocol rather than a property of CBC. Use Encrypt when the +// protocol pads by other means or not at all. +// +// Neither plainBytes nor iv is modified. +func (c *CBCCipher) EncryptPadded(plainBytes []byte, iv []byte) ([]byte, error) { + blockSize := c.block.BlockSize() + if err := validateIV(iv, blockSize); err != nil { + return nil, err + } + + return c.Encrypt(pad(plainBytes, blockSize), iv) +} + +// DecryptPadded decrypts cipherBytes and removes its PKCS#7 padding (RFC 5652 +// section 6.3). cipherBytes must be a non-zero multiple of the AES block size +// and iv must be exactly one AES block long. +// +// A padding error is NOT an integrity check. A modified ciphertext can decrypt +// to well formed padding just as easily as to malformed padding, so a nil error +// says nothing about authenticity. Do not surface padding errors to untrusted +// callers; doing so exposes a padding oracle. See CBCCipher. +// +// Neither cipherBytes nor iv is modified. +func (c *CBCCipher) DecryptPadded(cipherBytes []byte, iv []byte) ([]byte, error) { + paddedBytes, err := c.Decrypt(cipherBytes, iv) + if err != nil { + return nil, err + } + + return unpad(paddedBytes, c.block.BlockSize()) +} + +// CheckValue returns the key check value of the underlying key. It is derived +// from the key alone and is identical to the value Cipher reports for the same +// key, being independent of the mode of operation. +func (c *CBCCipher) CheckValue() string { + return deriveCheckValue(c.KeyBytes) +} + +// VerifyCheckValue reports whether the given key check value matches the one +// derived from the underlying key. The comparison is case insensitive. +func (c *CBCCipher) VerifyCheckValue(checkValue string) bool { + return verifyCheckValue(c.KeyBytes, checkValue) +} + +func validateIV(iv []byte, blockSize int) error { + if len(iv) != blockSize { + return fmt.Errorf("IV must be %d bytes but got %d", blockSize, len(iv)) + } + return nil +} + +func cloneBytes(src []byte) []byte { + dst := make([]byte, len(src)) + copy(dst, src) + return dst +} diff --git a/aes/aes_cbc_cipher_test.go b/aes/aes_cbc_cipher_test.go new file mode 100644 index 0000000..901143a --- /dev/null +++ b/aes/aes_cbc_cipher_test.go @@ -0,0 +1,457 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aes + +import ( + "bytes" + "encoding/hex" + "testing" +) + +// Known answer vectors from NIST SP 800-38A appendix F.2, the four block CBC +// examples for each AES key size. The plaintext and initialisation vector are +// shared by all six sub-examples. +const ( + nistIV = "000102030405060708090a0b0c0d0e0f" + nistPlaintext = "6bc1bee22e409f96e93d7e117393172a" + + "ae2d8a571e03ac9c9eb76fac45af8e51" + + "30c81c46a35ce411e5fbc1191a0a52ef" + + "f69f2445df4f9b17ad2b417be66c3710" +) + +var nistCBCVectors = []struct { + name string + key string + ciphertext string +}{ + { + // F.2.1 / F.2.2 CBC-AES128 + name: "AES-128", + key: "2b7e151628aed2a6abf7158809cf4f3c", + ciphertext: "7649abac8119b246cee98e9b12e9197d" + + "5086cb9b507219ee95db113a917678b2" + + "73bed6b8e3c1743b7116e69e22229516" + + "3ff1caa1681fac09120eca307586e1a7", + }, + { + // F.2.3 / F.2.4 CBC-AES192 + name: "AES-192", + key: "8e73b0f7da0e6452c810f32b809079e562f8ead2522c6b7b", + ciphertext: "4f021db243bc633d7178183a9fa071e8" + + "b4d9ada9ad7dedf4e5e738763f69145a" + + "571b242012fb7ae07fa9baac3df102e0" + + "08b0e27988598881d920a9e64f5615cd", + }, + { + // F.2.5 / F.2.6 CBC-AES256 + name: "AES-256", + key: "603deb1015ca71be2b73aef0857d77811f352c073b6108d72d9810a30914dff4", + ciphertext: "f58c4c04d6e5f1ba779eabfb5f7bfbd6" + + "9cfc4e967edb808d679f777bc6702c7d" + + "39f23369a9d9bacfa530e26304231461" + + "b2eb05e2c39be9fcda6c19078c6a9d1b", + }, +} + +func TestAESCBCCipher_EncryptMatchesNISTVectors(t *testing.T) { + iv, _ := hex.DecodeString(nistIV) + plainBytes, _ := hex.DecodeString(nistPlaintext) + + for _, vector := range nistCBCVectors { + t.Run(vector.name, func(t *testing.T) { + cipher, err := CreateCBCFromKeyString(vector.key) + if err != nil { + t.Fatalf("Did not expect an error but got %q", err) + } + + cipherBytes, err := cipher.Encrypt(plainBytes, iv) + if err != nil { + t.Fatalf("Did not expect an encryption error but got %q", err) + } + + if hex.EncodeToString(cipherBytes) != vector.ciphertext { + t.Errorf("Expected %s but get %s", vector.ciphertext, hex.EncodeToString(cipherBytes)) + } + }) + } +} + +func TestAESCBCCipher_DecryptMatchesNISTVectors(t *testing.T) { + iv, _ := hex.DecodeString(nistIV) + + for _, vector := range nistCBCVectors { + t.Run(vector.name, func(t *testing.T) { + cipher, err := CreateCBCFromKeyString(vector.key) + if err != nil { + t.Fatalf("Did not expect an error but got %q", err) + } + cipherBytes, _ := hex.DecodeString(vector.ciphertext) + + plainBytes, err := cipher.Decrypt(cipherBytes, iv) + if err != nil { + t.Fatalf("Did not expect a decryption error but got %q", err) + } + + if hex.EncodeToString(plainBytes) != nistPlaintext { + t.Errorf("Expected %s but get %s", nistPlaintext, hex.EncodeToString(plainBytes)) + } + }) + } +} + +func TestAESCBCCipher_EncryptRejectsInvalidInputLength(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) + iv, _ := hex.DecodeString(nistIV) + + testCases := []struct { + name string + length int + }{ + {"empty", 0}, + {"one byte", 1}, + {"one byte short of a block", 15}, + {"one byte over a block", 17}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + if _, err := cipher.Encrypt(make([]byte, testCase.length), iv); err == nil { + t.Error("Expected an error but got none") + } + if _, err := cipher.Decrypt(make([]byte, testCase.length), iv); err == nil { + t.Error("Expected an error but got none") + } + }) + } +} + +func TestAESCBCCipher_RejectsInvalidIVLength(t *testing.T) { + plainBytes, _ := hex.DecodeString(nistPlaintext) + + // The initialisation vector is one AES block for every key size, so a key + // sized initialisation vector is a mistake worth catching for AES-192 and + // AES-256 in particular. + for _, vector := range nistCBCVectors { + cipher, _ := CreateCBCFromKeyString(vector.key) + + for _, ivLength := range []int{0, 8, 15, 17, 24, 32} { + iv := make([]byte, ivLength) + + if _, err := cipher.Encrypt(plainBytes, iv); err == nil { + t.Errorf("%s: expected Encrypt to reject a %d byte IV", vector.name, ivLength) + } + if _, err := cipher.Decrypt(plainBytes, iv); err == nil { + t.Errorf("%s: expected Decrypt to reject a %d byte IV", vector.name, ivLength) + } + if _, err := cipher.EncryptPadded(plainBytes, iv); err == nil { + t.Errorf("%s: expected EncryptPadded to reject a %d byte IV", vector.name, ivLength) + } + if _, err := cipher.DecryptPadded(plainBytes, iv); err == nil { + t.Errorf("%s: expected DecryptPadded to reject a %d byte IV", vector.name, ivLength) + } + } + + if _, err := cipher.Encrypt(plainBytes, nil); err == nil { + t.Errorf("%s: expected Encrypt to reject a nil IV", vector.name) + } + } +} + +func TestAESCBCCipher_EncryptAndDecryptPaddedRoundTrip(t *testing.T) { + iv, _ := hex.DecodeString(nistIV) + + for _, vector := range nistCBCVectors { + t.Run(vector.name, func(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(vector.key) + + // Lengths either side of the block boundary, including empty input + // and exactly block aligned input. + for length := 0; length <= 33; length++ { + plainBytes := make([]byte, length) + for i := range plainBytes { + plainBytes[i] = byte(i) + } + + cipherBytes, err := cipher.EncryptPadded(plainBytes, iv) + if err != nil { + t.Errorf("Did not expect an encryption error for length %d but got %q", length, err) + continue + } + if len(cipherBytes)%16 != 0 || len(cipherBytes) <= length { + t.Errorf("Expected padded ciphertext longer than %d and block aligned but get %d", length, len(cipherBytes)) + } + + decryptedBytes, err := cipher.DecryptPadded(cipherBytes, iv) + if err != nil { + t.Errorf("Did not expect a decryption error for length %d but got %q", length, err) + continue + } + + if !bytes.Equal(plainBytes, decryptedBytes) { + t.Errorf("Expected %s but get %s", hex.EncodeToString(plainBytes), hex.EncodeToString(decryptedBytes)) + } + } + }) + } +} + +func TestAESCBCCipher_DecryptPaddedRejectsInvalidPKCS7Padding(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) + iv, _ := hex.DecodeString(nistIV) + + // A block whose final byte is zero can never be valid PKCS#7 padding, so + // encrypting it raw yields a ciphertext that DecryptPadded must reject. + // This is a padding check, not an integrity check: see + // TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext. + unpaddableBlock, _ := hex.DecodeString("000102030405060708090a0b0c0d0e00") + + cipherBytes, err := cipher.Encrypt(unpaddableBlock, iv) + if err != nil { + t.Fatalf("Did not expect an encryption error but got %q", err) + } + + if _, err := cipher.DecryptPadded(cipherBytes, iv); err == nil { + t.Error("Expected DecryptPadded to reject invalid PKCS#7 padding but got no error") + } + + // The same bytes decrypt without complaint when no padding is expected. + plainBytes, err := cipher.Decrypt(cipherBytes, iv) + if err != nil { + t.Errorf("Did not expect a raw decryption error but got %q", err) + } + if !bytes.Equal(plainBytes, unpaddableBlock) { + t.Errorf("Expected %s but get %s", hex.EncodeToString(unpaddableBlock), hex.EncodeToString(plainBytes)) + } +} + +// TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext records that CBC is +// unauthenticated. A modified ciphertext still decrypts without error, so a nil +// error from Decrypt is no evidence that the message is genuine. Callers must +// authenticate the initialisation vector and the ciphertext separately. +func TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) + iv, _ := hex.DecodeString(nistIV) + plainBytes, _ := hex.DecodeString(nistPlaintext) + + cipherBytes, err := cipher.Encrypt(plainBytes, iv) + if err != nil { + t.Fatalf("Did not expect an encryption error but got %q", err) + } + + modifiedCipherBytes := cloneBytes(cipherBytes) + modifiedCipherBytes[0] ^= 0x01 + + decryptedBytes, err := cipher.Decrypt(modifiedCipherBytes, iv) + if err != nil { + t.Errorf("Expected a modified ciphertext to still decrypt without error but got %q", err) + } + if bytes.Equal(decryptedBytes, plainBytes) { + t.Error("Expected the recovered plaintext to differ from the original") + } +} + +// TestAESCBCCipher_DecryptPaddedDoesNotDetectModifiedIV goes further: flipping a +// bit in the initialisation vector flips exactly the same bit in the first +// plaintext block and leaves every later block, including the padding block, +// untouched. DecryptPadded therefore returns attacker chosen plaintext with a +// nil error. Valid padding is not integrity. +func TestAESCBCCipher_DecryptPaddedDoesNotDetectModifiedIV(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) + iv, _ := hex.DecodeString(nistIV) + plainBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e0f") + + // One block of plaintext plus a whole block of PKCS#7 padding. + cipherBytes, err := cipher.EncryptPadded(plainBytes, iv) + if err != nil { + t.Fatalf("Did not expect an encryption error but got %q", err) + } + + modifiedIV := cloneBytes(iv) + modifiedIV[0] ^= 0xff + + decryptedBytes, err := cipher.DecryptPadded(cipherBytes, modifiedIV) + if err != nil { + t.Fatalf("Expected a modified IV to still decrypt without error but got %q", err) + } + + expectedBytes := cloneBytes(plainBytes) + expectedBytes[0] ^= 0xff + if !bytes.Equal(decryptedBytes, expectedBytes) { + t.Errorf("Expected the attacker chosen plaintext %s but get %s", hex.EncodeToString(expectedBytes), hex.EncodeToString(decryptedBytes)) + } +} + +func TestAESCBCCipher_DoesNotModifyInputs(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) + iv, _ := hex.DecodeString(nistIV) + plainBytes, _ := hex.DecodeString(nistPlaintext) + cipherBytes, _ := hex.DecodeString(nistCBCVectors[0].ciphertext) + paddedCipherBytes, _ := cipher.EncryptPadded(plainBytes, iv) + + operations := []struct { + name string + input []byte + run func(input []byte, iv []byte) ([]byte, error) + }{ + {"Encrypt", plainBytes, cipher.Encrypt}, + {"Decrypt", cipherBytes, cipher.Decrypt}, + {"EncryptPadded", plainBytes, cipher.EncryptPadded}, + {"DecryptPadded", paddedCipherBytes, cipher.DecryptPadded}, + } + + for _, operation := range operations { + t.Run(operation.name, func(t *testing.T) { + input := cloneBytes(operation.input) + operationIV := cloneBytes(iv) + + if _, err := operation.run(input, operationIV); err != nil { + t.Fatalf("Did not expect an error but got %q", err) + } + + if !bytes.Equal(input, operation.input) { + t.Errorf("Expected input to be left as %s but get %s", hex.EncodeToString(operation.input), hex.EncodeToString(input)) + } + if !bytes.Equal(operationIV, iv) { + t.Errorf("Expected IV to be left as %s but get %s", hex.EncodeToString(iv), hex.EncodeToString(operationIV)) + } + }) + } +} + +func TestAESCBCCipher_DifferentIVsProduceDifferentCiphertext(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) + plainBytes, _ := hex.DecodeString(nistPlaintext) + + firstIV, err := GenerateCBCIV() + if err != nil { + t.Fatalf("Did not expect an error but got %q", err) + } + secondIV, err := GenerateCBCIV() + if err != nil { + t.Fatalf("Did not expect an error but got %q", err) + } + + firstCipherBytes, _ := cipher.Encrypt(plainBytes, firstIV) + secondCipherBytes, _ := cipher.Encrypt(plainBytes, secondIV) + + if bytes.Equal(firstCipherBytes, secondCipherBytes) { + t.Error("Expected the same plaintext under different IVs to produce different ciphertext") + } + + // Each ciphertext must still decrypt under its own initialisation vector. + for _, testCase := range []struct { + cipherBytes []byte + iv []byte + }{{firstCipherBytes, firstIV}, {secondCipherBytes, secondIV}} { + decryptedBytes, err := cipher.Decrypt(testCase.cipherBytes, testCase.iv) + if err != nil { + t.Errorf("Did not expect a decryption error but got %q", err) + continue + } + if !bytes.Equal(decryptedBytes, plainBytes) { + t.Errorf("Expected %s but get %s", nistPlaintext, hex.EncodeToString(decryptedBytes)) + } + } +} + +func TestGenerateCBCIV(t *testing.T) { + seen := make(map[string]bool) + + for i := 0; i < 32; i++ { + iv, err := GenerateCBCIV() + if err != nil { + t.Fatalf("Did not expect an error but got %q", err) + } + if len(iv) != 16 { + t.Errorf("Expected a 16 byte IV but get %d", len(iv)) + } + + encodedIV := hex.EncodeToString(iv) + if seen[encodedIV] { + t.Errorf("Expected every generated IV to be unique but %s repeated", encodedIV) + } + seen[encodedIV] = true + } +} + +func TestCreateCBCFromKeyBytes(t *testing.T) { + for _, keyLength := range []int{16, 24, 32} { + keyBytes := make([]byte, keyLength) + + cipher, err := CreateCBCFromKeyBytes(keyBytes) + if err != nil { + t.Errorf("Did not expect an error for a %d byte key but got %q", keyLength, err) + continue + } + if !bytes.Equal(cipher.KeyBytes, keyBytes) { + t.Errorf("Expected key %s but get %s", hex.EncodeToString(keyBytes), hex.EncodeToString(cipher.KeyBytes)) + } + } + + for _, keyLength := range []int{0, 8, 15, 17, 23, 25, 31, 33, 64} { + if _, err := CreateCBCFromKeyBytes(make([]byte, keyLength)); err == nil { + t.Errorf("Expected a %d byte key to be rejected", keyLength) + } + } +} + +func TestCreateCBCFromKeyString(t *testing.T) { + cipher, err := CreateCBCFromKeyString(nistCBCVectors[0].key) + if err != nil { + t.Fatalf("Did not expect an error but got %q", err) + } + if hex.EncodeToString(cipher.KeyBytes) != nistCBCVectors[0].key { + t.Errorf("Expected key %s but get %s", nistCBCVectors[0].key, hex.EncodeToString(cipher.KeyBytes)) + } + + if _, err := CreateCBCFromKeyString("not hex"); err == nil { + t.Error("Expected a non hex key to be rejected") + } + if _, err := CreateCBCFromKeyString("2b7e151628aed2a6abf7158809cf4f"); err == nil { + t.Error("Expected a hex key of the wrong length to be rejected") + } +} + +// TestAESCBCCheckValueMatchesGCMCipher asserts that the key check value depends +// only on the key, so a key imported for CBC use reports the same value as the +// same key used with GCM. +func TestAESCBCCheckValueMatchesGCMCipher(t *testing.T) { + keyBytes, _ := hex.DecodeString("2b7e151628aed2a6abf7158809cf4f3c") + + cbcCipher, _ := CreateCBCFromKeyBytes(keyBytes) + gcmCipher, _ := CreateFromKeyBytes(keyBytes) + + if cbcCipher.CheckValue() != "7df76b" { + t.Errorf("Expected check value 7df76b but got %s", cbcCipher.CheckValue()) + } + if cbcCipher.CheckValue() != gcmCipher.CheckValue() { + t.Errorf("Expected check value %s but got %s", gcmCipher.CheckValue(), cbcCipher.CheckValue()) + } +} + +func TestAESCBCVerifyCheckValue(t *testing.T) { + cipher, _ := CreateCBCFromKeyString("2b7e151628aed2a6abf7158809cf4f3c") + + if !cipher.VerifyCheckValue("7df76b") { + t.Error("Expected check value to verify successfully") + } + if !cipher.VerifyCheckValue("7DF76B") { + t.Error("Expected check value verification to be case insensitive") + } + if cipher.VerifyCheckValue("abcdef") { + t.Error("Expected wrong check value to fail verification") + } + if cipher.VerifyCheckValue("") { + t.Error("Expected empty check value to fail verification") + } +} diff --git a/aes/aes_cbc_factory.go b/aes/aes_cbc_factory.go new file mode 100644 index 0000000..e90ca47 --- /dev/null +++ b/aes/aes_cbc_factory.go @@ -0,0 +1,63 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aes + +import ( + "crypto/aes" + "encoding/hex" + "errors" + + "github.com/hashicorp/go-uuid" +) + +// CreateCBCFromKeyBytes constructs a new AES CBC cipher using the raw key bytes provided, the raw bytes must be either 16, 24, or 32 bytes +func CreateCBCFromKeyBytes(keyBytes []byte) (CBCCipher, error) { + if len(keyBytes) != 16 && len(keyBytes) != 24 && len(keyBytes) != 32 { + return CBCCipher{}, errors.New("AES key must be 16, 24, or 32 bytes") + } + + keyBlock, err := aes.NewCipher(keyBytes) + if err != nil { + return CBCCipher{}, errors.New("invalid AES keyBlock") + } + + return CBCCipher{keyBlock, keyBytes}, nil +} + +// CreateCBCFromKeyString constructs a new AES CBC cipher using the hex-encoded key string provided +func CreateCBCFromKeyString(key string) (CBCCipher, error) { + keyBytes, err := hex.DecodeString(key) + if err != nil { + return CBCCipher{}, errors.New("AES key is not in correct hex format") + } + return CreateCBCFromKeyBytes(keyBytes) +} + +// GenerateCBCIV returns a cryptographically random initialisation vector of one +// AES block, suitable for a single CBC encryption. +// +// NIST SP 800-38A requires the initialisation vector to be unpredictable, and a +// fresh one must be generated for every message encrypted under the same key. +// The initialisation vector need not be kept secret and may travel alongside the +// ciphertext, but it must be integrity protected together with the ciphertext; +// see CBCCipher. +// +// The name is deliberately distinct from the GCM nonce that Cipher generates +// internally: the two are not interchangeable. +func GenerateCBCIV() ([]byte, error) { + iv, err := uuid.GenerateRandomBytes(aes.BlockSize) + if err != nil { + return nil, errors.New("fail to generate IV") + } + return iv, nil +} diff --git a/aes/aes_cipher.go b/aes/aes_cipher.go index 89e7f13..6b51eba 100644 --- a/aes/aes_cipher.go +++ b/aes/aes_cipher.go @@ -10,7 +10,11 @@ limitations under the License. */ -// Package aes provides wrapper methods on top of the AES GCM cipher for our own usage +// Package aes provides wrapper methods on top of the AES cipher for our own usage. +// +// Cipher wraps AES GCM, which is authenticated. CBCCipher wraps AES CBC, which +// provides confidentiality only and is not authenticated; see CBCCipher for the +// obligations that places on the caller. package aes import ( @@ -68,7 +72,18 @@ func (cipher *Cipher) Decrypt(cipherBytes []byte, nonce []byte) ([]byte, error) } func (c *Cipher) CheckValue() string { - block, err := aes.NewCipher(c.KeyBytes) + return deriveCheckValue(c.KeyBytes) +} + +func (c *Cipher) VerifyCheckValue(checkValue string) bool { + return verifyCheckValue(c.KeyBytes, checkValue) +} + +// deriveCheckValue returns the key check value of keyBytes: the first +// checkValueBytes bytes of a single AES ECB block encryption of all zeroes, +// hex encoded. It returns an empty string when keyBytes is not a valid AES key. +func deriveCheckValue(keyBytes []byte) string { + block, err := aes.NewCipher(keyBytes) if err != nil { return "" } @@ -77,12 +92,12 @@ func (c *Cipher) CheckValue() string { return hex.EncodeToString(cipherBytes[:checkValueBytes]) } -func (c *Cipher) VerifyCheckValue(checkValue string) bool { +func verifyCheckValue(keyBytes []byte, checkValue string) bool { if checkValue == "" { return false } - derivedCheckValue := c.CheckValue() + derivedCheckValue := deriveCheckValue(keyBytes) if derivedCheckValue == "" { return false } diff --git a/aes/pkcs7.go b/aes/pkcs7.go new file mode 100644 index 0000000..905e2a3 --- /dev/null +++ b/aes/pkcs7.go @@ -0,0 +1,65 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aes + +import "fmt" + +// pad appends PKCS#7 padding (RFC 5652 section 6.3) to plainBytes so that its +// length becomes a multiple of blockSize. Input that is already block aligned +// gains a whole extra block of padding, so the padding is always removable +// without ambiguity. An empty input therefore yields a single full block. +// +// The input slice is never modified; a new slice is always returned. +func pad(plainBytes []byte, blockSize int) []byte { + padLen := blockSize - len(plainBytes)%blockSize + + padded := make([]byte, 0, len(plainBytes)+padLen) + padded = append(padded, plainBytes...) + for i := 0; i < padLen; i++ { + padded = append(padded, byte(padLen)) + } + return padded +} + +// unpad removes PKCS#7 padding (RFC 5652 section 6.3) from paddedBytes. It +// returns an error when the padding is not well formed. +// +// A padding error is not evidence of tampering alone, and callers must not +// treat it as an integrity check: CBC is unauthenticated, so a modified +// ciphertext can just as easily decrypt to well formed padding. Reporting +// padding failures to an untrusted caller also exposes a padding oracle. +// +// The input slice is never modified; the returned slice is a copy. +func unpad(paddedBytes []byte, blockSize int) ([]byte, error) { + if len(paddedBytes) == 0 { + return nil, fmt.Errorf("padded input is empty") + } + if len(paddedBytes)%blockSize != 0 { + return nil, fmt.Errorf("padded input length %d is not a multiplier of block size %d", len(paddedBytes), blockSize) + } + + padLen := int(paddedBytes[len(paddedBytes)-1]) + if padLen == 0 || padLen > blockSize { + return nil, fmt.Errorf("invalid padding") + } + + for _, padByte := range paddedBytes[len(paddedBytes)-padLen:] { + if int(padByte) != padLen { + return nil, fmt.Errorf("invalid padding") + } + } + + plainBytes := make([]byte, len(paddedBytes)-padLen) + copy(plainBytes, paddedBytes) + return plainBytes, nil +} diff --git a/aes/pkcs7_test.go b/aes/pkcs7_test.go new file mode 100644 index 0000000..c12870b --- /dev/null +++ b/aes/pkcs7_test.go @@ -0,0 +1,167 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aes + +import ( + "bytes" + "encoding/hex" + "testing" +) + +const testBlockSize = 16 + +func TestPKCS7Pad(t *testing.T) { + testCases := []struct { + name string + plain string + expected string + }{ + // RFC 5652 section 6.3: block aligned input gains a whole extra block, + // so an empty input pads to a single full block. + {"empty", "", "10101010101010101010101010101010"}, + {"one byte", "00", "000f0f0f0f0f0f0f0f0f0f0f0f0f0f0f"}, + {"fifteen bytes", "000102030405060708090a0b0c0d0e", "000102030405060708090a0b0c0d0e01"}, + {"one full block", "000102030405060708090a0b0c0d0e0f", "000102030405060708090a0b0c0d0e0f10101010101010101010101010101010"}, + {"one block plus one byte", "000102030405060708090a0b0c0d0e0f10", "000102030405060708090a0b0c0d0e0f100f0f0f0f0f0f0f0f0f0f0f0f0f0f0f"}, + {"two full blocks", "000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f", "000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f10101010101010101010101010101010"}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + plainBytes, _ := hex.DecodeString(testCase.plain) + + padded := hex.EncodeToString(pad(plainBytes, testBlockSize)) + if padded != testCase.expected { + t.Errorf("Expected %s but get %s", testCase.expected, padded) + } + }) + } +} + +func TestPKCS7PadDoesNotModifyInput(t *testing.T) { + plainBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e0f") + original := hex.EncodeToString(plainBytes) + + pad(plainBytes, testBlockSize) + + if hex.EncodeToString(plainBytes) != original { + t.Errorf("Expected input to be left as %s but get %s", original, hex.EncodeToString(plainBytes)) + } +} + +func TestPKCS7Unpad(t *testing.T) { + testCases := []struct { + name string + padded string + expected string + }{ + {"empty plaintext", "10101010101010101010101010101010", ""}, + {"one byte", "000f0f0f0f0f0f0f0f0f0f0f0f0f0f0f", "00"}, + {"fifteen bytes", "000102030405060708090a0b0c0d0e01", "000102030405060708090a0b0c0d0e"}, + {"one full block", "000102030405060708090a0b0c0d0e0f10101010101010101010101010101010", "000102030405060708090a0b0c0d0e0f"}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + paddedBytes, _ := hex.DecodeString(testCase.padded) + + plainBytes, err := unpad(paddedBytes, testBlockSize) + if err != nil { + t.Errorf("Did not expect an error but got %q", err) + } + + if hex.EncodeToString(plainBytes) != testCase.expected { + t.Errorf("Expected %s but get %s", testCase.expected, hex.EncodeToString(plainBytes)) + } + }) + } +} + +func TestPKCS7UnpadRejectsMalformedPadding(t *testing.T) { + testCases := []struct { + name string + padded string + }{ + {"empty input", ""}, + {"length not a multiple of the block size", "000102030405060708090a0b0c0d0e"}, + {"pad byte of zero", "000102030405060708090a0b0c0d0e00"}, + {"pad byte larger than the block size", "000102030405060708090a0b0c0d0e11"}, + {"inconsistent pad bytes", "000102030405060708090a0b0c0d0203"}, + {"pad bytes not repeated for the full length", "0001020304050607080900000000ff04"}, + {"padding claims the whole block but is not uniform", "0102030405060708090a0b0c0d0e0f10"}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + paddedBytes, _ := hex.DecodeString(testCase.padded) + + if _, err := unpad(paddedBytes, testBlockSize); err == nil { + t.Error("Expected an error but got none") + } + }) + } +} + +func TestPKCS7UnpadDoesNotModifyInput(t *testing.T) { + paddedBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e01") + original := hex.EncodeToString(paddedBytes) + + if _, err := unpad(paddedBytes, testBlockSize); err != nil { + t.Errorf("Did not expect an error but got %q", err) + } + + if hex.EncodeToString(paddedBytes) != original { + t.Errorf("Expected input to be left as %s but get %s", original, hex.EncodeToString(paddedBytes)) + } +} + +func TestPKCS7UnpadReturnsCopyOfInput(t *testing.T) { + paddedBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e01") + + plainBytes, err := unpad(paddedBytes, testBlockSize) + if err != nil { + t.Errorf("Did not expect an error but got %q", err) + } + + plainBytes[0] = 0xff + if paddedBytes[0] == 0xff { + t.Error("Expected the returned slice not to alias the input") + } +} + +func TestPKCS7RoundTrip(t *testing.T) { + for length := 0; length <= 48; length++ { + plainBytes := make([]byte, length) + for i := range plainBytes { + plainBytes[i] = byte(i) + } + + padded := pad(plainBytes, testBlockSize) + if len(padded)%testBlockSize != 0 { + t.Errorf("Expected padded length to be a multiplier of %d but get %d", testBlockSize, len(padded)) + } + if len(padded) <= length { + t.Errorf("Expected padded length to be greater than %d but get %d", length, len(padded)) + } + + unpadded, err := unpad(padded, testBlockSize) + if err != nil { + t.Errorf("Did not expect an error for length %d but got %q", length, err) + continue + } + + if !bytes.Equal(plainBytes, unpadded) { + t.Errorf("Expected %s but get %s", hex.EncodeToString(plainBytes), hex.EncodeToString(unpadded)) + } + } +} From b6250c7eef1b8fda78b0487947faed2a752fef81 Mon Sep 17 00:00:00 2001 From: Keith Fong Date: Thu, 3 Sep 2026 16:10:44 +0800 Subject: [PATCH 2/4] feat: use ISO/IEC 9797-1 method 2 padding instead of PKCS#7 The PKCS#7 padded operation was chosen before the G+D CII specification was available, so it was never derived from a requirement. The CII data provider interface names its algorithm aes-256-cbc-iso2, and "ISO2" in card personalisation interfaces denotes ISO/IEC 9797-1 padding method 2: a single 0x80 marker followed by 0x00 filler to the block boundary. PKCS#7 is a different byte layout and must not be substituted merely because it also produces block-aligned output. EncryptPadded and DecryptPadded become EncryptISO9797M2Padded and DecryptISO9797M2Padded so a protocol requiring a different scheme, such as PKCS#7 or ISO 10126-2, cannot reach them by accident. Renaming is free because CBCCipher has not been released. The padded ciphertext is anchored to the NIST SP 800-38A F.2 vectors: CBC encrypts block by block, so appending a padding block cannot alter the preceding ciphertext, and the block-aligned NIST plaintext gains exactly one extra block. Tests also assert that PKCS#7 padding is rejected rather than silently accepted, and that messages ending in 0x80 or 0x00 still round trip. --- README.md | 39 ++++- aes/aes_cbc_cipher.go | 37 +++-- aes/aes_cbc_cipher_test.go | 132 +++++++++++----- aes/iso9797m2.go | 84 ++++++++++ aes/iso9797m2_test.go | 304 +++++++++++++++++++++++++++++++++++++ aes/pkcs7.go | 65 -------- aes/pkcs7_test.go | 167 -------------------- 7 files changed, 538 insertions(+), 290 deletions(-) create mode 100644 aes/iso9797m2.go create mode 100644 aes/iso9797m2_test.go delete mode 100644 aes/pkcs7.go delete mode 100644 aes/pkcs7_test.go diff --git a/README.md b/README.md index 5f71639..3db7d83 100644 --- a/README.md +++ b/README.md @@ -22,14 +22,15 @@ materially weaker guarantee. > - Callers **must** authenticate the initialisation vector together with the > ciphertext by separate means (for example an HMAC over both) and **must** > verify it before decrypting. -> - A PKCS#7 padding error is **not** an integrity check. Never surface padding +> - A padding error is **not** an integrity check. Never surface padding > failures to an untrusted caller: doing so exposes a > [padding oracle](https://en.wikipedia.org/wiki/Padding_oracle_attack) that > can be used to recover plaintext. - factory methods to construct an AES-CBC cipher from the raw key bytes or hex text - `Encrypt` / `Decrypt` operate on block-aligned input with no padding -- `EncryptPadded` / `DecryptPadded` apply and remove PKCS#7 padding (RFC 5652 §6.3) +- `EncryptISO9797M2Padded` / `DecryptISO9797M2Padded` apply and remove ISO/IEC + 9797-1 padding method 2 - `GenerateCBCIV` returns a random 16-byte initialisation vector - verify the constructed cipher against the check value; the value depends only on the key, so it matches the one `Cipher` reports for the same key @@ -41,10 +42,38 @@ Parameters: | Key | 16, 24, or 32 bytes | | Initialisation vector | Always exactly 16 bytes, for every key size | | Input to `Encrypt` / `Decrypt` | Non-zero multiple of 16 bytes | -| Input to `EncryptPadded` | Any length, including empty | -| Padding | PKCS#7 via the `*Padded` methods, otherwise agreed by the protocol | +| Input to `EncryptISO9797M2Padded` | Any length, including empty | +| Padding | ISO/IEC 9797-1 method 2 via the `*ISO9797M2Padded` methods, otherwise agreed by the protocol | | Authentication | Not provided. Supplied separately by the caller | +#### Padding + +Padding belongs to the surrounding protocol, not to CBC. The padded methods are +named after their scheme so a protocol that needs a different one cannot reach +them by accident. Use `Encrypt` and `Decrypt` when the protocol pads by other +means or not at all. + +ISO/IEC 9797-1 method 2 appends a single `0x80` byte followed by `0x00` filler to +the block boundary. At least one byte is always added, so input that is already +block aligned gains a whole extra block. That is what makes the padding +removable without ambiguity, even when the message itself ends in `0x80` or +`0x00`. The same construction appears in ISO/IEC 7816-4 and is what BouncyCastle +calls `ISO7816-4Padding`. + +Card personalisation interfaces often label this scheme `ISO2`, including the +`aes-256-cbc-iso2` algorithm name in the G+D CII data provider interface. + +Two schemes are commonly confused with it and neither is implemented here: + +- **PKCS#7** (RFC 5652 §6.3), which Java calls `PKCS5Padding` and OpenSSL uses by + default for CBC. It is a different byte layout and is **not** `ISO2`. Do not + substitute it merely because it also produces block-aligned data. +- **ISO 10126-2**, which pads with random bytes and stores the padding length in + the final byte. It was withdrawn in 2007 and, being non-deterministic, cannot + be checked by re-encrypting a known plaintext. + +#### Initialisation vector + The initialisation vector is always supplied by the caller and is never prefixed to, or stripped from, the ciphertext. How it is transported or derived is a property of the surrounding protocol, so that framing is left to the caller. @@ -61,7 +90,7 @@ option to pick freely, and it should be justified by the protocol specification. ```go cipher, err := aes.CreateCBCFromKeyString(hexKey) iv, err := aes.GenerateCBCIV() -cipherBytes, err := cipher.EncryptPadded(plainBytes, iv) +cipherBytes, err := cipher.EncryptISO9797M2Padded(plainBytes, iv) // Now authenticate iv and cipherBytes together, and transmit the tag alongside. ``` diff --git a/aes/aes_cbc_cipher.go b/aes/aes_cbc_cipher.go index 18dca0f..bb6b5b0 100644 --- a/aes/aes_cbc_cipher.go +++ b/aes/aes_cbc_cipher.go @@ -45,8 +45,8 @@ type CBCCipher struct { // Encrypt encrypts plainBytes under the given initialisation vector without // applying any padding. plainBytes must be a non-zero multiple of the AES block -// size; use EncryptPadded for arbitrary length input. iv must be exactly one AES -// block long, for every supported key size. +// size; use EncryptISO9797M2Padded for arbitrary length input. iv must be +// exactly one AES block long, for every supported key size. // // Neither plainBytes nor iv is modified. func (c *CBCCipher) Encrypt(plainBytes []byte, iv []byte) ([]byte, error) { @@ -91,27 +91,34 @@ func (c *CBCCipher) Decrypt(cipherBytes []byte, iv []byte) ([]byte, error) { return plainBytes, nil } -// EncryptPadded applies PKCS#7 padding (RFC 5652 section 6.3) to plainBytes and -// encrypts the result, so plainBytes may be of any length, including empty. iv -// must be exactly one AES block long. +// EncryptISO9797M2Padded applies ISO/IEC 9797-1 padding method 2 to plainBytes +// and encrypts the result, so plainBytes may be of any length, including empty. +// iv must be exactly one AES block long. // -// PKCS#7 is one padding scheme among several; it is a choice made by the -// surrounding protocol rather than a property of CBC. Use Encrypt when the -// protocol pads by other means or not at all. +// The padding is a single 0x80 byte followed by zero filler to the block +// boundary, and at least one byte is always added. The same construction appears +// in ISO/IEC 7816-4 and is labelled "ISO2" by several card personalisation +// interfaces, including the aes-256-cbc-iso2 algorithm name used by the G+D CII +// data provider interface. +// +// Padding is a choice made by the surrounding protocol rather than a property of +// CBC. This method is named after its scheme so that a protocol requiring a +// different one, such as PKCS#7 or ISO 10126-2, cannot reach it by accident. Use +// Encrypt when the protocol pads by other means or not at all. // // Neither plainBytes nor iv is modified. -func (c *CBCCipher) EncryptPadded(plainBytes []byte, iv []byte) ([]byte, error) { +func (c *CBCCipher) EncryptISO9797M2Padded(plainBytes []byte, iv []byte) ([]byte, error) { blockSize := c.block.BlockSize() if err := validateIV(iv, blockSize); err != nil { return nil, err } - return c.Encrypt(pad(plainBytes, blockSize), iv) + return c.Encrypt(padISO9797M2(plainBytes, blockSize), iv) } -// DecryptPadded decrypts cipherBytes and removes its PKCS#7 padding (RFC 5652 -// section 6.3). cipherBytes must be a non-zero multiple of the AES block size -// and iv must be exactly one AES block long. +// DecryptISO9797M2Padded decrypts cipherBytes and removes its ISO/IEC 9797-1 +// padding method 2. cipherBytes must be a non-zero multiple of the AES block +// size and iv must be exactly one AES block long. // // A padding error is NOT an integrity check. A modified ciphertext can decrypt // to well formed padding just as easily as to malformed padding, so a nil error @@ -119,13 +126,13 @@ func (c *CBCCipher) EncryptPadded(plainBytes []byte, iv []byte) ([]byte, error) // callers; doing so exposes a padding oracle. See CBCCipher. // // Neither cipherBytes nor iv is modified. -func (c *CBCCipher) DecryptPadded(cipherBytes []byte, iv []byte) ([]byte, error) { +func (c *CBCCipher) DecryptISO9797M2Padded(cipherBytes []byte, iv []byte) ([]byte, error) { paddedBytes, err := c.Decrypt(cipherBytes, iv) if err != nil { return nil, err } - return unpad(paddedBytes, c.block.BlockSize()) + return unpadISO9797M2(paddedBytes, c.block.BlockSize()) } // CheckValue returns the key check value of the underlying key. It is derived diff --git a/aes/aes_cbc_cipher_test.go b/aes/aes_cbc_cipher_test.go index 901143a..5fbb4ff 100644 --- a/aes/aes_cbc_cipher_test.go +++ b/aes/aes_cbc_cipher_test.go @@ -153,11 +153,11 @@ func TestAESCBCCipher_RejectsInvalidIVLength(t *testing.T) { if _, err := cipher.Decrypt(plainBytes, iv); err == nil { t.Errorf("%s: expected Decrypt to reject a %d byte IV", vector.name, ivLength) } - if _, err := cipher.EncryptPadded(plainBytes, iv); err == nil { - t.Errorf("%s: expected EncryptPadded to reject a %d byte IV", vector.name, ivLength) + if _, err := cipher.EncryptISO9797M2Padded(plainBytes, iv); err == nil { + t.Errorf("%s: expected EncryptISO9797M2Padded to reject a %d byte IV", vector.name, ivLength) } - if _, err := cipher.DecryptPadded(plainBytes, iv); err == nil { - t.Errorf("%s: expected DecryptPadded to reject a %d byte IV", vector.name, ivLength) + if _, err := cipher.DecryptISO9797M2Padded(plainBytes, iv); err == nil { + t.Errorf("%s: expected DecryptISO9797M2Padded to reject a %d byte IV", vector.name, ivLength) } } @@ -167,7 +167,7 @@ func TestAESCBCCipher_RejectsInvalidIVLength(t *testing.T) { } } -func TestAESCBCCipher_EncryptAndDecryptPaddedRoundTrip(t *testing.T) { +func TestAESCBCCipher_EncryptAndDecryptISO9797M2PaddedRoundTrip(t *testing.T) { iv, _ := hex.DecodeString(nistIV) for _, vector := range nistCBCVectors { @@ -182,7 +182,7 @@ func TestAESCBCCipher_EncryptAndDecryptPaddedRoundTrip(t *testing.T) { plainBytes[i] = byte(i) } - cipherBytes, err := cipher.EncryptPadded(plainBytes, iv) + cipherBytes, err := cipher.EncryptISO9797M2Padded(plainBytes, iv) if err != nil { t.Errorf("Did not expect an encryption error for length %d but got %q", length, err) continue @@ -191,7 +191,7 @@ func TestAESCBCCipher_EncryptAndDecryptPaddedRoundTrip(t *testing.T) { t.Errorf("Expected padded ciphertext longer than %d and block aligned but get %d", length, len(cipherBytes)) } - decryptedBytes, err := cipher.DecryptPadded(cipherBytes, iv) + decryptedBytes, err := cipher.DecryptISO9797M2Padded(cipherBytes, iv) if err != nil { t.Errorf("Did not expect a decryption error for length %d but got %q", length, err) continue @@ -205,32 +205,88 @@ func TestAESCBCCipher_EncryptAndDecryptPaddedRoundTrip(t *testing.T) { } } -func TestAESCBCCipher_DecryptPaddedRejectsInvalidPKCS7Padding(t *testing.T) { - cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) +// TestAESCBCCipher_EncryptISO9797M2PaddedMatchesNISTVectors ties the padded path +// back to the published known answer vectors. CBC encrypts block by block, so +// appending a padding block cannot alter the ciphertext of the blocks before it. +// The NIST plaintext is block aligned, which means ISO/IEC 9797-1 method 2 adds +// exactly one extra block and the leading blocks must match the vector byte for +// byte. +func TestAESCBCCipher_EncryptISO9797M2PaddedMatchesNISTVectors(t *testing.T) { iv, _ := hex.DecodeString(nistIV) + plainBytes, _ := hex.DecodeString(nistPlaintext) - // A block whose final byte is zero can never be valid PKCS#7 padding, so - // encrypting it raw yields a ciphertext that DecryptPadded must reject. - // This is a padding check, not an integrity check: see - // TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext. - unpaddableBlock, _ := hex.DecodeString("000102030405060708090a0b0c0d0e00") + for _, vector := range nistCBCVectors { + t.Run(vector.name, func(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(vector.key) + expectedBytes, _ := hex.DecodeString(vector.ciphertext) - cipherBytes, err := cipher.Encrypt(unpaddableBlock, iv) - if err != nil { - t.Fatalf("Did not expect an encryption error but got %q", err) - } + cipherBytes, err := cipher.EncryptISO9797M2Padded(plainBytes, iv) + if err != nil { + t.Fatalf("Did not expect an encryption error but got %q", err) + } - if _, err := cipher.DecryptPadded(cipherBytes, iv); err == nil { - t.Error("Expected DecryptPadded to reject invalid PKCS#7 padding but got no error") + if len(cipherBytes) != len(expectedBytes)+16 { + t.Fatalf("Expected %d bytes of ciphertext but got %d", len(expectedBytes)+16, len(cipherBytes)) + } + if !bytes.Equal(cipherBytes[:len(expectedBytes)], expectedBytes) { + t.Errorf("Expected leading blocks %s but get %s", hex.EncodeToString(expectedBytes), hex.EncodeToString(cipherBytes[:len(expectedBytes)])) + } + }) } +} - // The same bytes decrypt without complaint when no padding is expected. - plainBytes, err := cipher.Decrypt(cipherBytes, iv) - if err != nil { - t.Errorf("Did not expect a raw decryption error but got %q", err) +func TestAESCBCCipher_DecryptISO9797M2PaddedRejectsMalformedPadding(t *testing.T) { + cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) + iv, _ := hex.DecodeString(nistIV) + + // Each of these blocks fails the ISO/IEC 9797-1 method 2 rules, so + // encrypting it raw yields a ciphertext that DecryptISO9797M2Padded must + // reject. This is a padding check, not an integrity check: see + // TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext. + testCases := []struct { + name string + block string + }{ + { + name: "final byte is neither the marker nor zero filler", + block: "000102030405060708090a0b0c0d0e01", + }, + { + name: "zero filler runs back to a non marker byte", + block: "000102030405060708090a0b0c0d0e00", + }, + { + name: "block is all zeroes so the marker is missing", + block: "00000000000000000000000000000000", + }, + { + name: "PKCS#7 padding is not accepted in place of the marker", + block: "000102030405060708090a0b0c0d0202", + }, } - if !bytes.Equal(plainBytes, unpaddableBlock) { - t.Errorf("Expected %s but get %s", hex.EncodeToString(unpaddableBlock), hex.EncodeToString(plainBytes)) + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + unpaddableBlock, _ := hex.DecodeString(testCase.block) + + cipherBytes, err := cipher.Encrypt(unpaddableBlock, iv) + if err != nil { + t.Fatalf("Did not expect an encryption error but got %q", err) + } + + if _, err := cipher.DecryptISO9797M2Padded(cipherBytes, iv); err == nil { + t.Error("Expected DecryptISO9797M2Padded to reject malformed padding but got no error") + } + + // The same bytes decrypt without complaint when no padding is expected. + plainBytes, err := cipher.Decrypt(cipherBytes, iv) + if err != nil { + t.Errorf("Did not expect a raw decryption error but got %q", err) + } + if !bytes.Equal(plainBytes, unpaddableBlock) { + t.Errorf("Expected %s but get %s", hex.EncodeToString(unpaddableBlock), hex.EncodeToString(plainBytes)) + } + }) } } @@ -260,18 +316,18 @@ func TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext(t *testing.T) { } } -// TestAESCBCCipher_DecryptPaddedDoesNotDetectModifiedIV goes further: flipping a -// bit in the initialisation vector flips exactly the same bit in the first -// plaintext block and leaves every later block, including the padding block, -// untouched. DecryptPadded therefore returns attacker chosen plaintext with a -// nil error. Valid padding is not integrity. -func TestAESCBCCipher_DecryptPaddedDoesNotDetectModifiedIV(t *testing.T) { +// TestAESCBCCipher_DecryptISO9797M2PaddedDoesNotDetectModifiedIV goes further: +// flipping a bit in the initialisation vector flips exactly the same bit in the +// first plaintext block and leaves every later block, including the padding +// block, untouched. DecryptISO9797M2Padded therefore returns attacker chosen +// plaintext with a nil error. Valid padding is not integrity. +func TestAESCBCCipher_DecryptISO9797M2PaddedDoesNotDetectModifiedIV(t *testing.T) { cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) iv, _ := hex.DecodeString(nistIV) plainBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e0f") - // One block of plaintext plus a whole block of PKCS#7 padding. - cipherBytes, err := cipher.EncryptPadded(plainBytes, iv) + // Block aligned plaintext, so the padding occupies a whole extra block. + cipherBytes, err := cipher.EncryptISO9797M2Padded(plainBytes, iv) if err != nil { t.Fatalf("Did not expect an encryption error but got %q", err) } @@ -279,7 +335,7 @@ func TestAESCBCCipher_DecryptPaddedDoesNotDetectModifiedIV(t *testing.T) { modifiedIV := cloneBytes(iv) modifiedIV[0] ^= 0xff - decryptedBytes, err := cipher.DecryptPadded(cipherBytes, modifiedIV) + decryptedBytes, err := cipher.DecryptISO9797M2Padded(cipherBytes, modifiedIV) if err != nil { t.Fatalf("Expected a modified IV to still decrypt without error but got %q", err) } @@ -296,7 +352,7 @@ func TestAESCBCCipher_DoesNotModifyInputs(t *testing.T) { iv, _ := hex.DecodeString(nistIV) plainBytes, _ := hex.DecodeString(nistPlaintext) cipherBytes, _ := hex.DecodeString(nistCBCVectors[0].ciphertext) - paddedCipherBytes, _ := cipher.EncryptPadded(plainBytes, iv) + paddedCipherBytes, _ := cipher.EncryptISO9797M2Padded(plainBytes, iv) operations := []struct { name string @@ -305,8 +361,8 @@ func TestAESCBCCipher_DoesNotModifyInputs(t *testing.T) { }{ {"Encrypt", plainBytes, cipher.Encrypt}, {"Decrypt", cipherBytes, cipher.Decrypt}, - {"EncryptPadded", plainBytes, cipher.EncryptPadded}, - {"DecryptPadded", paddedCipherBytes, cipher.DecryptPadded}, + {"EncryptISO9797M2Padded", plainBytes, cipher.EncryptISO9797M2Padded}, + {"DecryptISO9797M2Padded", paddedCipherBytes, cipher.DecryptISO9797M2Padded}, } for _, operation := range operations { diff --git a/aes/iso9797m2.go b/aes/iso9797m2.go new file mode 100644 index 0000000..06cff46 --- /dev/null +++ b/aes/iso9797m2.go @@ -0,0 +1,84 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aes + +import "fmt" + +// iso9797M2Marker is the single 0x80 byte that ISO/IEC 9797-1 padding method 2 +// appends before the zero filler. At bit level the standard appends one 1 bit +// followed by 0 bits; on byte aligned data that is 0x80 then 0x00. +const iso9797M2Marker = 0x80 + +// padISO9797M2 appends ISO/IEC 9797-1 padding method 2 to plainBytes so that +// its length becomes a multiple of blockSize. The padding is a single 0x80 byte +// followed by as many 0x00 bytes as the block boundary requires. +// +// The same construction appears in ISO/IEC 7816-4 and is what BouncyCastle +// calls ISO7816-4Padding. Card personalisation interfaces often label it +// "ISO2", including the aes-256-cbc-iso2 algorithm name used by the G+D CII +// data provider interface. It is NOT ISO 10126-2, which pads with random bytes +// and stores the padding length in the final byte, and it is NOT PKCS#7. +// +// At least one byte is always added, so input that is already block aligned +// gains a whole extra block. That is what makes the padding removable without +// ambiguity, including when the message itself ends in 0x80 or 0x00. An empty +// input yields a single full block. +// +// The input slice is never modified; a new slice is always returned. +func padISO9797M2(plainBytes []byte, blockSize int) []byte { + padLen := blockSize - len(plainBytes)%blockSize + + padded := make([]byte, 0, len(plainBytes)+padLen) + padded = append(padded, plainBytes...) + padded = append(padded, iso9797M2Marker) + padded = append(padded, make([]byte, padLen-1)...) + return padded +} + +// unpadISO9797M2 removes ISO/IEC 9797-1 padding method 2 from paddedBytes by +// scanning back from the end over the zero filler to the 0x80 marker. It +// returns an error when the padding is not well formed. +// +// A padding error is not evidence of tampering alone, and callers must not +// treat it as an integrity check: CBC is unauthenticated, so a modified +// ciphertext can just as easily decrypt to well formed padding. Reporting +// padding failures to an untrusted caller also exposes a padding oracle. +// +// The input slice is never modified; the returned slice is a copy. +func unpadISO9797M2(paddedBytes []byte, blockSize int) ([]byte, error) { + if len(paddedBytes) == 0 { + return nil, fmt.Errorf("padded input is empty") + } + if len(paddedBytes)%blockSize != 0 { + return nil, fmt.Errorf("padded input length %d is not a multiplier of block size %d", len(paddedBytes), blockSize) + } + + markerIndex := len(paddedBytes) - 1 + for markerIndex >= 0 && paddedBytes[markerIndex] == 0x00 { + markerIndex-- + } + if markerIndex < 0 || paddedBytes[markerIndex] != iso9797M2Marker { + return nil, fmt.Errorf("invalid padding") + } + + // A well formed marker sits within the final block, because the padding is + // between 1 and blockSize bytes long. Anything longer means the trailing + // zeroes are message content and the padding is absent or corrupt. + if len(paddedBytes)-markerIndex > blockSize { + return nil, fmt.Errorf("invalid padding") + } + + plainBytes := make([]byte, markerIndex) + copy(plainBytes, paddedBytes) + return plainBytes, nil +} diff --git a/aes/iso9797m2_test.go b/aes/iso9797m2_test.go new file mode 100644 index 0000000..d88af08 --- /dev/null +++ b/aes/iso9797m2_test.go @@ -0,0 +1,304 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package aes + +import ( + "bytes" + "encoding/hex" + "testing" +) + +const testBlockSize = 16 + +func TestISO9797M2Pad(t *testing.T) { + testCases := []struct { + name string + input string + expected string + }{ + { + name: "empty input gains a whole block", + input: "", + expected: "80000000000000000000000000000000", + }, + { + name: "one byte short of a block gains only the marker", + input: "000102030405060708090a0b0c0d0e", + expected: "000102030405060708090a0b0c0d0e80", + }, + { + name: "block aligned input gains a whole extra block", + input: "000102030405060708090a0b0c0d0e0f", + expected: "000102030405060708090a0b0c0d0e0f" + "80000000000000000000000000000000", + }, + { + name: "single byte gains marker and fourteen zeroes", + input: "ff", + expected: "ff800000000000000000000000000000", + }, + { + name: "input ending in the marker byte is still unambiguous", + input: "0080", + expected: "00808000000000000000000000000000", + }, + { + name: "input ending in zeroes is still unambiguous", + input: "ff0000", + expected: "ff000080000000000000000000000000", + }, + { + name: "input spanning more than one block", + input: "000102030405060708090a0b0c0d0e0f" + "1011", + expected: "000102030405060708090a0b0c0d0e0f" + "10118000000000000000000000000000", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + inputBytes := mustDecodeHex(t, testCase.input) + expectedBytes := mustDecodeHex(t, testCase.expected) + + padded := padISO9797M2(inputBytes, testBlockSize) + + if !bytes.Equal(padded, expectedBytes) { + t.Fatalf("padded = %x, want %x", padded, expectedBytes) + } + if len(padded)%testBlockSize != 0 { + t.Fatalf("padded length %d is not a multiple of %d", len(padded), testBlockSize) + } + if len(padded) <= len(inputBytes) { + t.Fatalf("padding must always add at least one byte, input %d padded %d", len(inputBytes), len(padded)) + } + }) + } +} + +func TestISO9797M2PadAlwaysAddsBetweenOneAndBlockSizeBytes(t *testing.T) { + for inputLen := 0; inputLen <= 3*testBlockSize; inputLen++ { + inputBytes := make([]byte, inputLen) + padded := padISO9797M2(inputBytes, testBlockSize) + + padLen := len(padded) - inputLen + if padLen < 1 || padLen > testBlockSize { + t.Fatalf("input length %d: padding of %d bytes is out of range 1..%d", inputLen, padLen, testBlockSize) + } + if padded[inputLen] != iso9797M2Marker { + t.Fatalf("input length %d: marker byte = %#x, want %#x", inputLen, padded[inputLen], iso9797M2Marker) + } + for i, padByte := range padded[inputLen+1:] { + if padByte != 0x00 { + t.Fatalf("input length %d: filler byte %d = %#x, want 0x00", inputLen, i, padByte) + } + } + } +} + +func TestISO9797M2PadDoesNotModifyInput(t *testing.T) { + inputBytes := mustDecodeHex(t, "000102030405060708090a0b0c0d0e0f1011") + original := append([]byte(nil), inputBytes...) + + padISO9797M2(inputBytes, testBlockSize) + + if !bytes.Equal(inputBytes, original) { + t.Fatalf("input was modified: got %x, want %x", inputBytes, original) + } +} + +func TestISO9797M2Unpad(t *testing.T) { + testCases := []struct { + name string + input string + expected string + }{ + { + name: "whole block of padding yields empty output", + input: "80000000000000000000000000000000", + expected: "", + }, + { + name: "marker only", + input: "000102030405060708090a0b0c0d0e80", + expected: "000102030405060708090a0b0c0d0e", + }, + { + name: "extra block of padding after block aligned message", + input: "000102030405060708090a0b0c0d0e0f" + "80000000000000000000000000000000", + expected: "000102030405060708090a0b0c0d0e0f", + }, + { + name: "message ending in the marker byte", + input: "00808000000000000000000000000000", + expected: "0080", + }, + { + name: "message ending in zeroes", + input: "ff000080000000000000000000000000", + expected: "ff0000", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + inputBytes := mustDecodeHex(t, testCase.input) + expectedBytes := mustDecodeHex(t, testCase.expected) + + unpadded, err := unpadISO9797M2(inputBytes, testBlockSize) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !bytes.Equal(unpadded, expectedBytes) { + t.Fatalf("unpadded = %x, want %x", unpadded, expectedBytes) + } + }) + } +} + +func TestISO9797M2UnpadRejectsMalformedPadding(t *testing.T) { + testCases := []struct { + name string + input string + }{ + { + name: "empty input", + input: "", + }, + { + name: "input not a multiple of the block size", + input: "000102030405060708090a0b0c0d0e", + }, + { + name: "no marker at all", + input: "000102030405060708090a0b0c0d0e0f", + }, + { + name: "trailing byte is neither marker nor zero", + input: "000102030405060708090a0b0c0d0e01", + }, + { + name: "all zeroes, so the marker is missing", + input: "00000000000000000000000000000000", + }, + { + name: "final block is all zeroes with the marker too far back", + input: "000102030405060708090a0b0c0d0e80" + "00000000000000000000000000000000", + }, + { + name: "PKCS#7 padding is rejected rather than silently accepted", + input: "000102030405060708090a0b0c0d" + "0202", + }, + { + name: "PKCS#7 whole block of padding is rejected", + input: "10101010101010101010101010101010", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + inputBytes := mustDecodeHex(t, testCase.input) + + unpadded, err := unpadISO9797M2(inputBytes, testBlockSize) + if err == nil { + t.Fatalf("expected an error but got output %x", unpadded) + } + if unpadded != nil { + t.Fatalf("expected nil output alongside the error but got %x", unpadded) + } + }) + } +} + +func TestISO9797M2UnpadDoesNotModifyInput(t *testing.T) { + inputBytes := mustDecodeHex(t, "ff000080000000000000000000000000") + original := append([]byte(nil), inputBytes...) + + if _, err := unpadISO9797M2(inputBytes, testBlockSize); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !bytes.Equal(inputBytes, original) { + t.Fatalf("input was modified: got %x, want %x", inputBytes, original) + } +} + +func TestISO9797M2UnpadReturnsCopyOfInput(t *testing.T) { + inputBytes := mustDecodeHex(t, "ff112280000000000000000000000000") + + unpadded, err := unpadISO9797M2(inputBytes, testBlockSize) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + unpadded[0] = 0x00 + if inputBytes[0] != 0xff { + t.Fatal("mutating the returned slice also mutated the input") + } +} + +func TestISO9797M2RoundTrip(t *testing.T) { + for inputLen := 0; inputLen <= 3*testBlockSize; inputLen++ { + inputBytes := make([]byte, inputLen) + for i := range inputBytes { + inputBytes[i] = byte(i) + } + + padded := padISO9797M2(inputBytes, testBlockSize) + unpadded, err := unpadISO9797M2(padded, testBlockSize) + if err != nil { + t.Fatalf("input length %d: unexpected error: %v", inputLen, err) + } + if !bytes.Equal(unpadded, inputBytes) { + t.Fatalf("input length %d: round trip gave %x, want %x", inputLen, unpadded, inputBytes) + } + } +} + +func TestISO9797M2RoundTripWithTrailingMarkerAndZeroBytes(t *testing.T) { + // The awkward cases for this scheme are messages that already end in the + // bytes the padding uses. Because padding is always added, these still round + // trip exactly. + testCases := []string{ + "80", + "00", + "8000", + "0080", + "800000000000000000000000000000", + "00000000000000000000000000000080", + "ff" + "00000000000000000000000000000000", + "ff" + "80000000000000000000000000000000", + } + + for _, testCase := range testCases { + t.Run(testCase, func(t *testing.T) { + inputBytes := mustDecodeHex(t, testCase) + + padded := padISO9797M2(inputBytes, testBlockSize) + unpadded, err := unpadISO9797M2(padded, testBlockSize) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !bytes.Equal(unpadded, inputBytes) { + t.Fatalf("round trip gave %x, want %x", unpadded, inputBytes) + } + }) + } +} + +func mustDecodeHex(t *testing.T, s string) []byte { + t.Helper() + + decoded, err := hex.DecodeString(s) + if err != nil { + t.Fatalf("could not decode hex %q: %v", s, err) + } + return decoded +} diff --git a/aes/pkcs7.go b/aes/pkcs7.go deleted file mode 100644 index 905e2a3..0000000 --- a/aes/pkcs7.go +++ /dev/null @@ -1,65 +0,0 @@ -/* - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package aes - -import "fmt" - -// pad appends PKCS#7 padding (RFC 5652 section 6.3) to plainBytes so that its -// length becomes a multiple of blockSize. Input that is already block aligned -// gains a whole extra block of padding, so the padding is always removable -// without ambiguity. An empty input therefore yields a single full block. -// -// The input slice is never modified; a new slice is always returned. -func pad(plainBytes []byte, blockSize int) []byte { - padLen := blockSize - len(plainBytes)%blockSize - - padded := make([]byte, 0, len(plainBytes)+padLen) - padded = append(padded, plainBytes...) - for i := 0; i < padLen; i++ { - padded = append(padded, byte(padLen)) - } - return padded -} - -// unpad removes PKCS#7 padding (RFC 5652 section 6.3) from paddedBytes. It -// returns an error when the padding is not well formed. -// -// A padding error is not evidence of tampering alone, and callers must not -// treat it as an integrity check: CBC is unauthenticated, so a modified -// ciphertext can just as easily decrypt to well formed padding. Reporting -// padding failures to an untrusted caller also exposes a padding oracle. -// -// The input slice is never modified; the returned slice is a copy. -func unpad(paddedBytes []byte, blockSize int) ([]byte, error) { - if len(paddedBytes) == 0 { - return nil, fmt.Errorf("padded input is empty") - } - if len(paddedBytes)%blockSize != 0 { - return nil, fmt.Errorf("padded input length %d is not a multiplier of block size %d", len(paddedBytes), blockSize) - } - - padLen := int(paddedBytes[len(paddedBytes)-1]) - if padLen == 0 || padLen > blockSize { - return nil, fmt.Errorf("invalid padding") - } - - for _, padByte := range paddedBytes[len(paddedBytes)-padLen:] { - if int(padByte) != padLen { - return nil, fmt.Errorf("invalid padding") - } - } - - plainBytes := make([]byte, len(paddedBytes)-padLen) - copy(plainBytes, paddedBytes) - return plainBytes, nil -} diff --git a/aes/pkcs7_test.go b/aes/pkcs7_test.go deleted file mode 100644 index c12870b..0000000 --- a/aes/pkcs7_test.go +++ /dev/null @@ -1,167 +0,0 @@ -/* - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package aes - -import ( - "bytes" - "encoding/hex" - "testing" -) - -const testBlockSize = 16 - -func TestPKCS7Pad(t *testing.T) { - testCases := []struct { - name string - plain string - expected string - }{ - // RFC 5652 section 6.3: block aligned input gains a whole extra block, - // so an empty input pads to a single full block. - {"empty", "", "10101010101010101010101010101010"}, - {"one byte", "00", "000f0f0f0f0f0f0f0f0f0f0f0f0f0f0f"}, - {"fifteen bytes", "000102030405060708090a0b0c0d0e", "000102030405060708090a0b0c0d0e01"}, - {"one full block", "000102030405060708090a0b0c0d0e0f", "000102030405060708090a0b0c0d0e0f10101010101010101010101010101010"}, - {"one block plus one byte", "000102030405060708090a0b0c0d0e0f10", "000102030405060708090a0b0c0d0e0f100f0f0f0f0f0f0f0f0f0f0f0f0f0f0f"}, - {"two full blocks", "000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f", "000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f10101010101010101010101010101010"}, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - plainBytes, _ := hex.DecodeString(testCase.plain) - - padded := hex.EncodeToString(pad(plainBytes, testBlockSize)) - if padded != testCase.expected { - t.Errorf("Expected %s but get %s", testCase.expected, padded) - } - }) - } -} - -func TestPKCS7PadDoesNotModifyInput(t *testing.T) { - plainBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e0f") - original := hex.EncodeToString(plainBytes) - - pad(plainBytes, testBlockSize) - - if hex.EncodeToString(plainBytes) != original { - t.Errorf("Expected input to be left as %s but get %s", original, hex.EncodeToString(plainBytes)) - } -} - -func TestPKCS7Unpad(t *testing.T) { - testCases := []struct { - name string - padded string - expected string - }{ - {"empty plaintext", "10101010101010101010101010101010", ""}, - {"one byte", "000f0f0f0f0f0f0f0f0f0f0f0f0f0f0f", "00"}, - {"fifteen bytes", "000102030405060708090a0b0c0d0e01", "000102030405060708090a0b0c0d0e"}, - {"one full block", "000102030405060708090a0b0c0d0e0f10101010101010101010101010101010", "000102030405060708090a0b0c0d0e0f"}, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - paddedBytes, _ := hex.DecodeString(testCase.padded) - - plainBytes, err := unpad(paddedBytes, testBlockSize) - if err != nil { - t.Errorf("Did not expect an error but got %q", err) - } - - if hex.EncodeToString(plainBytes) != testCase.expected { - t.Errorf("Expected %s but get %s", testCase.expected, hex.EncodeToString(plainBytes)) - } - }) - } -} - -func TestPKCS7UnpadRejectsMalformedPadding(t *testing.T) { - testCases := []struct { - name string - padded string - }{ - {"empty input", ""}, - {"length not a multiple of the block size", "000102030405060708090a0b0c0d0e"}, - {"pad byte of zero", "000102030405060708090a0b0c0d0e00"}, - {"pad byte larger than the block size", "000102030405060708090a0b0c0d0e11"}, - {"inconsistent pad bytes", "000102030405060708090a0b0c0d0203"}, - {"pad bytes not repeated for the full length", "0001020304050607080900000000ff04"}, - {"padding claims the whole block but is not uniform", "0102030405060708090a0b0c0d0e0f10"}, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - paddedBytes, _ := hex.DecodeString(testCase.padded) - - if _, err := unpad(paddedBytes, testBlockSize); err == nil { - t.Error("Expected an error but got none") - } - }) - } -} - -func TestPKCS7UnpadDoesNotModifyInput(t *testing.T) { - paddedBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e01") - original := hex.EncodeToString(paddedBytes) - - if _, err := unpad(paddedBytes, testBlockSize); err != nil { - t.Errorf("Did not expect an error but got %q", err) - } - - if hex.EncodeToString(paddedBytes) != original { - t.Errorf("Expected input to be left as %s but get %s", original, hex.EncodeToString(paddedBytes)) - } -} - -func TestPKCS7UnpadReturnsCopyOfInput(t *testing.T) { - paddedBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e01") - - plainBytes, err := unpad(paddedBytes, testBlockSize) - if err != nil { - t.Errorf("Did not expect an error but got %q", err) - } - - plainBytes[0] = 0xff - if paddedBytes[0] == 0xff { - t.Error("Expected the returned slice not to alias the input") - } -} - -func TestPKCS7RoundTrip(t *testing.T) { - for length := 0; length <= 48; length++ { - plainBytes := make([]byte, length) - for i := range plainBytes { - plainBytes[i] = byte(i) - } - - padded := pad(plainBytes, testBlockSize) - if len(padded)%testBlockSize != 0 { - t.Errorf("Expected padded length to be a multiplier of %d but get %d", testBlockSize, len(padded)) - } - if len(padded) <= length { - t.Errorf("Expected padded length to be greater than %d but get %d", length, len(padded)) - } - - unpadded, err := unpad(padded, testBlockSize) - if err != nil { - t.Errorf("Did not expect an error for length %d but got %q", length, err) - continue - } - - if !bytes.Equal(plainBytes, unpadded) { - t.Errorf("Expected %s but get %s", hex.EncodeToString(plainBytes), hex.EncodeToString(unpadded)) - } - } -} From 1f5bbbb4ba1da704f03fb64e837c9c370b837762 Mon Sep 17 00:00:00 2001 From: Keith Fong Date: Thu, 3 Sep 2026 16:35:47 +0800 Subject: [PATCH 3/4] docs: simplify AES-CBC comments --- aes/aes_cbc_cipher.go | 79 ++++++++------------------------------ aes/aes_cbc_cipher_test.go | 43 +++++---------------- aes/aes_cbc_factory.go | 16 ++------ aes/aes_cipher.go | 11 ++---- aes/iso9797m2.go | 37 +++--------------- aes/iso9797m2_test.go | 4 +- 6 files changed, 37 insertions(+), 153 deletions(-) diff --git a/aes/aes_cbc_cipher.go b/aes/aes_cbc_cipher.go index bb6b5b0..f9d34c4 100644 --- a/aes/aes_cbc_cipher.go +++ b/aes/aes_cbc_cipher.go @@ -18,37 +18,18 @@ import ( "fmt" ) -// CBCCipher is a wrapper of the AES CBC cipher and stores the raw key bytes. +// CBCCipher encrypts and decrypts data with AES-CBC. // -// CBC provides confidentiality only. Unlike Cipher, which wraps AES GCM, it is -// NOT authenticated: -// -// - A modified initialisation vector or ciphertext will usually still decrypt -// without error, producing attacker influenced plaintext. Decryption -// succeeding is therefore no evidence that the message is genuine. -// - Callers MUST authenticate the initialisation vector together with the -// ciphertext by separate means, for example an HMAC over both, and MUST -// verify it before decrypting. -// - Reporting padding failures back to an untrusted caller exposes a padding -// oracle, which can be used to recover plaintext. -// -// Prefer Cipher (AES GCM) unless an external protocol mandates CBC. -// -// The initialisation vector is always supplied by the caller and is never -// prefixed to, or stripped from, the ciphertext by this type. How the -// initialisation vector is transported or derived is a property of the -// surrounding protocol, so it is left to the caller. +// CBC does not authenticate data. Callers must authenticate the IV and +// ciphertext separately and must not expose padding errors to untrusted callers. +// The caller supplies the IV; CBCCipher does not include it in the ciphertext. type CBCCipher struct { block cipher.Block KeyBytes []byte } -// Encrypt encrypts plainBytes under the given initialisation vector without -// applying any padding. plainBytes must be a non-zero multiple of the AES block -// size; use EncryptISO9797M2Padded for arbitrary length input. iv must be -// exactly one AES block long, for every supported key size. -// -// Neither plainBytes nor iv is modified. +// Encrypt encrypts non-empty, block-aligned plainBytes with a 16-byte IV. +// It does not add padding or modify its inputs. func (c *CBCCipher) Encrypt(plainBytes []byte, iv []byte) ([]byte, error) { blockSize := c.block.BlockSize() if err := validateIV(iv, blockSize); err != nil { @@ -66,14 +47,8 @@ func (c *CBCCipher) Encrypt(plainBytes []byte, iv []byte) ([]byte, error) { return cipherBytes, nil } -// Decrypt decrypts cipherBytes under the given initialisation vector without -// removing any padding. cipherBytes must be a non-zero multiple of the AES block -// size and iv must be exactly one AES block long. -// -// Decrypt does not, and cannot, detect modification of cipherBytes or iv. See -// CBCCipher for the caller's obligations. -// -// Neither cipherBytes nor iv is modified. +// Decrypt decrypts non-empty, block-aligned cipherBytes with a 16-byte IV. +// It does not remove padding or modify its inputs. func (c *CBCCipher) Decrypt(cipherBytes []byte, iv []byte) ([]byte, error) { blockSize := c.block.BlockSize() if err := validateIV(iv, blockSize); err != nil { @@ -91,22 +66,9 @@ func (c *CBCCipher) Decrypt(cipherBytes []byte, iv []byte) ([]byte, error) { return plainBytes, nil } -// EncryptISO9797M2Padded applies ISO/IEC 9797-1 padding method 2 to plainBytes -// and encrypts the result, so plainBytes may be of any length, including empty. -// iv must be exactly one AES block long. -// -// The padding is a single 0x80 byte followed by zero filler to the block -// boundary, and at least one byte is always added. The same construction appears -// in ISO/IEC 7816-4 and is labelled "ISO2" by several card personalisation -// interfaces, including the aes-256-cbc-iso2 algorithm name used by the G+D CII -// data provider interface. -// -// Padding is a choice made by the surrounding protocol rather than a property of -// CBC. This method is named after its scheme so that a protocol requiring a -// different one, such as PKCS#7 or ISO 10126-2, cannot reach it by accident. Use -// Encrypt when the protocol pads by other means or not at all. -// -// Neither plainBytes nor iv is modified. +// EncryptISO9797M2Padded pads plainBytes with ISO/IEC 9797-1 method 2, then +// encrypts it with a 16-byte IV. It accepts empty input and does not modify its +// inputs. func (c *CBCCipher) EncryptISO9797M2Padded(plainBytes []byte, iv []byte) ([]byte, error) { blockSize := c.block.BlockSize() if err := validateIV(iv, blockSize); err != nil { @@ -116,16 +78,8 @@ func (c *CBCCipher) EncryptISO9797M2Padded(plainBytes []byte, iv []byte) ([]byte return c.Encrypt(padISO9797M2(plainBytes, blockSize), iv) } -// DecryptISO9797M2Padded decrypts cipherBytes and removes its ISO/IEC 9797-1 -// padding method 2. cipherBytes must be a non-zero multiple of the AES block -// size and iv must be exactly one AES block long. -// -// A padding error is NOT an integrity check. A modified ciphertext can decrypt -// to well formed padding just as easily as to malformed padding, so a nil error -// says nothing about authenticity. Do not surface padding errors to untrusted -// callers; doing so exposes a padding oracle. See CBCCipher. -// -// Neither cipherBytes nor iv is modified. +// DecryptISO9797M2Padded decrypts cipherBytes with a 16-byte IV, then removes +// ISO/IEC 9797-1 method 2 padding. It does not modify its inputs. func (c *CBCCipher) DecryptISO9797M2Padded(cipherBytes []byte, iv []byte) ([]byte, error) { paddedBytes, err := c.Decrypt(cipherBytes, iv) if err != nil { @@ -135,15 +89,12 @@ func (c *CBCCipher) DecryptISO9797M2Padded(cipherBytes []byte, iv []byte) ([]byt return unpadISO9797M2(paddedBytes, c.block.BlockSize()) } -// CheckValue returns the key check value of the underlying key. It is derived -// from the key alone and is identical to the value Cipher reports for the same -// key, being independent of the mode of operation. +// CheckValue returns the AES key check value. func (c *CBCCipher) CheckValue() string { return deriveCheckValue(c.KeyBytes) } -// VerifyCheckValue reports whether the given key check value matches the one -// derived from the underlying key. The comparison is case insensitive. +// VerifyCheckValue reports whether checkValue matches the key, ignoring case. func (c *CBCCipher) VerifyCheckValue(checkValue string) bool { return verifyCheckValue(c.KeyBytes, checkValue) } diff --git a/aes/aes_cbc_cipher_test.go b/aes/aes_cbc_cipher_test.go index 5fbb4ff..b212bcd 100644 --- a/aes/aes_cbc_cipher_test.go +++ b/aes/aes_cbc_cipher_test.go @@ -18,9 +18,7 @@ import ( "testing" ) -// Known answer vectors from NIST SP 800-38A appendix F.2, the four block CBC -// examples for each AES key size. The plaintext and initialisation vector are -// shared by all six sub-examples. +// NIST SP 800-38A appendix F.2 uses the same plaintext and IV for each key size. const ( nistIV = "000102030405060708090a0b0c0d0e0f" nistPlaintext = "6bc1bee22e409f96e93d7e117393172a" + @@ -138,9 +136,7 @@ func TestAESCBCCipher_EncryptRejectsInvalidInputLength(t *testing.T) { func TestAESCBCCipher_RejectsInvalidIVLength(t *testing.T) { plainBytes, _ := hex.DecodeString(nistPlaintext) - // The initialisation vector is one AES block for every key size, so a key - // sized initialisation vector is a mistake worth catching for AES-192 and - // AES-256 in particular. + // An AES IV is always 16 bytes, regardless of key size. for _, vector := range nistCBCVectors { cipher, _ := CreateCBCFromKeyString(vector.key) @@ -174,8 +170,7 @@ func TestAESCBCCipher_EncryptAndDecryptISO9797M2PaddedRoundTrip(t *testing.T) { t.Run(vector.name, func(t *testing.T) { cipher, _ := CreateCBCFromKeyString(vector.key) - // Lengths either side of the block boundary, including empty input - // and exactly block aligned input. + // Cover both sides of two block boundaries. for length := 0; length <= 33; length++ { plainBytes := make([]byte, length) for i := range plainBytes { @@ -205,12 +200,7 @@ func TestAESCBCCipher_EncryptAndDecryptISO9797M2PaddedRoundTrip(t *testing.T) { } } -// TestAESCBCCipher_EncryptISO9797M2PaddedMatchesNISTVectors ties the padded path -// back to the published known answer vectors. CBC encrypts block by block, so -// appending a padding block cannot alter the ciphertext of the blocks before it. -// The NIST plaintext is block aligned, which means ISO/IEC 9797-1 method 2 adds -// exactly one extra block and the leading blocks must match the vector byte for -// byte. +// A trailing padding block must not change the preceding NIST ciphertext blocks. func TestAESCBCCipher_EncryptISO9797M2PaddedMatchesNISTVectors(t *testing.T) { iv, _ := hex.DecodeString(nistIV) plainBytes, _ := hex.DecodeString(nistPlaintext) @@ -239,10 +229,7 @@ func TestAESCBCCipher_DecryptISO9797M2PaddedRejectsMalformedPadding(t *testing.T cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) iv, _ := hex.DecodeString(nistIV) - // Each of these blocks fails the ISO/IEC 9797-1 method 2 rules, so - // encrypting it raw yields a ciphertext that DecryptISO9797M2Padded must - // reject. This is a padding check, not an integrity check: see - // TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext. + // Encrypt malformed padded plaintext to exercise the public decrypt path. testCases := []struct { name string block string @@ -278,7 +265,7 @@ func TestAESCBCCipher_DecryptISO9797M2PaddedRejectsMalformedPadding(t *testing.T t.Error("Expected DecryptISO9797M2Padded to reject malformed padding but got no error") } - // The same bytes decrypt without complaint when no padding is expected. + // Raw decryption does not validate padding. plainBytes, err := cipher.Decrypt(cipherBytes, iv) if err != nil { t.Errorf("Did not expect a raw decryption error but got %q", err) @@ -290,10 +277,7 @@ func TestAESCBCCipher_DecryptISO9797M2PaddedRejectsMalformedPadding(t *testing.T } } -// TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext records that CBC is -// unauthenticated. A modified ciphertext still decrypts without error, so a nil -// error from Decrypt is no evidence that the message is genuine. Callers must -// authenticate the initialisation vector and the ciphertext separately. +// CBC decryption cannot detect ciphertext changes. func TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext(t *testing.T) { cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) iv, _ := hex.DecodeString(nistIV) @@ -316,17 +300,13 @@ func TestAESCBCCipher_DecryptDoesNotDetectModifiedCiphertext(t *testing.T) { } } -// TestAESCBCCipher_DecryptISO9797M2PaddedDoesNotDetectModifiedIV goes further: -// flipping a bit in the initialisation vector flips exactly the same bit in the -// first plaintext block and leaves every later block, including the padding -// block, untouched. DecryptISO9797M2Padded therefore returns attacker chosen -// plaintext with a nil error. Valid padding is not integrity. +// Changing the IV changes the first plaintext block without invalidating padding. func TestAESCBCCipher_DecryptISO9797M2PaddedDoesNotDetectModifiedIV(t *testing.T) { cipher, _ := CreateCBCFromKeyString(nistCBCVectors[0].key) iv, _ := hex.DecodeString(nistIV) plainBytes, _ := hex.DecodeString("000102030405060708090a0b0c0d0e0f") - // Block aligned plaintext, so the padding occupies a whole extra block. + // Padding occupies a separate block. cipherBytes, err := cipher.EncryptISO9797M2Padded(plainBytes, iv) if err != nil { t.Fatalf("Did not expect an encryption error but got %q", err) @@ -404,7 +384,6 @@ func TestAESCBCCipher_DifferentIVsProduceDifferentCiphertext(t *testing.T) { t.Error("Expected the same plaintext under different IVs to produce different ciphertext") } - // Each ciphertext must still decrypt under its own initialisation vector. for _, testCase := range []struct { cipherBytes []byte iv []byte @@ -478,9 +457,7 @@ func TestCreateCBCFromKeyString(t *testing.T) { } } -// TestAESCBCCheckValueMatchesGCMCipher asserts that the key check value depends -// only on the key, so a key imported for CBC use reports the same value as the -// same key used with GCM. +// A KCV depends on the key, not the AES mode. func TestAESCBCCheckValueMatchesGCMCipher(t *testing.T) { keyBytes, _ := hex.DecodeString("2b7e151628aed2a6abf7158809cf4f3c") diff --git a/aes/aes_cbc_factory.go b/aes/aes_cbc_factory.go index e90ca47..1f5fb44 100644 --- a/aes/aes_cbc_factory.go +++ b/aes/aes_cbc_factory.go @@ -20,7 +20,7 @@ import ( "github.com/hashicorp/go-uuid" ) -// CreateCBCFromKeyBytes constructs a new AES CBC cipher using the raw key bytes provided, the raw bytes must be either 16, 24, or 32 bytes +// CreateCBCFromKeyBytes creates an AES-CBC cipher from a 16, 24, or 32-byte key. func CreateCBCFromKeyBytes(keyBytes []byte) (CBCCipher, error) { if len(keyBytes) != 16 && len(keyBytes) != 24 && len(keyBytes) != 32 { return CBCCipher{}, errors.New("AES key must be 16, 24, or 32 bytes") @@ -34,7 +34,7 @@ func CreateCBCFromKeyBytes(keyBytes []byte) (CBCCipher, error) { return CBCCipher{keyBlock, keyBytes}, nil } -// CreateCBCFromKeyString constructs a new AES CBC cipher using the hex-encoded key string provided +// CreateCBCFromKeyString creates an AES-CBC cipher from a hex-encoded key. func CreateCBCFromKeyString(key string) (CBCCipher, error) { keyBytes, err := hex.DecodeString(key) if err != nil { @@ -43,17 +43,7 @@ func CreateCBCFromKeyString(key string) (CBCCipher, error) { return CreateCBCFromKeyBytes(keyBytes) } -// GenerateCBCIV returns a cryptographically random initialisation vector of one -// AES block, suitable for a single CBC encryption. -// -// NIST SP 800-38A requires the initialisation vector to be unpredictable, and a -// fresh one must be generated for every message encrypted under the same key. -// The initialisation vector need not be kept secret and may travel alongside the -// ciphertext, but it must be integrity protected together with the ciphertext; -// see CBCCipher. -// -// The name is deliberately distinct from the GCM nonce that Cipher generates -// internally: the two are not interchangeable. +// GenerateCBCIV returns a random 16-byte IV. Generate a new IV for each message. func GenerateCBCIV() ([]byte, error) { iv, err := uuid.GenerateRandomBytes(aes.BlockSize) if err != nil { diff --git a/aes/aes_cipher.go b/aes/aes_cipher.go index 6b51eba..270d5d9 100644 --- a/aes/aes_cipher.go +++ b/aes/aes_cipher.go @@ -10,11 +10,7 @@ limitations under the License. */ -// Package aes provides wrapper methods on top of the AES cipher for our own usage. -// -// Cipher wraps AES GCM, which is authenticated. CBCCipher wraps AES CBC, which -// provides confidentiality only and is not authenticated; see CBCCipher for the -// obligations that places on the caller. +// Package aes provides AES-GCM and AES-CBC helpers. package aes import ( @@ -79,9 +75,8 @@ func (c *Cipher) VerifyCheckValue(checkValue string) bool { return verifyCheckValue(c.KeyBytes, checkValue) } -// deriveCheckValue returns the key check value of keyBytes: the first -// checkValueBytes bytes of a single AES ECB block encryption of all zeroes, -// hex encoded. It returns an empty string when keyBytes is not a valid AES key. +// deriveCheckValue encrypts a zero block with AES-ECB and returns the first +// three bytes as hex. It returns an empty string for an invalid key. func deriveCheckValue(keyBytes []byte) string { block, err := aes.NewCipher(keyBytes) if err != nil { diff --git a/aes/iso9797m2.go b/aes/iso9797m2.go index 06cff46..8b87f9b 100644 --- a/aes/iso9797m2.go +++ b/aes/iso9797m2.go @@ -14,27 +14,10 @@ package aes import "fmt" -// iso9797M2Marker is the single 0x80 byte that ISO/IEC 9797-1 padding method 2 -// appends before the zero filler. At bit level the standard appends one 1 bit -// followed by 0 bits; on byte aligned data that is 0x80 then 0x00. const iso9797M2Marker = 0x80 -// padISO9797M2 appends ISO/IEC 9797-1 padding method 2 to plainBytes so that -// its length becomes a multiple of blockSize. The padding is a single 0x80 byte -// followed by as many 0x00 bytes as the block boundary requires. -// -// The same construction appears in ISO/IEC 7816-4 and is what BouncyCastle -// calls ISO7816-4Padding. Card personalisation interfaces often label it -// "ISO2", including the aes-256-cbc-iso2 algorithm name used by the G+D CII -// data provider interface. It is NOT ISO 10126-2, which pads with random bytes -// and stores the padding length in the final byte, and it is NOT PKCS#7. -// -// At least one byte is always added, so input that is already block aligned -// gains a whole extra block. That is what makes the padding removable without -// ambiguity, including when the message itself ends in 0x80 or 0x00. An empty -// input yields a single full block. -// -// The input slice is never modified; a new slice is always returned. +// padISO9797M2 appends 0x80 and zeroes to the next block boundary. It always +// adds padding and returns a new slice. func padISO9797M2(plainBytes []byte, blockSize int) []byte { padLen := blockSize - len(plainBytes)%blockSize @@ -45,16 +28,8 @@ func padISO9797M2(plainBytes []byte, blockSize int) []byte { return padded } -// unpadISO9797M2 removes ISO/IEC 9797-1 padding method 2 from paddedBytes by -// scanning back from the end over the zero filler to the 0x80 marker. It -// returns an error when the padding is not well formed. -// -// A padding error is not evidence of tampering alone, and callers must not -// treat it as an integrity check: CBC is unauthenticated, so a modified -// ciphertext can just as easily decrypt to well formed padding. Reporting -// padding failures to an untrusted caller also exposes a padding oracle. -// -// The input slice is never modified; the returned slice is a copy. +// unpadISO9797M2 removes ISO/IEC 9797-1 method 2 padding. It rejects malformed +// padding and returns a new slice. func unpadISO9797M2(paddedBytes []byte, blockSize int) ([]byte, error) { if len(paddedBytes) == 0 { return nil, fmt.Errorf("padded input is empty") @@ -71,9 +46,7 @@ func unpadISO9797M2(paddedBytes []byte, blockSize int) ([]byte, error) { return nil, fmt.Errorf("invalid padding") } - // A well formed marker sits within the final block, because the padding is - // between 1 and blockSize bytes long. Anything longer means the trailing - // zeroes are message content and the padding is absent or corrupt. + // Valid padding starts within the final block. if len(paddedBytes)-markerIndex > blockSize { return nil, fmt.Errorf("invalid padding") } diff --git a/aes/iso9797m2_test.go b/aes/iso9797m2_test.go index d88af08..2b6a0fc 100644 --- a/aes/iso9797m2_test.go +++ b/aes/iso9797m2_test.go @@ -263,9 +263,7 @@ func TestISO9797M2RoundTrip(t *testing.T) { } func TestISO9797M2RoundTripWithTrailingMarkerAndZeroBytes(t *testing.T) { - // The awkward cases for this scheme are messages that already end in the - // bytes the padding uses. Because padding is always added, these still round - // trip exactly. + // Padding remains unambiguous when plaintext ends in 0x80 or 0x00. testCases := []string{ "80", "00", From 80705124cae7d74eb5684a01cfd4518b4b711509 Mon Sep 17 00:00:00 2001 From: Keith Fong Date: Thu, 3 Sep 2026 16:46:50 +0800 Subject: [PATCH 4/4] docs: simplify AES-CBC README --- README.md | 87 +++---------------------------------------------------- 1 file changed, 4 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index 3db7d83..c823a0e 100644 --- a/README.md +++ b/README.md @@ -6,93 +6,14 @@ This repo provides a list of utility modules for common crypto operations. - encrypt & decrypt methods, the output ciphertext is prefixed with the random nonce. - verify the constructed cipher against the check value -Prefer AES-GCM. Only reach for AES-CBC below when an external protocol mandates it. - ### AES-CBC -`CBCCipher` is provided for interoperability with external protocols that mandate -CBC. It is a separate type from the AES-GCM `Cipher` because it offers a -materially weaker guarantee. - -> **AES-CBC provides confidentiality only. It is not authenticated.** -> -> - A modified initialisation vector or ciphertext will usually still decrypt -> **without error**, producing attacker-influenced plaintext. Decryption -> succeeding is not evidence that a message is genuine. -> - Callers **must** authenticate the initialisation vector together with the -> ciphertext by separate means (for example an HMAC over both) and **must** -> verify it before decrypting. -> - A padding error is **not** an integrity check. Never surface padding -> failures to an untrusted caller: doing so exposes a -> [padding oracle](https://en.wikipedia.org/wiki/Padding_oracle_attack) that -> can be used to recover plaintext. - - factory methods to construct an AES-CBC cipher from the raw key bytes or hex text -- `Encrypt` / `Decrypt` operate on block-aligned input with no padding -- `EncryptISO9797M2Padded` / `DecryptISO9797M2Padded` apply and remove ISO/IEC - 9797-1 padding method 2 -- `GenerateCBCIV` returns a random 16-byte initialisation vector -- verify the constructed cipher against the check value; the value depends only - on the key, so it matches the one `Cipher` reports for the same key - -Parameters: - -| Parameter | Requirement | -| --- | --- | -| Key | 16, 24, or 32 bytes | -| Initialisation vector | Always exactly 16 bytes, for every key size | -| Input to `Encrypt` / `Decrypt` | Non-zero multiple of 16 bytes | -| Input to `EncryptISO9797M2Padded` | Any length, including empty | -| Padding | ISO/IEC 9797-1 method 2 via the `*ISO9797M2Padded` methods, otherwise agreed by the protocol | -| Authentication | Not provided. Supplied separately by the caller | - -#### Padding - -Padding belongs to the surrounding protocol, not to CBC. The padded methods are -named after their scheme so a protocol that needs a different one cannot reach -them by accident. Use `Encrypt` and `Decrypt` when the protocol pads by other -means or not at all. - -ISO/IEC 9797-1 method 2 appends a single `0x80` byte followed by `0x00` filler to -the block boundary. At least one byte is always added, so input that is already -block aligned gains a whole extra block. That is what makes the padding -removable without ambiguity, even when the message itself ends in `0x80` or -`0x00`. The same construction appears in ISO/IEC 7816-4 and is what BouncyCastle -calls `ISO7816-4Padding`. - -Card personalisation interfaces often label this scheme `ISO2`, including the -`aes-256-cbc-iso2` algorithm name in the G+D CII data provider interface. - -Two schemes are commonly confused with it and neither is implemented here: - -- **PKCS#7** (RFC 5652 §6.3), which Java calls `PKCS5Padding` and OpenSSL uses by - default for CBC. It is a different byte layout and is **not** `ISO2`. Do not - substitute it merely because it also produces block-aligned data. -- **ISO 10126-2**, which pads with random bytes and stores the padding length in - the final byte. It was withdrawn in 2007 and, being non-deterministic, cannot - be checked by re-encrypting a known plaintext. - -#### Initialisation vector - -The initialisation vector is always supplied by the caller and is never prefixed -to, or stripped from, the ciphertext. How it is transported or derived is a -property of the surrounding protocol, so that framing is left to the caller. - -Generate a fresh initialisation vector for every message with `GenerateCBCIV`. -NIST SP 800-38A requires it to be unpredictable; reusing one under the same key -leaks whether two messages share a leading block. It need not be secret and may -travel beside the ciphertext, but it must be integrity protected along with it. - -A fixed or all-zero initialisation vector is accepted only because an external -protocol may mandate one. That is an exception to the requirement above, not an -option to pick freely, and it should be justified by the protocol specification. +- encrypt & decrypt methods for block-aligned or ISO/IEC 9797-1 method 2 padded input +- generate a random 16-byte initialisation vector +- verify the constructed cipher against the check value -```go -cipher, err := aes.CreateCBCFromKeyString(hexKey) -iv, err := aes.GenerateCBCIV() -cipherBytes, err := cipher.EncryptISO9797M2Padded(plainBytes, iv) -// Now authenticate iv and cipherBytes together, and transmit the tag alongside. -``` +AES-CBC does not authenticate ciphertext. Callers must authenticate the initialisation vector and ciphertext separately. ### DES