5
\$\begingroup\$

I am new to Go and the web token / JTW concept. In order to learn Go and learn about web tokens, I am making my own implementation in Go (never to be used, of course).

If a user gets authenticated (assume that part is secure), I generate a web token, [header].[payload].[signature]. The header contains a timestamp of when the token was created. The payload has the user id and maybe role in there too, and the signature is there to show that the payload hasn't been altered.

  1. Are there any security concerns with this?
  2. Is there a way to improve the performance / speed of this?

    package gwt
    
    import (
        "crypto/hmac"
        "crypto/sha512"
        "crypto/subtle"
        "encoding/base64"
        "encoding/json"
        "errors"
        "fmt"
        "os"
        "strings"
        "time"
    )
    
    var Expired error = errors.New("Token expired")
    var Invalid error = errors.New("Invalid token")
    
    var expireAfter int = 60 * 60 * 24 * 7
    var refreshAfter int = 60 * 60 * 24 * 3
    var secret []byte
    
    func init() {
        gwtSecret := os.Getenv("GWT_SECRET")
        if gwtSecret == "" {
            fmt.Println("You must set GWT_SECRET environment variable: export GWT_SECRET=change_this")
            os.Exit(0)
        } else if len(gwtSecret) < 16 {
            fmt.Println("GWT_SECRET must be longer than 16 characters")
            os.Exit(0)
        }
    
        secret = []byte(gwtSecret)
    }
    
    func Expire(e int, r int) {
        expireAfter = e
        refreshAfter = r
    }
    
    func Make(object map[string]interface{}) (string, error) {
        exp := time.Now().Format(time.UnixDate)
        header := base64.StdEncoding.EncodeToString([]byte(exp))
    
        payloadBytes, err := json.Marshal(object)
        if err != nil {
            return "", err
        }
    
        payload := base64.StdEncoding.EncodeToString(payloadBytes)
    
        return header + "." + payload + "." + sign(header+payload), nil
    }
    
    func GetPayload(token string) (map[string]interface{}, string, error) {
        parts := strings.Split(token, ".")
        if len(parts) != 3 || subtle.ConstantTimeCompare([]byte(sign(parts[0]+parts[1])), []byte(parts[2])) != 1 {
            return nil, "", Invalid
        }
    
        headerBytes, err := base64.StdEncoding.DecodeString(parts[0])
        if err != nil {
            return nil, "", Invalid
        }
    
        header := string(headerBytes)
    
        created, err := time.Parse(time.UnixDate, header)
        if err != nil {
            return nil, "", Invalid
        }
    
        expired := created.Add(time.Second * time.Duration(expireAfter)).Before(time.Now())
        if expired {
            return nil, "", Expired
        }
    
        payloadBytes, err := base64.StdEncoding.DecodeString(parts[1])
        if err != nil {
            return nil, "", Invalid
        }
    
        var payload map[string]interface{}
        if err := json.Unmarshal(payloadBytes, &payload); err != nil {
            return nil, "", Invalid
        }
    
        var newToken string
        stale := created.Add(time.Second * time.Duration(refreshAfter)).Before(time.Now())
        if stale {
            newToken, err = Make(payload)
            if err != nil {
                return nil, "", Invalid
            }
        } else {
            newToken = ""
        }
    
        return payload, newToken, nil
    }
    
    func sign(s string) string {
        h := hmac.New(sha512.New, secret)
        h.Write([]byte(s))
    
        return base64.StdEncoding.EncodeToString(h.Sum(nil))
    }
    

The token will be stored locally on the client, sent via http headers on the necessary requests, and everything is https.

An updated version can be found here.

\$\endgroup\$
  • \$\begingroup\$ Your IsValid function is vulnerable to timing attacks (see stackoverflow.com/a/20663629/3207406 for constant time string comparison) \$\endgroup\$ – oliverpool Jul 25 '17 at 10:52
  • 1
    \$\begingroup\$ @oliverpool Are you suggesting I change the return line to return subtle.ConstantTimeCompare(sign(parts[0]), parts[1]) == 1? \$\endgroup\$ – twharmon Jul 26 '17 at 4:22
4
\$\begingroup\$

Go Code Review Comments

Don't Panic

Don't use panic for normal error handling. Use error and multiple return values.

func Make(object interface{}) string {
    payloadBytes, err := json.Marshal(object)
    if err != nil {
        panic(err)
    }
    payload := base64.StdEncoding.EncodeToString(payloadBytes)
    return payload + "." + sign(payload)
}

I'm going to exploit this, an exported function that panics, to crash your application.

func sign(s string) string {
    h := hmac.New(sha256.New, []byte("secret"))
    h.Write([]byte(s))
    return base64.StdEncoding.EncodeToString(h.Sum(nil))
}

I'm going use a literal embedded in your executable, []byte("secret"), to break your authentication.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ "a literal embedded in your executable" you mean I should use os.Getenv("WT_SECRET") or something? How would you break my authentication, by looking at the code on the server to see my "secret"? If you can look at the code in my executable, I've got much bigger problems. \$\endgroup\$ – twharmon Jun 8 '17 at 12:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.