Improve decryption failure message (#24573)

Help some users like #16832 #1851

There are many users reporting similar problem: if the SECRET_KEY
mismatches, some operations (like 2FA login) only reports unclear 500
error and unclear "base64 decode error" log (some maintainers ever spent
a lot of time on debugging such problem)

The SECRET_KEY was not well-designed and it is also a kind of technical
debt. Since it couldn't be fixed easily, it's good to add clearer error
messages, then at least users could know what the real problem is.

---------

Co-authored-by: delvh <dev.lh@web.de>
This commit is contained in:
wxiaoguang 2023-05-07 19:29:43 +08:00 committed by GitHub
parent 56ae853ca0
commit 0bb52883eb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 16 deletions

View file

@ -10,6 +10,7 @@ import (
"encoding/base64" "encoding/base64"
"encoding/hex" "encoding/hex"
"errors" "errors"
"fmt"
"io" "io"
"github.com/minio/sha256-simd" "github.com/minio/sha256-simd"
@ -19,13 +20,13 @@ import (
func AesEncrypt(key, text []byte) ([]byte, error) { func AesEncrypt(key, text []byte) ([]byte, error) {
block, err := aes.NewCipher(key) block, err := aes.NewCipher(key)
if err != nil { if err != nil {
return nil, err return nil, fmt.Errorf("AesEncrypt invalid key: %v", err)
} }
b := base64.StdEncoding.EncodeToString(text) b := base64.StdEncoding.EncodeToString(text)
ciphertext := make([]byte, aes.BlockSize+len(b)) ciphertext := make([]byte, aes.BlockSize+len(b))
iv := ciphertext[:aes.BlockSize] iv := ciphertext[:aes.BlockSize]
if _, err := io.ReadFull(rand.Reader, iv); err != nil { if _, err = io.ReadFull(rand.Reader, iv); err != nil {
return nil, err return nil, fmt.Errorf("AesEncrypt unable to read IV: %w", err)
} }
cfb := cipher.NewCFBEncrypter(block, iv) cfb := cipher.NewCFBEncrypter(block, iv)
cfb.XORKeyStream(ciphertext[aes.BlockSize:], []byte(b)) cfb.XORKeyStream(ciphertext[aes.BlockSize:], []byte(b))
@ -39,7 +40,7 @@ func AesDecrypt(key, text []byte) ([]byte, error) {
return nil, err return nil, err
} }
if len(text) < aes.BlockSize { if len(text) < aes.BlockSize {
return nil, errors.New("ciphertext too short") return nil, errors.New("AesDecrypt ciphertext too short")
} }
iv := text[:aes.BlockSize] iv := text[:aes.BlockSize]
text = text[aes.BlockSize:] text = text[aes.BlockSize:]
@ -47,7 +48,7 @@ func AesDecrypt(key, text []byte) ([]byte, error) {
cfb.XORKeyStream(text, text) cfb.XORKeyStream(text, text)
data, err := base64.StdEncoding.DecodeString(string(text)) data, err := base64.StdEncoding.DecodeString(string(text))
if err != nil { if err != nil {
return nil, err return nil, fmt.Errorf("AesDecrypt invalid decrypted base64 string: %w", err)
} }
return data, nil return data, nil
} }
@ -58,21 +59,21 @@ func EncryptSecret(key, str string) (string, error) {
plaintext := []byte(str) plaintext := []byte(str)
ciphertext, err := AesEncrypt(keyHash[:], plaintext) ciphertext, err := AesEncrypt(keyHash[:], plaintext)
if err != nil { if err != nil {
return "", err return "", fmt.Errorf("failed to encrypt by secret: %w", err)
} }
return hex.EncodeToString(ciphertext), nil return hex.EncodeToString(ciphertext), nil
} }
// DecryptSecret decrypts a previously encrypted hex string // 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)) keyHash := sha256.Sum256([]byte(key))
ciphertext, err := hex.DecodeString(cipherhex) ciphertext, err := hex.DecodeString(cipherHex)
if err != nil { if err != nil {
return "", err return "", fmt.Errorf("failed to decrypt by secret, invalid hex string: %w", err)
} }
plaintext, err := AesDecrypt(keyHash[:], ciphertext) plaintext, err := AesDecrypt(keyHash[:], ciphertext)
if err != nil { if err != nil {
return "", err return "", fmt.Errorf("failed to decrypt by secret, the key (maybe SECRET_KEY?) might be incorrect: %w", err)
} }
return string(plaintext), nil return string(plaintext), nil
} }

View file

@ -10,14 +10,22 @@ import (
) )
func TestEncryptDecrypt(t *testing.T) { func TestEncryptDecrypt(t *testing.T) {
var hex string hex, err := EncryptSecret("foo", "baz")
var str string assert.NoError(t, err)
str, _ := DecryptSecret("foo", hex)
hex, _ = EncryptSecret("foo", "baz")
str, _ = DecryptSecret("foo", hex)
assert.Equal(t, "baz", str) assert.Equal(t, "baz", str)
hex, _ = EncryptSecret("bar", "baz") hex, err = EncryptSecret("bar", "baz")
assert.NoError(t, err)
str, _ = DecryptSecret("foo", hex) str, _ = DecryptSecret("foo", hex)
assert.NotEqual(t, "baz", str) assert.NotEqual(t, "baz", str)
_, err = DecryptSecret("a", "b")
assert.ErrorContains(t, err, "invalid hex string")
_, err = DecryptSecret("a", "bb")
assert.ErrorContains(t, err, "the key (maybe SECRET_KEY?) might be incorrect: AesDecrypt ciphertext too short")
_, err = DecryptSecret("a", "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")
assert.ErrorContains(t, err, "the key (maybe SECRET_KEY?) might be incorrect: AesDecrypt invalid decrypted base64 string")
} }