Fixed token synchronization problem

totp.go

@@ -27,7 +27,8 @@ const (
 )
 
 var (
-	initialization_failed_error = errors.New("Totp has not been initialized correctly")
+	initializationFailedError = errors.New("Totp has not been initialized correctly")
+	LockDownError             = errors.New("The verification is locked down, because of too many trials.")
 )
 
 // WARNING: The `Totp` struct should never be instantiated manually!
@@ -128,7 +129,7 @@ func (otp *Totp) Validate(userCode string) error {
 
 	// check against the total amount of failures
 	if otp.totalVerificationFailures >= max_failures && !validBackoffTime(otp.lastVerificationTime) {
-		return errors.New("The verification is locked down, because of too many trials.")
+		return LockDownError
 	}
 
 	if otp.totalVerificationFailures >= max_failures && validBackoffTime(otp.lastVerificationTime) {
@@ -145,7 +146,7 @@ func (otp *Totp) Validate(userCode string) error {
 	token0Hash := sha256.Sum256([]byte(calculateTOTP(otp, -1)))
 	token1Hash := sha256.Sum256([]byte(calculateTOTP(otp, 0)))
 	token2Hash := sha256.Sum256([]byte(calculateTOTP(otp, 1)))
-	tokens[0] = hex.EncodeToString(token0Hash[:]) // sha256.Sum256() // 30 seconds ago token
+	tokens[0] = hex.EncodeToString(token0Hash[:]) // 30 seconds ago token
 	tokens[1] = hex.EncodeToString(token1Hash[:]) // current token
 	tokens[2] = hex.EncodeToString(token2Hash[:]) // next 30 seconds token
 
@@ -154,7 +155,7 @@ func (otp *Totp) Validate(userCode string) error {
 		return nil
 	}
 
-	// if the let's say 30 seconds ago token is valid then return nil, but re-synchronize
+	// if the 30 seconds ago token is valid then return nil, but re-synchronize
 	if tokens[0] == userToken {
 		otp.synchronizeCounter(-1)
 		return nil
@@ -191,8 +192,7 @@ func validBackoffTime(lastVerification time.Time) bool {
 func (otp *Totp) incrementCounter(index int) {
 	// Unix returns t as a Unix time, the number of seconds elapsed since January 1, 1970 UTC.
 	counterOffset := time.Duration(index*otp.stepSize) * time.Second
-	clientOffset := time.Duration(otp.clientOffset*otp.stepSize) * time.Second
+	now := time.Now().UTC().Add(counterOffset).Unix()
-	now := time.Now().UTC().Add(counterOffset).Add(clientOffset).Unix()
 	otp.counter = bigEndianUint64(increment(now, otp.stepSize))
 }
 
@@ -572,7 +572,7 @@ func TOTPFromBytes(data []byte) (*Totp, error) {
 // this method checks the proper initialization of the Totp object
 func totpHasBeenInitialized(otp *Totp) error {
 	if otp.key == nil || len(otp.key) == 0 {
-		return initialization_failed_error
+		return initializationFailedError
 	}
 	return nil
 }

totp_test.go

@@ -137,8 +137,8 @@ func TestVerificationFailures(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// verify the wrong token for 3 times and check the internal counters values
+	// verify the wrong token for 10 times and check the internal counters values
-	for i := 0; i < 3; i++ {
+	for i := 0; i < 10; i++ {
 		if err := otp.Validate("1234567"); err == nil {
 			t.Fatal(err)
 		}
@@ -155,11 +155,37 @@ func TestVerificationFailures(t *testing.T) {
 		}
 	}
 
-	// set the lastVerificationTime ahead in the future.
+	// test the validBackoffTime function
+	if validBackoffTime(otp.lastVerificationTime) {
+		t.Error("validBackoffTime should return false")
+	}
+
+	// serialize and deserialize the object and verify again
+	data, err := otp.ToBytes()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	restoredOtp, err := TOTPFromBytes(data)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// test the validBackoffTime function
+	if validBackoffTime(restoredOtp.lastVerificationTime) {
+		t.Error("validBackoffTime should return false")
+	}
+
+	// set the lastVerificationTime back in the past.
 	// it should at this point pass
 	back10Minutes := time.Duration(-10) * time.Minute
 	otp.lastVerificationTime = time.Now().UTC().Add(back10Minutes)
 
+	// test the validBackoffTime function
+	if !validBackoffTime(otp.lastVerificationTime) {
+		t.Error("validBackoffTime should return true")
+	}
+
 	for i := 0; i < 10; i++ {
 		if err := otp.Validate(expectedToken); err != nil {
 			t.Fatal(err)
@@ -313,3 +339,55 @@ func TestProperInitialization(t *testing.T) {
 		t.Fatal("Totp is not properly initialized and the method did not catch it")
 	}
 }
+
+func TestCounterSynchronization(t *testing.T) {
+
+	// create totp
+	otp, err := NewTOTP("info@sec51.com", "Sec51", crypto.SHA512, 8)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	token0 := calculateTOTP(otp, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	token_1 := calculateTOTP(otp, -1)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	token1 := calculateTOTP(otp, 1)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = otp.Validate(token0)
+	if err != nil {
+		t.Error(err)
+	}
+	// check the values
+	if otp.clientOffset != 0 {
+		t.Errorf("Client offset should be 0, instead we've got %d\n", otp.clientOffset)
+	}
+
+	err = otp.Validate(token_1)
+	if err != nil {
+		t.Error(err)
+	}
+	// check the values
+	if otp.clientOffset != -1 {
+		t.Errorf("Client offset should be -1, instead we've got %d\n", otp.clientOffset)
+	}
+
+	err = otp.Validate(token1)
+	if err != nil {
+		t.Error(err)
+	}
+	// check the values
+	if otp.clientOffset != 1 {
+		t.Errorf("Client offset should be 0, instead we've got %d\n", otp.clientOffset)
+	}
+
+}