From 6dca637a1623cc9b5f8fdd66ebfb58a3908bd0e4 Mon Sep 17 00:00:00 2001 From: MrMelon54 Date: Mon, 16 Oct 2023 16:47:18 +0100 Subject: [PATCH] Swap out TOTP library --- database/init.sql | 3 +- database/tx.go | 22 +++--- go.mod | 6 +- go.sum | 12 ++-- pages/edit-otp.go.html | 2 + server/otp.go | 155 ++++++++++++++++++----------------------- 6 files changed, 86 insertions(+), 114 deletions(-) diff --git a/database/init.sql b/database/init.sql index ed54fae..3fe86e8 100644 --- a/database/init.sql +++ b/database/init.sql @@ -35,6 +35,7 @@ CREATE TABLE IF NOT EXISTS client_store CREATE TABLE IF NOT EXISTS otp ( subject TEXT PRIMARY KEY UNIQUE NOT NULL, - raw BLOB NOT NULL, + secret TEXT NOT NULL, + digits INTEGER NOT NULL, FOREIGN KEY (subject) REFERENCES users (subject) ); diff --git a/database/tx.go b/database/tx.go index 2b005e6..507ac00 100644 --- a/database/tx.go +++ b/database/tx.go @@ -4,7 +4,6 @@ import ( "database/sql" "fmt" "github.com/1f349/tulip/password" - "github.com/1f349/twofactor" "github.com/go-oauth2/oauth2/v4" "github.com/google/uuid" "time" @@ -167,23 +166,20 @@ WHERE subject = ?`, return nil } -func (t *Tx) SetTwoFactor(sub uuid.UUID, totp *twofactor.Totp) error { - u, err := totp.ToBytes() - if err != nil { - return err - } - _, err = t.tx.Exec(`INSERT INTO otp(subject, raw) VALUES (?, ?) ON CONFLICT(subject) DO UPDATE SET raw = excluded.raw`, sub.String(), u) +func (t *Tx) SetTwoFactor(sub uuid.UUID, secret string, digits int) error { + _, err := t.tx.Exec(`INSERT INTO otp(subject, secret, digits) VALUES (?, ?, ?) ON CONFLICT(subject) DO UPDATE SET secret = excluded.secret, digits = excluded.digits`, sub.String(), secret, digits) return err } -func (t *Tx) GetTwoFactor(sub uuid.UUID, issuer string) (*twofactor.Totp, error) { - var u []byte - row := t.tx.QueryRow(`SELECT raw FROM otp WHERE subject = ?`, sub.String()) - err := row.Scan(&u) +func (t *Tx) GetTwoFactor(sub uuid.UUID) (string, int, error) { + var secret string + var digits int + row := t.tx.QueryRow(`SELECT secret, digits FROM otp WHERE subject = ?`, sub.String()) + err := row.Scan(&secret, &digits) if err != nil { - return nil, err + return "", 0, err } - return twofactor.TOTPFromBytes(u, issuer) + return secret, digits, nil } func (t *Tx) HasTwoFactor(sub uuid.UUID) (bool, error) { diff --git a/go.mod b/go.mod index 72a594f..a2bb484 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ go 1.21.1 require ( github.com/1f349/cache v0.0.2 github.com/1f349/overlapfs v0.0.1 - github.com/1f349/twofactor v1.0.4 github.com/1f349/violet v0.0.9 github.com/MrMelon54/exit-reload v0.0.1 github.com/MrMelon54/pronouns v1.0.1 @@ -18,7 +17,9 @@ require ( github.com/google/uuid v1.3.1 github.com/julienschmidt/httprouter v1.3.0 github.com/mattn/go-sqlite3 v1.14.17 + github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e github.com/stretchr/testify v1.8.4 + github.com/xlzd/gotp v0.1.0 golang.org/x/crypto v0.13.0 golang.org/x/text v0.13.0 ) @@ -29,9 +30,6 @@ require ( github.com/emersion/go-textwrapper v0.0.0-20200911093747-65d896831594 // indirect github.com/golang-jwt/jwt v3.2.2+incompatible // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/sec51/convert v1.0.2 // indirect - github.com/sec51/gf256 v0.0.0-20160126143050-2454accbeb9e // indirect - github.com/sec51/qrcode v0.0.0-20160126144534-b7779abbcaf1 // indirect github.com/tidwall/btree v1.6.0 // indirect github.com/tidwall/buntdb v1.3.0 // indirect github.com/tidwall/gjson v1.16.0 // indirect diff --git a/go.sum b/go.sum index 1a8b16f..2f7b99a 100644 --- a/go.sum +++ b/go.sum @@ -3,8 +3,6 @@ github.com/1f349/cache v0.0.2 h1:27QD6zPd9xYyvh9V1qqWq+EAt5+N+qvyGWKfnjMrhP8= github.com/1f349/cache v0.0.2/go.mod h1:LibAMy13dF0KO1fQA9aEjZPBCB6Y4b5kKYEQJUqc2rQ= github.com/1f349/overlapfs v0.0.1 h1:LAxBolrXFAgU0yqZtXg/C/aaPq3eoQSPpBc49BHuTp0= github.com/1f349/overlapfs v0.0.1/go.mod h1:I6aItQycr7nrzplmfNXp/QF9tTmKRSgY3fXmu/7Ky2o= -github.com/1f349/twofactor v1.0.4 h1:kN4EEGFlKRa7fGrxS+FpgwJI+tllES6YzXqCqurk4Uk= -github.com/1f349/twofactor v1.0.4/go.mod h1:gnG80vElwqLWNMnLT57yu4o4L1GdXGPP6pcIPlapXZs= github.com/1f349/violet v0.0.9 h1:eQfc5fDMKJXVFUjS2UiAGTkOVVBamppD5dguhmU4GeU= github.com/1f349/violet v0.0.9/go.mod h1:Uzu6I1pLBP5UEzcUCTQBbk/NTfI5TAABSrowa8DSpR0= github.com/MrMelon54/exit-reload v0.0.1 h1:sxHa59tNEQMcikwuX2+93lw6Vi1+R7oCRF8a0C3alXc= @@ -96,14 +94,10 @@ github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1y github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/sclevine/agouti v3.0.0+incompatible/go.mod h1:b4WX9W9L1sfQKXeJf1mUTLZKJ48R1S7H23Ji7oFO5Bw= -github.com/sec51/convert v1.0.2 h1:NoKWIRARjM3rQglNypMpcXSLLqPsN/uTTzaGeqDKbeg= -github.com/sec51/convert v1.0.2/go.mod h1:5qL/cT/oiOIvWXy2SccQ7LnacYftqqy9wdyFkTc1k2w= -github.com/sec51/gf256 v0.0.0-20160126143050-2454accbeb9e h1:wKXba8dfsFjbxkMpzZBKt8gkJAMSm1fIf1OSWQFQrVA= -github.com/sec51/gf256 v0.0.0-20160126143050-2454accbeb9e/go.mod h1:hCjOqSOB9PBw5MdJ+0uSLCBV7FbLy0xwOR+c193HkcE= -github.com/sec51/qrcode v0.0.0-20160126144534-b7779abbcaf1 h1:CI9zS8HvMiibvXM/F3IthY797GW77fNYgioJl/8Xzzk= -github.com/sec51/qrcode v0.0.0-20160126144534-b7779abbcaf1/go.mod h1:uPm44Rj3uXSSOvmKmoeRuAUNUgwH2JHW5KIzqFFS/j4= github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= +github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e h1:MRM5ITcdelLK2j1vwZ3Je0FKVCfqOLp5zO6trqMLYs0= +github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e/go.mod h1:XV66xRDqSt+GTGFMVlhk3ULuV0y9ZmzeVGR4mloJI3M= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d h1:zE9ykElWQ6/NYmHa3jpm/yHnI4xSofP+UP6SpjHcSeM= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= github.com/smartystreets/goconvey v1.6.4 h1:fv0U8FUIMPNf1L9lnHLvLhgicrIVChEkdzIKYqbNC9s= @@ -155,6 +149,8 @@ github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHo github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74= github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y= +github.com/xlzd/gotp v0.1.0 h1:37blvlKCh38s+fkem+fFh7sMnceltoIEBYTVXyoa5Po= +github.com/xlzd/gotp v0.1.0/go.mod h1:ndLJ3JKzi3xLmUProq4LLxCuECL93dG9WASNLpHz8qg= github.com/yalp/jsonpath v0.0.0-20180802001716-5cc68e5049a0 h1:6fRhSjgLCkTD3JnJxvaJ4Sj+TYblw757bqYgZaOq5ZY= github.com/yalp/jsonpath v0.0.0-20180802001716-5cc68e5049a0/go.mod h1:/LWChgwKmvncFJFHJ7Gvn9wZArjbV5/FppcK2fKk/tI= github.com/yudai/gojsondiff v1.0.0 h1:27cbfqXLVEJ1o8I6v3y9lg8Ydm53EKqHXAOMxEGlCOA= diff --git a/pages/edit-otp.go.html b/pages/edit-otp.go.html index bcbb1a5..6b067ac 100644 --- a/pages/edit-otp.go.html +++ b/pages/edit-otp.go.html @@ -9,6 +9,8 @@
+ +

OTP QR code not loading

diff --git a/server/otp.go b/server/otp.go index 5f04949..4856022 100644 --- a/server/otp.go +++ b/server/otp.go @@ -2,17 +2,17 @@ package server import ( "bytes" - "crypto" "encoding/base64" "github.com/1f349/tulip/database" "github.com/1f349/tulip/pages" - "github.com/1f349/twofactor" "github.com/google/uuid" "github.com/julienschmidt/httprouter" + "github.com/skip2/go-qrcode" + "github.com/xlzd/gotp" "html/template" "image/png" - "log" "net/http" + "time" ) func (h *HttpServer) LoginOtpGet(rw http.ResponseWriter, req *http.Request, _ httprouter.Params, auth UserAuth) { @@ -49,14 +49,15 @@ func (h *HttpServer) LoginOtpPost(rw http.ResponseWriter, req *http.Request, _ h func (h *HttpServer) fetchAndValidateOtp(rw http.ResponseWriter, sub uuid.UUID, code string) bool { var hasOtp bool - var otp *twofactor.Totp + var secret string + var digits int if h.DbTx(rw, func(tx *database.Tx) (err error) { hasOtp, err = tx.HasTwoFactor(sub) if err != nil { return } if hasOtp { - otp, err = tx.GetTwoFactor(sub, h.conf.OtpIssuer) + secret, digits, err = tx.GetTwoFactor(sub) } return }) { @@ -64,14 +65,8 @@ func (h *HttpServer) fetchAndValidateOtp(rw http.ResponseWriter, sub uuid.UUID, } if hasOtp { - defer func() { - h.DbTx(rw, func(tx *database.Tx) error { - return tx.SetTwoFactor(sub, otp) - }) - }() - - err := otp.Validate(code) - if err != nil { + totp := gotp.NewTOTP(secret, digits, 30, nil) + if !verifyTotp(totp, code) { http.Error(rw, "400 Bad Request: Invalid OTP code", http.StatusBadRequest) return true } @@ -81,7 +76,7 @@ func (h *HttpServer) fetchAndValidateOtp(rw http.ResponseWriter, sub uuid.UUID, } func (h *HttpServer) EditOtpGet(rw http.ResponseWriter, req *http.Request, _ httprouter.Params, auth UserAuth) { - var digits = 0 + var digits int switch req.URL.Query().Get("digits") { case "6": digits = 6 @@ -94,102 +89,86 @@ func (h *HttpServer) EditOtpGet(rw http.ResponseWriter, req *http.Request, _ htt return } - var otp *twofactor.Totp - - otpRaw, ok := auth.Session.Get("temp-otp") - if ok { - if otp, ok = otpRaw.(*twofactor.Totp); !ok { - http.Error(rw, "400 Bad Request: invalid session, try clearing your cookies", http.StatusBadRequest) - return - } - - // check OTP code matches number of digits - tempCode, err := otp.OTP() - if err != nil || len(tempCode) != digits { - otp = nil - } - } - - // make a new otp handler if needed - if otp == nil { - // get user email - var email string - if h.DbTx(rw, func(tx *database.Tx) error { - var err error - email, err = tx.GetUserEmail(auth.Data.ID) - return err - }) { - return - } - - // generate OTP key + // get user email + var email string + if h.DbTx(rw, func(tx *database.Tx) error { var err error - otp, err = twofactor.NewTOTP(email, h.conf.OtpIssuer, crypto.SHA512, digits) - if err != nil { - http.Error(rw, "500 Internal Server Error: Failed to generate OTP key", http.StatusInternalServerError) - return - } - - // save otp key - auth.Session.Set("temp-otp", otp) - err = auth.Session.Save() - if err != nil { - http.Error(rw, "500 Internal Server Error: Failed to save session", http.StatusInternalServerError) - return - } - } - - // get qr and url - otpQr, err := otp.QR() - if err != nil { - http.Error(rw, "500 Internal Server Error: Failed to generate OTP QR code", http.StatusInternalServerError) + email, err = tx.GetUserEmail(auth.Data.ID) + return err + }) { return } - decode, err := png.Decode(bytes.NewReader(otpQr)) - if err != nil { + + secret := gotp.RandomSecret(64) + if secret == "" { + http.Error(rw, "500 Internal Server Error: failed to generate OTP secret", http.StatusInternalServerError) return } - b := decode.Bounds() - qrWidth := b.Dx() / 4 - otpUrl, err := otp.URL() + totp := gotp.NewTOTP(secret, digits, 30, nil) + otpUri := totp.ProvisioningUri(email, h.conf.OtpIssuer) + code, err := qrcode.New(otpUri, qrcode.Medium) if err != nil { - http.Error(rw, "500 Internal Server Error: Failed to generate OTP URL", http.StatusInternalServerError) + http.Error(rw, "500 Internal Server Error: failed to generate QR code", http.StatusInternalServerError) + return + } + qrImg := code.Image(60 * 4) + qrBounds := qrImg.Bounds() + qrWidth := qrBounds.Dx() + + qrBuf := new(bytes.Buffer) + if png.Encode(qrBuf, qrImg) != nil { + http.Error(rw, "500 Internal Server Error: failed to generate PNG image of QR code", http.StatusInternalServerError) return } // render page pages.RenderPageTemplate(rw, "edit-otp", map[string]any{ "ServiceName": h.conf.ServiceName, - "OtpQr": template.URL("data:image/png;base64," + base64.StdEncoding.EncodeToString(otpQr)), + "OtpQr": template.URL("data:qrImg/png;base64," + base64.StdEncoding.EncodeToString(qrBuf.Bytes())), "QrWidth": qrWidth, - "OtpUrl": otpUrl, + "OtpUrl": otpUri, + "OtpSecret": secret, + "OtpDigits": digits, }) } func (h *HttpServer) EditOtpPost(rw http.ResponseWriter, req *http.Request, _ httprouter.Params, auth UserAuth) { - var otp *twofactor.Totp - - otpRaw, ok := auth.Session.Get("temp-otp") - if !ok { - http.Error(rw, "400 Bad Request: invalid session, try clearing your cookies", http.StatusBadRequest) - return - } - if otp, ok = otpRaw.(*twofactor.Totp); !ok { - http.Error(rw, "400 Bad Request: invalid session, try clearing your cookies", http.StatusBadRequest) - return - } - err := otp.Validate(req.FormValue("code")) - if err != nil { - http.Error(rw, "400 Bad Request: invalid OTP code: "+err.Error(), http.StatusBadRequest) - log.Println() + var digits int + switch req.FormValue("digits") { + case "6": + digits = 6 + case "7": + digits = 7 + case "8": + digits = 8 + default: + http.Error(rw, "400 Bad Request: Invalid number of digits for OTP code", http.StatusBadRequest) return } - if h.DbTx(rw, func(tx *database.Tx) error { - return tx.SetTwoFactor(auth.Data.ID, otp) - }) { + secret := req.FormValue("secret") + if !gotp.IsSecretValid(secret) { + http.Error(rw, "400 Bad Request: Invalid secret", http.StatusBadRequest) + return + } + + totp := gotp.NewTOTP(secret, digits, 30, nil) + + if !verifyTotp(totp, req.FormValue("code")) { + http.Error(rw, "400 Bad Request: invalid OTP code", http.StatusBadRequest) return } http.Redirect(rw, req, "/", http.StatusFound) } + +func verifyTotp(totp *gotp.TOTP, code string) bool { + t := time.Now() + if totp.VerifyTime(code, t) { + return true + } + if totp.VerifyTime(code, t.Add(-30*time.Second)) { + return true + } + return totp.VerifyTime(code, t.Add(30*time.Second)) +}