ITS#9437 Implement TOTP drift correction

diff --git a/doc/man/man5/slapo-otp_2fa.5 b/doc/man/man5/slapo-otp_2fa.5
index 205cd19a2496b811847f1c36b724d8942949a2b9..ea49f3d4f3cb343c90d8636d03bd0f1ec4a7b99c 100644 (file)
--- a/doc/man/man5/slapo-otp_2fa.5
+++ b/doc/man/man5/slapo-otp_2fa.5
@@ -119,9 +119,14 @@ The length of the time-step period for TOTP calculation.
 .B oathTOTPLastTimeStep: <number>
 The order of the last TOTP token successfully redeemed by the user.
 .TP
-.B oathTOTPGrace: <number>
+.B oathTOTPTimeStepWindow: <number>
 The number of time periods around the current time to try when checking the
 password provided by the user.
+.TP
+.B oathTOTPTimeStepDrift: <number>
+If the client didn't provide the correct token but it still fit with
+oathTOTPTimeStepWindow above, this attribute records the current offset to
+provide for slow clock drift of the client device.
 .RE
 
 .SH "SEE ALSO"

diff --git a/servers/slapd/overlays/otp_2fa.c b/servers/slapd/overlays/otp_2fa.c
index e5117800ff1c29ef7cdb4dcaf52191d7b1d026a0..4aaca990044a32bce8ea2e97b7acc33fdea82984 100644 (file)
--- a/servers/slapd/overlays/otp_2fa.c
+++ b/servers/slapd/overlays/otp_2fa.c
@@ -130,6 +130,7 @@ AttributeDescription *ad_oathTOTPParams;
 AttributeDescription *ad_oathTOTPToken;
 AttributeDescription *ad_oathTOTPLastTimeStep;
 AttributeDescription *ad_oathTOTPTimeStepWindow;
+AttributeDescription *ad_oathTOTPTimeStepDrift;
 
 static struct otp_at {
        char                                    *schema;
@@ -327,7 +328,8 @@ static struct otp_at {
         "DESC 'OATH-LDAP: Last observed time step shift seen for TOTP' "
         "X-ORIGIN 'OATH-LDAP' "
         "SINGLE-VALUE "
-        "SUP oathCounter )" },
+        "SUP oathCounter )",
+               &ad_oathTOTPTimeStepDrift },
 
        { "( oath-ldap-at:11 "
         "NAME 'oathSecretLength' "
@@ -452,7 +454,7 @@ static struct otp_oc {
                "AUXILIARY "
                "SUP oathParams "
                "MUST ( oathTOTPTimeStepPeriod ) "
-               "MAY ( oathTOTPTimeStepWindow $ oathTOTPTimeStepDrift ) )",
+               "MAY ( oathTOTPTimeStepWindow ) )",
                &oc_oathTOTPParams },
        { "( oath-ldap-oc:3 "
                "NAME 'oathToken' "
@@ -476,7 +478,7 @@ static struct otp_oc {
                "X-ORIGIN 'OATH-LDAP' "
                "AUXILIARY "
                "SUP oathToken "
-               "MAY ( oathTOTPParams $ oathTOTPLastTimeStep ) )",
+               "MAY ( oathTOTPParams $ oathTOTPLastTimeStep $ oathTOTPTimeStepDrift ) )",
                &oc_oathTOTPToken },
        { NULL }
 };
@@ -672,13 +674,14 @@ done:
 }
 
 static long
-otp_totp( Operation *op, Entry *token )
+otp_totp( Operation *op, Entry *token, long *drift )
 {
+       char outbuf[MAX_DIGITS + 1];
        Entry *params = NULL;
        Attribute *a;
        BerValue *secret, client_otp;
        const void *mech;
-       long t, last_step = -1, found = -1, window = 0;
+       long t, last_step = -1, found = -1, window = 0, old_drift;
        int i, otp_len, time_step;
 
        a = attr_find( token->e_attrs, ad_oathSecret );
@@ -706,7 +709,6 @@ otp_totp( Operation *op, Entry *token )
                                a->a_vals[0].bv_val );
                goto done;
        }
-       t = op->o_time / time_step;
 
        a = attr_find( params->e_attrs, ad_oathTOTPTimeStepWindow );
        if ( a && lutil_atol( &window, a->a_vals[0].bv_val ) != 0 ) {
@@ -716,6 +718,16 @@ otp_totp( Operation *op, Entry *token )
                goto done;
        }
 
+       a = attr_find( params->e_attrs, ad_oathTOTPTimeStepDrift );
+       if ( a && lutil_atol( drift, a->a_vals[0].bv_val ) != 0 ) {
+               Debug( LDAP_DEBUG_ANY, "otp_totp: "
+                               "could not parse oathTOTPTimeStepDrift value %s\n",
+                               a->a_vals[0].bv_val );
+               goto done;
+       }
+       old_drift = *drift;
+       t = op->o_time / time_step + *drift;
+
        a = attr_find( params->e_attrs, ad_oathOTPLength );
        if ( lutil_atoi( &otp_len, a->a_vals[0].bv_val ) != 0 ) {
                Debug( LDAP_DEBUG_ANY, "otp_totp: "
@@ -740,38 +752,32 @@ otp_totp( Operation *op, Entry *token )
        client_otp.bv_val = op->orb_cred.bv_val + op->orb_cred.bv_len - otp_len;
 
        /* If check succeeds, advance the step counter accordingly */
-       for ( i = -window; i <= window; i++ ) {
-               char outbuf[MAX_DIGITS + 1];
+       /* Negation of A001057 series that enumerates all integers:
+        * (0, -1, 1, -2, 2, ...) */
+       for ( i = 0; i >= -window; i = ( i < 0 ) ? -i : ~i ) {
                BerValue out = { .bv_val = outbuf, .bv_len = sizeof(outbuf) };
 
-               if ( t + i < 0 ) continue;
+               if ( t + i <= last_step ) continue;
 
                generate( secret, t + i, otp_len, &out, mech );
                if ( !ber_bvcmp( &out, &client_otp ) ) {
                        found = t + i;
+                       *drift = old_drift + i;
+                       /* Would we leak information if we stopped right now? */
                }
        }
 
+       /* OTP check passed, trim the password */
        if ( found >= 0 ) {
-               int offset = found - t;
-
-               if ( found <= last_step ) {
-                       /* Token re-used, refuse */
-                       found = -1;
-                       Debug( LDAP_DEBUG_TRACE, "%s client tried to reuse old TOTP token %s, offset %d\n",
-                                       op->o_log_prefix, token->e_name.bv_val, offset );
-               } else {
-                       /* OTP check passed, trim the password */
-                       op->orb_cred.bv_len -= otp_len;
-                       Debug( LDAP_DEBUG_TRACE, "%s TOTP token %s redeemed with offset %d\n",
-                                       op->o_log_prefix, token->e_name.bv_val, offset );
-               }
-       } else {
-               Debug( LDAP_DEBUG_TRACE, "%s TOTP token was not valid\n",
-                               op->o_log_prefix );
+               assert( found > last_step );
+
+               op->orb_cred.bv_len -= otp_len;
+               Debug( LDAP_DEBUG_TRACE, "%s TOTP token %s redeemed with new drift of %ld\n",
+                               op->o_log_prefix, token->e_name.bv_val, *drift );
        }
 
 done:
+       memset( outbuf, 0, sizeof(outbuf) );
        if ( params ) {
                be_entry_release_r( op, params );
        }
@@ -784,9 +790,9 @@ otp_op_bind( Operation *op, SlapReply *rs )
        slap_overinst *on = (slap_overinst *)op->o_bd->bd_info;
        BerValue totpdn = BER_BVNULL, hotpdn = BER_BVNULL, ndn;
        Entry *user = NULL, *token = NULL;
-       AttributeDescription *ad = NULL;
+       AttributeDescription *ad = NULL, *drift_ad = NULL;
        Attribute *a;
-       long t = -1;
+       long t = -1, drift = 0;
        int rc = SLAP_CB_CONTINUE;
 
        if ( op->oq_bind.rb_method != LDAP_AUTH_SIMPLE ) {
@@ -818,7 +824,8 @@ otp_op_bind( Operation *op, SlapReply *rs )
                                        &token ) == LDAP_SUCCESS ) {
                ndn = totpdn;
                ad = ad_oathTOTPLastTimeStep;
-               t = otp_totp( op, token );
+               drift_ad = ad_oathTOTPTimeStepDrift;
+               t = otp_totp( op, token, &drift );
                be_entry_release_r( op, token );
                token = NULL;
        }
@@ -832,27 +839,44 @@ otp_op_bind( Operation *op, SlapReply *rs )
                token = NULL;
        }
 
-       /* If check succeeds, advance the step counter accordingly */
+       /* If check succeeds, advance the step counter and drift accordingly */
        if ( t >= 0 ) {
-               char outbuf[32];
+               char outbuf[32], drift_buf[32];
                Operation op2;
                Opheader oh;
-               Modifications mod;
+               Modifications mod[2], *m = mod;
                SlapReply rs2 = { REP_RESULT };
                slap_callback cb = { .sc_response = &slap_null_cb };
-               BerValue bv[2];
+               BerValue bv[2], bv_drift[2];
 
                bv[0].bv_val = outbuf;
                bv[0].bv_len = snprintf( bv[0].bv_val, sizeof(outbuf), "%ld", t );
                BER_BVZERO( &bv[1] );
 
-               mod.sml_numvals = 1;
-               mod.sml_values = bv;
-               mod.sml_nvalues = NULL;
-               mod.sml_desc = ad;
-               mod.sml_op = LDAP_MOD_REPLACE;
-               mod.sml_flags = SLAP_MOD_INTERNAL;
-               mod.sml_next = NULL;
+               m->sml_numvals = 1;
+               m->sml_values = bv;
+               m->sml_nvalues = NULL;
+               m->sml_desc = ad;
+               m->sml_op = LDAP_MOD_REPLACE;
+               m->sml_flags = SLAP_MOD_INTERNAL;
+
+               if ( drift_ad ) {
+                       m->sml_next = &mod[1];
+
+                       bv_drift[0].bv_val = drift_buf;
+                       bv_drift[0].bv_len = snprintf(
+                                       bv_drift[0].bv_val, sizeof(drift_buf), "%ld", drift );
+                       BER_BVZERO( &bv_drift[1] );
+
+                       m++;
+                       m->sml_numvals = 1;
+                       m->sml_values = bv_drift;
+                       m->sml_nvalues = NULL;
+                       m->sml_desc = drift_ad;
+                       m->sml_op = LDAP_MOD_REPLACE;
+                       m->sml_flags = SLAP_MOD_INTERNAL;
+               }
+               m->sml_next = NULL;
 
                op2 = *op;
                oh = *op->o_hdr;
@@ -861,7 +885,7 @@ otp_op_bind( Operation *op, SlapReply *rs )
                op2.o_callback = &cb;
 
                op2.o_tag = LDAP_REQ_MODIFY;
-               op2.orm_modlist = &mod;
+               op2.orm_modlist = mod;
                op2.o_dn = op->o_bd->be_rootdn;
                op2.o_ndn = op->o_bd->be_rootndn;
                op2.o_req_dn = ndn;