Debugging of simple authentication handler

7 files changed, 169 insertions, 90 deletions

diff --git a/daemon/digest.c b/daemon/digest.c
index 79b4ff3..099ca49 100644
--- a/daemon/digest.c
+++ b/daemon/digest.c
@@ -87,16 +87,13 @@ digest_record_t* digest_makerec(unsigned char* nonce, const char* user)
   return rec;
 }
 
-const char* digest_challenge(ha_buffer_t* buf, unsigned char* nonce,
+const char* digest_challenge(ha_buffer_t* buf, const char* nonce_str,
                              const char* realm, const char* domains, int stale)
 {
-  ASSERT(buf && realm && nonce);
+  ASSERT(buf && realm && nonce_str);
 
-  ha_bufmcat(buf, HA_PREFIX_DIGEST, " realm=\"", realm, "\", nonce=\"", NULL);
-  ha_bufjoin(buf);
-  ha_bufenc64(buf, nonce, DIGEST_NONCE_LEN);
-  ha_bufjoin(buf);
-  ha_bufmcat(buf, "\", qop=\"auth\", algorithm=\"MD5\"", NULL);
+  ha_bufmcat(buf, HA_PREFIX_DIGEST, " realm=\"", realm, "\", nonce=\"",
+              nonce_str, "\", qop=\"auth\", algorithm=\"MD5\"", NULL);
 
   if(domains)
   {
@@ -143,7 +140,7 @@ int digest_parse(char* header, ha_buffer_t* buf, digest_header_t* rec,
   if(!header)
     return HA_ERROR;
 
-  memset(rec, 0, sizeof(rec));
+  memset(rec, 0, sizeof(*rec));
 
   while(header[0])
   {
@@ -178,8 +175,6 @@ int digest_parse(char* header, ha_buffer_t* buf, digest_header_t* rec,
 
     if(next == '=')
     {
-      header++;
-
       while(header[0] && isspace(header[0]))
         header++;
 
@@ -191,6 +186,7 @@ int digest_parse(char* header, ha_buffer_t* buf, digest_header_t* rec,
         while(header[0] && header[0] != '\"')
           header++;
 
+        next = header[0];
         header[0] = 0;
         header++;
       }
@@ -202,15 +198,19 @@ int digest_parse(char* header, ha_buffer_t* buf, digest_header_t* rec,
         while(header[0] && header[0] != ',' && !isspace(header[0]))
           header++;
 
+        next = header[0];
         header[0] = 0;
         header++;
       }
 
-      while(header[0] && header[0] != ',')
-        header++;
+      if(next != ',')
+      {
+        while(header[0] && header[0] != ',')
+          header++;
 
-      if(header[0])
-        header++;
+        if(header[0])
+          header++;
+      }
 
       if(!strcasecmp(key, "username"))
         rec->username = value;
@@ -320,7 +320,7 @@ int digest_check(const char* realm, const char* method, const char* uri,
       char* e;
       long nc = strtol(dg->nc, &e, 10);
 
-      if(e != (dg->nc + strlen(e)) || nc != rec->nc)
+      if(*e || nc != rec->nc)
       {
         ha_messagex(LOG_WARNING, "digest response has invalid nc value: %s",
                     dg->nc);
@@ -455,6 +455,9 @@ const char* digest_respond(ha_buffer_t* buf, digest_header_t* dg,
 
   ASSERT(buf && dg && rec);
 
+  /* This makes a new buffer */
+  ha_bufcpy(buf, "");
+
   if(next)
   {
     nextnonce = ha_bufenc64(buf, next, DIGEST_NONCE_LEN);
@@ -464,7 +467,7 @@ const char* digest_respond(ha_buffer_t* buf, digest_header_t* dg,
   }
 
   /* For older clients RFC 2069 */
-  if(dg->qop)
+  if(!dg->qop)
   {
     if(nextnonce)
       ha_bufmcat(buf, "nextnonce=\"", nextnonce, "\"", NULL);
diff --git a/daemon/digest.h b/daemon/digest.h
index 6bd7acc..0f5e6b2 100644
--- a/daemon/digest.h
+++ b/daemon/digest.h
@@ -48,7 +48,7 @@ int ha_digestcheck(const char* realm, const char* method, const char* uri,
 const char* digest_respond(ha_buffer_t* buf, digest_header_t* dg,
                            digest_record_t* rec, unsigned char* next);
 
-const char* digest_challenge(ha_buffer_t* buf, unsigned char* nonce,
+const char* digest_challenge(ha_buffer_t* buf, const char* nonce_str,
                              const char* realm, const char* domains, int stale);
 
 void digest_makeha1(unsigned char* digest, const char* user,
diff --git a/daemon/httpauthd.c b/daemon/httpauthd.c
index a2985a3..465ac68 100644
--- a/daemon/httpauthd.c
+++ b/daemon/httpauthd.c
@@ -68,7 +68,7 @@ const char* kAuthHeaders[] =
 /* The command definitions */
 const httpauth_command_t kCommands[] =
 {
-  { "auth", REQTYPE_AUTH, 3, kAuthHeaders },
+  { "auth", REQTYPE_AUTH, 4, kAuthHeaders },
   { "quit", REQTYPE_QUIT, 0, 0            },
   { NULL,   -1, -1                        }
 };
@@ -434,6 +434,12 @@ int httpauth_read(int ifd, ha_request_t* req,
     }
   }
 
+  else
+  {
+    req->type = REQTYPE_IGNORE;
+    return more;
+  }
+
   /* Check for invalid command */
   if(req->type == -1)
     return more;
@@ -734,6 +740,8 @@ int httpauth_processor(int ifd, int ofd)
       result = 0;
       break;
 
+    case REQTYPE_IGNORE:
+      break;
 
     default:
       if(httpauth_respond(ofd, HA_SERVER_BADREQ, "Unknown command") == -1)
diff --git a/daemon/httpauthd.h b/daemon/httpauthd.h
index 55a4d90..7f51895 100644
--- a/daemon/httpauthd.h
+++ b/daemon/httpauthd.h
@@ -6,11 +6,13 @@
  * Memory Buffers
  */
 
+struct ha_buffer_internal;
+
 /* A buffer which owns memory */
 typedef struct ha_buffer
 {
-  int _al;
-  char* _dt;
+  struct ha_buffer_internal* _ft;
+  struct ha_buffer_internal* _dt;
   char* _pp;
   char* _rp;
 }
@@ -191,7 +193,7 @@ ha_context_t;
  * should be no need to change it unless we're
  * adding or removing commands
  */
-#define MAX_ARGS        6
+#define MAX_ARGS        4
 
 /*
  * The maximum number of pertinent headers to read
@@ -222,9 +224,9 @@ ha_header_t;
 #define REQTYPE_QUIT    1
 #define REQTYPE_AUTH    2
 
-#define AUTH_ARG_CONN   0
-#define AUTH_ARG_METHOD 1
-#define AUTH_ARG_URI    2
+#define AUTH_ARG_CONN   1
+#define AUTH_ARG_METHOD 2
+#define AUTH_ARG_URI    3
 
 /* A single request from client */
 typedef struct ha_request
diff --git a/daemon/ldap.c b/daemon/ldap.c
index 2474d09..440c531 100644
--- a/daemon/ldap.c
+++ b/daemon/ldap.c
@@ -920,15 +920,20 @@ static int digest_ldap_challenge(ldap_context_t* ctx, ha_response_t* resp,
                                  ha_buffer_t* buf, int stale)
 {
   unsigned char nonce[DIGEST_NONCE_LEN];
+  const char* nonce_str;
   const char* header;
 
   ASSERT(ctx && resp && buf);
 
   /* Generate an nonce */
   digest_makenonce(nonce, g_ldap_secret, NULL);
+  nonce_str = ha_bufenchex(buf, nonce, DIGEST_NONCE_LEN);
+
+  if(!nonce_str)
+    return HA_ERROR;
 
   /* Now generate a message to send */
-  header = digest_challenge(buf, nonce, ctx->realm, ctx->domains, stale);
+  header = digest_challenge(buf, nonce_str, ctx->realm, ctx->domains, stale);
 
   if(!header)
     return HA_ERROR;
diff --git a/daemon/misc.c b/daemon/misc.c
index c3f5ff4..9dba389 100644
--- a/daemon/misc.c
+++ b/daemon/misc.c
@@ -208,8 +208,7 @@ int ha_confint(const char* name, const char* conf, int min, int max, int* value)
   errno = 0;
   *value = strtol(conf, &p, 10);
 
-  if(p != (name + strlen(name)) || errno == ERANGE ||
-     (*value < min) || (*value > max))
+  if(*p || errno == ERANGE || (*value < min) || (*value > max))
   {
     ha_messagex(LOG_ERR, "invalid configuration value '%s': must be a number between %d and %d", name, min, max);
     return HA_ERROR;
diff --git a/daemon/simple.c b/daemon/simple.c
index d51e9d9..e33e833 100644
--- a/daemon/simple.c
+++ b/daemon/simple.c
@@ -33,20 +33,13 @@ typedef struct simple_context
 
   /* Context ----------------------------------------------------------- */
   hash_t* cache;            /* Some cached records or basic */
+
+#ifdef _DEBUG
+  const char* debug_nonce;
+#endif
 }
 simple_context_t;
 
-/* The defaults for the context */
-static const simple_context_t simple_defaults =
-{
-  NULL,               /* filename */
-  NULL,               /* realm */
-  NULL,               /* domains */
-  0,                  /* cache_max */
-  0,                  /* cache_timeout */
-  NULL                /* cache */
-};
-
 
 /* -------------------------------------------------------------------------------
  * Internal Functions
@@ -156,6 +149,7 @@ static int complete_digest_ha1(simple_context_t* ctx, digest_record_t* rec,
   FILE* f;
   int found = 0;
   char* t;
+  char* t2;
   char line[SIMPLE_MAXLINE];
 
   ASSERT(ctx && rec && buf && user && user[0] && code);
@@ -195,33 +189,39 @@ static int complete_digest_ha1(simple_context_t* ctx, digest_record_t* rec,
       /* Check the user */
       if(strcmp(line, user) == 0)
       {
-        /* Now decode the rest of the line and see if it matches up */
-        t = ha_bufdechex(buf, t, MD5_LEN);
-
-        if(t != NULL)
+        /* Otherwise it might be a digest type ha1. */
+        t2 = strchr(t, ':');
+        if(t2)
         {
-          memcpy(rec->ha1, t, MD5_LEN);
-          found = 1;
-          break;
+          *t2 = 0;
+          t2++;
+
+          /* Check the realm */
+          if(strcmp(t, ctx->realm) == 0)
+          {
+            /* Now try antd decode the ha1  */
+            t = ha_bufdechex(buf, t2, MD5_LEN);
+            if(t != NULL)
+            {
+              memcpy(rec->ha1, t, MD5_LEN);
+              found = 1;
+              break;
+            }
+          }
         }
 
-        else
-        {
-          if(ha_buferr(buf))
-            break;
-
+        if(!t2 || !found)
           ha_messagex(LOG_WARNING, "user '%s' found in file, but password not in digest format", user);
-        }
       }
     }
-
-    fclose(f);
   }
 
+  fclose(f);
+
   if(ha_buferr(buf))
     return HA_ERROR;
 
-  return found ? HA_FALSE : HA_OK;
+  return found ? HA_OK : HA_FALSE;
 }
 
 static int validate_user_password(simple_context_t* ctx, ha_buffer_t* buf,
@@ -264,6 +264,14 @@ static int validate_user_password(simple_context_t* ctx, ha_buffer_t* buf,
       break;
     }
 
+    /* Take white space off end of line */
+    t = line + strlen(line);
+    while(t != line && isspace(*(t - 1)))
+    {
+      *(t - 1) = 0;
+      t--;
+    }
+
     t = strchr(line, ':');
     if(t)
     {
@@ -274,40 +282,48 @@ static int validate_user_password(simple_context_t* ctx, ha_buffer_t* buf,
       /* Check the user */
       if(strcmp(line, user) == 0)
       {
-        /* We can validate against an ha1, so check if it decodes as one */
-        t2 = ha_bufdechex(buf, t, MD5_LEN);
+        /* Not sure if crypt is thread safe so we lock */
+        ha_lock(NULL);
 
-        if(t2 && memcmp(ha1, t2, MD5_LEN) == 0)
+          /* Check the password */
+          t2 = crypt(clearpw, t);
+
+        ha_unlock(NULL);
+
+        if(strcmp(crypt(clearpw, t), t) == 0)
         {
-          memcpy(ha1, t2, MD5_LEN);
           found = 1;
           break;
         }
 
-        /* Otherwise we try a nice crypt style password */
-        else
+        /* Otherwise it might be a digest type ha1. */
+        t2 = strchr(t, ':');
+        if(t2)
         {
-          /* Not sure if crypt is thread safe so we lock */
-          ha_lock();
-
-            /* Check the password */
-            if(strcmp(crypt(clearpw, t), t) == 0)
+          *t2 = 0;
+          t2++;
+
+          /* Check the realm */
+          if(strcmp(t, ctx->realm) == 0)
+          {
+            /* Now try antd decode the ha1  */
+            t = ha_bufdechex(buf, t2, MD5_LEN);
+            if(t && memcmp(ha1, t, MD5_LEN) == 0)
+            {
               found = 1;
-
-          ha_unlock();
-
-          if(found)
-            break;
+              break;
+            }
+          }
         }
 
         if(ha_buferr(buf))
           break;
       }
     }
-
-    fclose(f);
   }
 
+  fclose(f);
+
   if(ha_buferr(buf))
     return HA_ERROR;
 
@@ -347,8 +363,9 @@ static int simple_basic_response(simple_context_t* ctx, const char* header,
 
 finally:
 
-  if(resp->code == HA_SERVER_ACCEPT)
+  if(ret = HA_OK)
   {
+    resp->code = HA_SERVER_ACCEPT;
     resp->detail = basic.user;
 
     /* We put this connection into the successful connections */
@@ -361,16 +378,33 @@ finally:
 static int simple_digest_challenge(simple_context_t* ctx, ha_response_t* resp,
                             ha_buffer_t* buf, int stale)
 {
-  unsigned char nonce[DIGEST_NONCE_LEN];
+  const char* nonce_str;
   const char* header;
 
   ASSERT(ctx && resp && buf);
 
   /* Generate an nonce */
-  digest_makenonce(nonce, g_simple_secret, NULL);
+
+#ifdef _DEBUG
+  if(ctx->debug_nonce)
+  {
+    nonce_str = ctx->debug_nonce;
+    ha_messagex(LOG_WARNING, "using debug nonce. security non-existant.");
+  }
+  else
+#endif
+  {
+    unsigned char nonce[DIGEST_NONCE_LEN];
+    digest_makenonce(nonce, g_simple_secret, NULL);
+
+    nonce_str = ha_bufenchex(buf, nonce, DIGEST_NONCE_LEN);
+    if(!nonce_str)
+      return HA_ERROR;
+  }
+
 
   /* Now generate a message to send */
-  header = digest_challenge(buf, nonce, ctx->realm, ctx->domains, stale);
+  header = digest_challenge(buf, nonce_str, ctx->realm, ctx->domains, stale);
 
   if(!header)
     return HA_ERROR;
@@ -403,14 +437,34 @@ static int simple_digest_response(simple_context_t* ctx, const char* header,
   if(digest_parse(header, buf, &dg, nonce) == HA_ERROR)
     return HA_ERROR;
 
-  r = digest_checknonce(nonce, g_simple_secret, &expiry);
-  if(r != HA_OK)
+#ifdef _DEBUG
+  if(ctx->debug_nonce)
   {
-    if(r == HA_FALSE)
+    if(dg.nonce && strcmp(dg.nonce, ctx->debug_nonce) != 0)
+    {
+      ret = HA_FALSE;
       ha_messagex(LOG_WARNING, "digest response contains invalid nonce");
+      goto finally;
+    }
 
-    ret = r;
-    goto finally;
+    /* Do a rough hash into the real nonce, for use as a key */
+    md5_string(nonce, ctx->debug_nonce);
+
+    /* Debug nonce's never expire */
+    expiry = time(NULL);
+  }
+  else
+#endif
+  {
+    r = digest_checknonce(nonce, g_simple_secret, &expiry);
+    if(r != HA_OK)
+    {
+      if(r == HA_FALSE)
+        ha_messagex(LOG_WARNING, "digest response contains invalid nonce");
+
+      ret = r;
+      goto finally;
+    }
   }
 
   rec = get_cached_digest(ctx, nonce);
@@ -520,6 +574,14 @@ int simple_config(ha_context_t* context, const char* name, const char* value)
     return HA_OK;
   }
 
+#ifdef _DEBUG
+  else if(strcmp(name, "digestdebugnonce") == 0)
+  {
+    ctx->debug_nonce = value;
+    return HA_OK;
+  }
+#endif
+
   return HA_FALSE;
 }
 
@@ -600,7 +662,7 @@ int simple_process(ha_context_t* context, ha_request_t* req,
                  ha_response_t* resp, ha_buffer_t* buf)
 {
   simple_context_t* ctx = (simple_context_t*)(context->data);
-  const char* header;
+  const char* header = NULL;
   int ret = HA_FALSE;
   int found = 0;
   basic_header_t basic;
@@ -652,13 +714,6 @@ int simple_process(ha_context_t* context, ha_request_t* req,
   {
     resp->code = HA_SERVER_DECLINE;
 
-    if(context->types & HA_TYPE_DIGEST)
-    {
-      ret = simple_digest_challenge(ctx, resp, buf, 0);
-      if(ret == HA_ERROR)
-        return ret;
-    }
-
     if(context->types & HA_TYPE_BASIC)
     {
       ha_bufmcat(buf, "BASIC realm=\"", ctx->realm , "\"", NULL);
@@ -668,6 +723,13 @@ int simple_process(ha_context_t* context, ha_request_t* req,
 
       ha_addheader(resp, "WWW-Authenticate", ha_bufdata(buf));
     }
+
+    if(context->types & HA_TYPE_DIGEST)
+    {
+      ret = simple_digest_challenge(ctx, resp, buf, 0);
+      if(ret == HA_ERROR)
+        return ret;
+    }
   }
 
   return ret;
@@ -685,7 +747,7 @@ ha_handler_t simple_handler =
   simple_destroy,          /* Uninitialization routine */
   simple_config,           /* Config routine */
   simple_process,          /* Processing routine */
-  &simple_defaults,        /* A default context */
+  NULL,                    /* A default context */
   sizeof(simple_context_t) /* Size of the context */
 };