Decode Argon2 parameters and handling decode errors

Question

I am writing a function to decode Argon2 hashed string to extract parameters. I need to handle many decode errors, and handle those errors lead to write many repeating if err == nil check code. Is there a better way to do this kind of check?

The encodedHash string follows the format, separated by $

"$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s"

so an example string looks like

$argon2id$v=19$m=65536,t=3,p=4$ruDgwQK24h0wGXI87+lVWAbHmgNidUNPVSTdSloOlfM$py20HR7L4K6LllGsZNDbkrbh89x2tIF8JCIG0DAaoi8

What the decode function do is convert the string into this struct

type Argon2Params struct {
 Memory uint32
 Time uint32
 Threads uint8
 SaltLength uint32
 KeyLength uint32
}

Here is the decode function, I need to do a serious check during the decoding process. Every filed need to be checked to make sure no error rises.

func decodeHash(encodedHash string) (p *Params, salt, hash []byte, err error) {
 fileds := strings.Split(encodedHash, "$")
 if len(fileds) != 6 {
 return nil, nil, nil, errors.New("The encoded hash is not in the correct format")
 }
 var version int
// error check
 if _, err = fmt.Sscanf(fileds[2], "v=%d", &version); err != nil {
 return
 }
// error check
 if version != argon2.Version {
 return nil, nil, nil, errors.New("Argon2 version not match")
 }
 p = &Params{}
 _, err = fmt.Sscanf(fileds[3], "m=%d,t=%d,p=%d", &p.memory, &p.time, &p.threads)
// error check
 if err != nil {
 return
 }
 salt, p.saltLength, err = decodeBase64WithLength(fileds[4])
// error check
 if err != nil {
 return
 }
// error check
 hash, p.keyLength, err = decodeBase64WithLength(fileds[5])
 return
}
// a helper function
func decodeBase64WithLength(encodeString string) (str []byte, length uint32, err error) {
 str, err = base64.RawStdEncoding.DecodeString(encodeString)
 length = uint32(len(str))
 return
}

Answer 1

Because you use named parameters, the error checking can indeed be simplified.

While you critique the use of if err != nil, I enjoy the consistent error handling, and prefer it to the concepts used in many other languages.

Returning errors

Rather than explicitly returning values, such as

return nil, nil, nil, errors.New("...")

You can instead set the named return value err:

if len(fields) != 6 {
 err = errors.New("incorrect hash format")
 return
}

This does not set the other values to nil, and that shouldn't matter. In terms of consuming this API, it would be bad practice to use any other return values if err is not nil. It would be the equivalent in C of using the return value of strtoul() without checking errno.

Having the comment // error check each time is too verbose.

Misspelling

The variable fileds is misspelled, it should be fields.

Helper function

Using named return values in your helper function is superfluous. You can easily return those variables directly.

func decodeBase64WithLength(encStr string) ([]byte, uint32, error) {
 s, err := base64.RawStdEncoding.DecodeString(encStr)
 return s, uint32(len(s)), err
}

But now that the function has been cleaned up, it's easy to see that it really isn't needed. If this is the only place you use the function, then doing something small twice doesn't justify a helper function in my opinion.

Params

You reference Argon2Params as Params, and both are exported types, but you refer to its members as unexported types. I'll assume it's exported, and capitalize the member variables.

Conclusion

Here is the code I ended up with:

package main
import (
 "encoding/base64"
 "errors"
 "fmt"
 "strings"
 "golang.org/x/crypto/argon2"
)
// Argon2Params are ...
type Argon2Params struct {
 Memory uint32
 Time uint32
 Threads uint8
 SaltLength uint32
 KeyLength uint32
}
func decodeHash(encodedHash string) (p *Argon2Params, salt, hash []byte, err error) {
 fields := strings.Split(encodedHash, "$")
 if len(fields) != 6 {
 err = errors.New("incorrect hash format")
 return
 }
 var version int
 if _, err = fmt.Sscanf(fields[2], "v=%d", &version); err != nil {
 return
 }
 if version != argon2.Version {
 err = errors.New("argon2 version mismatch")
 return
 }
 p = &Argon2Params{}
 if _, err = fmt.Sscanf(fields[3], "m=%d,t=%d,p=%d", &p.Memory, &p.Time,
 &p.Threads); err != nil {
 return
 }
 salt, err = base64.RawStdEncoding.DecodeString(fields[4])
 p.SaltLength = uint32(len(salt))
 if err != nil {
 return
 }
 hash, err = base64.RawStdEncoding.DecodeString(fields[5])
 p.KeyLength = uint32(len(hash))
 return
}
func main() {
 decodeHash("$argon2id$v=19$m=65536,t=3,p=4$ruDgwQK24h0wGXI87+lVWAbHmgNidUNPVSTdSloOlfM$py20HR7L4K6LllGsZNDbkrbh89x2tIF8JCIG0DAaoi8")
}

Hope this helps!

• Li Jinyao: Wow! such a detailed answer! Yes the Params is same as Argon2Params .