From cb29e3074df69773110c5d550de336fe33d06d1e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 7 May 2023 18:03:43 +0800 Subject: [PATCH 1/3] improve decryption failure message --- modules/secret/secret.go | 21 +++++++++++---------- modules/secret/secret_test.go | 20 ++++++++++++++------ 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/modules/secret/secret.go b/modules/secret/secret.go index 628ae505a5c0b..bc4a9f6116a75 100644 --- a/modules/secret/secret.go +++ b/modules/secret/secret.go @@ -10,6 +10,7 @@ import ( "encoding/base64" "encoding/hex" "errors" + "fmt" "io" "github.com/minio/sha256-simd" @@ -19,13 +20,13 @@ import ( func AesEncrypt(key, text []byte) ([]byte, error) { block, err := aes.NewCipher(key) if err != nil { - return nil, err + return nil, fmt.Errorf("AesEncrypt invalid key: %v", err) } b := base64.StdEncoding.EncodeToString(text) ciphertext := make([]byte, aes.BlockSize+len(b)) iv := ciphertext[:aes.BlockSize] - if _, err := io.ReadFull(rand.Reader, iv); err != nil { - return nil, err + if _, err = io.ReadFull(rand.Reader, iv); err != nil { + return nil, fmt.Errorf("AesEncrypt unable to read iv: %w", err) } cfb := cipher.NewCFBEncrypter(block, iv) cfb.XORKeyStream(ciphertext[aes.BlockSize:], []byte(b)) @@ -39,7 +40,7 @@ func AesDecrypt(key, text []byte) ([]byte, error) { return nil, err } if len(text) < aes.BlockSize { - return nil, errors.New("ciphertext too short") + return nil, errors.New("AesDecrypt ciphertext too short") } iv := text[:aes.BlockSize] text = text[aes.BlockSize:] @@ -47,7 +48,7 @@ func AesDecrypt(key, text []byte) ([]byte, error) { cfb.XORKeyStream(text, text) data, err := base64.StdEncoding.DecodeString(string(text)) if err != nil { - return nil, err + return nil, fmt.Errorf("AesDecrypt invalid decrtyped base64 string: %w", err) } return data, nil } @@ -58,21 +59,21 @@ func EncryptSecret(key, str string) (string, error) { plaintext := []byte(str) ciphertext, err := AesEncrypt(keyHash[:], plaintext) if err != nil { - return "", err + return "", fmt.Errorf("failed to encrypt by secret: %w", err) } return hex.EncodeToString(ciphertext), nil } // DecryptSecret decrypts a previously encrypted hex string -func DecryptSecret(key, cipherhex string) (string, error) { +func DecryptSecret(key, cipherHex string) (string, error) { keyHash := sha256.Sum256([]byte(key)) - ciphertext, err := hex.DecodeString(cipherhex) + ciphertext, err := hex.DecodeString(cipherHex) if err != nil { - return "", err + return "", fmt.Errorf("failed to decrtyp by secret, invalid hex string: %w", err) } plaintext, err := AesDecrypt(keyHash[:], ciphertext) if err != nil { - return "", err + return "", fmt.Errorf("failed to decrtyp by secret, secret key (SECRET_KEY) might be incorrect: %w", err) } return string(plaintext), nil } diff --git a/modules/secret/secret_test.go b/modules/secret/secret_test.go index 4b018fddb6783..7262a9bbeb664 100644 --- a/modules/secret/secret_test.go +++ b/modules/secret/secret_test.go @@ -10,14 +10,22 @@ import ( ) func TestEncryptDecrypt(t *testing.T) { - var hex string - var str string - - hex, _ = EncryptSecret("foo", "baz") - str, _ = DecryptSecret("foo", hex) + hex, err := EncryptSecret("foo", "baz") + assert.NoError(t, err) + str, _ := DecryptSecret("foo", hex) assert.Equal(t, "baz", str) - hex, _ = EncryptSecret("bar", "baz") + hex, err = EncryptSecret("bar", "baz") + assert.NoError(t, err) str, _ = DecryptSecret("foo", hex) assert.NotEqual(t, "baz", str) + + _, err = DecryptSecret("a", "b") + assert.ErrorContains(t, err, "invalid hex string") + + _, err = DecryptSecret("a", "bb") + assert.ErrorContains(t, err, "secret key (SECRET_KEY) might be incorrect: AesDecrypt ciphertext too short") + + _, err = DecryptSecret("a", "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef") + assert.ErrorContains(t, err, "secret key (SECRET_KEY) might be incorrect: AesDecrypt invalid decrtyped base64 string") } From a99e8303a5234a7a650e7ad58963264793a7bb0d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 7 May 2023 18:14:37 +0800 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: delvh --- modules/secret/secret.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/secret/secret.go b/modules/secret/secret.go index bc4a9f6116a75..46aef89fbc0ed 100644 --- a/modules/secret/secret.go +++ b/modules/secret/secret.go @@ -26,7 +26,7 @@ func AesEncrypt(key, text []byte) ([]byte, error) { ciphertext := make([]byte, aes.BlockSize+len(b)) iv := ciphertext[:aes.BlockSize] if _, err = io.ReadFull(rand.Reader, iv); err != nil { - return nil, fmt.Errorf("AesEncrypt unable to read iv: %w", err) + return nil, fmt.Errorf("AesEncrypt unable to read IV: %w", err) } cfb := cipher.NewCFBEncrypter(block, iv) cfb.XORKeyStream(ciphertext[aes.BlockSize:], []byte(b)) @@ -48,7 +48,7 @@ func AesDecrypt(key, text []byte) ([]byte, error) { cfb.XORKeyStream(text, text) data, err := base64.StdEncoding.DecodeString(string(text)) if err != nil { - return nil, fmt.Errorf("AesDecrypt invalid decrtyped base64 string: %w", err) + return nil, fmt.Errorf("AesDecrypt invalid decrypted base64 string: %w", err) } return data, nil } @@ -69,11 +69,11 @@ func DecryptSecret(key, cipherHex string) (string, error) { keyHash := sha256.Sum256([]byte(key)) ciphertext, err := hex.DecodeString(cipherHex) if err != nil { - return "", fmt.Errorf("failed to decrtyp by secret, invalid hex string: %w", err) + return "", fmt.Errorf("failed to decrypt by secret, invalid hex string: %w", err) } plaintext, err := AesDecrypt(keyHash[:], ciphertext) if err != nil { - return "", fmt.Errorf("failed to decrtyp by secret, secret key (SECRET_KEY) might be incorrect: %w", err) + return "", fmt.Errorf("failed to decrypt by secret, secret key (SECRET_KEY) might be incorrect: %w", err) } return string(plaintext), nil } From dc96becfe9952649a22d9d866a2bc640585280a5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 7 May 2023 18:21:46 +0800 Subject: [PATCH 3/3] fine tune error message --- modules/secret/secret.go | 2 +- modules/secret/secret_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/secret/secret.go b/modules/secret/secret.go index 46aef89fbc0ed..9c2ecd181d2ca 100644 --- a/modules/secret/secret.go +++ b/modules/secret/secret.go @@ -73,7 +73,7 @@ func DecryptSecret(key, cipherHex string) (string, error) { } plaintext, err := AesDecrypt(keyHash[:], ciphertext) if err != nil { - return "", fmt.Errorf("failed to decrypt by secret, secret key (SECRET_KEY) might be incorrect: %w", err) + return "", fmt.Errorf("failed to decrypt by secret, the key (maybe SECRET_KEY?) might be incorrect: %w", err) } return string(plaintext), nil } diff --git a/modules/secret/secret_test.go b/modules/secret/secret_test.go index 7262a9bbeb664..d4fb46955b7f2 100644 --- a/modules/secret/secret_test.go +++ b/modules/secret/secret_test.go @@ -24,8 +24,8 @@ func TestEncryptDecrypt(t *testing.T) { assert.ErrorContains(t, err, "invalid hex string") _, err = DecryptSecret("a", "bb") - assert.ErrorContains(t, err, "secret key (SECRET_KEY) might be incorrect: AesDecrypt ciphertext too short") + assert.ErrorContains(t, err, "the key (maybe SECRET_KEY?) might be incorrect: AesDecrypt ciphertext too short") _, err = DecryptSecret("a", "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef") - assert.ErrorContains(t, err, "secret key (SECRET_KEY) might be incorrect: AesDecrypt invalid decrtyped base64 string") + assert.ErrorContains(t, err, "the key (maybe SECRET_KEY?) might be incorrect: AesDecrypt invalid decrypted base64 string") }