Encrypting strings in Golang

Question

This is a an example application for testing some encryption and decryption functions in Golang. The functions 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_i1m6KxKRs7YT12O-o47PrIKXTOHtk7BpSRrYr4jtwrfHU5jIduS23BZHMCtaw0w=="

Output:

Decrypting '0bWnVHDPIls_a-MWUs_i1m6KxKRs7YT12O-o47PrIKXTOHtk7BpSRrYr4jtwrfHU5jIduS23BZHMCtaw0w==' with key 'I am a key'
Output: 'This is some text that needs to be encrypted.'

Code

package main

import (
    "fmt"
    "os"
    "crypto/sha256"
    "crypto/cipher"
    "crypto/aes"
    "crypto/rand"
    "encoding/base64"
    "io"
    "errors"
)

// 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) {

    // Format the keyString so that it's 32 bytes.
    newKeyString, err := hashTo32Bytes(keyString)

    // Encode the cryptoText to base 64.
    cipherText, _ := base64.URLEncoding.DecodeString(cryptoText)

    block, err := aes.NewCipher([]byte(newKeyString))

    if err != nil {
        panic(err)
    }

    if len(cipherText) < aes.BlockSize {
        panic("cipherText too short")
    }

    iv := cipherText[:aes.BlockSize]
    cipherText = cipherText[aes.BlockSize:]

    stream := cipher.NewCFBDecrypter(block, iv)

    stream.XORKeyStream(cipherText, cipherText)

    return string(cipherText), 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) {

    // Format the keyString so that it's 32 bytes.
    newKeyString, err := hashTo32Bytes(keyString)

    if err != nil {
        return "", err
    }

    key := []byte(newKeyString)
    value := []byte(plainText)

    block, err := aes.NewCipher(key)

    if err != nil {
        panic(err)
    }

    cipherText := make([]byte, aes.BlockSize + len(value))

    iv := cipherText[:aes.BlockSize]
    if _, err = io.ReadFull(rand.Reader, iv); err != nil {
        return
    }

    cfb := cipher.NewCFBEncrypter(block, iv)
    cfb.XORKeyStream(cipherText[aes.BlockSize:], value)

    return base64.URLEncoding.EncodeToString(cipherText), nil
}

// 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
}

func main() {

    // Get the amount of arguments from the command line.
    argumentsCount := len(os.Args)

    // Expected usage:
    // encrypt.go -e|-d --key "key here" --value "value here"

    if argumentsCount != 6 {
        fmt.Printf("Usage:\n-e to encrypt, -d to decrypt.\n")
        fmt.Printf("--key \"I am a key\" to load the key.\n")
        fmt.Printf("--value \"I am a text to be encrypted or decrypted\".\n")
        return
    }

    // Set up some flags to check against arguments.
    encrypt := false
    decrypt := false
    key := false
    expectKeyString := 0
    keyString := false
    value := false
    expectValueString := 0
    valueString := false

    // Set the input variables up.
    encryptionFlag := ""
    stringToEncrypt := ""
    encryptionKey := ""

    // Get the arguments from the command line.
    // If any issues are detected, alert the user and exit.
    for index, element := range os.Args {

        if element == "-e" {
            // Ensure that decrypt has not also been set.
            if decrypt == true {
                fmt.Printf("Can't set both -e and -d.\nBye!\n")
                return
            }
            encrypt = true
            encryptionFlag = "-e"

        } else if element == "-d" {
            // Ensure that encrypt has not also been set.
            if encrypt == true {
                fmt.Printf("Can't set both -e and -d.\nBye!\n")
                return
            }
            decrypt = true
            encryptionFlag = "-d"

        } else if element == "--key" {
            key = true
            expectKeyString++

        } else if element == "--value" {
            value = true
            expectValueString++

        } else if expectKeyString == 1 {
            encryptionKey = os.Args[index]
            keyString = true
            expectKeyString = 0

        } else if expectValueString == 1 {
            stringToEncrypt = os.Args[index]
            valueString = true
            expectValueString = 0
        }

        if expectKeyString >= 2 {
            fmt.Printf("Something went wrong, too many keys entered.\bBye!\n")
            return

        } else if expectValueString >= 2 {
            fmt.Printf("Something went wrong, too many keys entered.\bBye!\n")
            return
        }
    }


    // On error, output some useful information.
    if !(encrypt == true || decrypt == true) || key == false || keyString == false || value == false || valueString == false {
        fmt.Printf("Incorrect usage!\n")
        fmt.Printf("---------\n")
        fmt.Printf("-e or -d -> %v\n", (encrypt == true || decrypt == true))
        fmt.Printf("--key -> %v\n", key)
        fmt.Printf("Key string? -> %v\n", keyString)
        fmt.Printf("--value -> %v\n", value)
        fmt.Printf("Value string? -> %v\n", valueString)
        fmt.Printf("---------")
        fmt.Printf("\nUsage:\n-e to encrypt, -d to decrypt.\n")
        fmt.Printf("--key \"I am a key\" to load the key.\n")
        fmt.Printf("--value \"I am a text to be encrypted or decrypted\".\n")
        return
    }


    // Check the encrpytion flag.
    if false == (encryptionFlag == "-e" || encryptionFlag == "-d") {
        fmt.Println("Sorry but the first argument has to be either -e or -d")
        fmt.Println("for either encryption or decryption.")
        return
    }

    if encryptionFlag == "-e" {
        // Encrypt!

        fmt.Printf("Encrypting '%s' with key '%s'\n", stringToEncrypt, encryptionKey)

        encryptedString, _ := encryptString(stringToEncrypt,encryptionKey)

        fmt.Printf("Output: '%s'\n", encryptedString)

    } else if encryptionFlag == "-d" {
        // Decrypt!

        fmt.Printf("Decrypting '%s' with key '%s'\n", stringToEncrypt, encryptionKey)

        decryptedString, _ := decryptString(stringToEncrypt,encryptionKey)

        fmt.Printf("Output: '%s'\n", decryptedString)

    }
}

Community · Accepted Answer · 2017-04-13 12:40:47Z

Nice question.

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-based processes, gives:

// 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
}

Testing

go test is a really convenient way to test code. I wrote the following when working with your code. It's a mess, but, it will give you some ideas:

func TestEnc(t *testing.T) {
    data := []struct {
        input string
        key   string
    }{
        {"Foo", "Boo"},
        {"Bar", "Car"},
        {"Bar", ""},
        {"", "Car"},
        {"Long input with more than 16 characters", "Car"},
    }
    for _, d := range data {
        enc, err := encryptString(d.input, d.key)
        if err != nil {
            t.Errorf("Unable to encrypt '%v' with key '%v': %v", d.input, d.key, err)
            continue
        }
        dec, err := decryptString(enc, d.key)
        if err != nil {
            t.Errorf("Unable to decrypt '%v' with key '%v': %v", enc, d.key, err)
            continue
        }
        if dec != d.input {
            t.Errorf("Decrypt Key %v\n  Input: %v\n  Expect: %v\n  Actual: %v", d.key, enc, d.input, enc)
        }
    }
}

As a result, I never needed to run your main method...

Conclusion

The encryptString and decryptString methods can be simplified by removing the actual []byte manipulation in to separate methods.

The 32-byte hash method was horrible - it lost a lot of "randomness" from the key, and it was not using the right argument types.

The Block cyphers are really good, by the way. AES is a fine way to go.

Because this answer has a lot of attention, I thought people might be interested in this question about mode implementations: security.stackexchange.com/questions/27776/… — hermancain, Commented Jan 22, 2017 at 18:58

Stack Exchange Network

Encrypting strings in Golang

What I would like feedback on

How to use program to encrypt

How to use program to decrypt

Code

1 Answer 1

Consistent error handling.

Crypto Key

Decrypting

Encrypting

Testing

Conclusion

Linked

Hot Network Questions

Encrypting strings in Golang

What I would like feedback on

How to use program to encrypt

How to use program to decrypt

Code

1 Answer 1

Consistent error handling.

Crypto Key

Decrypting

Encrypting

Testing

Conclusion

Linked

Related

Hot Network Questions