From 151ae5cd1c6fc7ee4b056bbae2b372cffde9a902 Mon Sep 17 00:00:00 2001 From: Zenichi Amano Date: Thu, 9 Jul 2026 09:32:34 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20=E3=82=B3=E3=83=BC=E3=83=89=E6=9C=80?= =?UTF-8?q?=E9=81=A9=E5=8C=96=E3=80=81=E3=83=86=E3=82=B9=E3=83=88=E8=BF=BD?= =?UTF-8?q?=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Zenichi Amano --- contentencoding.go | 40 +++++++-------- contentencoding_test.go | 51 +++++++++++++++++++ decrypt.go | 13 +++-- decrypt_test.go | 21 ++++++++ encrypt.go | 8 +-- encrypt_test.go | 32 ++++++++++++ keys.go | 10 +--- keys_test.go | 105 ++++++++++++++++++++++++++++++++++++++++ options_test.go | 70 +++++++++++++++++++++++++++ utils.go | 52 ++------------------ utils_test.go | 19 +++----- 11 files changed, 324 insertions(+), 97 deletions(-) create mode 100644 options_test.go diff --git a/contentencoding.go b/contentencoding.go index aa10afe..a83b23e 100644 --- a/contentencoding.go +++ b/contentencoding.go @@ -44,13 +44,11 @@ func (i ContentEncoding) overhead(gcm cipher.AEAD) int { } func (i ContentEncoding) calculateRecordPadSize(pad, baseRecordSize int) int { - padSize := i.Padding() - // Pad so that at least one data byte is in a block. - recordPad := min(baseRecordSize-1, pad) + recordPad := min(pad, baseRecordSize-1) if i != AES128GCM { - recordPad = min((1<<(padSize*8))-1, recordPad) + recordPad = min(recordPad, (1<<(i.Padding()*8))-1) } if pad > 0 && recordPad == 0 { recordPad++ // Deal with perverse case of rs=overhead+1 with padding. @@ -63,14 +61,11 @@ func (i ContentEncoding) calculateCipherBlockEnd(gcm cipher.AEAD, start, content tagSize := gcm.Overhead() if i != AES128GCM { blockSize += tagSize + if start+blockSize == contentLen { + return 0, ErrTruncated + } } - end := start + blockSize - - if i != AES128GCM && end == contentLen { - return 0, ErrTruncated - } - - end = min(end, contentLen) + end := min(start+blockSize, contentLen) if end-start <= tagSize { return 0, ErrTruncated } @@ -103,8 +98,8 @@ func (i ContentEncoding) appendPadding(plaintext []byte, pad int, last bool) ([] func (i ContentEncoding) unpad(plaintext []byte, last bool) ([]byte, error) { switch i { case AES128GCM: - for i := len(plaintext) - 1; i >= 0; i-- { - c := plaintext[i] + for j := len(plaintext) - 1; j >= 0; j-- { + c := plaintext[j] switch { case c == 0: continue @@ -113,7 +108,7 @@ func (i ContentEncoding) unpad(plaintext []byte, last bool) ([]byte, error) { case !last && c != 1: return nil, ErrInvalidPaddingNonLast default: - return plaintext[:i], nil + return plaintext[:j], nil } } @@ -136,14 +131,13 @@ func (i ContentEncoding) unpad(plaintext []byte, last bool) ([]byte, error) { } // isLastBlock returns true when last block -func (i ContentEncoding) isLastBlock(pad, contentLen, blockEnd int) bool { - var last bool - switch i { - case AES128GCM: - last = blockEnd >= contentLen - default: - // The > here ensures that we write out a padding-only block at the end of a buffer. - last = blockEnd > contentLen +func (i ContentEncoding) isLastBlock(pad, plaintextLen, blockEnd int) bool { + if pad != 0 { + return false + } + if i == AES128GCM { + return blockEnd >= plaintextLen } - return last && pad == 0 + // The > here ensures that we write out a padding-only block at the end of a buffer. + return blockEnd > plaintextLen } diff --git a/contentencoding_test.go b/contentencoding_test.go index 8881ade..7e2e817 100644 --- a/contentencoding_test.go +++ b/contentencoding_test.go @@ -16,4 +16,55 @@ import ( func TestContentEncoding_Padding(t *testing.T) { assert.Equal(t, 2, AESGCM.Padding()) assert.Equal(t, 1, AES128GCM.Padding()) + assert.Equal(t, 0, ContentEncoding("unknown").Padding()) +} + +func TestContentEncoding_Unpad_Errors(t *testing.T) { + // AES128GCM error cases + _, err := AES128GCM.unpad([]byte{0, 0, 0}, true) + assert.Equal(t, ErrAllZeroPlaintext, err) + + _, err = AES128GCM.unpad([]byte{0, 0, 3}, true) + assert.Equal(t, ErrInvalidPaddingLast, err) + + _, err = AES128GCM.unpad([]byte{0, 0, 3}, false) + assert.Equal(t, ErrInvalidPaddingNonLast, err) + + // Other (AESGCM) error cases + _, err = ContentEncoding("unknown").unpad([]byte{0, 0, 0}, true) + assert.Contains(t, err.Error(), "unknown padding size 0") + + _, err = AESGCM.unpad([]byte{0, 10}, true) + assert.Contains(t, err.Error(), "padding exceeds block size: 12") +} + +func TestContentEncoding_AppendPadding_Error(t *testing.T) { + _, err := AESGCM.appendPadding([]byte("test"), -1, true) + assert.Error(t, err) + + _, err = AESGCM.appendPadding([]byte("test"), 70000, true) + assert.Error(t, err) +} + +func TestContentEncoding_IsLastBlock(t *testing.T) { + assert.True(t, AES128GCM.isLastBlock(0, 10, 10)) + assert.False(t, AES128GCM.isLastBlock(1, 10, 10)) + assert.True(t, AESGCM.isLastBlock(0, 10, 11)) + assert.False(t, AESGCM.isLastBlock(0, 10, 10)) +} + +func TestContentEncoding_Padding_Misc(t *testing.T) { + // rs=overhead+1 case in calculateRecordPadSize + gcm, _ := createCipher(make([]byte, 16)) + _ = AES128GCM.overhead(gcm) + // baseRecordSize = 1 + pad := AES128GCM.calculateRecordPadSize(10, 1) + assert.Equal(t, 1, pad) +} + +func TestContentEncoding_CalculateCipherBlockEnd_Truncated(t *testing.T) { + gcm, _ := createCipher(make([]byte, 16)) + // end-start <= tagSize case + _, err := AES128GCM.calculateCipherBlockEnd(gcm, 0, 10, 4096) + assert.Equal(t, ErrTruncated, err) } diff --git a/decrypt.go b/decrypt.go index a86541e..62724ef 100644 --- a/decrypt.go +++ b/decrypt.go @@ -76,13 +76,20 @@ func Decrypt(content []byte, opts ...Option) ([]byte, error) { } func readHeader(opt *options, content []byte) []byte { + contentLen := len(content) if opt.encoding == AES128GCM { - baseOffset := uint32(keyLen + recodeSizeLen) - idLen := uint32(content[baseOffset]) + baseOffset := keyLen + recodeSizeLen + if contentLen <= baseOffset { + return content + } + idLen := int(content[baseOffset]) opt.salt = content[0:keyLen] opt.recordSize = binary.BigEndian.Uint32(content[keyLen:baseOffset]) baseOffset++ + if contentLen < baseOffset+idLen { + return content + } opt.keyID = content[baseOffset : baseOffset+idLen] return content[baseOffset+idLen:] @@ -91,7 +98,7 @@ func readHeader(opt *options, content []byte) []byte { } func decryptRecord(opt *options, gcm cipher.AEAD, nonce []byte, content []byte, last bool) ([]byte, error) { - result, err := gcm.Open(nil, nonce, content, nil) + result, err := gcm.Open(content[:0], nonce, content, nil) if err != nil { return nil, err } diff --git a/decrypt_test.go b/decrypt_test.go index 017ac8c..91e1bb3 100644 --- a/decrypt_test.go +++ b/decrypt_test.go @@ -32,6 +32,27 @@ func TestDecryptWithAESGCM(t *testing.T) { assert.Equal(t, "hello world", string(plaintext)) } +func TestDecryptWithAESGCM_Truncated(t *testing.T) { + authSecret := d(t, "9HcXsQe3xLMG/w2HsYKrOA==") + salt := d(t, "mRGYnIzSJGeZnJ19lgQcfw==") + privateKey := d(t, "yfSYB+/vCEoWklHCG7F99cQ1vRwemFYn87jZc8PHBwU=") + senderPublicKey := d(t, "BGJXZ4zDA04RfSgTufdauZXcNYbe3oF/yEri5ETSuZLDx70gYi7w2ytak8U82H01P1HYnIvr2fEeX7NZpeHdnhM=") + + // BaseRecordSize for AESGCM (rs=4096) is 4096 - 2 = 4094. + // Total block size including tag is 4096 + 16 = 4112. + content := make([]byte, 4112) + plaintext, err := Decrypt(content, + WithEncoding(AESGCM), + WithSalt(salt), + WithAuthSecret(authSecret), + WithPrivate(privateKey), + WithDh(senderPublicKey), + ) + + assert.Equal(t, ErrTruncated, err) + assert.Nil(t, plaintext) +} + func TestDecryptWithAESGCM_2record(t *testing.T) { authSecret := d(t, "9HcXsQe3xLMG/w2HsYKrOA==") salt := d(t, "mRGYnIzSJGeZnJ19lgQcfw==") diff --git a/encrypt.go b/encrypt.go index ff366b0..6f23be6 100644 --- a/encrypt.go +++ b/encrypt.go @@ -9,6 +9,7 @@ package httpece import ( "crypto/cipher" + "encoding/binary" "fmt" "math" ) @@ -19,7 +20,8 @@ func Encrypt(plaintext []byte, opts ...Option) ([]byte, error) { var err error // Options - if opt, err = parseOptions(encrypt, opts); err != nil { + opt, err = parseOptions(encrypt, opts) + if err != nil { return nil, err } @@ -103,7 +105,7 @@ func encryptRecord(opt *options, gcm cipher.AEAD, nonce, plaintext []byte, recor if err != nil { return nil, err } - return gcm.Seal(nil, nonce, plaintextWithPadding, nil), nil + return gcm.Seal(plaintextWithPadding[:0], nonce, plaintextWithPadding, nil), nil } func writeHeader(opt *options, results [][]byte) ([][]byte, error) { @@ -119,7 +121,7 @@ func writeHeader(opt *options, results [][]byte) ([][]byte, error) { } buffer := make([]byte, saltLen+4+1+keyIDLen) copy(buffer, opt.salt) - copy(buffer[saltLen:], uint32ToBytes(opt.recordSize)) + binary.BigEndian.PutUint32(buffer[saltLen:], opt.recordSize) buffer[saltLen+4] = uint8(keyIDLen) copy(buffer[saltLen+5:], opt.keyID) return append(results, buffer), nil diff --git a/encrypt_test.go b/encrypt_test.go index fa6cdef..80d3513 100644 --- a/encrypt_test.go +++ b/encrypt_test.go @@ -35,6 +35,38 @@ func TestEncryptWithAESGCM(t *testing.T) { assert.Equal(t, "vOjpVgZE4IYn/uEJKk3DzZ4X+Qr1dgSSUkuIzQE=", e(content)) } +func TestEncryptDecrypt_Errors(t *testing.T) { + // Encrypt error: record size too small + _, err := Encrypt([]byte("test"), WithRecordSize(1)) + assert.Error(t, err) + + // Decrypt error: record size too small + _, err = Decrypt([]byte("test"), WithRecordSize(1)) + assert.Error(t, err) + + // Encrypt error: invalid salt length + _, err = Encrypt([]byte("test"), WithSalt(make([]byte, 5))) + assert.Error(t, err) +} + +func TestEncrypt_HeaderErrors(t *testing.T) { + // writeHeader with too long keyID + opt := &options{ + encoding: AES128GCM, + keyID: make([]byte, 300), + } + _, err := writeHeader(opt, nil) + assert.Error(t, err) + + // writeHeader with too long salt + opt = &options{ + encoding: AES128GCM, + salt: make([]byte, 300), + } + _, err = writeHeader(opt, nil) + assert.Error(t, err) +} + func TestEncryptWithAESGCM4093Byte(t *testing.T) { authSecret := d(t, "9HcXsQe3xLMG/w2HsYKrOA==") salt := d(t, "mRGYnIzSJGeZnJ19lgQcfw==") diff --git a/keys.go b/keys.go index 2653649..f7a16ee 100644 --- a/keys.go +++ b/keys.go @@ -8,6 +8,7 @@ package httpece import ( + "bytes" "crypto/aes" "crypto/cipher" "crypto/ecdh" @@ -176,14 +177,7 @@ func extractSecret(opt *options) ([]byte, error) { } func buildInfo(base []byte, context []byte) string { - baseLen := len(base) - contextLen := len(context) - result := make([]byte, 0, baseLen+contextLen) - result = append(result, base...) - if contextLen > 0 { - result = append(result, context...) - } - return string(result) + return string(bytes.Join([][]byte{base, context}, nil)) } func extractDH(opt *options) (secret []byte, context []byte, err error) { diff --git a/keys_test.go b/keys_test.go index a9e13ad..c6d32b9 100644 --- a/keys_test.go +++ b/keys_test.go @@ -29,3 +29,108 @@ func TestRandomKey(t *testing.T) { assert.NotEqual(t, private, private2) assert.NotEqual(t, public, public2) } + +func TestKeys_Errors(t *testing.T) { + // Test extractSecret with invalid key length + opt := &options{ + key: make([]byte, 10), // invalid length (not 16) + } + _, err := extractSecret(opt) + assert.Contains(t, err.Error(), "an explicit Key must be 16 bytes") + + // Test extractSecret with no saved key + opt = &options{ + keyID: []byte("missing"), + keyMap: func(b []byte) []byte { return nil }, + } + _, err = extractSecret(opt) + assert.Contains(t, err.Error(), "no saved key") + + // Test extractSecret with no auth secret + priv, _ := randomKey() + opt = &options{ + privateKey: priv, + authSecret: nil, + } + _, err = extractSecret(opt) + assert.Equal(t, ErrNoAuthSecret, err) + + // Test extractDH with invalid receiver public key length + opt = &options{ + mode: encrypt, + privateKey: priv, + publicKey: priv.PublicKey(), + dh: make([]byte, 70000), + keyLabel: []byte("test"), + } + // Note: We need to bypass getSecret check or use a valid public key but mock the length check + // But getSecret calls curve.NewPublicKey which will fail for 70000 bytes. + // So it will fail early at getSecret. + _, _, err = extractDH(opt) + assert.Error(t, err) + + // Test getSecret with invalid public key + _, err = opt.getSecret([]byte("invalid")) + assert.Error(t, err) +} + +func TestKeys_AdditionalCoverage(t *testing.T) { + // extractSecretAndContext with keyID and keyMap + km := func(b []byte) []byte { + if string(b) == "test" { + return make([]byte, 16) + } + return nil + } + opt := &options{ + keyID: []byte("test"), + keyMap: km, + } + secret, context, err := extractSecretAndContext(opt) + assert.Nil(t, err) + assert.NotNil(t, secret) + assert.Nil(t, context) + + // extractSecretAndContext with invalid key length + opt = &options{ + key: make([]byte, 10), + } + _, _, err = extractSecretAndContext(opt) + assert.Error(t, err) + + // deriveKeyAndNonce with invalid encoding + opt = &options{ + encoding: ContentEncoding("invalid"), + } + _, _, err = deriveKeyAndNonce(opt) + assert.Error(t, err) + + // extractSecret (AES128GCM) decrypt mode + priv, _ := randomKey() + opt = &options{ + mode: decrypt, + encoding: AES128GCM, + privateKey: priv, + publicKey: priv.PublicKey(), + keyID: priv.PublicKey().Bytes(), + authSecret: make([]byte, 16), + } + _, err = extractSecret(opt) + assert.Nil(t, err) +} + +func TestKeys_CreateCipher_Error(t *testing.T) { + _, err := createCipher(make([]byte, 10)) // invalid key length for AES + assert.Error(t, err) +} + +func TestKeys_ExtractSecretAndContext_Misc(t *testing.T) { + // AESGCM with keyID only (no keyMap result) + opt := &options{ + encoding: AESGCM, + keyID: []byte("test"), + keyMap: func(b []byte) []byte { return nil }, + } + _, _, err := extractSecretAndContext(opt) + assert.Equal(t, ErrUnableDetermineKey, err) +} diff --git a/options_test.go b/options_test.go new file mode 100644 index 0000000..7b1d56b --- /dev/null +++ b/options_test.go @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2026 Zenichi Amano + * + * This file is part of http-ece, which is MIT licensed. + * See http://opensource.org/licenses/MIT + */ + +package httpece + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOptions_Coverage(t *testing.T) { + // Test WithPadSize error + opt := WithPadSize(-1) + err := opt(&options{}) + assert.Error(t, err) + + // Test WithRecordSize error + opt = WithRecordSize(-1) + err = opt(&options{}) + assert.Error(t, err) + + opt = WithRecordSize(recordSizeMax + 1) + err = opt(&options{}) + assert.Error(t, err) + + // Test WithKey + opt = WithKey([]byte("key")) + o := &options{} + err = opt(o) + assert.Nil(t, err) + assert.Equal(t, []byte("key"), o.key) + + // Test WithKeyID + opt = WithKeyID(make([]byte, 300)) + err = opt(o) + assert.Equal(t, ErrKeyIDTooLong, err) + + opt = WithKeyID([]byte("id")) + err = opt(o) + assert.Nil(t, err) + assert.Equal(t, []byte("id"), o.keyID) + + // Test WithKeyLabel + opt = WithKeyLabel([]byte("label")) + err = opt(o) + assert.Nil(t, err) + assert.Equal(t, []byte("label"), o.keyLabel) + + // Test WithKeyMap + km := func(b []byte) []byte { return b } + opt = WithKeyMap(km) + err = opt(o) + assert.Nil(t, err) + // Function comparison is not possible, but we can call it + assert.Equal(t, []byte("test"), o.keyMap([]byte("test"))) +} + +func TestOptions_ParseOptions_Error(t *testing.T) { + // This is hard to trigger as randomKey() usually succeeds, + // but we can try with a custom curve if we could inject it. + // For now, let's at least cover parseOptions error. + _, err := parseOptions(encrypt, []Option{func(o *options) error { return fmt.Errorf("error") }}) + assert.Error(t, err) +} diff --git a/utils.go b/utils.go index 998ce8c..325da89 100644 --- a/utils.go +++ b/utils.go @@ -8,6 +8,7 @@ package httpece import ( + "bytes" "crypto/rand" "encoding/base64" "encoding/binary" @@ -61,60 +62,17 @@ func uint16ToBytes(i uint16) []byte { return x } -func uint32ToBytes(i uint32) []byte { - x := make([]byte, 4) - binary.BigEndian.PutUint32(x, i) - return x -} - func generateNonce(baseNonce []byte, counter uint32) []byte { x := make([]byte, nonceLen) binary.BigEndian.PutUint32(x[8:], counter) - xor12(x, baseNonce, x) + for i := 0; i < nonceLen; i++ { + x[i] ^= baseNonce[i] + } return x } -func xor12(dst []byte, a []byte, b []byte) { - _ = dst[11] - _ = a[11] - _ = b[11] - - dst[0] = a[0] ^ b[0] - dst[1] = a[1] ^ b[1] - dst[2] = a[2] ^ b[2] - dst[3] = a[3] ^ b[3] - dst[4] = a[4] ^ b[4] - dst[5] = a[5] ^ b[5] - dst[6] = a[6] ^ b[6] - dst[7] = a[7] ^ b[7] - dst[8] = a[8] ^ b[8] - dst[9] = a[9] ^ b[9] - dst[10] = a[10] ^ b[10] - dst[11] = a[11] ^ b[11] -} - func join(s [][]byte) []byte { - if len(s) == 0 { - return []byte{} - } - if len(s) == 1 { - return append([]byte(nil), s[0]...) - } - - var n int - for _, v := range s { - if len(v) > maxInt-n { - panic("join output length overflow") - } - n += len(v) - } - - b := make([]byte, n) - o := 0 - for _, v := range s { - o += copy(b[o:], v) - } - return b + return bytes.Join(s, nil) } func randomSalt() ([]byte, error) { diff --git a/utils_test.go b/utils_test.go index da67a22..05d0eee 100644 --- a/utils_test.go +++ b/utils_test.go @@ -26,19 +26,6 @@ func TestUint16ToBytes(t *testing.T) { f(0x01, 0x01) } -func TestUint32ToBytes(t *testing.T) { - f := func(v ...byte) { - assert.Equal(t, v, uint32ToBytes(binary.BigEndian.Uint32(v))) - } - - f(0x00, 0x00, 0x00, 0x01) - f(0x00, 0x00, 0x00, 0x11) - f(0x00, 0x00, 0x01, 0x01) - f(0x00, 0x01, 0x01, 0x01) - f(0x00, 0x11, 0x01, 0x01) - f(0x01, 0x01, 0x01, 0x01) -} - func TestGenerateNonce(t *testing.T) { expect := []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00} tmp := []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00} @@ -58,6 +45,12 @@ func TestGenerateNonce(t *testing.T) { assert.Equal(t, expect, generateNonce(tmp, 0xffffffff)) } +func TestDebug_Coverage(t *testing.T) { + // These functions don't do much when debug is false, but we can call them for coverage + debug.dumpBinary("test", []byte("data")) + debug.dumpInfo("test", "info") +} + func TestJoin(t *testing.T) { assert.Equal(t, []byte{0x01}, join([][]byte{{0x01}})) assert.Equal(t, []byte{0x01, 0x02}, join([][]byte{{0x01}, {0x02}}))