Hello,

I would like to submit some patchs I made for oath-toolkit.
I am not good in english so there may(/must) be some spelling in my comments.
If you have any question on those patchs don't hesitate to ask.

0001-usersfile-rewrite
======================
I rewrite liboath/usersfile.c to lock and modify usersfile inplace
instead of creating lock and temporary file.

This patch is related to the problems already described in previous mails and 
bugs :
* pam-oath, private usersfiles (feature request)
* issue (bug?) in update_usersfile
* RFE: Configurable lock file location (for SELinux compatiblity) :
    https://savannah.nongnu.org/support/?108723

I first did those patchs because I want to use pam_oath with postgres
(which doesn't run as root).
So I need the usersfile to have 660 permission, owner "root" and group
"oath" (postgres is a member of oath).

I take advantage of the code hacking to make some other changes (patchs 0002 & 
0003).

0002-different-usersfile-field-5-if-HOTP-TOTP
=============================================

As it is mansion in the userfile google specification, field 5 is different if 
the line is related to HOTP or TOTP.
https://code.google.com/p/mod-authn-otp/wiki/UsersFile

Currently that's not the case. This patch correct this issue and use the 5th 
field value to improve the TOTP replay verification.

0003-usersfile-fields-5-present-6-and-7-mandatory
=================================================

This patch make the userfile 6th and 7th fields mandatory if the 5th field is 
present.
That's simplified the code and make things more understandable (from my point 
of view :) ).

-- 
Regards
Maxime de Roucy
From eda645f7e0af2dcb02a13e1a1ce6c56049c30b83 Mon Sep 17 00:00:00 2001
From: Maxime de Roucy <[email protected]>
Date: Sun, 4 Jan 2015 12:47:03 +0100
Subject: [PATCH 1/3] usersfile : rewrite

modify the usersfile inplace to keep owner and permissions
---
 NEWS                          |   3 +
 liboath/errors.c              |  54 ++--
 liboath/oath.h.in             |  71 +++--
 liboath/tests/tst_usersfile.c |   2 +-
 liboath/totp.c                |   6 +-
 liboath/usersfile.c           | 701 ++++++++++++++++++++++++------------------
 pam_oath/README               |   2 +-
 7 files changed, 481 insertions(+), 358 deletions(-)

diff --git a/NEWS b/NEWS
index 559d9d8..d84f8bf 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,9 @@ Copyright (C) 2009-2014 Simon Josefsson.  Licensed under the GPLv3+.
 
 * Version 2.6.0 (unreleased)
 
+** liboath: usersfile is now updated inplace, so there is no owner/permission
+modifications.
+
 ** liboath: Support TOTP with HMAC-SHA256 and HMAC-SHA512.
 This adds new APIs oath_totp_generate2, oath_totp_validate4 and
 oath_totp_validate4_callback.
diff --git a/liboath/errors.c b/liboath/errors.c
index 87292e1..1257bad 100644
--- a/liboath/errors.c
+++ b/liboath/errors.c
@@ -31,32 +31,34 @@ typedef struct
 } err_t;
 
 static const err_t errors[] = {
-  ERR (OATH_OK, "Successful return"),
-  ERR (OATH_CRYPTO_ERROR, "Internal error in crypto functions"),
-  ERR (OATH_INVALID_DIGITS, "Unsupported number of OTP digits"),
-  ERR (OATH_PRINTF_ERROR, "Error from system printf call"),
-  ERR (OATH_INVALID_HEX, "Hex string is invalid"),
-  ERR (OATH_TOO_SMALL_BUFFER, "The output buffer is too small"),
-  ERR (OATH_INVALID_OTP, "The OTP is not valid"),
-  ERR (OATH_REPLAYED_OTP, "The OTP has been replayed"),
-  ERR (OATH_BAD_PASSWORD, "The password does not match"),
-  ERR (OATH_INVALID_COUNTER, "The counter value is corrupt"),
-  ERR (OATH_INVALID_TIMESTAMP, "The timestamp is corrupt"),
-  ERR (OATH_NO_SUCH_FILE, "The supplied filename does not exist"),
-  ERR (OATH_UNKNOWN_USER, "Cannot find information about user"),
-  ERR (OATH_FILE_SEEK_ERROR, "System error when seeking in file"),
-  ERR (OATH_FILE_CREATE_ERROR, "System error when creating file"),
-  ERR (OATH_FILE_LOCK_ERROR, "System error when locking file"),
-  ERR (OATH_FILE_RENAME_ERROR, "System error when renaming file"),
-  ERR (OATH_FILE_UNLINK_ERROR, "System error when removing file"),
-  ERR (OATH_TIME_ERROR, "System error for time manipulation"),
-  ERR (OATH_STRCMP_ERROR, "A strcmp callback returned an error"),
-  ERR (OATH_INVALID_BASE32, "Base32 string is invalid"),
-  ERR (OATH_BASE32_OVERFLOW, "Base32 encoding would overflow"),
-  ERR (OATH_MALLOC_ERROR, "Memory allocation failed"),
-  ERR (OATH_FILE_FLUSH_ERROR, "System error when flushing file buffer"),
-  ERR (OATH_FILE_SYNC_ERROR, "System error when syncing file to disk"),
-  ERR (OATH_FILE_CLOSE_ERROR, "System error when closing file")
+  ERR (OATH_OK, " Successful return"),
+  ERR (OATH_CRYPTO_ERROR, " Internal error in crypto functions"),
+  ERR (OATH_INVALID_DIGITS, " Unsupported number of OTP digits"),
+  ERR (OATH_INVALID_HEX, " Hex string is invalid"),
+  ERR (OATH_TOO_SMALL_BUFFER, " The output buffer is too small"),
+  ERR (OATH_INVALID_OTP, " The OTP is not valid"),
+  ERR (OATH_REPLAYED_OTP, " The OTP has been replayed"),
+  ERR (OATH_BAD_PASSWORD, " The password does not match"),
+  ERR (OATH_INVALID_COUNTER, " The counter value is corrupt"),
+  ERR (OATH_INVALID_TIMESTAMP, " The timestamp is corrupt"),
+  ERR (OATH_UNKNOWN_USER, " Cannot find information about user"),
+  ERR (OATH_WRONG_TOKEN_TYPE, " Bad formated token type"),
+  ERR (OATH_NO_USERNAME, " Can't read username from file"),
+  ERR (OATH_NO_PASSWORD, " Can't read password from file"),
+  ERR (OATH_NO_SECRET, " Can't read secret key from file"),
+  ERR (OATH_PRINTF_ERROR, " Error from system *printf call"),
+  ERR (OATH_FILE_OPEN_ERROR, " System error when opening file"),
+  ERR (OATH_FILE_READ_ERROR, " System error when reading in file"),
+  ERR (OATH_FILE_WRITE_ERROR, " System error when writting in file"),
+  ERR (OATH_FILE_TELL_ERROR, " Error from system ftell call"),
+  ERR (OATH_FILE_SEEK_ERROR, " System error when seeking in file"),
+  ERR (OATH_FILE_TRUNCATE_ERROR, " System error when truncating file"),
+  ERR (OATH_FILE_LOCK_ERROR, " System error when locking file"),
+  ERR (OATH_TIME_ERROR, " System error for time manipulation"),
+  ERR (OATH_STRCMP_ERROR, " A strcmp callback returned an error"),
+  ERR (OATH_INVALID_BASE32, " Base32 string is invalid"),
+  ERR (OATH_BASE32_OVERFLOW, " Base32 encoding would overflow"),
+  ERR (OATH_MALLOC_ERROR, " Memory allocation failed")
 };
 
 /**
diff --git a/liboath/oath.h.in b/liboath/oath.h.in
index 173592a..a6bb8d1 100644
--- a/liboath/oath.h.in
+++ b/liboath/oath.h.in
@@ -68,7 +68,6 @@ extern "C"
  * @OATH_OK: Successful return
  * @OATH_CRYPTO_ERROR: Internal error in crypto functions
  * @OATH_INVALID_DIGITS: Unsupported number of OTP digits
- * @OATH_PRINTF_ERROR: Error from system printf call
  * @OATH_INVALID_HEX: Hex string is invalid
  * @OATH_TOO_SMALL_BUFFER: The output buffer is too small
  * @OATH_INVALID_OTP: The OTP is not valid
@@ -76,22 +75,26 @@ extern "C"
  * @OATH_BAD_PASSWORD: The password does not match
  * @OATH_INVALID_COUNTER: The counter value is corrupt
  * @OATH_INVALID_TIMESTAMP: The timestamp is corrupt
- * @OATH_NO_SUCH_FILE: The supplied filename does not exist
  * @OATH_UNKNOWN_USER: Cannot find information about user
+ * @OATH_WRONG_TOKEN_TYPE: Bad formated token type
+ * @OATH_NO_USERNAME: Can't read username from file
+ * @OATH_NO_PASSWORD: Can't read password from file
+ * @OATH_NO_SECRET: Can't read secret key from file
+ * @OATH_PRINTF_ERROR: Error from system *printf call
+ * @OATH_FILE_OPEN_ERROR: System error when opening file
+ * @OATH_FILE_READ_ERROR: System error when reading in file
+ * @OATH_FILE_WRITE_ERROR: System error when writting in file
+ * @OATH_FILE_TELL_ERROR: Error from system ftell call
  * @OATH_FILE_SEEK_ERROR: System error when seeking in file
- * @OATH_FILE_CREATE_ERROR: System error when creating file
+ * @OATH_FILE_TRUNCATE_ERROR: System error when truncating file
  * @OATH_FILE_LOCK_ERROR: System error when locking file
- * @OATH_FILE_RENAME_ERROR: System error when renaming file
- * @OATH_FILE_UNLINK_ERROR: System error when removing file
  * @OATH_TIME_ERROR: System error for time manipulation
  * @OATH_STRCMP_ERROR: A strcmp callback returned an error
  * @OATH_INVALID_BASE32: Base32 string is invalid
  * @OATH_BASE32_OVERFLOW: Base32 encoding would overflow
  * @OATH_MALLOC_ERROR: Memory allocation failed
- * @OATH_FILE_FLUSH_ERROR: System error when flushing file buffer
- * @OATH_FILE_SYNC_ERROR: System error when syncing file to disk
- * @OATH_FILE_CLOSE_ERROR: System error when closing file
  * @OATH_LAST_ERROR: Meta-error indicating the last error code, for use
+ *
  *   when iterating over all error codes or similar.
  *
  * Return codes for OATH functions.  All return codes are negative
@@ -106,32 +109,34 @@ typedef enum
   OATH_OK = 0,
   OATH_CRYPTO_ERROR = -1,
   OATH_INVALID_DIGITS = -2,
-  OATH_PRINTF_ERROR = -3,
-  OATH_INVALID_HEX = -4,
-  OATH_TOO_SMALL_BUFFER = -5,
-  OATH_INVALID_OTP = -6,
-  OATH_REPLAYED_OTP = -7,
-  OATH_BAD_PASSWORD = -8,
-  OATH_INVALID_COUNTER = -9,
-  OATH_INVALID_TIMESTAMP = -10,
-  OATH_NO_SUCH_FILE = -11,
-  OATH_UNKNOWN_USER = -12,
-  OATH_FILE_SEEK_ERROR = -13,
-  OATH_FILE_CREATE_ERROR = -14,
-  OATH_FILE_LOCK_ERROR = -15,
-  OATH_FILE_RENAME_ERROR = -16,
-  OATH_FILE_UNLINK_ERROR = -17,
-  OATH_TIME_ERROR = -18,
-  OATH_STRCMP_ERROR = -19,
-  OATH_INVALID_BASE32 = -20,
-  OATH_BASE32_OVERFLOW = -21,
-  OATH_MALLOC_ERROR = -22,
-  OATH_FILE_FLUSH_ERROR = -23,
-  OATH_FILE_SYNC_ERROR = -24,
-  OATH_FILE_CLOSE_ERROR = -25,
+  OATH_INVALID_HEX = -3,
+  OATH_TOO_SMALL_BUFFER = -4,
+  OATH_INVALID_OTP = -5,
+  OATH_REPLAYED_OTP = -6,
+  OATH_BAD_PASSWORD = -7,
+  OATH_INVALID_COUNTER = -8,
+  OATH_INVALID_TIMESTAMP = -9,
+  OATH_UNKNOWN_USER = -10,
+  OATH_WRONG_TOKEN_TYPE = -11,
+  OATH_NO_USERNAME = -12,
+  OATH_NO_PASSWORD = -13,
+  OATH_NO_SECRET = -14,
+  OATH_PRINTF_ERROR = -15,
+  OATH_FILE_OPEN_ERROR = -16,
+  OATH_FILE_READ_ERROR = -17,
+  OATH_FILE_WRITE_ERROR = -18,
+  OATH_FILE_TELL_ERROR = -19,
+  OATH_FILE_SEEK_ERROR = -20,
+  OATH_FILE_TRUNCATE_ERROR = -21,
+  OATH_FILE_LOCK_ERROR = -22,
+  OATH_TIME_ERROR = -23,
+  OATH_STRCMP_ERROR = -24,
+  OATH_INVALID_BASE32 = -25,
+  OATH_BASE32_OVERFLOW = -26,
+  OATH_MALLOC_ERROR = -27,
   /* When adding anything here, update OATH_LAST_ERROR, errors.c
-     and tests/tst_errors.c. */
-  OATH_LAST_ERROR = -25
+   * and tests/tst_errors.c. */
+  OATH_LAST_ERROR = -27
 } oath_rc;
 
 /* Global */
diff --git a/liboath/tests/tst_usersfile.c b/liboath/tests/tst_usersfile.c
index b018707..615e376 100644
--- a/liboath/tests/tst_usersfile.c
+++ b/liboath/tests/tst_usersfile.c
@@ -53,7 +53,7 @@ main (void)
 
   rc = oath_authenticate_usersfile ("no-such-file", "joe", "755224",
 				    0, "1234", &last_otp);
-  if (rc != OATH_NO_SUCH_FILE)
+  if (rc != OATH_FILE_OPEN_ERROR)
     {
       printf ("oath_authenticate_usersfile[1]: %s (%d)\n",
 	      oath_strerror_name (rc), rc);
diff --git a/liboath/totp.c b/liboath/totp.c
index 0d796ae..2b3be04 100644
--- a/liboath/totp.c
+++ b/liboath/totp.c
@@ -162,8 +162,10 @@ oath_totp_validate (const char *secret,
 		    unsigned time_step_size,
 		    time_t start_offset, size_t window, const char *otp)
 {
-  return oath_totp_validate3 (secret, secret_length, now, time_step_size,
-			      start_offset, window, NULL, NULL, otp);
+  return oath_totp_validate4_callback (secret, secret_length, now,
+				       time_step_size, start_offset,
+				       strlen (otp), window, NULL, NULL, 0,
+				       _oath_strcmp_callback, (void *) otp);
 }
 
 /**
diff --git a/liboath/usersfile.c b/liboath/usersfile.c
index 8dfeffa..816053d 100644
--- a/liboath/usersfile.c
+++ b/liboath/usersfile.c
@@ -31,8 +31,25 @@
 #include <errno.h>		/* For errno. */
 #include <sys/stat.h>		/* For S_IRUSR, S_IWUSR. */
 
+#define IF_ERROR_GOTO(test, rc_value, goto_label) if (test) { rc = rc_value; goto goto_label; }
+
+static const char *whitespace = " \t\r\n";
+#define TIME_FORMAT_STRING "%Y-%m-%dT%H:%M:%SL"
+#define TIME_BUFFER_SIZE 30
+#define BUFFER_SIZE 1024
+
+/*
+ * parse_usersfile :
+ * @str: string with token type to parse
+ * @digits: output variable holding the length of OTP (6, 7 or 8)
+ * @totpstepsize: output variable holding the interval (in second) of TOTP
+ *
+ * internal fonction
+ *
+ * Returns: 0 on success, -1 on error
+ **/
 static int
-parse_type (const char *str, unsigned *digits, unsigned *totpstepsize)
+parse_type (const char *str, unsigned int *digits, unsigned int *totpstepsize)
 {
   *totpstepsize = 0;
   if (strcmp (str, "HOTP/E/6") == 0
@@ -72,59 +89,127 @@ parse_type (const char *str, unsigned *digits, unsigned *totpstepsize)
   return 0;
 }
 
-static const char *whitespace = " \t\r\n";
-#define TIME_FORMAT_STRING "%Y-%m-%dT%H:%M:%SL"
+/* compute the timestamp */
+static int
+compute_timestamp (char timestamp[])
+{
+  struct tm now;
+  time_t t;
+  size_t l;
+
+  if (time (&t) == (time_t) - 1)
+    return OATH_TIME_ERROR;
+
+  if (localtime_r (&t, &now) == NULL)
+    return OATH_TIME_ERROR;
+
+  l = strftime (timestamp, TIME_BUFFER_SIZE, TIME_FORMAT_STRING, &now);
+  if (l != 20)
+    return OATH_TIME_ERROR;
+
+  return OATH_OK;
+}
+
+
+/*
+ * parse_usersfile :
+ * @username: string with name of user
+ * @otp: string with one-time password to authenticate
+ * @window: how many past/future OTPs to search
+ * @passwd: string with password, or NULL if password checking is disabled
+ * @usersfile_fd: input file descriptor for usersfile
+ * @last_otp_timestamp: output variable holding last successful authentication timestamp
+ * @old_log_start: output variable holding the file position of the beginning of the "log" of the validation line
+ * @old_log_end: output variable holding the file position of the end of the "log" of the validation line
+ *
+ * internal fonction
+ *
+ * by "log" (old_log_start/end) I mean the end of line that should contain a moving_factor, an otp and a timestamp.
+ *
+ * Returns: %OATH_OK on success, negative value on error (see oath.h)
+ **/
 
 static int
 parse_usersfile (const char *username,
 		 const char *otp,
-		 size_t window,
+		 const size_t window,
 		 const char *passwd,
-		 time_t * last_otp,
-		 FILE * infh,
-		 char **lineptr, size_t * n, uint64_t * new_moving_factor,
-		 size_t * skipped_users)
+		 FILE * usersfile_fd,
+		 time_t * last_otp_timestamp,
+		 off_t * old_log_start,
+		 off_t * old_log_end, char *new_log_buffer)
 {
-  int bad_password = 0;
+  int rc = OATH_OK;
+
+  // to record the fact we read a line matching the username
+  int matching_user_line = 0;
+  // to record the fact we read a line matching the username and the passwd
+  int matching_user_and_passwd_line = 0;
 
-  *skipped_users = 0;
+  char *line_buffer = NULL;
+  size_t line_buffer_size = 0;
+  ssize_t line_size = 0;
 
-  while (getline (lineptr, n, infh) != -1)
+  // for each line of userfile
+  while ((line_size =
+	  getline (&line_buffer, &line_buffer_size, usersfile_fd)) != -1)
     {
+      // used internally by strtok_r in order to maintain context
+      // between successive calls that parse the same string.
       char *saveptr;
-      char *p = strtok_r (*lineptr, whitespace, &saveptr);
-      unsigned digits, totpstepsize;
-      char secret[32];
-      size_t secret_length = sizeof (secret);
-      uint64_t start_moving_factor = 0;
-      int rc = 0;
-      char *prev_otp = NULL;
 
-      if (p == NULL)
+      // read the first token/element of the line
+      char *p = strtok_r (line_buffer, whitespace, &saveptr);
+
+      if (p == NULL || *p == '#')
+	// blank line or comment
 	continue;
 
-      /* Read token type */
+      // read token type
+      unsigned int digits, totpstepsize;
       if (parse_type (p, &digits, &totpstepsize) != 0)
-	continue;
+	{
+	  // wrong formated token type
+	  rc = OATH_WRONG_TOKEN_TYPE;
+	  break;
+	}
 
-      /* Read username */
+      // read username
       p = strtok_r (NULL, whitespace, &saveptr);
-      if (p == NULL || strcmp (p, username) != 0)
+      if (p == NULL)
+	{
+	  // there is no username in the current line of usersfile
+	  rc = OATH_NO_USERNAME;
+	  break;
+	}
+
+      if (strcmp (p, username) != 0)
+	// username doesn't match
 	continue;
 
-      /* Read password. */
+      // we record the fact we read a line matching the username
+      matching_user_line = 1;
+
+      // read password
       p = strtok_r (NULL, whitespace, &saveptr);
+
       if (passwd)
 	{
+	  // password checking is enabled
+
 	  if (p == NULL)
-	    continue;
+	    {
+	      // there is no password in the usersfile
+	      rc = OATH_NO_PASSWORD;
+	      break;
+	    }
+
 	  if (strcmp (p, "-") == 0)
 	    {
 	      if (*passwd != '\0')
-		{
-		  bad_password = 1;
-		  rc = OATH_BAD_PASSWORD;
-		}
+		// the user supply a non empty password but
+		// there is no password ("-") in the current usersfile line
+		rc = OATH_BAD_PASSWORD;
 	    }
 	  else if (strcmp (p, "+") == 0)
 	    {
@@ -132,285 +217,235 @@ parse_usersfile (const char *username,
 	    }
 	  else if (strcmp (p, passwd) != 0)
 	    {
-	      bad_password = 1;
+	      // the password supply by the user doesn't match the one in the
+	      // current usersfile line
 	      rc = OATH_BAD_PASSWORD;
 	    }
+
 	  if (rc == OATH_BAD_PASSWORD)
 	    {
-	      (*skipped_users)++;
+	      // the user supply a non empty password but there is no password ("-") in the current usersfile line
+	      // or
+	      // the password supply by the user doesn't match the one in the current usersfile line
+
+	      // we continue because a user can have multiple password
+	      rc = OATH_OK;
 	      continue;
 	    }
-	  bad_password = 0;
+
+	  // we record the fact we read a line matching the username and the passwd
+	  matching_user_and_passwd_line = 1;
 	}
 
-      /* Read key. */
+      // read secret
       p = strtok_r (NULL, whitespace, &saveptr);
       if (p == NULL)
-	continue;
+	{
+	  // there is no secret key in the current line of usersfile
+	  rc = OATH_NO_SECRET;
+	  break;
+	}
+
+      // convert the secret key in binary format
+      char secret[32];
+      size_t secret_length = sizeof (secret);
       rc = oath_hex2bin (p, secret, &secret_length);
       if (rc != OATH_OK)
-	return rc;
+	// the secret key can't be converted the binary format
+	break;
+
+      // record the size of the "log" of the current line
+      long old_log_size = line_size - strlen (p) - (p - line_buffer);
+
+      // read (optional) moving factor
+      unsigned long long start_moving_factor = 0;
 
-      /* Read (optional) moving factor. */
       p = strtok_r (NULL, whitespace, &saveptr);
-      if (p && *p)
+      if (p)
 	{
+	  // the current line contain a moving factor
+
+	  // convert the string to unsigned long long
 	  char *endptr;
-	  unsigned long long int ull = strtoull (p, &endptr, 10);
+	  start_moving_factor = strtoull (p, &endptr, 10);
 	  if (endptr && *endptr != '\0')
-	    return OATH_INVALID_COUNTER;
-	  start_moving_factor = ull;
+	    {
+	      // the moving factor is bad formated and
+	      // can't be converted to unsigned long long
+	      rc = OATH_INVALID_COUNTER;
+	      break;
+	    }
 	}
 
-      /* Read (optional) last OTP */
-      prev_otp = strtok_r (NULL, whitespace, &saveptr);
+      // read (optional) last OTP
+      char *last_otp = NULL;
+      last_otp = strtok_r (NULL, whitespace, &saveptr);
 
-      /* Read (optional) last_otp */
+      // Read (optional) last OTP timestamp
       p = strtok_r (NULL, whitespace, &saveptr);
-      if (p)
+      if (p && last_otp_timestamp)
 	{
+	  // the current line contain a timestamp
+	  // and the caller off the function want to get it
+
+	  // we convert it to tm
 	  struct tm tm;
 	  char *ts;
-
 	  ts = strptime (p, TIME_FORMAT_STRING, &tm);
 	  if (ts == NULL || *ts != '\0')
-	    return OATH_INVALID_TIMESTAMP;
+	    {
+	      // the timestamp is bad formatted
+	      rc = OATH_INVALID_TIMESTAMP;
+	      break;
+	    }
 	  tm.tm_isdst = -1;
-	  if (last_otp)
+
+	  // convert the tm into time_t and
+	  // store it in last_otp_timestamp
+	  *last_otp_timestamp = mktime (&tm);
+	  if (*last_otp_timestamp == (time_t) - 1)
 	    {
-	      *last_otp = mktime (&tm);
-	      if (*last_otp == (time_t) - 1)
-		return OATH_INVALID_TIMESTAMP;
+	      // the tm is wrong
+	      rc = OATH_INVALID_TIMESTAMP;
+	      break;
 	    }
 	}
 
-      if (prev_otp && strcmp (prev_otp, otp) == 0)
-	return OATH_REPLAYED_OTP;
+      if (last_otp && strcmp (last_otp, otp) == 0)
+	{
+	  // the OTP supply by the user is the
+	  // same that is stored in usersfile
+	  rc = OATH_REPLAYED_OTP;
+	  break;
+	}
 
       if (totpstepsize == 0)
-	rc = oath_hotp_validate (secret, secret_length,
-				 start_moving_factor, window, otp);
-      else if (prev_otp)
 	{
-	  int prev_otp_pos, this_otp_pos, tmprc;
+	  // token type algorithm is HOTP
+
+	  // check if the suppied OTP is valid
+	  rc = oath_hotp_validate (secret, secret_length,
+				   start_moving_factor, window, otp);
+	}
+      else if (last_otp)
+	{
+	  // token type algorithm is TOTP
+	  // and the current line contain a "log"
+
+	  // check if the suppied OTP is valid
+	  int totp_position;
 	  rc = oath_totp_validate2 (secret, secret_length,
 				    time (NULL), totpstepsize, 0, window,
-				    &this_otp_pos, otp);
-	  if (rc == OATH_INVALID_OTP)
+				    &totp_position, otp);
+
+	  if (rc == OATH_OK)
 	    {
-	      (*skipped_users)++;
-	      continue;
+	      // the supplied OTP is valide
+	      // but it may have been already by played
+	      // since it's valide for a periode of time in which a new
+	      // OTP could have been played :
+	      //
+	      //                OTP1 → OTP2 → OTP1(replay)
+	      // OTP1 validity   |--------------------------|
+	      //
+	      // in that case OTP1(replay) should be rejected
+
+	      // get the time validity of the last recorded OTP
+	      int last_totp_position, tmprc;
+	      tmprc = oath_totp_validate2 (secret, secret_length,
+					   time (NULL), totpstepsize, 0,
+					   window, &last_totp_position,
+					   last_otp);
+
+	      if (tmprc >= 0 && last_totp_position >= totp_position)
+		{
+		  // last recorded otp is newer than the one supplied by the user
+		  rc = OATH_REPLAYED_OTP;
+		  break;
+		}
 	    }
-	  if (rc < 0)
-	    return rc;
-	  tmprc = oath_totp_validate2 (secret, secret_length,
-				       time (NULL), totpstepsize, 0, window,
-				       &prev_otp_pos, prev_otp);
-	  if (tmprc >= 0 && prev_otp_pos >= this_otp_pos)
-	    return OATH_REPLAYED_OTP;
 	}
       else
-	rc = oath_totp_validate (secret, secret_length,
-				 time (NULL), totpstepsize, 0, window, otp);
-      if (rc == OATH_INVALID_OTP)
 	{
-	  (*skipped_users)++;
-	  continue;
-	}
-      if (rc < 0)
-	return rc;
-      *new_moving_factor = start_moving_factor + rc;
-      return OATH_OK;
-    }
-
-  if (*skipped_users)
-    {
-      if (bad_password)
-	return OATH_BAD_PASSWORD;
-      else
-	return OATH_INVALID_OTP;
-    }
-
-  return OATH_UNKNOWN_USER;
-}
-
-static int
-update_usersfile2 (const char *username,
-		   const char *otp,
-		   FILE * infh,
-		   FILE * outfh,
-		   char **lineptr,
-		   size_t * n, char *timestamp, uint64_t new_moving_factor,
-		   size_t skipped_users)
-{
-  size_t got_users = 0;
-
-  while (getline (lineptr, n, infh) != -1)
-    {
-      char *saveptr;
-      char *origline;
-      const char *user, *type, *passwd, *secret;
-      int r;
-      unsigned digits, totpstepsize;
-
-      origline = strdup (*lineptr);
-
-      type = strtok_r (*lineptr, whitespace, &saveptr);
-      if (type == NULL)
-	goto skip_line;
-
-      /* Read token type */
-      if (parse_type (type, &digits, &totpstepsize) != 0)
-	goto skip_line;
-
-      /* Read username */
-      user = strtok_r (NULL, whitespace, &saveptr);
-      if (user == NULL || strcmp (user, username) != 0
-	  || got_users++ != skipped_users)
-	goto skip_line;
-
-      passwd = strtok_r (NULL, whitespace, &saveptr);
-      if (passwd == NULL)
-	passwd = "-";
-
-      secret = strtok_r (NULL, whitespace, &saveptr);
-      if (secret == NULL)
-	secret = "-";
-
-      r = fprintf (outfh, "%s\t%s\t%s\t%s\t%llu\t%s\t%s\n",
-		   type, username, passwd, secret,
-		   (unsigned long long) new_moving_factor, otp, timestamp);
-      free (origline);
-      if (r <= 0)
-	return OATH_PRINTF_ERROR;
-      continue;
-
-    skip_line:
-      r = fprintf (outfh, "%s", origline);
-      free (origline);
-      if (r <= 0)
-	return OATH_PRINTF_ERROR;
-      continue;
-    }
+	  // token type algorithm is TOTP
+	  // but the current line doesn't contain a "log"
+	  // it's the first OTP the user supply
 
-  return OATH_OK;
-}
-
-static int
-update_usersfile (const char *usersfile,
-		  const char *username,
-		  const char *otp,
-		  FILE * infh,
-		  char **lineptr,
-		  size_t * n, char *timestamp, uint64_t new_moving_factor,
-		  size_t skipped_users)
-{
-  FILE *outfh, *lockfh;
-  int rc;
-  char *newfilename, *lockfile;
-
-  /* Rewind input file. */
-  {
-    int pos;
-
-    pos = fseeko (infh, 0L, SEEK_SET);
-    if (pos == -1)
-      return OATH_FILE_SEEK_ERROR;
-    clearerr (infh);
-  }
-
-  /* Open lockfile. */
-  {
-    int l;
+	  // check if the suppied OTP is valid
+	  rc = oath_totp_validate (secret, secret_length,
+				   time (NULL), totpstepsize, 0, window, otp);
+	}
 
-    l = asprintf (&lockfile, "%s.lock", usersfile);
-    if (lockfile == NULL || ((size_t) l) != strlen (usersfile) + 5)
-      return OATH_PRINTF_ERROR;
+      if (rc == OATH_INVALID_OTP)
+	{
+	  // the supplied otp doesn't match the current line
 
-    lockfh = fopen (lockfile, "w");
-    if (!lockfh)
-      {
-	free (lockfile);
-	return OATH_FILE_CREATE_ERROR;
-      }
-  }
+	  // we continue because we can have multiple secret key for the
+	  // same pair <user,passwd>
+	  rc = OATH_OK;
+	  continue;
+	}
 
-  /* Lock the lockfile. */
-  {
-    struct flock l;
+      if (rc < 0)
+	// there were an error other than OATH_INVALID_OTP
+	break;
 
-    memset (&l, 0, sizeof (l));
-    l.l_whence = SEEK_SET;
-    l.l_start = 0;
-    l.l_len = 0;
-    l.l_type = F_WRLCK;
+      // OTP is valide
 
-    while ((rc = fcntl (fileno (lockfh), F_SETLKW, &l)) < 0 && errno == EINTR)
-      continue;
-    if (rc == -1)
-      {
-	fclose (lockfh);
-	free (lockfile);
-	return OATH_FILE_LOCK_ERROR;
-      }
-  }
+      // compute the current timestamp
+      char timestamp_buffer[TIME_BUFFER_SIZE];
+      rc = compute_timestamp (timestamp_buffer);
 
-  /* Open the "new" file. */
-  {
-    int l;
+      if (rc != OATH_OK)
+	// timestamp can't be generated
+	break;
 
-    l = asprintf (&newfilename, "%s.new", usersfile);
-    if (newfilename == NULL || ((size_t) l) != strlen (usersfile) + 4)
-      {
-	fclose (lockfh);
-	free (lockfile);
-	return OATH_PRINTF_ERROR;
-      }
+      // compute the new moving factor
+      unsigned long long new_moving_factor = start_moving_factor + rc;
 
-    outfh = fopen (newfilename, "w");
-    if (!outfh)
-      {
-	free (newfilename);
-	fclose (lockfh);
-	free (lockfile);
-	return OATH_FILE_CREATE_ERROR;
-      }
-  }
+      // record the new "log" in new_log_buffer
+      rc =
+	snprintf (new_log_buffer, BUFFER_SIZE, "\t%llu\t%s\t%s\n",
+		  new_moving_factor, otp, timestamp_buffer);
 
-  /* Create the new usersfile content. */
-  rc = update_usersfile2 (username, otp, infh, outfh, lineptr, n,
-			  timestamp, new_moving_factor, skipped_users);
+      if (rc < 0)
+	{
+	  rc = OATH_PRINTF_ERROR;
+	  break;
+	}
 
-  /* On success, flush the buffers. */
-  if (rc == OATH_OK && fflush (outfh) != 0)
-    rc = OATH_FILE_FLUSH_ERROR;
+      // save the file position of the start and end
+      // of the old "log"
+      *old_log_end = ftello (usersfile_fd);
+      *old_log_start = *old_log_end - old_log_size;
 
-  /* On success, sync the disks. */
-  if (rc == OATH_OK && fsync (fileno (outfh)) != 0)
-    rc = OATH_FILE_SYNC_ERROR;
+      if (*old_log_end == -1)
+	{
+	  rc = OATH_FILE_TELL_ERROR;
+	  break;
+	}
 
-  /* Close the file regardless of success. */
-  if (fclose (outfh) != 0)
-    rc = OATH_FILE_CLOSE_ERROR;
+      free (line_buffer);
+      return OATH_OK;
+    }
 
-  /* On success, overwrite the usersfile with the new copy. */
-  if (rc == OATH_OK && rename (newfilename, usersfile) != 0)
-    rc = OATH_FILE_RENAME_ERROR;
+  free (line_buffer);
 
-  /* Something has failed, don't leave garbage lying around. */
   if (rc != OATH_OK)
-    unlink (newfilename);
+    return rc;
 
-  free (newfilename);
+  // the usersfile was parse entirely without error
+  // but no matching OTP have been found
 
-  /* Complete, close the lockfile */
-  if (fclose (lockfh) != 0)
-    rc = OATH_FILE_CLOSE_ERROR;
-  if (unlink (lockfile) != 0)
-    rc = OATH_FILE_UNLINK_ERROR;
-  free (lockfile);
+  if (matching_user_and_passwd_line)
+    // there were line(s) matchine username and password
+    return OATH_INVALID_OTP;
+  else if (matching_user_line)
+    // there were line(s) matchine username (but not password)
+    return OATH_BAD_PASSWORD;
 
-  return rc;
+  // there were no line matchine the username
+  return OATH_UNKNOWN_USER;
 }
 
 /**
@@ -420,7 +455,7 @@ update_usersfile (const char *usersfile,
  * @otp: string with one-time password to authenticate
  * @window: how many past/future OTPs to search
  * @passwd: string with password, or NULL to disable password checking
- * @last_otp: output variable holding last successful authentication
+ * @last_otp_timestamp: output variable holding last successful authentication timestamp, or NULL to not record this value
  *
  * Authenticate user named @username with the one-time password @otp
  * and (optional) password @passwd.  Credentials are read (and
@@ -434,7 +469,7 @@ update_usersfile (const char *usersfile,
  * Returns: On successful validation, %OATH_OK is returned.  If the
  *   supplied @otp is the same as the last successfully authenticated
  *   one-time password, %OATH_REPLAYED_OTP is returned and the
- *   timestamp of the last authentication is returned in @last_otp.
+ *   timestamp of the last authentication is returned in @last_otp_timestamp.
  *   If the one-time password is not found in the indicated search
  *   window, %OATH_INVALID_OTP is returned.  Otherwise, an error code
  *   is returned.
@@ -444,52 +479,128 @@ oath_authenticate_usersfile (const char *usersfile,
 			     const char *username,
 			     const char *otp,
 			     size_t window,
-			     const char *passwd, time_t * last_otp)
+			     const char *passwd, time_t * last_otp_timestamp)
 {
-  FILE *infh;
-  char *line = NULL;
-  size_t n = 0;
-  uint64_t new_moving_factor;
-  int rc;
-  size_t skipped_users;
-
-  infh = fopen (usersfile, "r");
-  if (!infh)
-    return OATH_NO_SUCH_FILE;
-
-  rc = parse_usersfile (username, otp, window, passwd, last_otp,
-			infh, &line, &n, &new_moving_factor, &skipped_users);
-
-  if (rc == OATH_OK)
-    {
-      char timestamp[30];
-      size_t max = sizeof (timestamp);
-      struct tm now;
-      time_t t;
-      size_t l;
-      mode_t old_umask;
-
-      if (time (&t) == (time_t) - 1)
-	return OATH_TIME_ERROR;
-
-      if (localtime_r (&t, &now) == NULL)
-	return OATH_TIME_ERROR;
-
-      l = strftime (timestamp, max, TIME_FORMAT_STRING, &now);
-      if (l != 20)
-	return OATH_TIME_ERROR;
-
-      old_umask = umask (~(S_IRUSR | S_IWUSR));
-
-      rc = update_usersfile (usersfile, username, otp, infh,
-			     &line, &n, timestamp, new_moving_factor,
-			     skipped_users);
-
-      umask (old_umask);
-    }
-
-  free (line);
-  fclose (infh);
+  int rc = OATH_OK;
+  // temporary variable to check various syscall output for error
+  int syscall_output = 0;
+
+  // open usersfile
+  FILE *usersfile_fd;
+  usersfile_fd = fopen (usersfile, "r+");
+  IF_ERROR_GOTO (usersfile_fd == NULL, OATH_FILE_OPEN_ERROR, end);
+
+  {				// this context was created to avoid goto crosses variable declaration
+
+    // put a read lock on usersfile
+    struct flock lock;
+    memset (&lock, 0, sizeof (lock));
+    lock.l_whence = SEEK_SET;
+    lock.l_start = 0;
+    lock.l_len = 0;
+    lock.l_type = F_RDLCK;
+    syscall_output = fcntl (fileno (usersfile_fd), F_SETLKW, &lock);
+    IF_ERROR_GOTO (syscall_output == -1, OATH_FILE_LOCK_ERROR, close_end);
+
+    // parse usersfile and check if the supplied otp is valide
+    off_t old_log_start, old_log_end;
+    char new_log_buffer[BUFFER_SIZE];
+    rc =
+      parse_usersfile (username, otp, window, passwd, usersfile_fd,
+		       last_otp_timestamp, &old_log_start, &old_log_end,
+		       new_log_buffer);
+    if (rc != OATH_OK)
+      // supplied otp is not valide or
+      // the were error during usersfile the parsing
+      goto close_end;
+
+    // the otp is valide
+    // now we have to record the new "log" in usersfile
+
+    // put a write lock on usersfile
+    lock.l_type = F_WRLCK;
+    syscall_output = fcntl (fileno (usersfile_fd), F_SETLKW, &lock);
+    IF_ERROR_GOTO (syscall_output == -1, OATH_FILE_LOCK_ERROR, close_end);
+
+    size_t new_log_len = strlen (new_log_buffer);
+    if ((size_t) (old_log_end - old_log_start) == new_log_len)
+      {
+	// the new and old "log" have the same size
+	// we write directly the new log in the usersfile inplace
+	// of the old "log"
+
+	// go the the start possition of the old "log"
+	syscall_output = fseeko (usersfile_fd, old_log_start, SEEK_SET);
+	IF_ERROR_GOTO (syscall_output == -1, OATH_FILE_SEEK_ERROR, close_end);
+
+	// write the new "log"
+	fwrite (new_log_buffer, sizeof (char), strlen (new_log_buffer),
+		usersfile_fd);
+	IF_ERROR_GOTO (ferror (usersfile_fd), OATH_FILE_WRITE_ERROR,
+		       close_end);
+      }
+    else
+      {
+	// the new and old "log" doesn't have the same size
+	// we will load in memory the end of the usersfile from the end of the old "log" to the end of the file
+	// write the new "log" from the starting point of the old "log"
+	// and append the end of the usersfile previously loaded in memory
+
+	// compute the amont of memory we will need
+	syscall_output = fseeko (usersfile_fd, 0, SEEK_END);
+	IF_ERROR_GOTO (syscall_output == -1, OATH_FILE_SEEK_ERROR, close_end);
+
+	off_t usersfile_buffer_size = ftello (usersfile_fd) - old_log_end;
+	IF_ERROR_GOTO (usersfile_buffer_size < 0, OATH_FILE_TELL_ERROR,
+		       close_end);
+
+	// allocate the dynamic memory
+	char *usersfile_buffer =
+	  malloc (usersfile_buffer_size * sizeof (char));
+	IF_ERROR_GOTO (usersfile_buffer == NULL, OATH_MALLOC_ERROR,
+		       close_end);
+
+	// load usersfile from the end of the old "log" to the end of the file
+	syscall_output = fseeko (usersfile_fd, old_log_end, SEEK_SET);
+	IF_ERROR_GOTO (syscall_output == -1, OATH_FILE_SEEK_ERROR,
+		       free_close_end);
+
+	syscall_output =
+	  fread (usersfile_buffer, sizeof (char), usersfile_buffer_size,
+		 usersfile_fd);
+	IF_ERROR_GOTO (syscall_output != usersfile_buffer_size,
+		       OATH_FILE_READ_ERROR, free_close_end);
+
+	// write the new log in the usersfile from the start of the old "log"
+	syscall_output = fseeko (usersfile_fd, old_log_start, SEEK_SET);
+	IF_ERROR_GOTO (syscall_output == -1, OATH_FILE_SEEK_ERROR,
+		       free_close_end);
+
+	fwrite (new_log_buffer, sizeof (char), strlen (new_log_buffer),
+		usersfile_fd);
+	IF_ERROR_GOTO (ferror (usersfile_fd), OATH_FILE_WRITE_ERROR,
+		       free_close_end);
+
+	// write the end of the usersfile previously loaded in memory
+	fwrite (usersfile_buffer, sizeof (char), usersfile_buffer_size,
+		usersfile_fd);
+	IF_ERROR_GOTO (ferror (usersfile_fd), OATH_FILE_WRITE_ERROR,
+		       free_close_end);
+
+	// in case the old log was longer than the new one we truncate the end of the file
+	syscall_output =
+	  ftruncate (fileno (usersfile_fd), ftello (usersfile_fd));
+	IF_ERROR_GOTO (syscall_output == -1, OATH_FILE_TRUNCATE_ERROR,
+		       free_close_end);
+
+      free_close_end:
+	free (usersfile_buffer);
+      }
+  }
 
+close_end:
+  // close the usersfile and remove the lock
+  fclose (usersfile_fd);
+end:
   return rc;
 }
diff --git a/pam_oath/README b/pam_oath/README
index 5fc534b..639cd73 100644
--- a/pam_oath/README
+++ b/pam_oath/README
@@ -34,7 +34,7 @@ file:
 ---------
 # cat>/etc/users.oath
 HOTP root - 00
-# chmod go-rw /etc/users.oath
+# chmod 600 /etc/users.oath
 #
 ---------
 
-- 
2.2.1

From 6005d6e62f9ea0bfe3a00fdcaabd84874196ad23 Mon Sep 17 00:00:00 2001
From: Maxime de Roucy <[email protected]>
Date: Sun, 4 Jan 2015 12:52:11 +0100
Subject: [PATCH 2/3] different usersfile field 5 if HOTP / TOTP

Counter/Offset : Next expected counter value (event tokens) or
counter offset (time tokens)

Use the last_otp_timestamp to verify check for replayed otp (TOTP)
---
 liboath/usersfile.c | 134 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 99 insertions(+), 35 deletions(-)

diff --git a/liboath/usersfile.c b/liboath/usersfile.c
index 816053d..200f7b8 100644
--- a/liboath/usersfile.c
+++ b/liboath/usersfile.c
@@ -118,7 +118,7 @@ compute_timestamp (char timestamp[])
  * @window: how many past/future OTPs to search
  * @passwd: string with password, or NULL if password checking is disabled
  * @usersfile_fd: input file descriptor for usersfile
- * @last_otp_timestamp: output variable holding last successful authentication timestamp
+ * @parameter_last_otp_timestamp: output variable holding last successful authentication timestamp
  * @old_log_start: output variable holding the file position of the beginning of the "log" of the validation line
  * @old_log_end: output variable holding the file position of the end of the "log" of the validation line
  *
@@ -135,7 +135,7 @@ parse_usersfile (const char *username,
 		 const size_t window,
 		 const char *passwd,
 		 FILE * usersfile_fd,
-		 time_t * last_otp_timestamp,
+		 time_t * parameter_last_otp_timestamp,
 		 off_t * old_log_start,
 		 off_t * old_log_end, char *new_log_buffer)
 {
@@ -257,23 +257,46 @@ parse_usersfile (const char *username,
       // record the size of the "log" of the current line
       long old_log_size = line_size - strlen (p) - (p - line_buffer);
 
-      // read (optional) moving factor
+      // read (optional)
+      // if HOTP : moving factor
       unsigned long long start_moving_factor = 0;
+      // if TOTP : search position in search windowa
+      int last_totp_position = 0;
 
       p = strtok_r (NULL, whitespace, &saveptr);
       if (p)
 	{
-	  // the current line contain a moving factor
+	  // the current line contain a moving factor/search position
 
-	  // convert the string to unsigned long long
-	  char *endptr;
-	  start_moving_factor = strtoull (p, &endptr, 10);
-	  if (endptr && *endptr != '\0')
+	  if (totpstepsize == 0)
 	    {
-	      // the moving factor is bad formated and
-	      // can't be converted to unsigned long long
-	      rc = OATH_INVALID_COUNTER;
-	      break;
+	      // token type algorithm is HOTP
+
+	      // convert the string to unsigned long long
+	      char *endptr;
+	      start_moving_factor = strtoull (p, &endptr, 10);
+	      if (endptr && *endptr != '\0')
+		{
+		  // the moving factor is bad formated and
+		  // can't be converted to unsigned long long
+		  rc = OATH_INVALID_COUNTER;
+		  break;
+		}
+	    }
+	  else
+	    {
+	      // token type algorithm is TOTP
+
+	      // convert the string to long
+	      char *endptr;
+	      last_totp_position = strtol (p, &endptr, 10);
+	      if (endptr && *endptr != '\0')
+		{
+		  // the search position is bad formated and
+		  // can't be converted to long
+		  rc = OATH_INVALID_COUNTER;
+		  break;
+		}
 	    }
 	}
 
@@ -282,11 +305,11 @@ parse_usersfile (const char *username,
       last_otp = strtok_r (NULL, whitespace, &saveptr);
 
       // Read (optional) last OTP timestamp
+      time_t last_otp_timestamp = (time_t) - 1;
       p = strtok_r (NULL, whitespace, &saveptr);
-      if (p && last_otp_timestamp)
+      if (p)
 	{
 	  // the current line contain a timestamp
-	  // and the caller off the function want to get it
 
 	  // we convert it to tm
 	  struct tm tm;
@@ -302,13 +325,21 @@ parse_usersfile (const char *username,
 
 	  // convert the tm into time_t and
 	  // store it in last_otp_timestamp
-	  *last_otp_timestamp = mktime (&tm);
-	  if (*last_otp_timestamp == (time_t) - 1)
+	  last_otp_timestamp = mktime (&tm);
+	  if (last_otp_timestamp == (time_t) - 1)
 	    {
 	      // the tm is wrong
 	      rc = OATH_INVALID_TIMESTAMP;
 	      break;
 	    }
+
+	  if (parameter_last_otp_timestamp)
+	    {
+	      // if the caller off the function want to record
+	      // the last OTP timestamp
+
+	      *parameter_last_otp_timestamp = last_otp_timestamp;
+	    }
 	}
 
       if (last_otp && strcmp (last_otp, otp) == 0)
@@ -319,6 +350,7 @@ parse_usersfile (const char *username,
 	  break;
 	}
 
+      int totp_position = 0;
       if (totpstepsize == 0)
 	{
 	  // token type algorithm is HOTP
@@ -333,7 +365,6 @@ parse_usersfile (const char *username,
 	  // and the current line contain a "log"
 
 	  // check if the suppied OTP is valid
-	  int totp_position;
 	  rc = oath_totp_validate2 (secret, secret_length,
 				    time (NULL), totpstepsize, 0, window,
 				    &totp_position, otp);
@@ -341,7 +372,7 @@ parse_usersfile (const char *username,
 	  if (rc == OATH_OK)
 	    {
 	      // the supplied OTP is valide
-	      // but it may have been already by played
+	      // but it may have been already be played
 	      // since it's valide for a periode of time in which a new
 	      // OTP could have been played :
 	      //
@@ -350,18 +381,38 @@ parse_usersfile (const char *username,
 	      //
 	      // in that case OTP1(replay) should be rejected
 
-	      // get the time validity of the last recorded OTP
-	      int last_totp_position, tmprc;
-	      tmprc = oath_totp_validate2 (secret, secret_length,
-					   time (NULL), totpstepsize, 0,
-					   window, &last_totp_position,
-					   last_otp);
-
-	      if (tmprc >= 0 && last_totp_position >= totp_position)
+	      if (last_otp_timestamp != (time_t) - 1)
 		{
-		  // last recorded otp is newer than the one supplied by the user
-		  rc = OATH_REPLAYED_OTP;
-		  break;
+		  unsigned long long totp_time_step_number,
+		    last_totp_time_step_number;
+		  totp_time_step_number =
+		    (time (NULL) / totpstepsize) + totp_position;
+		  last_totp_time_step_number =
+		    (last_otp_timestamp / totpstepsize) + last_totp_position;
+
+		  if (last_totp_time_step_number >= totp_time_step_number)
+		    {
+		      // last recorded otp is newer than the one supplied by the user
+		      rc = OATH_REPLAYED_OTP;
+		      break;
+		    }
+		}
+	      else
+		{
+		  // get the time validity of the last recorded OTP
+		  int tmprc;
+		  tmprc = oath_totp_validate2 (secret, secret_length,
+					       time (NULL), totpstepsize, 0,
+					       window, &last_totp_position,
+					       last_otp);
+
+		  if (tmprc >= 0 && last_totp_position >= totp_position)
+		    {
+		      // last recorded otp is newer than the one supplied by the user
+		      rc = OATH_REPLAYED_OTP;
+		      break;
+		    }
+
 		}
 	    }
 	}
@@ -392,6 +443,9 @@ parse_usersfile (const char *username,
 
       // OTP is valide
 
+      // compute the new moving factor
+      unsigned long long new_moving_factor = start_moving_factor + rc;
+
       // compute the current timestamp
       char timestamp_buffer[TIME_BUFFER_SIZE];
       rc = compute_timestamp (timestamp_buffer);
@@ -400,13 +454,23 @@ parse_usersfile (const char *username,
 	// timestamp can't be generated
 	break;
 
-      // compute the new moving factor
-      unsigned long long new_moving_factor = start_moving_factor + rc;
-
       // record the new "log" in new_log_buffer
-      rc =
-	snprintf (new_log_buffer, BUFFER_SIZE, "\t%llu\t%s\t%s\n",
-		  new_moving_factor, otp, timestamp_buffer);
+      if (totpstepsize == 0)
+	{
+	  // token type algorithm is HOTP
+
+	  rc =
+	    snprintf (new_log_buffer, BUFFER_SIZE, "\t%llu\t%s\t%s\n",
+		      new_moving_factor, otp, timestamp_buffer);
+	}
+      else
+	{
+	  // token type algorithm is TOTP
+
+	  rc =
+	    snprintf (new_log_buffer, BUFFER_SIZE, "\t%d\t%s\t%s\n",
+		      totp_position, otp, timestamp_buffer);
+	}
 
       if (rc < 0)
 	{
-- 
2.2.1

From 8c934f6420285ba2c7f7444905a651a4337cc369 Mon Sep 17 00:00:00 2001
From: Maxime de Roucy <[email protected]>
Date: Sun, 4 Jan 2015 13:14:25 +0100
Subject: [PATCH 3/3] =?UTF-8?q?usersfile=20:=20fields=205=20present=20?=
 =?UTF-8?q?=E2=86=92=206=20and=207=20mandatory?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If usersfile fields 5 is present, fields 6 and 7 are now mandatory
---
 NEWS                |   2 +
 liboath/errors.c    |   1 +
 liboath/oath.h.in   |  42 +++++++++--------
 liboath/usersfile.c | 129 +++++++++++++++++++++-------------------------------
 pam_oath/README     |  10 ++--
 5 files changed, 85 insertions(+), 99 deletions(-)

diff --git a/NEWS b/NEWS
index d84f8bf..bd2687f 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,8 @@ Copyright (C) 2009-2014 Simon Josefsson.  Licensed under the GPLv3+.
 
 * Version 2.6.0 (unreleased)
 
+** liboath: if usersfile fields 5 is present, fields 6 and 7 are now mandatory
+
 ** liboath: usersfile is now updated inplace, so there is no owner/permission
 modifications.
 
diff --git a/liboath/errors.c b/liboath/errors.c
index 1257bad..3ba55f8 100644
--- a/liboath/errors.c
+++ b/liboath/errors.c
@@ -40,6 +40,7 @@ static const err_t errors[] = {
   ERR (OATH_REPLAYED_OTP, " The OTP has been replayed"),
   ERR (OATH_BAD_PASSWORD, " The password does not match"),
   ERR (OATH_INVALID_COUNTER, " The counter value is corrupt"),
+  ERR (OATH_INVALID_LAST_OTP, " The last OTP value is corrupt"),
   ERR (OATH_INVALID_TIMESTAMP, " The timestamp is corrupt"),
   ERR (OATH_UNKNOWN_USER, " Cannot find information about user"),
   ERR (OATH_WRONG_TOKEN_TYPE, " Bad formated token type"),
diff --git a/liboath/oath.h.in b/liboath/oath.h.in
index a6bb8d1..8752ea9 100644
--- a/liboath/oath.h.in
+++ b/liboath/oath.h.in
@@ -74,6 +74,7 @@ extern "C"
  * @OATH_REPLAYED_OTP: The OTP has been replayed
  * @OATH_BAD_PASSWORD: The password does not match
  * @OATH_INVALID_COUNTER: The counter value is corrupt
+ * @OATH_INVALID_LAST_OTP: The last OTP value is corrupt
  * @OATH_INVALID_TIMESTAMP: The timestamp is corrupt
  * @OATH_UNKNOWN_USER: Cannot find information about user
  * @OATH_WRONG_TOKEN_TYPE: Bad formated token type
@@ -115,28 +116,29 @@ typedef enum
   OATH_REPLAYED_OTP = -6,
   OATH_BAD_PASSWORD = -7,
   OATH_INVALID_COUNTER = -8,
-  OATH_INVALID_TIMESTAMP = -9,
-  OATH_UNKNOWN_USER = -10,
-  OATH_WRONG_TOKEN_TYPE = -11,
-  OATH_NO_USERNAME = -12,
-  OATH_NO_PASSWORD = -13,
-  OATH_NO_SECRET = -14,
-  OATH_PRINTF_ERROR = -15,
-  OATH_FILE_OPEN_ERROR = -16,
-  OATH_FILE_READ_ERROR = -17,
-  OATH_FILE_WRITE_ERROR = -18,
-  OATH_FILE_TELL_ERROR = -19,
-  OATH_FILE_SEEK_ERROR = -20,
-  OATH_FILE_TRUNCATE_ERROR = -21,
-  OATH_FILE_LOCK_ERROR = -22,
-  OATH_TIME_ERROR = -23,
-  OATH_STRCMP_ERROR = -24,
-  OATH_INVALID_BASE32 = -25,
-  OATH_BASE32_OVERFLOW = -26,
-  OATH_MALLOC_ERROR = -27,
+  OATH_INVALID_LAST_OTP = -9,
+  OATH_INVALID_TIMESTAMP = -10,
+  OATH_UNKNOWN_USER = -11,
+  OATH_WRONG_TOKEN_TYPE = -12,
+  OATH_NO_USERNAME = -13,
+  OATH_NO_PASSWORD = -14,
+  OATH_NO_SECRET = -15,
+  OATH_PRINTF_ERROR = -16,
+  OATH_FILE_OPEN_ERROR = -17,
+  OATH_FILE_READ_ERROR = -18,
+  OATH_FILE_WRITE_ERROR = -19,
+  OATH_FILE_TELL_ERROR = -20,
+  OATH_FILE_SEEK_ERROR = -21,
+  OATH_FILE_TRUNCATE_ERROR = -22,
+  OATH_FILE_LOCK_ERROR = -23,
+  OATH_TIME_ERROR = -24,
+  OATH_STRCMP_ERROR = -25,
+  OATH_INVALID_BASE32 = -26,
+  OATH_BASE32_OVERFLOW = -27,
+  OATH_MALLOC_ERROR = -28,
   /* When adding anything here, update OATH_LAST_ERROR, errors.c
    * and tests/tst_errors.c. */
-  OATH_LAST_ERROR = -27
+  OATH_LAST_ERROR = -28
 } oath_rc;
 
 /* Global */
diff --git a/liboath/usersfile.c b/liboath/usersfile.c
index 200f7b8..d9bd09c 100644
--- a/liboath/usersfile.c
+++ b/liboath/usersfile.c
@@ -124,7 +124,8 @@ compute_timestamp (char timestamp[])
  *
  * internal fonction
  *
- * by "log" (old_log_start/end) I mean the end of line that should contain a moving_factor, an otp and a timestamp.
+ * By "log" (old_log_start/end) I mean the end of line that should contain a moving factor/search position, an otp and a timestamp.
+ * The "log" contain all the fields from the 5th ; it can also be empty if the line contain only 4 fields.
  *
  * Returns: %OATH_OK on success, negative value on error (see oath.h)
  **/
@@ -237,7 +238,7 @@ parse_usersfile (const char *username,
 	  matching_user_and_passwd_line = 1;
 	}
 
-      // read secret
+      // read secret key
       p = strtok_r (NULL, whitespace, &saveptr);
       if (p == NULL)
 	{
@@ -257,16 +258,16 @@ parse_usersfile (const char *username,
       // record the size of the "log" of the current line
       long old_log_size = line_size - strlen (p) - (p - line_buffer);
 
-      // read (optional)
-      // if HOTP : moving factor
-      unsigned long long start_moving_factor = 0;
-      // if TOTP : search position in search windowa
+      // read the (optional) "log"
+      unsigned long long last_moving_factor = 0;
       int last_totp_position = 0;
+      char *last_otp = NULL;
+      time_t last_otp_timestamp = (time_t) - 1;
 
       p = strtok_r (NULL, whitespace, &saveptr);
       if (p)
 	{
-	  // the current line contain a moving factor/search position
+	  // the current line contain a non empty "log"
 
 	  if (totpstepsize == 0)
 	    {
@@ -274,11 +275,10 @@ parse_usersfile (const char *username,
 
 	      // convert the string to unsigned long long
 	      char *endptr;
-	      start_moving_factor = strtoull (p, &endptr, 10);
+	      last_moving_factor = strtoull (p, &endptr, 10);
 	      if (endptr && *endptr != '\0')
 		{
-		  // the moving factor is bad formated and
-		  // can't be converted to unsigned long long
+		  // the moving factor can't be converted to unsigned long long
 		  rc = OATH_INVALID_COUNTER;
 		  break;
 		}
@@ -287,29 +287,32 @@ parse_usersfile (const char *username,
 	    {
 	      // token type algorithm is TOTP
 
-	      // convert the string to long
+	      // convert the string to int
 	      char *endptr;
 	      last_totp_position = strtol (p, &endptr, 10);
 	      if (endptr && *endptr != '\0')
 		{
-		  // the search position is bad formated and
-		  // can't be converted to long
+		  // the search position can't be converted to long
 		  rc = OATH_INVALID_COUNTER;
 		  break;
 		}
 	    }
-	}
 
-      // read (optional) last OTP
-      char *last_otp = NULL;
-      last_otp = strtok_r (NULL, whitespace, &saveptr);
+	  // read last OTP
+	  last_otp = strtok_r (NULL, whitespace, &saveptr);
+	  if (!last_otp)
+	    {
+	      rc = OATH_INVALID_LAST_OTP;
+	      break;
+	    }
 
-      // Read (optional) last OTP timestamp
-      time_t last_otp_timestamp = (time_t) - 1;
-      p = strtok_r (NULL, whitespace, &saveptr);
-      if (p)
-	{
-	  // the current line contain a timestamp
+	  // Read last OTP timestamp
+	  p = strtok_r (NULL, whitespace, &saveptr);
+	  if (!p)
+	    {
+	      rc = OATH_INVALID_TIMESTAMP;
+	      break;
+	    }
 
 	  // we convert it to tm
 	  struct tm tm;
@@ -334,12 +337,9 @@ parse_usersfile (const char *username,
 	    }
 
 	  if (parameter_last_otp_timestamp)
-	    {
-	      // if the caller off the function want to record
-	      // the last OTP timestamp
-
-	      *parameter_last_otp_timestamp = last_otp_timestamp;
-	    }
+	    // if the function caller want to record
+	    // the last OTP timestamp
+	    *parameter_last_otp_timestamp = last_otp_timestamp;
 	}
 
       if (last_otp && strcmp (last_otp, otp) == 0)
@@ -350,14 +350,14 @@ parse_usersfile (const char *username,
 	  break;
 	}
 
-      int totp_position = 0;
+      int new_totp_position = 0;
       if (totpstepsize == 0)
 	{
 	  // token type algorithm is HOTP
 
 	  // check if the suppied OTP is valid
 	  rc = oath_hotp_validate (secret, secret_length,
-				   start_moving_factor, window, otp);
+				   last_moving_factor, window, otp);
 	}
       else if (last_otp)
 	{
@@ -367,7 +367,7 @@ parse_usersfile (const char *username,
 	  // check if the suppied OTP is valid
 	  rc = oath_totp_validate2 (secret, secret_length,
 				    time (NULL), totpstepsize, 0, window,
-				    &totp_position, otp);
+				    &new_totp_position, otp);
 
 	  if (rc == OATH_OK)
 	    {
@@ -381,38 +381,18 @@ parse_usersfile (const char *username,
 	      //
 	      // in that case OTP1(replay) should be rejected
 
-	      if (last_otp_timestamp != (time_t) - 1)
-		{
-		  unsigned long long totp_time_step_number,
-		    last_totp_time_step_number;
-		  totp_time_step_number =
-		    (time (NULL) / totpstepsize) + totp_position;
-		  last_totp_time_step_number =
-		    (last_otp_timestamp / totpstepsize) + last_totp_position;
-
-		  if (last_totp_time_step_number >= totp_time_step_number)
-		    {
-		      // last recorded otp is newer than the one supplied by the user
-		      rc = OATH_REPLAYED_OTP;
-		      break;
-		    }
-		}
-	      else
-		{
-		  // get the time validity of the last recorded OTP
-		  int tmprc;
-		  tmprc = oath_totp_validate2 (secret, secret_length,
-					       time (NULL), totpstepsize, 0,
-					       window, &last_totp_position,
-					       last_otp);
-
-		  if (tmprc >= 0 && last_totp_position >= totp_position)
-		    {
-		      // last recorded otp is newer than the one supplied by the user
-		      rc = OATH_REPLAYED_OTP;
-		      break;
-		    }
+	      unsigned long long new_totp_time_step_number,
+		last_totp_time_step_number;
+	      last_totp_time_step_number =
+		(last_otp_timestamp / totpstepsize) + last_totp_position;
+	      new_totp_time_step_number =
+		(time (NULL) / totpstepsize) + new_totp_position;
 
+	      if (last_totp_time_step_number >= new_totp_time_step_number)
+		{
+		  // last recorded otp is newer than the one supplied by the user
+		  rc = OATH_REPLAYED_OTP;
+		  break;
 		}
 	    }
 	}
@@ -444,7 +424,7 @@ parse_usersfile (const char *username,
       // OTP is valide
 
       // compute the new moving factor
-      unsigned long long new_moving_factor = start_moving_factor + rc;
+      unsigned long long new_moving_factor = last_moving_factor + rc;
 
       // compute the current timestamp
       char timestamp_buffer[TIME_BUFFER_SIZE];
@@ -459,17 +439,15 @@ parse_usersfile (const char *username,
 	{
 	  // token type algorithm is HOTP
 
-	  rc =
-	    snprintf (new_log_buffer, BUFFER_SIZE, "\t%llu\t%s\t%s\n",
-		      new_moving_factor, otp, timestamp_buffer);
+	  rc = snprintf (new_log_buffer, BUFFER_SIZE, "\t%llu\t%s\t%s\n",
+			 new_moving_factor, otp, timestamp_buffer);
 	}
       else
 	{
 	  // token type algorithm is TOTP
 
-	  rc =
-	    snprintf (new_log_buffer, BUFFER_SIZE, "\t%d\t%s\t%s\n",
-		      totp_position, otp, timestamp_buffer);
+	  rc = snprintf (new_log_buffer, BUFFER_SIZE, "\t%d\t%s\t%s\n",
+			 new_totp_position, otp, timestamp_buffer);
 	}
 
       if (rc < 0)
@@ -569,17 +547,16 @@ oath_authenticate_usersfile (const char *usersfile,
     // parse usersfile and check if the supplied otp is valide
     off_t old_log_start, old_log_end;
     char new_log_buffer[BUFFER_SIZE];
-    rc =
-      parse_usersfile (username, otp, window, passwd, usersfile_fd,
-		       last_otp_timestamp, &old_log_start, &old_log_end,
-		       new_log_buffer);
+    rc = parse_usersfile (username, otp, window, passwd, usersfile_fd,
+			  last_otp_timestamp, &old_log_start, &old_log_end,
+			  new_log_buffer);
     if (rc != OATH_OK)
       // supplied otp is not valide or
       // the were error during usersfile the parsing
       goto close_end;
 
     // the otp is valide
-    // now we have to record the new "log" in usersfile
+    // now we have to write the new "log" in usersfile
 
     // put a write lock on usersfile
     lock.l_type = F_WRLCK;
@@ -593,7 +570,7 @@ oath_authenticate_usersfile (const char *usersfile,
 	// we write directly the new log in the usersfile inplace
 	// of the old "log"
 
-	// go the the start possition of the old "log"
+	// go the the start position of the old "log"
 	syscall_output = fseeko (usersfile_fd, old_log_start, SEEK_SET);
 	IF_ERROR_GOTO (syscall_output == -1, OATH_FILE_SEEK_ERROR, close_end);
 
diff --git a/pam_oath/README b/pam_oath/README
index 639cd73..ba2445f 100644
--- a/pam_oath/README
+++ b/pam_oath/README
@@ -38,13 +38,17 @@ HOTP root - 00
 #
 ---------
 
+WARNING!  The above added an OATH secret of all-zeros, which leads to
+no security.  In production, replace "00" with a randomly generate hex
+encoded data of say, 20 bytes in size.
+
 The file format handles any whitespace as field separator.  You may
 also add lines starting with '#' for comments.  The file format is
 documented here: http://code.google.com/p/mod-authn-otp/wiki/UsersFile
+Differences are :
 
-WARNING!  The above added an OATH secret of all-zeros, which leads to
-no security.  In production, replace "00" with a randomly generate hex
-encoded data of say, 20 bytes in size.
+* we use only field "Token Type", "Usernam", "PIN", "Token Key", "Counter/Offset", "Last OTP" and "Time of Last OTP"
+* if field "Counter/Offset" is present, "Last OTP" and "Time of Last OTP" are mandatory
 
 To test the setup, we need to generate some one-time passwords.  The
 "oathtool" is handy for this purpose.  Replace 00 with the key you
-- 
2.2.1

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to