OTP Generated by Google Authenticator not validated by the Library's OTP #94
Comments
|
I can not reproduce in my environment and all the authenticator apps below returns as success, "OTP verified successfully!"
In my experience, the time zone and/or NTP are not set correctly in such cases. |
|
Hi, @Cprime50. I reproduced your code and it worked for me. Go version (Latest) I don't think this issue has anything to do with Go's version. Package version |
|
I've found that sometimes the While it is funny that Google asks me to use Apple software because they suck, maybe there's good reason for this package to make sure the secret is the first parameter in the query. |
secret at the end -> gauth throws in the towel |
|
https://github.com/pquerna/otp/blob/master/internal/encode.go#L21 this is the culprit. |
|
i'll create a PR |
|
According to the below specs, "secret" is the only required parameter and all the examples begins with "secret" parameter.
Due to the random nature of Golang maps, "secret" does not appear statically in the first parameter. Therefore, I was almost convinced that GoogleAuthenticator had a rule that the first parameter had to be "secret". But on the other hand, the specifications do not stipulate that the first parameter must be "secret". I have looked at the old source code of GoogleAuthenticator for both Android and iOS and it seems that neither of them check/restricts the order of the parameters in the OTP URI. So even if it does no harm if the first parameter is forced to be "secret", certain precautions should be taken. Because I suspect that there is a deeper reason for this. I created the below test code. It creates QR code images for both URI with "secret" parameter at the head and the tail. And they both worked fine on my Android and iPhone authenticators (Google Authenticator and Microsoft Authenticator). @yogo1212 Does the blow reproduce the problem? TEST CODEpackage main
import (
"image"
"image/png"
"os"
"github.com/boombuler/barcode"
"github.com/boombuler/barcode/qr"
)
func main() {
const (
// valid URI which begins with a secret parameter
validURI = "otpauth://totp/localhost:8000:[email protected]?" +
"secret=3GTPYMOZFQGN26SXJRB3RUHFR4NESG4PRNFX2PBGRDBGKRYNRE4A&algorithm=SHA1&digits=6&issuer=localhost:8000&period=30"
// invalid URI which ends with a secret parameter
invalidURI = "otpauth://totp/localhost:8000:[email protected]?" +
"algorithm=SHA1&digits=6&issuer=localhost:8000&period=30&secret=3GTPYMOZFQGN26SXJRB3RUHFR4NESG4PRNFX2PBGRDBGKRYNRE4A"
)
// generate QR code for valid URI and save it to file
okImage, err := getQR(validURI, 256, 256)
panicOnError(err)
err = saveImg(okImage, "valid.png")
panicOnError(err)
// generate QR code for invalid URI and save it to file
ngImage, err := getQR(invalidURI, 256, 256)
panicOnError(err)
err = saveImg(ngImage, "invalid.png")
panicOnError(err)
}
func saveImg(img image.Image, filename string) error {
file, err := os.Create(filename)
if err != nil {
return err
}
defer file.Close()
if err := png.Encode(file, img); err != nil {
return err
}
return nil
}
func getQR(orig string, width int, height int) (image.Image, error) {
b, err := qr.Encode(orig, qr.M, qr.Auto)
if err != nil {
return nil, err
}
b, err = barcode.Scale(b, width, height)
if err != nil {
return nil, err
}
return b, nil
}
func panicOnError(err error) {
if err != nil {
panic(err)
}
} |
|
@KEINOS there's a draft RFC that (seemingly) didn't go through (err.. it expired. where is my mind). the rfc links to a blog post exploring the situation. google worked on the thing until they were happy with out, producing a de-facto-but-not-really standard, and left the cleanup for the rest of the world. i created #95 in the meantime. |
|
That's very informative! Thank you.
Indeed, the blog explains everything.
I do understand and agree. But what I'm pointing out here is that I can not reproduce the problem on any device and AuthApps that I have. So, I have a hunch that the order of the parameters itself is not the cause of the problem. However, let's assume that in the case of GoogleAuthenticator, the first parameter must be "secret" and is mandatory. If this is due to the order of the parameters, it will lead to conflicts in the future if other providers choose different specifications. As noted in the blog, different providers have different specifications. To continue the discussion for closing, shouldn't the order of the optional parameters be configurable as follows? - func EncodeQuery(v url.Values) string {...}
+ func EncodeQuery(v url.Values, paramKeys ...string) string {
+ if len(paramKeys) == 0 {
+ return ...(original behaviour)...
+ }
+
+ for _, paramKey := range paramKeys {
+ // append to the URI if paramKey exists in v
+ }
+ return ...(custom ordered)...
+ } |
|
@KEINOS most authenticators don't impose any particular order - which is the sane thing to do. since we aren't aware of any other restrictions when it comes to ordering params, the solution you suggested is probably overkill. then, the workaround could even be removed from here. |
|
since google, apple, etc. brought us into this age, i see little reason against creating a negative review on any of the app stores, explaining this problem. in fact, i got my hands on a relative's iphone just to do that.
for those of you want to their spend their time being annoyed by this issue more productively, i can only recommend doing the same. |
|
I agree that my propose may be a bit over the top.
Can you provide us the URI and the QR code of that particular example? I'm not trying to be offensive, just looking for a reproducible example. So we can find the actual problem and make this package handle one thing and well. I like this package and believe that it will make my application more secure. |
|
doesn't work (QR code):
does work:
this is for the cross-checking the generated otp code:
|
|
|
urgh. that looks horrible in here... |
Indeed, the camera didn't recognize the QR code. 😄 I'll write a test code with the TOTP URIs given and compare the image information. P.S. |
|
I made a simple test code. Here are the produced images. Can you read them?
Test Code
package main
import (
"crypto/sha1"
"fmt"
"image"
"image/png"
"os"
"os/signal"
"syscall"
"time"
"github.com/pquerna/otp"
"github.com/pquerna/otp/totp"
)
func main() {
const (
// invalid URI which ends with a secret parameter
invalidURI = "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X"
// valid URI which begins with a secret parameter
validURI = "otpauth://totp/domain.com:[email protected]?" +
"secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=domain.com&period=45"
// Image size of the QR code
imgSize = 256
)
// Create a new key from the URI
keyInvalid, err := otp.NewKeyFromURL(invalidURI) // Invalid case
panicOnError(err)
keyValid, err := otp.NewKeyFromURL(validURI) // Valid case
panicOnError(err)
// Generate QR code images
imgInvalid, err := keyInvalid.Image(imgSize, imgSize)
panicOnError(err)
imgValid, err := keyValid.Image(imgSize, imgSize)
panicOnError(err)
// Save the QR code to a file
panicOnError(saveImg(imgInvalid, "invalid.png"))
panicOnError(saveImg(imgValid, "valid.png"))
// Print the SHA1 hash of the QR code images for debugging
fmt.Println("QR code images saved. (Below are the SHA1 hash of the images)")
panicOnError(printFileHash("invalid.png"))
panicOnError(printFileHash("valid.png"))
fmt.Println("\n" + "Open the QR code images and scan it with the Authenticator app to register.")
fmt.Println("- Invalid QR code img: invalid.png")
fmt.Println("- Valid QR code img : valid.png")
// Watch cancel signal
done := watchCancel()
defer func() {
fmt.Println("\nCancelled")
}()
// Print the current pass code on every second
fmt.Println("\n" + "Current expected pass-code (Press Ctrl+C to cancel)")
for {
select {
case <-done:
return // monitor cancelled
default:
timeNow := time.Now()
// Get the current pass code
passInvalid, err := totp.GenerateCodeCustom(
keyInvalid.Secret(),
timeNow,
totp.ValidateOpts{
Period: uint(keyInvalid.Period()),
Digits: keyInvalid.Digits(),
Algorithm: keyInvalid.Algorithm(),
},
)
panicOnError(err)
passValid, err := totp.GenerateCodeCustom(
keyValid.Secret(),
timeNow,
totp.ValidateOpts{
Period: uint(keyValid.Period()),
Digits: keyValid.Digits(),
Algorithm: keyValid.Algorithm(),
},
)
panicOnError(err)
// Print the pass code
fmt.Print("\r- Invalid: ", passInvalid, " | Valid: ", passValid)
time.Sleep(time.Second)
}
}
}
func printFileHash(pathImg string) error {
imgData, err := os.ReadFile(pathImg)
if err != nil {
return err
}
imgHash := sha1.Sum(imgData)
fmt.Printf("- Hash:%x (%s)\n", imgHash, pathImg)
return nil
}
func watchCancel() chan bool {
// Create a channel to catch signals
sigChan := make(chan os.Signal, 1)
signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
// Channel to receive signals
done := make(chan bool, 1)
// Spawn a goroutine to receive signals
go func() {
<-sigChan
done <- true
}()
return done
}
func saveImg(img image.Image, filename string) error {
file, err := os.Create(filename)
if err != nil {
return err
}
defer file.Close()
if err := png.Encode(file, img); err != nil {
return err
}
return nil
}
func panicOnError(err error) {
if err != nil {
panic(err)
}
}
module example/reproduce
go 1.23.2
require github.com/boombuler/barcode v1.0.2 // indirect
require github.com/pquerna/otp v1.4.0
github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8=
github.com/boombuler/barcode v1.0.2 h1:79yrbttoZrLGkL/oOI8hBrUKucwOL0oOjUgEguGMcJ4=
github.com/boombuler/barcode v1.0.2/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8=
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
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/pquerna/otp v1.4.0 h1:wZvl1TIVxKRThZIBiwOOHOGP/1+nZyWBil9Y2XNEDzg=
github.com/pquerna/otp v1.4.0/go.mod h1:dkJfzwRKNiegxyNb54X/3fLwhCynbMspSyWKnvi1AEg=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=P.S. |
|
@KEINOS yeah, ms authenticator is 💩 otherwise, i'm only talking about google authenticator. see here for a comparison of providers: https://en.wikipedia.org/wiki/Comparison_of_OTP_applications |
Indeed. Let's stick with GoogleAuthenticator for now. Were you able to create the PNG QR code rather than the CLI output one? |
|
@KEINOS qr codes can be created using different parameters, like size, fault tolerance, or encoding. since camera access, and qr code parsing as well, are usually implemented using OS libraries, i wouldn't expect any problems from that end. i use |
We need to isolate the problem one by one. Can you provide us that QR code image? As you say, if the arguments and order are the same, the output image should be the same regardless of how the QR code is generated. That's why I am asking for the QR code image. If the images differ then the problem is in the The fact that the problem is not reproduced with exactly the same URI indicates that the problem may not be in the Or at least please try the test code that I provided and tell us the results? Am I asking too much or asking some thing nonsense? > all |
|
I tested with the latest But again, both It can be read by GoogleAuthenticator (iPhone, iPad and Android) without errors and generates an 8-digit passcode that is identical to To ensure reproducibility I used Docker , but the same local version of #!/bin/bash
set -e
docker build --quiet -t qrencode:local .
clear;
echo "* qrencode version"
docker run --rm qrencode:local --version
invalidURI="otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X"
validURI="otpauth://totp/domain.com:[email protected]?secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=domain.com&period=45"
echo; echo "* qrencode invalid URI"
echo -n "${invalidURI}" | docker run --rm -i qrencode:local -o - > invalid_qrencode.png
sha1sum invalid_qrencode.png
echo; echo "* qrencode valid URI"
echo -n "${validURI}" | docker run --rm -i qrencode:local -o - > valid_qrencode.png
sha1sum valid_qrencode.png$ ls
Dockerfile test.sh
$ ./test.sh
* qrencode version
qrencode version 4.1.1
Copyright (C) 2006-2017 Kentaro Fukuchi
* qrencode invalid URI
f0662befd805a31761ff2fa6df5deef78a17187d invalid_qrencode.png
* qrencode valid URI
bcd15753a083db7aa67c6866cccb5e339cf2d543 valid_qrencode.png
$ ls
Dockerfile test.sh valid_qrencode.png valid_qrencode.pngDockerfileFROM alpine:latest AS build
ARG LIBQRENCODE_VERSION=v4.1.1
WORKDIR /root
RUN apk add --no-cache \
alpine-sdk \
build-base \
cmake \
libpng-dev
RUN \
git clone https://github.com/fukuchi/libqrencode.git && \
cd libqrencode && \
git checkout -b build refs/tags/${LIBQRENCODE_VERSION} && \
cmake BUILD_SHARED_LIBS=ON -Wno-dev . && \
make && \
make install
# Smoketest
RUN qrencode --version
ENTRYPOINT [ "qrencode" ] |
|
@KEINOS this is not necessarily a question of asking "too much" :-) to me, it was unclear whether we were talking about the same thing, which is important for the decision to divert time. they also do with my "work" iphone. it has google authenticator 4.2.1. the versions don't compare because the app for ios is different from that for android. strange. i tried around a little and, doing nothing more than replacing the secret in your URI with one freshly generated (using the code in this comment), created this: and it doesn't work. i moved the secret to the start and it doesn't work either?!
so, i assume you're right about it not really being about the position of the parameter. |
|
wait a second, going back to these two: the "invalid uri" does not work when generated using but why would the result be different depending on the position of the secret? |
|
indeed it is! they all work with looks like google does their own qr code parsing and that fails with low redundancy codes if the secret comes first (sometimes, but not always)! |
|
wait. now the same thing works with |
|
entirely stupid.
doesn't work. taking lines from my terminal scrollback buffer (has timestamps and comments). this worked (when everything worked): i sent the png to a friend who has both a modern iphone and a modern android and he reports that neither code works with any of them. |
|
everything works reliably with other apps like the rocketchat authenticator or apple keychain. |
|
the exact same command (taken from my shell history) produces different outcomes. this is the kind of thing that drives you crazy. at least there's my colleague who confirmed the qr code isn't working for him as well (he was the one who triggered the investigation because he tested a totp API and originally thought that there was something with the API checking the generated totp code). the qr code that prompted me to join this discussion was generated by https://github.com/piglig/go-qr and shown on a web frontend (no qrencode). other apps work. this is 100% a "google authenticator" issue. since that is not open source, i'm choosing not to work on this anymore (probing a black box is no fun). this is a "tell your customers that google authenticator doesn't work reliably" situation. |
Glad that now we are on the same page! And I did reproduce your error for the first time! 🎉
Indeed, I fed to my GoogleAuthenticator and got food poisoning as expected! 😄 Reading the QR code image that you provided; I also analyzed the QR code image from the DENSO Wave's QR reader and it returned the below URI, which is the exact same URI. Even though more digging is needed, I have a feeling that "error correction" of the QR code is causing the unstable-ness. What version of |
|
@KEINOS i was about to test whether the order of the parameters still held as reason for the failure to parse the qr code. depending on the rest of the qr code's content (issuer etc.), the secret parameter can influence the 'version' of the qr code when using qrencode. my patience was exhausted when the very same qr code image (png) which was not working earlier started working out of the blue and then, later, stopped working again. i don't want to spend days scanning a series of qr codes with the google authenticator before i know whether a qr code is good or not. one successful scan is not enough to determine whether a qr code works or not i don't know how much time your willing to invest in this. we don't know what is causing the error message. we don't even know whether the image parser or the uri parser is causing it. depending on which one it is, further testing looks very different. someone with source access can debug this issue efficiently. we can not (assuming you don't have access to the source code). |
|
I did some digging. And yes, GoogleAuthenticatior su*ks. As a conclusion, THERE IS A PARSING BUG in GoogleAuthenticator's QR Code image decoder. And we can avoid that bug by appending a parameter "&gauthsucks" in the URI. Yea, really. In bash, try: uriTailSec="otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X"
qrencode -t PNG -o ./qr_tailing_sec_A.png -l Q <<< "${uriTailSec}" # errors
qrencode -t PNG -o ./qr_tailing_sec_B.png -l Q <<< "${uriTailSec}&gauthsucks" # succeedstl;drI found that it was the other way around. It is not that the "secret" parameter should be placed at the beginning of the URI, but rather that it should not be placed at the end in some occasions. With GoogleAuthenticatior;
The solution is simple, but thanks to @yogo1212 and @Cprime50 for issueing out the problem. Because of this issue, I was able to fix the bug before any potential issues arose. Thank you very much. My customers trust Google more than I do. So using GoogleAuthenticator is a must. They don't listen to me because they think such a bug can't possibly exist in an app of a security-conscious company. My Opinion to fix this issueForce append "&google" (or something) in // EncodeQuery is a copy-paste of url.Values.Encode, except it uses %20 instead
// of + to encode spaces. This is necessary to correctly render spaces in some
// authenticator apps, like Google Authenticator.
func EncodeQuery(v url.Values) string {
if v == nil {
return ""
}
var buf strings.Builder
keys := make([]string, 0, len(v))
for k := range v {
keys = append(keys, k)
}
sort.Strings(keys)
for _, k := range keys {
vs := v[k]
keyEscaped := url.PathEscape(k) // changed from url.QueryEscape
for _, v := range vs {
if buf.Len() > 0 {
buf.WriteByte('&')
}
buf.WriteString(keyEscaped)
buf.WriteByte('=')
buf.WriteString(url.PathEscape(v)) // changed from url.QueryEscape
}
}
+ // Append extra parameter for GoogleAuthenticator parsing bug
+ // (Fix #94)
+ buf.WriteString("&google")
return buf.String()
}ts;dr
|
| Heading secret |
|---|
 |
otpauth://totp/domain.com:[email protected]?secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=domain.com&period=45 |
| Middle secret |
|---|
 |
otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=domain.com&period=45 |
| Tailing secret |
|---|
 |
otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X |
If GoogleAuthenticator restricts the order of the "secret" parameter, the last two QR codes should error, but they didn't.
In other words, the order of the parameters in the URI does not matter as long as the QR code can be read correctly. Also at this point, we can confirm that GoogleAuthenticator does support SHA-256 as well.
Go code to reproduce the above image
Note that we are not using our pquerna/otp, but the boombuler/barcode package on which it depends.
package main
import (
"image"
"image/png"
"os"
"github.com/boombuler/barcode"
"github.com/boombuler/barcode/qr"
)
func main() {
const (
// URI which begins with a secret parameter
uriHeadSec = "otpauth://totp/domain.com:[email protected]?" +
"secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=domain.com&period=45"
// URI which a secret parameter is in the middle
uriMiddleSec = "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=domain.com&period=45"
// URI which ends with a secret parameter
uriTailSec = "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X"
imgSize = 195 // 195x195 pixels
)
// Generate QR code images
const (
qr_level = qr.Q // Error correction level (choices: L, M, Q, H)
qr_mode = qr.Unicode // Not only UPPER case
)
imgHeadSec, err := customImage(uriHeadSec, qr_level, qr_mode, imgSize, imgSize)
panicOnError(err)
imgMiddleSec, err := customImage(uriMiddleSec, qr_level, qr_mode, imgSize, imgSize)
panicOnError(err)
imgTailSec, err := customImage(uriTailSec, qr_level, qr_mode, imgSize, imgSize)
panicOnError(err)
// Save the QR code to a file
panicOnError(saveImg(imgHeadSec, "qr_go_heading_sec.png"))
panicOnError(saveImg(imgMiddleSec, "qr_go_middle_sec.png"))
panicOnError(saveImg(imgTailSec, "qr_go_tailing_sec.png"))
}
// Note "mode"(encoding mode) are: qr.Auto, qr.AlphaNumeric, qr.Numeric, qr.Unicode
// But `qr.AlphaNumeric` only encodes:
//
// UPPERCASE letters, numbers and `[Space], $, %, *, +, -, ., /, :`.
//
// Therefore, we must use `qr.Auto` or `qr.Unicode` instead of `qr.AlphaNumeric` to
// encode as bytes (as is).
//
// See: https://en.wikipedia.org/wiki/QR_code#Information_capacity
func customImage(content string, level qr.ErrorCorrectionLevel, mode qr.Encoding, width int, height int) (image.Image, error) {
b, err := qr.Encode(content, level, mode)
if err != nil {
return nil, err
}
b, err = barcode.Scale(b, width, height)
if err != nil {
return nil, err
}
return b, nil
}
func panicOnError(err error) {
if err != nil {
panic(err)
}
}
func saveImg(img image.Image, filename string) error {
file, err := os.Create(filename)
if err != nil {
return err
}
defer file.Close()
if err := png.Encode(file, img); err != nil {
return err
}
return nil
}QR Code Generation
The next question is whether there are any problems with the creation of QR codes. This is also the reason why the topic is so complex.
The QR codes shown previously ware generated in Go with the package boombuler/barcode and works well.
However, if a QR code with the same URI is generated with the qrencode command, GoogleAuthenticator starts to fail reading.
The first two (heading and middle secret) succeeds reading but the last one.
| Heading secret (w/qrencode) -> SUCCEED |
|---|
 |
otpauth://totp/domain.com:[email protected]?secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=domain.com&period=45 |
| Middle secret (w/qrencode) -> SUCCEED |
|---|
 |
otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=domain.com&period=45 |
| Tailing secret (w/qrencode) -> FAILURE |
|---|
 |
otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X |
But, at this point, we know that the order of the "secret" parameters proved to be irrelevant.
This means that the problem can be narrowed down to either; a problem with the QR code generation with "qrencode" or a reading problem with GoogleAuthenticator.
So, did "qrencode" inadvertently generate the wrong QR code under niche conditions?
I'd say the answer as NO.
Because, as scanning the failing QR code (w/tailing secret param) with the below apps, all apps read the QR code fine.
- The official DENSO Wave's QR reader app from arara Inc.
- It displayed the same URI as the original
- Apple Password app via iPhone camera
- Once adding a dummy user name and password to a account, it generates the right passcode
- qrscan command
- Giving the generated QR code image, it returns the same URI as the original
This means that the QR code generated via qrencode CAN be read and decoded (not corrupted).
[!NOTE]
Above all, however, the library "libqrencode", on which "qrencode" is based, is widely used and well tested.The only concern is that the library has not been updated for more than four years and may not meet the current ISO/IEC 18004:2024 standards of the QR code. But this is not a critical point for backward compatibility per se.
Bash script to reproduce the above image
#!/bin/bash
set -e
type qrencode || { echo "qrencode not found"; exit 1; }
qrencode --version
#rm -f ./qr_*.png
# URI which begins with a secret parameter
uriHeadSec="otpauth://totp/domain.com:[email protected]?secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=domain.com&period=45"
# URI which a secret parameter is in the middle
uriMiddleSec="otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=domain.com&period=45"
# URI which ends with a secret parameter
uriTailSec="otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X"
qrencode -t PNG -o ./qr_bash_heading_sec.png -l Q <<< "$uriHeadSec"
qrencode -t PNG -o ./qr_bash_middle_sec.png -l Q <<< "$uriMiddleSec"
qrencode -t PNG -o ./qr_bash_tailing_sec.png -l Q <<< "$uriTailSec"
Error Correction
So far, the combination of URI with tailing "secret" parameter and qrencode causes the problem.
I then assumed that there might be a problem with error correction when scanning with GoogleAuthenticator.
I created QR codes with tailing secret URIs with error correction levels L, M, Q and H. But none of them could be read by GoogleAuthenticator.
This proves that scanning error is not causing the problem rather than parsing the image.
Script to reproduce
#!/bin/bash
set -e
uriTailSec="otpauth://totp/domain.com:[email protected]?algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X"
qrencode -t PNG -o ./qr_tailing_sec_L.png -l L <<< "$uriTailSec"
qrencode -t PNG -o ./qr_tailing_sec_M.png -l M <<< "$uriTailSec"
qrencode -t PNG -o ./qr_tailing_sec_Q.png -l Q <<< "$uriTailSec"
qrencode -t PNG -o ./qr_tailing_sec_H.png -l H <<< "$uriTailSec"
qrencode -t PNG -o ./qr_tailing_sec_L_2.png -l L <<< "${uriTailSec}&gauthsucks"
qrencode -t PNG -o ./qr_tailing_sec_M_2.png -l M <<< "${uriTailSec}&gauthsucks"
qrencode -t PNG -o ./qr_tailing_sec_Q_2.png -l Q <<< "${uriTailSec}&gauthsucks"
qrencode -t PNG -o ./qr_tailing_sec_H_2.png -l H <<< "${uriTailSec}&gauthsucks"
|
Simply adding an extra "&" delimiter at the end fixes the problem as well. // Append extra parameter for GoogleAuthenticator parsing bug
// (Fix #94)
- buf.WriteString("&google")
+ buf.WriteString("&") |
|
@KEINOS wow! so far, at least. i'll be checking again tomorrow because i'm not approaching this problem without a sheet of aluminium foil on my noggin. |
|
I’m glad the simple “&” managed to break our QR code (Google Authenticator) curse ! 🎉 Let’s keep the aluminium foil close, though—one can never be too careful when it comes to production mysteries. Looking forward to hearing tomorrow’s results. Thanks for giving it a shot! |
|
still works. |
|
Here's my two cents for those who want the "GoogleAuthenticator-safe URI" for an external application to generate QR code images. // Fix94GAuthBug appends an extra parameter delimiter "&" to the URI if necessary.
//
// This function provides a workaround for a known bug in Google Authenticator.
// (See issue #94: https://github.com/pquerna/otp/issues/94#issuecomment-2524954588 )
//
// The bug affects QR codes generated with certain combinations of standards
// and parameters, causing them to fail when scanned by Google Authenticator.
// Use this function if you need to generate a QR code using an external tool
// (outside this library) and require the URI to be compatible with Google Authenticator.
// It adjusts the URI to ensure proper functionality.
func Fix94GAuthBug(uri string) string {
chunks := strings.Split(uri, "&")
if !strings.HasPrefix(chunks[len(chunks)-1], "secret=") {
return uri // last param is not a secret
}
return uri + "&" // avoid GoogleAuthenticator bug
}Testsfunc TestFix94GAuthBug(t *testing.T) {
for index, test := range []struct {
title string
uriInput string
uriExpect string
}{
{
title: "heading secret",
uriInput: "otpauth://totp/domain.com:[email protected]?" +
"secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=domain.com&period=45",
uriExpect: "otpauth://totp/domain.com:[email protected]?" +
"secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&algorithm=SHA256&digits=8&issuer=domain.com&period=45",
},
{
title: "middle secret",
uriInput: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=domain.com&period=45",
uriExpect: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&issuer=domain.com&period=45",
},
{
title: "tailing secret (require '&' to be added)",
uriInput: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X",
uriExpect: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&",
},
{
title: "tailing secret (fixed w/& only)",
uriInput: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&",
uriExpect: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&",
},
{
title: "tailing secret (fixed w/single param)",
uriInput: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo",
uriExpect: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo",
},
{
title: "tailing secret (fixed w/key-value param)",
uriInput: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo=bar",
uriExpect: "otpauth://totp/domain.com:[email protected]?" +
"algorithm=SHA256&digits=8&issuer=domain.com&period=45&secret=DEOXGYTNWD3D6J3RNBEGCI2R45X3XO3X&foo=bar",
},
} {
expect := test.uriExpect
actual := Fix94GAuthBug(test.uriInput) // Apply bug fix
if expect != actual {
t.Errorf("test case #%d: %s", index+1, test.title)
}
}
}
By the way, @Cprime50, have you solved your problem? Still not working? |
|
@KEINOS would you mind turning that into a PR? |
|
@yogo1212 (CC: @Cprime50 , @pquerna ) Happy holidays! I created a PR #99 to close this issue. Since the bug itself does not rely on our package, I have not changed the function |
I am using this library to generate a QR code that I connect to Google Authenticator. However, the OTP generated by Google Authenticator doesn't get validated by the library.
version of package
go version
code:
to reproduce:
Run the code provided above.
Scan the generated QR code using Google Authenticator.
Enter the OTP code displayed in Google Authenticator into the terminal.
The text was updated successfully, but these errors were encountered: