diff --git a/services/mailer/token/token.go b/services/mailer/token/token.go index 8a5a762d6b..1a52bce803 100644 --- a/services/mailer/token/token.go +++ b/services/mailer/token/token.go @@ -22,9 +22,16 @@ import ( // // The payload is verifiable by the generated HMAC using the user secret. It contains: // | Timestamp | Action/Handler Type | Action/Handler Data | +// +// +// Version changelog +// +// v1 -> v2: +// Use 128 instead of 80 bits of the HMAC-SHA256 output. const ( tokenVersion1 byte = 1 + tokenVersion2 byte = 2 tokenLifetimeInYears int = 1 ) @@ -70,7 +77,7 @@ func CreateToken(ht HandlerType, user *user_model.User, data []byte) (string, er return "", err } - return encodingWithoutPadding.EncodeToString(append([]byte{tokenVersion1}, packagedData...)), nil + return encodingWithoutPadding.EncodeToString(append([]byte{tokenVersion2}, packagedData...)), nil } // ExtractToken extracts the action/user tuple from the token and verifies the content @@ -84,7 +91,7 @@ func ExtractToken(ctx context.Context, token string) (HandlerType, *user_model.U return UnknownHandlerType, nil, nil, &ErrToken{"no data"} } - if data[0] != tokenVersion1 { + if data[0] != tokenVersion2 { return UnknownHandlerType, nil, nil, &ErrToken{fmt.Sprintf("unsupported token version: %v", data[0])} } @@ -124,5 +131,8 @@ func generateHmac(secret, payload []byte) []byte { mac.Write(payload) hmac := mac.Sum(nil) - return hmac[:10] // RFC2104 recommends not using less then 80 bits + // RFC2104 section 5 recommends that if you do HMAC truncation, you should use + // the max(80, hash_len/2) of the leftmost bits. + // For SHA256 this works out to using 128 of the leftmost bits. + return hmac[:16] } diff --git a/tests/integration/incoming_email_test.go b/tests/integration/incoming_email_test.go index fdc0425b54..66f833b28d 100644 --- a/tests/integration/incoming_email_test.go +++ b/tests/integration/incoming_email_test.go @@ -4,6 +4,7 @@ package integration import ( + "encoding/base32" "io" "net" "net/smtp" @@ -75,6 +76,51 @@ func TestIncomingEmail(t *testing.T) { assert.Equal(t, payload, p) }) + tokenEncoding := base32.StdEncoding.WithPadding(base32.NoPadding) + t.Run("Deprecated token version", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + payload := []byte{1, 2, 3, 4, 5} + + token, err := token_service.CreateToken(token_service.ReplyHandlerType, user, payload) + require.NoError(t, err) + assert.NotEmpty(t, token) + + // Set the token to version 1. + unencodedToken, err := tokenEncoding.DecodeString(token) + require.NoError(t, err) + unencodedToken[0] = 1 + token = tokenEncoding.EncodeToString(unencodedToken) + + ht, u, p, err := token_service.ExtractToken(db.DefaultContext, token) + require.ErrorContains(t, err, "unsupported token version: 1") + assert.Equal(t, token_service.UnknownHandlerType, ht) + assert.Nil(t, u) + assert.Nil(t, p) + }) + + t.Run("MAC check", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + payload := []byte{1, 2, 3, 4, 5} + + token, err := token_service.CreateToken(token_service.ReplyHandlerType, user, payload) + require.NoError(t, err) + assert.NotEmpty(t, token) + + // Modify the MAC. + unencodedToken, err := tokenEncoding.DecodeString(token) + require.NoError(t, err) + unencodedToken[len(unencodedToken)-1] ^= 0x01 + token = tokenEncoding.EncodeToString(unencodedToken) + + ht, u, p, err := token_service.ExtractToken(db.DefaultContext, token) + require.ErrorContains(t, err, "verification failed") + assert.Equal(t, token_service.UnknownHandlerType, ht) + assert.Nil(t, u) + assert.Nil(t, p) + }) + t.Run("Handler", func(t *testing.T) { t.Run("Reply", func(t *testing.T) { checkReply := func(t *testing.T, payload []byte, issue *issues_model.Issue, commentType issues_model.CommentType) {