]> granicus.if.org Git - linux-pam/commitdiff
Rewrite of the field parsing in pam_group and pam_time.
authorTomas Mraz <tmraz@fedoraproject.org>
Mon, 6 Jun 2011 10:22:02 +0000 (12:22 +0200)
committerTomas Mraz <tmraz@fedoraproject.org>
Mon, 6 Jun 2011 10:22:02 +0000 (12:22 +0200)
ChangeLog
modules/pam_group/pam_group.c
modules/pam_time/pam_time.c

index 0be8f055fd2c5620f6047a5b7499967d884b2a2a..b0cb53a223b5a8273e99a8b3befeb18b6b9a2088 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2011-06-06  Tomas Mraz  <tm@t8m.info>
+
+       * modules/pam_group/pam_group.c (shift_bytes): Removed.
+       (shift_buf, trim_spaces): Added new functions.
+       (read_field): Thorough rewrite of the parsing.
+       (check_account): read_field() now uses state information. No
+       extra read_field() call at the end of configuration line.
+       * modules/pam_time/pam_time.c (shift_bytes): Removed.
+       (shift_buf, trim_spaces): Added new functions.
+       (read_field): Thorough rewrite of the parsing.
+       (check_account): read_field() now uses state information. No
+       extra read_field() call at the end of configuration line.
+
 2011-06-06  Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
 
        * po/LINGUAS: Add vietnamese.
index 310b26227b99b07b6feb0ae5480142fa12cc2422..be5f20f3937fc9c19a8feb337d7babf653c732f7 100644 (file)
@@ -2,6 +2,7 @@
 
 /*
  * Written by Andrew Morgan <morgan@linux.kernel.org> 1996/7/6
+ * Field parsing rewritten by Tomas Mraz <tm@t8m.info>
  */
 
 #include "config.h"
@@ -50,166 +51,156 @@ typedef enum { AND, OR } operator;
 
 /* --- static functions for checking whether the user should be let in --- */
 
-static void shift_bytes(char *mem, int from, int by)
+static char *
+shift_buf(char *mem, int from)
 {
-    while (by-- > 0) {
-       *mem = mem[from];
+    char *start = mem;
+    while ((*mem = mem[from]) != '\0')
        ++mem;
+    memset(mem, '\0', PAM_GROUP_BUFLEN - (mem - start));
+    return mem;
+}
+
+static void
+trim_spaces(char *buf, char *from)
+{
+    while (from > buf) {
+       --from;
+       if (*from == ' ')
+           *from = '\0';
+       else
+           break;
     }
 }
 
-/* This function should initially be called with buf = NULL. If
- * an error occurs, the file descriptor is closed. Subsequent
- * calls with a closed descriptor will cause buf to be deallocated.
- * Therefore, always check buf after calling this to see if an error
- * occurred.
- */
+#define STATE_NL       0 /* new line starting */
+#define STATE_COMMENT  1 /* inside comment */
+#define STATE_FIELD    2 /* field following */
+#define STATE_EOF      3 /* end of file or error */
+
 static int
-read_field (const pam_handle_t *pamh, int fd, char **buf, int *from, int *to)
+read_field(const pam_handle_t *pamh, int fd, char **buf, int *from, int *state)
 {
-    /* is buf set ? */
+    char *to;
+    char *src;
+    int i;
+    char c;
+    int onspace;
 
+    /* is buf set ? */
     if (! *buf) {
-       *buf = (char *) malloc(PAM_GROUP_BUFLEN);
+       *buf = (char *) calloc(1, PAM_GROUP_BUFLEN+1);
        if (! *buf) {
            pam_syslog(pamh, LOG_ERR, "out of memory");
+           D(("no memory"));
+           *state = STATE_EOF;
            return -1;
        }
-       *from = *to = 0;
+       *from = 0;
+        *state = STATE_NL;
        fd = open(PAM_GROUP_CONF, O_RDONLY);
+       if (fd < 0) {
+           pam_syslog(pamh, LOG_ERR, "error opening %s: %m", PAM_GROUP_CONF);
+           _pam_drop(*buf);
+           *state = STATE_EOF;
+           return -1;
+       }
     }
 
-    /* do we have a file open ? return error */
-
-    if (fd < 0 && *to <= 0) {
-       pam_syslog(pamh, LOG_ERR, "%s not opened", PAM_GROUP_CONF);
-       memset(*buf, 0, PAM_GROUP_BUFLEN);
-       _pam_drop(*buf);
-       return -1;
-    }
-
-    /* check if there was a newline last time */
-
-    if ((*to > *from) && (*to > 0)
-       && ((*buf)[*from] == '\0')) {   /* previous line ended */
-       (*from)++;
-       (*buf)[0] = '\0';
-       return fd;
-    }
-
-    /* ready for more data: first shift the buffer's remaining data */
-
-    *to -= *from;
-    shift_bytes(*buf, *from, *to);
-    *from = 0;
-    (*buf)[*to] = '\0';
-
-    while (fd >= 0 && *to < PAM_GROUP_BUFLEN) {
-       int i;
-
-       /* now try to fill the remainder of the buffer */
+    if (*from > 0)
+       to = shift_buf(*buf, *from);
+    else
+       to = *buf;
 
-       i = read(fd, *to + *buf, PAM_GROUP_BUFLEN - *to);
+    while (fd != -1 && to - *buf < PAM_GROUP_BUFLEN) {
+       i = pam_modutil_read(fd, to, PAM_GROUP_BUFLEN - (to - *buf));
        if (i < 0) {
            pam_syslog(pamh, LOG_ERR, "error reading %s: %m", PAM_GROUP_CONF);
            close(fd);
+           memset(*buf, 0, PAM_GROUP_BUFLEN);
+           _pam_drop(*buf);
+           *state = STATE_EOF;
            return -1;
        } else if (!i) {
            close(fd);
            fd = -1;          /* end of file reached */
-       } else
-           *to += i;
+       }
 
-       /*
-        * contract the buffer. Delete any comments, and replace all
-        * multiple spaces with single commas
-        */
+       to += i;
+    }
 
-       i = 0;
-#ifdef DEBUG_DUMP
-       D(("buffer=<%s>",*buf));
-#endif
-       while (i < *to) {
-           if ((*buf)[i] == ',') {
-               int j;
-
-               for (j=++i; j<*to && (*buf)[j] == ','; ++j);
-               if (j!=i) {
-                   shift_bytes(i + (*buf), j-i, (*to) - j);
-                   *to -= j-i;
+    if (to == *buf) {
+       /* nothing previously in buf, nothing read */
+       _pam_drop(*buf);
+       *state = STATE_EOF;
+       return -1;
+    }
+
+    memset(to, '\0', PAM_GROUP_BUFLEN - (to - *buf));
+
+    to = *buf;
+    onspace = 1; /* delete any leading spaces */
+
+    for (src = to; (c=*src) != '\0'; ++src) {
+       if (*state == STATE_COMMENT && c != '\n') {
+               continue;
+       }
+
+       switch (c) {
+           case '\n':
+               *state = STATE_NL;
+                *to = '\0';
+               *from = (src - *buf) + 1;
+               trim_spaces(*buf, to);
+               return fd;
+
+           case '\t':
+            case ' ':
+               if (!onspace) {
+                   onspace = 1;
+                   *to++ = ' ';
                }
-           }
-           switch ((*buf)[i]) {
-               int j, c;
+               break;
+
+            case '!':
+               onspace = 1; /* ignore following spaces */
+               *to++ = '!';
+               break;
+
            case '#':
-               c = 0;
-               for (j=i; j < *to && (c = (*buf)[j]) != '\n'; ++j);
-               if (j >= *to) {
-                   (*buf)[*to = ++i] = '\0';
-               } else if (c == '\n') {
-                   shift_bytes(i + (*buf), j-i, (*to) - j);
-                   *to -= j-i;
-                   ++i;
-               } else {
-                   pam_syslog(pamh, LOG_CRIT,
-                              "internal error in file %s at line %d",
-                              __FILE__, __LINE__);
-                   close(fd);
-                   return -1;
-               }
+               *state = STATE_COMMENT;
                break;
+
+           case FIELD_SEPARATOR:
+               *state = STATE_FIELD;
+                *to = '\0';
+               *from = (src - *buf) + 1;
+               trim_spaces(*buf, to);
+               return fd;
+
            case '\\':
-               if ((*buf)[i+1] == '\n') {
-                   shift_bytes(i + *buf, 2, *to - (i+2));
-                   *to -= 2;
-               } else {
-                   ++i;   /* we don't escape non-newline characters */
+               if (src[1] == '\n') {
+                   ++src; /* skip it */
+                   break;
                }
-               break;
-           case '!':
-           case ' ':
-           case '\t':
-               if ((*buf)[i] != '!')
-                   (*buf)[i] = ',';
-               /* delete any trailing spaces */
-               for (j=++i; j < *to && ( (c = (*buf)[j]) == ' '
-                                        || c == '\t' ); ++j);
-               shift_bytes(i + *buf, j-i, (*to)-j );
-               *to -= j-i;
-               break;
            default:
-               ++i;
-           }
+               *to++ = c;
+               onspace = 0;
        }
+       if (src > to)
+           *src = '\0'; /* clearing */
     }
 
-    (*buf)[*to] = '\0';
-
-    /* now return the next field (set the from/to markers) */
-    {
-       int i;
-
-       for (i=0; i<*to; ++i) {
-           switch ((*buf)[i]) {
-           case '#':
-           case '\n':               /* end of the line/file */
-               (*buf)[i] = '\0';
-               *from = i;
-               return fd;
-           case FIELD_SEPARATOR:    /* end of the field */
-               (*buf)[i] = '\0';
-           *from = ++i;
-           return fd;
-           }
-       }
-       *from = i;
-       (*buf)[*from] = '\0';
+    if (*state != STATE_COMMENT) {
+       *state = STATE_COMMENT;
+       pam_syslog(pamh, LOG_ERR, "field too long - ignored");
+       **buf = '\0';
+    } else {
+       *to = '\0';
+       trim_spaces(*buf, to);
     }
 
-    if (*to <= 0) {
-       D(("[end of text]"));
-       *buf = NULL;
-    }
+    *from = 0;
     return fd;
 }
 
@@ -582,7 +573,7 @@ static int mkgrplist(pam_handle_t *pamh, char *buf, gid_t **list, int len)
 static int check_account(pam_handle_t *pamh, const char *service,
                         const char *tty, const char *user)
 {
-    int from=0,to=0,fd=-1;
+    int from=0, state=STATE_NL, fd=-1;
     char *buffer=NULL;
     int count=0;
     TIME here_and_now;
@@ -627,7 +618,7 @@ static int check_account(pam_handle_t *pamh, const char *service,
 
        /* here we get the service name field */
 
-       fd = read_field(pamh,fd,&buffer,&from,&to);
+       fd = read_field(pamh, fd, &buffer, &from, &state);
        if (!buffer || !buffer[0]) {
            /* empty line .. ? */
            continue;
@@ -635,15 +626,21 @@ static int check_account(pam_handle_t *pamh, const char *service,
        ++count;
        D(("working on rule #%d",count));
 
+       if (state != STATE_FIELD) {
+           pam_syslog(pamh, LOG_ERR,
+                      "%s: malformed rule #%d", PAM_GROUP_CONF, count);
+           continue;
+       }
+
        good = logic_field(pamh,service, buffer, count, is_same);
        D(("with service: %s", good ? "passes":"fails" ));
 
        /* here we get the terminal name field */
 
-       fd = read_field(pamh,fd,&buffer,&from,&to);
-       if (!buffer || !buffer[0]) {
+       fd = read_field(pamh, fd, &buffer, &from, &state);
+       if (state != STATE_FIELD) {
            pam_syslog(pamh, LOG_ERR,
-                      "%s: no tty entry #%d", PAM_GROUP_CONF, count);
+                      "%s: malformed rule #%d", PAM_GROUP_CONF, count);
            continue;
        }
        good &= logic_field(pamh,tty, buffer, count, is_same);
@@ -651,10 +648,10 @@ static int check_account(pam_handle_t *pamh, const char *service,
 
        /* here we get the username field */
 
-       fd = read_field(pamh,fd,&buffer,&from,&to);
-       if (!buffer || !buffer[0]) {
+       fd = read_field(pamh, fd, &buffer, &from, &state);
+       if (state != STATE_FIELD) {
            pam_syslog(pamh, LOG_ERR,
-                      "%s: no user entry #%d", PAM_GROUP_CONF, count);
+                      "%s: malformed rule #%d", PAM_GROUP_CONF, count);
            continue;
        }
        /* If buffer starts with @, we are using netgroups */
@@ -669,20 +666,20 @@ static int check_account(pam_handle_t *pamh, const char *service,
 
        /* here we get the time field */
 
-       fd = read_field(pamh,fd,&buffer,&from,&to);
-       if (!buffer || !buffer[0]) {
+       fd = read_field(pamh, fd, &buffer, &from, &state);
+       if (state != STATE_FIELD) {
            pam_syslog(pamh, LOG_ERR,
-                      "%s: no time entry #%d", PAM_GROUP_CONF, count);
+                      "%s: malformed rule #%d", PAM_GROUP_CONF, count);
            continue;
        }
 
        good &= logic_field(pamh,&here_and_now, buffer, count, check_time);
        D(("with time: %s", good ? "passes":"fails" ));
 
-       fd = read_field(pamh,fd,&buffer,&from,&to);
-       if (!buffer || !buffer[0]) {
+       fd = read_field(pamh, fd, &buffer, &from, &state);
+       if (state == STATE_FIELD) {
            pam_syslog(pamh, LOG_ERR,
-                      "%s: no listed groups for rule #%d", PAM_GROUP_CONF, count);
+                      "%s: poorly terminated rule #%d", PAM_GROUP_CONF, count);
            continue;
        }
 
@@ -701,14 +698,6 @@ static int check_account(pam_handle_t *pamh, const char *service,
            }
        }
 
-       /* check the line is terminated correctly */
-
-       fd = read_field(pamh,fd,&buffer,&from,&to);
-       if (buffer && buffer[0]) {
-           pam_syslog(pamh, LOG_ERR,
-                       "%s: poorly terminated rule #%d", PAM_GROUP_CONF, count);
-       }
-
        if (good > 0) {
            D(("rule #%d passed, added %d groups", count, good));
        } else if (good < 0) {
@@ -717,7 +706,7 @@ static int check_account(pam_handle_t *pamh, const char *service,
            D(("rule #%d failed", count));
        }
 
-    } while (buffer);
+    } while (state != STATE_EOF);
 
     /* now set the groups for the user */
 
index 7e418808036e88cd1f4df8d6abf74ef4643ee6be..dff4a6da012ed7e8ae6e3bffd72b1ec69e2ba00d 100644 (file)
@@ -4,6 +4,7 @@
  * Written by Andrew Morgan <morgan@linux.kernel.org> 1996/6/22
  * (File syntax and much other inspiration from the shadow package
  * shadow-960129)
+ * Field parsing rewritten by Tomas Mraz <tm@t8m.info>
  */
 
 #include "config.h"
@@ -23,7 +24,7 @@
 #include <netdb.h>
 
 #ifdef HAVE_LIBAUDIT
-#include <libaudit.h>                                                                                                                                            
+#include <libaudit.h>
 #endif
 
 #define PAM_TIME_BUFLEN        1000
@@ -79,163 +80,157 @@ _pam_parse (const pam_handle_t *pamh, int argc, const char **argv)
 
 /* --- static functions for checking whether the user should be let in --- */
 
-static void
-shift_bytes(char *mem, int from, int by)
+static char *
+shift_buf(char *mem, int from)
 {
-    while (by-- > 0) {
-       *mem = mem[from];
+    char *start = mem;
+    while ((*mem = mem[from]) != '\0')
        ++mem;
+    memset(mem, '\0', PAM_TIME_BUFLEN - (mem - start));
+    return mem;
+}
+
+static void
+trim_spaces(char *buf, char *from)
+{
+    while (from > buf) {
+       --from;
+       if (*from == ' ')
+           *from = '\0';
+       else
+           break;
     }
 }
 
+#define STATE_NL       0 /* new line starting */
+#define STATE_COMMENT  1 /* inside comment */
+#define STATE_FIELD    2 /* field following */
+#define STATE_EOF      3 /* end of file or error */
+
 static int
-read_field(const pam_handle_t *pamh, int fd, char **buf, int *from, int *to)
+read_field(const pam_handle_t *pamh, int fd, char **buf, int *from, int *state)
 {
-    /* is buf set ? */
+    char *to;
+    char *src;
+    int i;
+    char c;
+    int onspace;
 
+    /* is buf set ? */
     if (! *buf) {
-       *buf = (char *) malloc(PAM_TIME_BUFLEN);
+       *buf = (char *) calloc(1, PAM_TIME_BUFLEN+1);
        if (! *buf) {
            pam_syslog(pamh, LOG_ERR, "out of memory");
            D(("no memory"));
+           *state = STATE_EOF;
            return -1;
        }
-       *from = *to = 0;
+       *from = 0;
+        *state = STATE_NL;
        fd = open(PAM_TIME_CONF, O_RDONLY);
+       if (fd < 0) {
+           pam_syslog(pamh, LOG_ERR, "error opening %s: %m", PAM_TIME_CONF);
+           _pam_drop(*buf);
+           *state = STATE_EOF;
+           return -1;
+       }
     }
 
-    /* do we have a file open ? return error */
-
-    if (fd < 0 && *to <= 0) {
-       pam_syslog(pamh, LOG_ERR, "error opening %s: %m", PAM_TIME_CONF);
-       memset(*buf, 0, PAM_TIME_BUFLEN);
-       _pam_drop(*buf);
-       return -1;
-    }
-
-    /* check if there was a newline last time */
-
-    if ((*to > *from) && (*to > 0)
-       && ((*buf)[*from] == '\0')) { /* previous line ended */
-       (*from)++;
-       (*buf)[0] = '\0';
-       return fd;
-    }
-
-    /* ready for more data: first shift the buffer's remaining data */
-
-    *to -= *from;
-    shift_bytes(*buf, *from, *to);
-    *from = 0;
-    (*buf)[*to] = '\0';
-
-    while (fd >= 0 && *to < PAM_TIME_BUFLEN) {
-       int i;
-
-       /* now try to fill the remainder of the buffer */
+    if (*from > 0)
+       to = shift_buf(*buf, *from);
+    else
+       to = *buf;
 
-       i = read(fd, *to + *buf, PAM_TIME_BUFLEN - *to);
+    while (fd != -1 && to - *buf < PAM_TIME_BUFLEN) {
+       i = pam_modutil_read(fd, to, PAM_TIME_BUFLEN - (to - *buf));
        if (i < 0) {
            pam_syslog(pamh, LOG_ERR, "error reading %s: %m", PAM_TIME_CONF);
            close(fd);
+           memset(*buf, 0, PAM_TIME_BUFLEN);
+           _pam_drop(*buf);
+           *state = STATE_EOF;
            return -1;
        } else if (!i) {
            close(fd);
            fd = -1;          /* end of file reached */
-       } else
-           *to += i;
+       }
 
-       /*
-        * contract the buffer. Delete any comments, and replace all
-        * multiple spaces with single commas
-        */
+       to += i;
+    }
 
-       i = 0;
-#ifdef DEBUG_DUMP
-       D(("buffer=<%s>",*buf));
-#endif
-       while (i < *to) {
-           if ((*buf)[i] == ',') {
-               int j;
-
-               for (j=++i; j<*to && (*buf)[j] == ','; ++j);
-               if (j!=i) {
-                   shift_bytes(i + (*buf), j-i, (*to) - j);
-                   *to -= j-i;
-               }
-           }
-           switch ((*buf)[i]) {
-               int j,c;
-           case '#':
-                c = 0;
-               for (j=i; j < *to && (c = (*buf)[j]) != '\n'; ++j);
-               if (j >= *to) {
-                   (*buf)[*to = ++i] = '\0';
-               } else if (c == '\n') {
-                   shift_bytes(i + (*buf), j-i, (*to) - j);
-                   *to -= j-i;
-                   ++i;
-               } else {
-                   pam_syslog(pamh, LOG_CRIT,
-                              "internal error in file %s at line %d",
-                              __FILE__, __LINE__);
-                   close(fd);
-                   return -1;
-               }
-               break;
-           case '\\':
-               if ((*buf)[i+1] == '\n') {
-                   shift_bytes(i + *buf, 2, *to - (i+2));
-                   *to -= 2;
-               } else {
-                   ++i;   /* we don't escape non-newline characters */
-               }
-               break;
-           case '!':
-           case ' ':
-           case '\t':
-               if ((*buf)[i] != '!')
-                   (*buf)[i] = ',';
-               /* delete any trailing spaces */
-               for (j=++i; j < *to && ( (c = (*buf)[j]) == ' '
-                                        || c == '\t' ); ++j);
-               shift_bytes(i + *buf, j-i, (*to)-j );
-               *to -= j-i;
-               break;
-           default:
-               ++i;
-           }
-       }
+    if (to == *buf) {
+       /* nothing previously in buf, nothing read */
+       _pam_drop(*buf);
+       *state = STATE_EOF;
+       return -1;
     }
 
-    (*buf)[*to] = '\0';
+    memset(to, '\0', PAM_TIME_BUFLEN - (to - *buf));
 
-    /* now return the next field (set the from/to markers) */
-    {
-       int i;
+    to = *buf;
+    onspace = 1; /* delete any leading spaces */
+
+    for (src = to; (c=*src) != '\0'; ++src) {
+       if (*state == STATE_COMMENT && c != '\n') {
+               continue;
+       }
+
+       switch (c) {
+           case '\n':
+               *state = STATE_NL;
+                *to = '\0';
+               *from = (src - *buf) + 1;
+               trim_spaces(*buf, to);
+               return fd;
+
+           case '\t':
+            case ' ':
+               if (!onspace) {
+                   onspace = 1;
+                   *to++ = ' ';
+               }
+               break;
+
+            case '!':
+               onspace = 1; /* ignore following spaces */
+               *to++ = '!';
+               break;
 
-       for (i=0; i<*to; ++i) {
-           switch ((*buf)[i]) {
            case '#':
-           case '\n':               /* end of the line/file */
-               (*buf)[i] = '\0';
-               *from = i;
+               *state = STATE_COMMENT;
+               break;
+
+           case FIELD_SEPARATOR:
+               *state = STATE_FIELD;
+                *to = '\0';
+               *from = (src - *buf) + 1;
+               trim_spaces(*buf, to);
                return fd;
-           case FIELD_SEPARATOR:    /* end of the field */
-               (*buf)[i] = '\0';
-           *from = ++i;
-           return fd;
-           }
+
+           case '\\':
+               if (src[1] == '\n') {
+                   ++src; /* skip it */
+                   break;
+               }
+           default:
+               *to++ = c;
+               onspace = 0;
        }
-       *from = i;
-       (*buf)[*from] = '\0';
+       if (src > to)
+           *src = '\0'; /* clearing */
     }
 
-    if (*to <= 0) {
-       D(("[end of text]"));
-       *buf = NULL;
+    if (*state != STATE_COMMENT) {
+       *state = STATE_COMMENT;
+       pam_syslog(pamh, LOG_ERR, "field too long - ignored");
+       **buf = '\0';
+    } else {
+       *to = '\0';
+       trim_spaces(*buf, to);
     }
 
+    *from = 0;
     return fd;
 }
 
@@ -511,7 +506,7 @@ static int
 check_account(pam_handle_t *pamh, const char *service,
              const char *tty, const char *user)
 {
-     int from=0,to=0,fd=-1;
+     int from=0, state=STATE_NL, fd=-1;
      char *buffer=NULL;
      int count=0;
      TIME here_and_now;
@@ -523,23 +518,28 @@ check_account(pam_handle_t *pamh, const char *service,
 
          /* here we get the service name field */
 
-         fd = read_field(pamh, fd, &buffer, &from, &to);
-
+         fd = read_field(pamh, fd, &buffer, &from, &state);
          if (!buffer || !buffer[0]) {
               /* empty line .. ? */
               continue;
          }
          ++count;
 
+         if (state != STATE_FIELD) {
+              pam_syslog(pamh, LOG_ERR,
+                         "%s: malformed rule #%d", PAM_TIME_CONF, count);
+              continue;
+         }
+
          good = logic_field(pamh, service, buffer, count, is_same);
          D(("with service: %s", good ? "passes":"fails" ));
 
          /* here we get the terminal name field */
 
-         fd = read_field(pamh, fd, &buffer, &from, &to);
-         if (!buffer || !buffer[0]) {
+         fd = read_field(pamh, fd, &buffer, &from, &state);
+         if (state != STATE_FIELD) {
               pam_syslog(pamh, LOG_ERR,
-                         "%s: no tty entry #%d", PAM_TIME_CONF, count);
+                         "%s: malformed rule #%d", PAM_TIME_CONF, count);
               continue;
          }
          good &= logic_field(pamh, tty, buffer, count, is_same);
@@ -547,10 +547,10 @@ check_account(pam_handle_t *pamh, const char *service,
 
          /* here we get the username field */
 
-         fd = read_field(pamh, fd, &buffer, &from, &to);
-         if (!buffer || !buffer[0]) {
+         fd = read_field(pamh, fd, &buffer, &from, &state);
+         if (state != STATE_FIELD) {
               pam_syslog(pamh, LOG_ERR,
-                         "%s: no user entry #%d", PAM_TIME_CONF, count);
+                         "%s: malformed rule #%d", PAM_TIME_CONF, count);
               continue;
          }
          /* If buffer starts with @, we are using netgroups */
@@ -562,23 +562,16 @@ check_account(pam_handle_t *pamh, const char *service,
 
          /* here we get the time field */
 
-         fd = read_field(pamh, fd, &buffer, &from, &to);
-         if (!buffer || !buffer[0]) {
+         fd = read_field(pamh, fd, &buffer, &from, &state);
+         if (state == STATE_FIELD) {
               pam_syslog(pamh, LOG_ERR,
-                         "%s: no time entry #%d", PAM_TIME_CONF, count);
+                         "%s: poorly terminated rule #%d", PAM_TIME_CONF, count);
               continue;
          }
 
          intime = logic_field(pamh, &here_and_now, buffer, count, check_time);
          D(("with time: %s", intime ? "passes":"fails" ));
 
-         fd = read_field(pamh, fd, &buffer, &from, &to);
-         if (buffer && buffer[0]) {
-              pam_syslog(pamh, LOG_ERR,
-                          "%s: poorly terminated rule #%d", PAM_TIME_CONF, count);
-              continue;
-         }
-
          if (good && !intime) {
               /*
                * for security parse whole file..  also need to ensure
@@ -588,7 +581,7 @@ check_account(pam_handle_t *pamh, const char *service,
          } else {
               D(("rule passed"));
          }
-     } while (buffer);
+     } while (state != STATE_EOF);
 
      return retval;
 }