From 5898f9c3903b0e8a2e4d9c9035a01fb5f626a61b Mon Sep 17 00:00:00 2001
From: Thomas Darimont <thomas.darimont@googlemail.com>
Date: Sun, 22 Aug 2021 22:31:04 +0200
Subject: [PATCH] KEYCLOAK-18880 TimeBasedOTP should use look-around to
 mitigate clock skew

Previously the TimeBasedOTP only looked behind to mitigate clock skew.
We now look around (look ahead + look behind) to better accommodate clock skew.
---
 .../keycloak/models/utils/TimeBasedOTP.java   | 28 +++++++++++++------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/TimeBasedOTP.java b/server-spi-private/src/main/java/org/keycloak/models/utils/TimeBasedOTP.java
index 0e29a26675..497cb85e6f 100755
--- a/server-spi-private/src/main/java/org/keycloak/models/utils/TimeBasedOTP.java
+++ b/server-spi-private/src/main/java/org/keycloak/models/utils/TimeBasedOTP.java
@@ -42,10 +42,10 @@ public class TimeBasedOTP extends HmacOTP {
      * @param algorithm the encryption algorithm
      * @param numberDigits the number of digits for tokens
      * @param timeIntervalInSeconds the number of seconds a token is valid
-     * @param lookAheadWindow the number of previous intervals that should be used to validate tokens.
+     * @param lookAroundWindow the number of previous and following intervals that should be used to validate tokens.
      */
-    public TimeBasedOTP(String algorithm, int numberDigits, int timeIntervalInSeconds, int lookAheadWindow) {
-        super(numberDigits, algorithm, lookAheadWindow);
+    public TimeBasedOTP(String algorithm, int numberDigits, int timeIntervalInSeconds, int lookAroundWindow) {
+        super(numberDigits, algorithm, lookAroundWindow);
         this.clock = new Clock(timeIntervalInSeconds);
     }
 
@@ -60,8 +60,9 @@ public class TimeBasedOTP extends HmacOTP {
         String steps = Long.toHexString(T).toUpperCase();
 
         // Just get a 16 digit string
-        while (steps.length() < 16)
+        while (steps.length() < 16) {
             steps = "0" + steps;
+        }
 
         return generateOTP(secretKey, steps, this.numberDigits, this.algorithm);
     }
@@ -71,17 +72,21 @@ public class TimeBasedOTP extends HmacOTP {
      *
      * @param token  OTP string to validate
      * @param secret Shared secret
-     * @return
+     * @return true of the token is valid
      */
     public boolean validateTOTP(String token, byte[] secret) {
         long currentInterval = this.clock.getCurrentInterval();
 
-        for (int i = this.lookAheadWindow; i >= 0; --i) {
-            String steps = Long.toHexString(currentInterval - i).toUpperCase();
+        for (int i = 0; i <= (lookAheadWindow * 2); i++) {
+            long delta = clockSkewIndexToDelta(i);
+            long adjustedInterval = currentInterval + delta;
+
+            String steps = Long.toHexString(adjustedInterval).toUpperCase();
 
             // Just get a 16 digit string
-            while (steps.length() < 16)
+            while (steps.length() < 16) {
                 steps = "0" + steps;
+            }
 
             String candidate = generateOTP(new String(secret), steps, this.numberDigits, this.algorithm);
 
@@ -93,6 +98,13 @@ public class TimeBasedOTP extends HmacOTP {
         return false;
     }
 
+    /**
+     * maps 0, 1, 2, 3, 4, 5, 6, 7, ... to 0, -1, 1, -2, 2, -3, 3, ...
+     */
+    public static long clockSkewIndexToDelta(int idx) {
+        return (idx + 1) / 2 * (1 - (idx % 2) * 2);
+    }
+
     public void setCalendar(Calendar calendar) {
         this.clock.setCalendar(calendar);
     }