Generation of TOTP secret now uses crypto/rand instead of math/rand and is unit-tested
MODIFY
commands/commandimpls/tfa.go
commands/commandimpls/tfa.go
@@ -2,13 +2,15 @@ package commandimpls
import (
"fmt"
+ "io"
+
+ "github.com/Bios-Marcel/discordgo"
+
"github.com/Bios-Marcel/cordless/commands"
"github.com/Bios-Marcel/cordless/config"
"github.com/Bios-Marcel/cordless/ui"
"github.com/Bios-Marcel/cordless/ui/tviewutil"
"github.com/Bios-Marcel/cordless/util/text"
- "github.com/Bios-Marcel/discordgo"
- "io"
)
const tfaHelpPage = `[::b]NAME
@@ -139,7 +141,10 @@ func (cmd *TFAEnableCmd) Execute(writer io.Writer, parameters []string) {
if cmd.session.MFA {
fmt.Fprintln(writer, "TFA is already enabled on this account.")
} else {
- cmd.window.ShowTFASetup()
+ tfaError := cmd.window.ShowTFASetup()
+ if tfaError != nil {
+ commands.PrintError(writer, "error showing tfa gui", tfaError.Error())
+ }
}
}

MODIFY
ui/window.go
ui/window.go
@@ -3,19 +3,26 @@ package ui
import (
"bytes"
"fmt"
- "github.com/Bios-Marcel/cordless/util/text"
- "github.com/mattn/go-runewidth"
- "github.com/mdp/qrterminal/v3"
"log"
"strings"
"time"
"unicode"
+ "github.com/mattn/go-runewidth"
+ "github.com/mdp/qrterminal/v3"
+
+ "github.com/Bios-Marcel/cordless/util/text"
+
"github.com/Bios-Marcel/discordemojimap"
"github.com/Bios-Marcel/goclipimg"
"github.com/atotto/clipboard"
+ "github.com/Bios-Marcel/discordgo"
+ "github.com/Bios-Marcel/tview"
+ "github.com/gdamore/tcell"
+ "github.com/gen2brain/beeep"
+
"github.com/Bios-Marcel/cordless/commands"
"github.com/Bios-Marcel/cordless/config"
"github.com/Bios-Marcel/cordless/discordutil"
@@ -26,10 +33,6 @@ import (
"github.com/Bios-Marcel/cordless/ui/tviewutil"
"github.com/Bios-Marcel/cordless/util/fuzzy"
"github.com/Bios-Marcel/cordless/util/maths"
- "github.com/Bios-Marcel/discordgo"
- "github.com/Bios-Marcel/tview"
- "github.com/gdamore/tcell"
- "github.com/gen2brain/beeep"
)
const (
@@ -2127,8 +2130,12 @@ func (window *Window) ExecuteCommand(input string) {
// ShowTFASetup generates a new TFA-Secret and shows a QR-Code. The QR-Code can
// be scanned and the resulting TFA-Token can be entered into cordless and used
// to enable TFA on this account.
-func (window *Window) ShowTFASetup() {
- tfaSecret := text.GenerateBase32Key()
+func (window *Window) ShowTFASetup() error {
+ tfaSecret, secretError := text.GenerateBase32Key()
+ if secretError != nil {
+ return secretError
+ }
+
qrURL := fmt.Sprintf("otpauth://totp/%s?secret=%s&issuer=Discord", window.session.State.User.Email, tfaSecret)
qrCodeText := text.GenerateQRCode(qrURL, qrterminal.M)
qrCodeImage := tview.NewTextView().SetText(qrCodeText).SetTextAlign(tview.AlignCenter)
@@ -2207,6 +2214,8 @@ func (window *Window) ShowTFASetup() {
qrCodeView.AddItem(tviewutil.CreateCenteredComponent(tokenInput, 68), 3, 0, false)
window.app.SetRoot(qrCodeView, true)
window.app.SetFocus(tokenInput)
+
+ return nil
}
func (window *Window) startEditingMessage(message *discordgo.Message) {

MODIFY
util/text/totp.go
util/text/totp.go
package text
import (
+ "crypto/rand"
"errors"
"fmt"
- "math/rand"
"strconv"
"strings"
- "time"
)
// ParseTFACodes takes an arbitrary string and checks whether it's a valid 6
@@ -32,16 +31,22 @@ func ParseTFACode(text string) (string, error) {
}
// GenerateBase32Key generates a 16 character key containing 2-7 and A-Z.
-func GenerateBase32Key() string {
- tfaSecretRaw := make([]rune, 16, 16)
+// The implementation uses crypto/rand.
+func GenerateBase32Key() (string, error) {
+ randomBytes := make([]byte, 16, 16)
+ _, err := rand.Read(randomBytes)
+ if err != nil {
+ return "", err
+ }
+
availableCharacters := [...]rune{
'2', '3', '4', '5', '6', '7',
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
}
- rand.Seed(time.Now().Unix())
- for i := 0; i < 16; i++ {
- tfaSecretRaw[i] = availableCharacters[rand.Int31n(int32(len(availableCharacters)))]
+ tfaSecretRaw := make([]rune, 16, 16)
+ for index, randomByte := range randomBytes {
+ tfaSecretRaw[index] = availableCharacters[rune(int(randomByte)%len(availableCharacters))]
}
- return string(tfaSecretRaw)
+ return string(tfaSecretRaw), nil
}

MODIFY
util/text/totp_test.go
util/text/totp_test.go
@@ -74,3 +74,42 @@ func TestParseTFACode(t *testing.T) {
})
}
}
+
+// TestGenerateBase32Key tests whether 100 unique keys can be generated
+// generated in a row. This is simple in order to make sure that the keys
+// aren't always the same. On top of that, keys are checked against the
+// valid base 32 characters and a length of 16.
+func TestGenerateBase32Key(t *testing.T) {
+ availableCharacters := [...]rune{
+ '2', '3', '4', '5', '6', '7',
+ 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
+ }
+
+ iterations := 50
+ generated := make(map[string]bool, iterations)
+ for i := 0; i < iterations; i++ {
+ newKey, keyError := GenerateBase32Key()
+ if keyError != nil {
+ t.Errorf("Error generating key: %s", keyError)
+ }
+ _, ok := generated[newKey]
+ if ok {
+ t.Errorf("Duplicated key: %s", newKey)
+ }
+ generated[newKey] = true
+
+ if len(newKey) != 16 {
+ t.Errorf("Keylength is invalid: %d", len(newKey))
+ }
+
+ OUTER_LOOP:
+ for _, char := range []rune(newKey) {
+ for _, validChar := range availableCharacters {
+ if validChar == char {
+ continue OUTER_LOOP
+ }
+ }
+ t.Errorf("Generated key contains invalid character: %c", char)
+ }
+ }
+}

Generation of TOTP secret now uses crypto/rand instead of math/rand and is unit-tested