patterngoMinor
Encrypting strings in Golang
Viewed 0 times
golangencryptingstrings
Problem
This is a an example application for testing some encryption and decryption functions in Golang. The functions
-
I want to have as little involvement in the encryption process as possible and I certainly do not want to be rolling my own crypto. Unfortunately, from the various examples and tutorials about on the web, I can't seem to make the functions smaller than they are here.
-
One condition here is that the encryption key is supplied by the user and should be of any length. As my research into these encryption and decryption functions suggests that block ciphers are the way to go, this forces me to manipulate the user's key. I built function
What I would like feedback on
-
Can the encryption and decryption functions be reduced even more? So less chance of any mistakes I make that could compromise users data.
-
If (1) is as simple as they can get, are there any third party Go libraries that are trusted enough for my use case?
-
If (2) isn't viable, can you please let me know if my functions
-
For lengthening/shortening the users key, is my function
-
In fact, do I have to use a block cipher? Can I use another cipher built into standard Go that removes the need for
How to use program to encrypt
Input:
Output:
How to use program to decrypt
Input:
```
go run encrypt.go -d --key "I am a key" --value "0bWnVHDPIls_a-MWUs_i1m6KxKRs
encryptString(), decryptString() and hashTo32Bytes() will potentially make it into a live application, so I need to know that these functions are safe and if not, what I can do to make them safe.-
I want to have as little involvement in the encryption process as possible and I certainly do not want to be rolling my own crypto. Unfortunately, from the various examples and tutorials about on the web, I can't seem to make the functions smaller than they are here.
-
One condition here is that the encryption key is supplied by the user and should be of any length. As my research into these encryption and decryption functions suggests that block ciphers are the way to go, this forces me to manipulate the user's key. I built function
hashTo32Bytes() to do deal with this.What I would like feedback on
-
Can the encryption and decryption functions be reduced even more? So less chance of any mistakes I make that could compromise users data.
-
If (1) is as simple as they can get, are there any third party Go libraries that are trusted enough for my use case?
-
If (2) isn't viable, can you please let me know if my functions
encryptString() and decryptString() are safe?-
For lengthening/shortening the users key, is my function
hashTo32Bytes() suitable?-
In fact, do I have to use a block cipher? Can I use another cipher built into standard Go that removes the need for
hashTo32Bytes()?How to use program to encrypt
Input:
$ go run encrypt.go -e --key "I am a key" --value "This is some text that needs to be encrypted."Output:
Encrypting 'This is some text that needs to be encrypted.' with key 'I am a key'
Output: '0bWnVHDPIls_a-MWUs_i1m6KxKRs7YT12O-o47PrIKXTOHtk7BpSRrYr4jtwrfHU5jIduS23BZHMCtaw0w=='How to use program to decrypt
Input:
```
go run encrypt.go -d --key "I am a key" --value "0bWnVHDPIls_a-MWUs_i1m6KxKRs
Solution
Nice question.
Consistent error handling.
Regardless of the language, the key to a good program is consistency. Your code combines a combination of
Additionally, you have a copy/paste and error handling problem here too:
If you write comments, you should do it properly.... the comment is obviously wrong, you are decoding the data, not encoding it.... and, it is so obvious that you are decoding things, why have the comment at all? Comments should explain the "why" (see Code Smells, and Comments concerning)
But, that code.... why the hell are you ignoring the error from the decode? All user input should be treated as hostile, and you should expect users to enter the wrong thing. This Decode is the first place where you get to validate the user input, and you ignore the error... oh dear.
Then, in your main method, you ignore all the errors anyway.... I guess it's a good thing you had the panics in the long run.... they are the only things that work.
In your
I looked in to the documentation for AES in go, and I see you copied most of it from the examples in there - code in the go documentation is not necessarily good code!
Crypto Key
You select to convert the key to a 32-byte hash. This is a good thing, but what you are missing is that you can use the sha-256 algorithm so much better.... in fact, it's a fantastically convenient hash, but you are doing it wrong... here's your function:
Look at what you do ....:
Question: sha-256 creates a 32-byte hash - why don't you just use it, instead of converting it to a string, and truncating half of it?
Also, an empty string creates a valid hash.... checking for an empty string is OK if you want to check for empty passwords, but then it should be somewhere else as a check, not inside a hashing function.
And moreover, the method is called
Using sha256 appropriately, you can have:
This simplifies your encrypt/decrypt functions too - they don't need to do horrible string manipulations.
Decrypting
I tidied this function up a bit, renamed the variables, but then decided it should be split to make the logic clearer. Consider your
Encrypting
Similarly, the encryption is over complicated in the method. Splitting up the byte-b
Consistent error handling.
Regardless of the language, the key to a good program is consistency. Your code combines a combination of
panic and error handling mechanisms. You should only be using error values - panicing is an over-the-top reaction and is "not cool".Additionally, you have a copy/paste and error handling problem here too:
// Encode the cryptoText to base 64.
cipherText, _ := base64.URLEncoding.DecodeString(cryptoText)If you write comments, you should do it properly.... the comment is obviously wrong, you are decoding the data, not encoding it.... and, it is so obvious that you are decoding things, why have the comment at all? Comments should explain the "why" (see Code Smells, and Comments concerning)
But, that code.... why the hell are you ignoring the error from the decode? All user input should be treated as hostile, and you should expect users to enter the wrong thing. This Decode is the first place where you get to validate the user input, and you ignore the error... oh dear.
Then, in your main method, you ignore all the errors anyway.... I guess it's a good thing you had the panics in the long run.... they are the only things that work.
In your
decryptString method, it's declared to return an error, but you never actually return one.... you just panic. Your encryptString does, in fact, occasionally return an error.I looked in to the documentation for AES in go, and I see you copied most of it from the examples in there - code in the go documentation is not necessarily good code!
Crypto Key
You select to convert the key to a 32-byte hash. This is a good thing, but what you are missing is that you can use the sha-256 algorithm so much better.... in fact, it's a fantastically convenient hash, but you are doing it wrong... here's your function:
// As we cannot use a variable length key, we must cut the users key
// up to or down to 32 bytes. To do this the function takes a hash
// of the key and cuts it down to 32 bytes.
func hashTo32Bytes(input string) (output string, err error) {
if len(input) == 0 {
return "", errors.New("No input supplied")
}
hasher := sha256.New()
hasher.Write([]byte(input))
stringToSHA256 := base64.URLEncoding.EncodeToString(hasher.Sum(nil))
// Cut the length down to 32 bytes and return.
return stringToSHA256[:32], nil
}Look at what you do ....:
- you run the key through a hash function.
- you convert the hash to a string
- you take the first 32 characters from the string
- you then convert those 32 characters back to a 32-byte slice.
Question: sha-256 creates a 32-byte hash - why don't you just use it, instead of converting it to a string, and truncating half of it?
Also, an empty string creates a valid hash.... checking for an empty string is OK if you want to check for empty passwords, but then it should be somewhere else as a check, not inside a hashing function.
And moreover, the method is called
hashTo32Bytes but it returns a string... what's with that?Using sha256 appropriately, you can have:
// hashTo32Bytes will compute a cryptographically useful hash of the input string.
func hashTo32Bytes(input string) []byte {
data := sha256.Sum256([]byte(input))
return data[0:]
}This simplifies your encrypt/decrypt functions too - they don't need to do horrible string manipulations.
Decrypting
cypherText implies a text value, but in your function, it contains a binary []byte slice. It should be called encrypted or something.I tidied this function up a bit, renamed the variables, but then decided it should be split to make the logic clearer. Consider your
decryptString method decomposed as:// Takes two strings, cryptoText and keyString.
// cryptoText is the text to be decrypted and the keyString is the key to use for the decryption.
// The function will output the resulting plain text string with an error variable.
func decryptString(cryptoText string, keyString string) (plainTextString string, err error) {
encrypted, err := base64.URLEncoding.DecodeString(cryptoText)
if err != nil {
return "", err
}
if len(encrypted) < aes.BlockSize {
return "", fmt.Errorf("cipherText too short. It decodes to %v bytes but the minimum length is 16", len(encrypted))
}
decrypted, err := decryptAES(hashTo32Bytes(keyString), encrypted)
if err != nil {
return "", err
}
return string(decrypted), nil
}
func decryptAES(key, data []byte) ([]byte, error) {
// split the input up in to the IV seed and then the actual encrypted data.
iv := data[:aes.BlockSize]
data = data[aes.BlockSize:]
block, err := aes.NewCipher(key)
if err != nil {
return nil, err
}
stream := cipher.NewCFBDecrypter(block, iv)
stream.XORKeyStream(data, data)
return data, nil
}Encrypting
Similarly, the encryption is over complicated in the method. Splitting up the byte-b
Code Snippets
// Encode the cryptoText to base 64.
cipherText, _ := base64.URLEncoding.DecodeString(cryptoText)// As we cannot use a variable length key, we must cut the users key
// up to or down to 32 bytes. To do this the function takes a hash
// of the key and cuts it down to 32 bytes.
func hashTo32Bytes(input string) (output string, err error) {
if len(input) == 0 {
return "", errors.New("No input supplied")
}
hasher := sha256.New()
hasher.Write([]byte(input))
stringToSHA256 := base64.URLEncoding.EncodeToString(hasher.Sum(nil))
// Cut the length down to 32 bytes and return.
return stringToSHA256[:32], nil
}// hashTo32Bytes will compute a cryptographically useful hash of the input string.
func hashTo32Bytes(input string) []byte {
data := sha256.Sum256([]byte(input))
return data[0:]
}// Takes two strings, cryptoText and keyString.
// cryptoText is the text to be decrypted and the keyString is the key to use for the decryption.
// The function will output the resulting plain text string with an error variable.
func decryptString(cryptoText string, keyString string) (plainTextString string, err error) {
encrypted, err := base64.URLEncoding.DecodeString(cryptoText)
if err != nil {
return "", err
}
if len(encrypted) < aes.BlockSize {
return "", fmt.Errorf("cipherText too short. It decodes to %v bytes but the minimum length is 16", len(encrypted))
}
decrypted, err := decryptAES(hashTo32Bytes(keyString), encrypted)
if err != nil {
return "", err
}
return string(decrypted), nil
}
func decryptAES(key, data []byte) ([]byte, error) {
// split the input up in to the IV seed and then the actual encrypted data.
iv := data[:aes.BlockSize]
data = data[aes.BlockSize:]
block, err := aes.NewCipher(key)
if err != nil {
return nil, err
}
stream := cipher.NewCFBDecrypter(block, iv)
stream.XORKeyStream(data, data)
return data, nil
}// Takes two string, plainText and keyString.
// plainText is the text that needs to be encrypted by keyString.
// The function will output the resulting crypto text and an error variable.
func encryptString(plainText string, keyString string) (cipherTextString string, err error) {
key := hashTo32Bytes(keyString)
encrypted, err := encryptAES(key, []byte(plainText))
if err != nil {
return "", err
}
return base64.URLEncoding.EncodeToString(encrypted), nil
}
func encryptAES(key, data []byte) ([]byte, error) {
block, err := aes.NewCipher(key)
if err != nil {
return nil, err
}
// create two 'windows' in to the output slice.
output := make([]byte, aes.BlockSize+len(data))
iv := output[:aes.BlockSize]
encrypted := output[aes.BlockSize:]
// populate the IV slice with random data.
if _, err = io.ReadFull(rand.Reader, iv); err != nil {
return nil, err
}
stream := cipher.NewCFBEncrypter(block, iv)
// note that encrypted is still a window in to the output slice
stream.XORKeyStream(encrypted, data)
return output, nil
}Context
StackExchange Code Review Q#125846, answer score: 9
Revisions (0)
No revisions yet.