More debugging fixes.

1 files changed, 99 insertions, 70 deletions

diff --git a/daemon/ldap.c b/daemon/ldap.c
index b6da6d4..9a79d43 100644
--- a/daemon/ldap.c
+++ b/daemon/ldap.c
@@ -96,14 +96,14 @@ static const ldap_context_t ldap_defaults =
 {
   NULL,               /* servers */
   NULL,               /* filter */
-  "",                 /* base */
+  NULL,               /* base */
   "userPassword",     /* pw_attr */
   NULL,               /* ha1_attr */
   NULL,               /* user */
   NULL,               /* password */
   NULL,               /* dnmap */
   389,                /* port */
-  LDAP_SCOPE_DEFAULT, /* scope */
+  LDAP_SCOPE_SUBTREE, /* scope */
   "",                 /* realm */
   NULL,               /* domains */
   1,                  /* dobind */
@@ -130,7 +130,7 @@ static void free_hash_object(void* arg, void* val)
     free(val);
 }
 
-static int report_ldap(const char* msg, int code, ha_response_t* resp)
+static int report_ldap(const char* msg, int code)
 {
   ASSERT(code != LDAP_SUCCESS);
 
@@ -142,13 +142,10 @@ static int report_ldap(const char* msg, int code, ha_response_t* resp)
   switch(code)
   {
   case LDAP_NO_MEMORY:
-    return HA_ERROR;
+    return HA_CRITERROR;
 
   default:
-    if(resp)
-      resp->code = HA_SERVER_ERROR;
-
-    return HA_FALSE;
+    return HA_FAILED;
   };
 }
 
@@ -197,7 +194,10 @@ static int save_cached_digest(ldap_context_t* ctx, digest_record_t* rec)
   ASSERT(ctx && rec);
 
   if(ctx->cache_max == 0)
+  {
+    free_hash_object(NULL, rec);
     return HA_FALSE;
+  }
 
   ha_lock(NULL);
 
@@ -211,7 +211,7 @@ static int save_cached_digest(ldap_context_t* ctx, digest_record_t* rec)
   if(!r)
   {
     ha_messagex(LOG_CRIT, "out of memory");
-    return HA_ERROR;
+    return HA_CRITERROR;
   }
 
   return HA_OK;
@@ -238,7 +238,7 @@ static int add_cached_basic(ldap_context_t* ctx, unsigned char* key)
   if(!r)
   {
     ha_messagex(LOG_CRIT, "out of memory");
-    return HA_ERROR;
+    return HA_CRITERROR;
   }
 
   return HA_OK;
@@ -282,6 +282,12 @@ static const char* substitute_params(ldap_context_t* ctx, ha_buffer_t* buf,
       ha_bufcpy(buf, ctx->realm);
       t++;
       break;
+
+    case '%':
+      ha_bufjoin(buf);
+      ha_bufcpy(buf, "%");
+      t++;
+      break;
     };
 
     str = t;
@@ -351,7 +357,7 @@ static int parse_ldap_password(const char** password)
   /* find a scheme in our map */
   for(i = 0; i < countof(kLDAPPWTypes); i++)
   {
-    if(strncasecmp(kLDAPPWTypes[i].name, scheme, pw - scheme))
+    if(strncasecmp(kLDAPPWTypes[i].name, scheme, pw - scheme) == 0)
       return kLDAPPWTypes[i].type;
   }
 
@@ -412,13 +418,14 @@ static int parse_ldap_ha1(ha_buffer_t* buf, struct berval* bv, unsigned char* ha
     }
   }
 
-  return ha_buferr(buf) ? HA_ERROR : HA_FALSE;
+  return ha_buferr(buf) ? HA_CRITERROR : HA_FALSE;
 }
 
 static int validate_ldap_password(ldap_context_t* ctx, LDAP* ld, LDAPMessage* entry,
                                   ha_buffer_t* buf, const char* user, const char* clearpw)
 {
   char** pws;
+  char** t;
   const char* pw;
   const char* p;
   int type;
@@ -432,9 +439,9 @@ static int validate_ldap_password(ldap_context_t* ctx, LDAP* ld, LDAPMessage* en
 
   if(pws)
   {
-    for( ; *pws; pws++)
+    for(t = pws ; *t; t++)
     {
-      pw = *pws;
+      pw = *t;
       type = parse_ldap_password(&pw);
 
       switch(type)
@@ -470,7 +477,7 @@ static int validate_ldap_password(ldap_context_t* ctx, LDAP* ld, LDAPMessage* en
 
       if(!p)
       {
-        res = HA_ERROR;
+        res = HA_CRITERROR;
         break;
       }
 
@@ -494,6 +501,7 @@ static int validate_ldap_ha1(ldap_context_t* ctx, LDAP* ld, LDAPMessage* entry,
                              ha_buffer_t* buf, const char* user, const char* clearpw)
 {
   struct berval** ha1s;
+  struct berval** b;
   unsigned char key[MD5_LEN];
   unsigned char k[MD5_LEN];
   int r, first = 1;
@@ -510,10 +518,10 @@ static int validate_ldap_ha1(ldap_context_t* ctx, LDAP* ld, LDAPMessage* entry,
   {
     digest_makeha1(key, user, ctx->realm, clearpw);
 
-    for( ; *ha1s; ha1s++)
+    for(b = ha1s ; *b; b++)
     {
-      r = parse_ldap_ha1(buf, *ha1s, k);
-      if(r == HA_ERROR)
+      r = parse_ldap_ha1(buf, *b, k);
+      if(r < 0)
       {
         res = r;
         break;
@@ -578,7 +586,7 @@ static LDAP* get_ldap_connection(ldap_context_t* ctx)
                            ctx->password ? ctx->password : "");
     if(r != LDAP_SUCCESS)
     {
-      report_ldap("couldn't bind to LDAP server", r, NULL);
+      report_ldap("couldn't bind to LDAP server", r);
       ldap_unbind_s(ld);
       return NULL;
     }
@@ -645,7 +653,7 @@ static int retrieve_user_entry(ldap_context_t* ctx, ha_buffer_t* buf, LDAP* ld,
     /* Filters can also have %u and %r */
     filter = substitute_params(ctx, buf, user, ctx->filter);
     if(!filter)
-      return HA_ERROR;
+      return HA_CRITERROR;
   }
   else
   {
@@ -664,7 +672,15 @@ static int retrieve_user_entry(ldap_context_t* ctx, ha_buffer_t* buf, LDAP* ld,
                      filter, (char**)attrs, 0, &tv, result);
 
   if(r != LDAP_SUCCESS)
-    return report_ldap("couldn't search LDAP server", r, NULL);
+  {
+    if(r == LDAP_NO_SUCH_OBJECT)
+    {
+      ha_messagex(LOG_WARNING, "user not found in LDAP: %s", user);
+      return HA_FALSE;
+    }
+
+    return report_ldap("couldn't search LDAP server", r);
+  }
 
 
   /* Only one result should exist */
@@ -685,28 +701,27 @@ static int retrieve_user_entry(ldap_context_t* ctx, ha_buffer_t* buf, LDAP* ld,
     break;
   };
 
-  ldap_msgfree(*result);
   return HA_FALSE;
 }
 
 static int complete_digest_ha1(ldap_context_t* ctx, digest_record_t* rec,
-                               ha_buffer_t* buf, const char* user, int* code)
+                               ha_buffer_t* buf, const char* user)
 {
   LDAP* ld = NULL;                /* freed in finally */
   LDAPMessage* results = NULL;    /* freed in finally */
   LDAPMessage* entry = NULL;      /* no need to free */
-  struct berval** ha1s;            /* freed manually */
+  struct berval** ha1s = NULL;    /* freed manually */
   char** pws;
   int ret = HA_FALSE;
-  const char* dn;
+  const char* dn = NULL;
   int r;
 
-  ASSERT(ctx && rec && buf && user && code);
+  ASSERT(ctx && rec && buf && user);
 
   ld = get_ldap_connection(ctx);
   if(!ld)
   {
-    *code = HA_SERVER_ERROR;
+    ret = HA_FAILED;
     goto finally;
   }
 
@@ -721,7 +736,7 @@ static int complete_digest_ha1(ldap_context_t* ctx, digest_record_t* rec,
     dn = substitute_params(ctx, buf, user, ctx->dnmap);
     if(!dn)
     {
-      ret = HA_ERROR;
+      ret = HA_CRITERROR;
       goto finally;
     }
   }
@@ -742,12 +757,10 @@ static int complete_digest_ha1(ldap_context_t* ctx, digest_record_t* rec,
   {
     if(*ha1s)
     {
-      r = parse_ldap_ha1(buf, *ha1s, rec->ha1);
-      if(r != HA_OK)
+      ret = parse_ldap_ha1(buf, *ha1s, rec->ha1);
+      if(ret != HA_OK)
       {
-        ret = r;
-
-        if(ret != HA_FALSE)
+        if(ret == HA_FALSE)
           ha_messagex(LOG_ERR, "LDAP contains invalid HA1 digest hash for user: %s", user);
       }
     }
@@ -764,14 +777,16 @@ static int complete_digest_ha1(ldap_context_t* ctx, digest_record_t* rec,
     /* Find a cleartext password */
     const char* t = find_cleartext_password(buf, (const char**)pws);
 
-    ldap_value_free(pws);
-
     if(t)
     {
       digest_makeha1(rec->ha1, user, ctx->realm, t);
       ret = HA_OK;
-      goto finally;
     }
+
+    ldap_value_free(pws);
+
+    if(ret == HA_OK)
+      goto finally;
   }
 
   ha_messagex(LOG_ERR, "LDAP contains no cleartext password for user: %s", user);
@@ -794,15 +809,15 @@ static int basic_ldap_response(ldap_context_t* ctx, const char* header,
   LDAP* ld = NULL;
   LDAPMessage* entry = NULL;
   LDAPMessage* results = NULL;
-  const char* dn;
+  const char* dn = NULL;
   int ret = HA_FALSE;
   int found = 0;
   int r;
 
   ASSERT(buf && header && resp && buf);
 
-  if(basic_parse(header, buf, &basic) == HA_ERROR)
-    return HA_ERROR;
+  if((r = basic_parse(header, buf, &basic)) < 0)
+    return r;
 
   /* Past this point we don't return directly */
 
@@ -823,7 +838,7 @@ static int basic_ldap_response(ldap_context_t* ctx, const char* header,
   ld = get_ldap_connection(ctx);
   if(!ld)
   {
-    resp->code = HA_SERVER_ERROR;
+    ret = HA_FAILED;
     goto finally;
   }
 
@@ -839,7 +854,7 @@ static int basic_ldap_response(ldap_context_t* ctx, const char* header,
     dn = substitute_params(ctx, buf, basic.user, ctx->dnmap);
     if(!dn)
     {
-      ret = HA_ERROR;
+      ret = HA_CRITERROR;
       goto finally;
     }
   }
@@ -882,7 +897,7 @@ static int basic_ldap_response(ldap_context_t* ctx, const char* header,
       if(r == LDAP_INVALID_CREDENTIALS)
         ha_messagex(LOG_WARNING, "invalid login for: %s", basic.user);
       else
-        report_ldap("couldn't bind to LDAP server", r, resp);
+        report_ldap("couldn't bind to LDAP server", r);
 
       goto finally;
     }
@@ -903,7 +918,7 @@ static int basic_ldap_response(ldap_context_t* ctx, const char* header,
       found = 1;
 
     else
-      ha_messagex(LOG_WARNING, "invalid or unrecognized password for user: %s", basic.user);
+      ha_messagex(LOG_WARNING, "invalid, unreadable or unrecognized password for user: %s", basic.user);
   }
 
 
@@ -915,7 +930,7 @@ finally:
   if(results)
     ldap_msgfree(results);
 
-  if(found && ret != HA_ERROR)
+  if(found && ret >= 0)
   {
     resp->code = HA_SERVER_ACCEPT;
     resp->detail = basic.user;
@@ -950,14 +965,14 @@ static int digest_ldap_challenge(ldap_context_t* ctx, ha_response_t* resp,
 
     nonce_str = ha_bufenchex(buf, nonce, DIGEST_NONCE_LEN);
     if(!nonce_str)
-      return HA_ERROR;
+      return HA_CRITERROR;
   }
 
   /* Now generate a message to send */
   header = digest_challenge(buf, nonce_str, ctx->realm, ctx->domains, stale);
 
   if(!header)
-    return HA_ERROR;
+    return HA_CRITERROR;
 
   /* And append it nicely */
   resp->code = HA_SERVER_DECLINE;
@@ -984,8 +999,8 @@ static int digest_ldap_response(ldap_context_t* ctx, const char* header,
   /* We use this below to send a default response */
   resp->code = -1;
 
-  if(digest_parse(header, buf, &dg, nonce) == HA_ERROR)
-    return HA_ERROR;
+  if((r = digest_parse(header, buf, &dg, nonce)) < 0)
+    return r;
 
 #ifdef _DEBUG
   if(ctx->debug_nonce)
@@ -993,6 +1008,7 @@ static int digest_ldap_response(ldap_context_t* ctx, const char* header,
     if(dg.nonce && strcmp(dg.nonce, ctx->debug_nonce) != 0)
     {
       ret = HA_FALSE;
+      resp->code = HA_SERVER_BADREQ;
       ha_messagex(LOG_WARNING, "digest response contains invalid nonce");
       goto finally;
     }
@@ -1010,9 +1026,11 @@ static int digest_ldap_response(ldap_context_t* ctx, const char* header,
     if(r != HA_OK)
     {
       if(r == HA_FALSE)
+      {
+        resp->code = HA_SERVER_BADREQ;
         ha_messagex(LOG_WARNING, "digest response contains invalid nonce");
+      }
 
-      ret = r;
       goto finally;
     }
   }
@@ -1037,11 +1055,11 @@ static int digest_ldap_response(ldap_context_t* ctx, const char* header,
     rec = digest_makerec(nonce, dg.username);
     if(!rec)
     {
-      ret = HA_ERROR;
+      ret = HA_CRITERROR;
       goto finally;
     }
 
-    r = complete_digest_ha1(ctx, rec, buf, dg.username, &(resp->code));
+    r = complete_digest_ha1(ctx, rec, buf, dg.username);
     if(r != HA_OK)
     {
       ret = r;
@@ -1054,7 +1072,13 @@ static int digest_ldap_response(ldap_context_t* ctx, const char* header,
 
   ret = digest_check(ctx->realm, method, uri, buf, &dg, rec);
 
-  if(ret == HA_OK)
+  if(ret == HA_BADREQ)
+  {
+    ret = HA_FALSE;
+    resp->code = HA_SERVER_BADREQ;
+  }
+
+  else if(ret == HA_OK)
   {
     resp->code = HA_SERVER_ACCEPT;
     resp->detail = dg.username;
@@ -1069,7 +1093,7 @@ static int digest_ldap_response(ldap_context_t* ctx, const char* header,
     t = digest_respond(buf, &dg, rec, stale ? nonce : NULL);
     if(!t)
     {
-      ret = HA_ERROR;
+      ret = HA_CRITERROR;
       goto finally;
     }
 
@@ -1077,10 +1101,10 @@ static int digest_ldap_response(ldap_context_t* ctx, const char* header,
       ha_addheader(resp, "Authentication-Info", t);
 
     /* Put the connection into the cache */
-    if(save_cached_digest(ctx, rec) == HA_ERROR)
-      ret = HA_ERROR;
-    else
-      rec = NULL;
+    if((r = save_cached_digest(ctx, rec)) < 0)
+      ret = r;
+
+    rec = NULL;
   }
 
 finally:
@@ -1179,7 +1203,7 @@ int ldap_config(ha_context_t* context, const char* name, const char* value)
     else
     {
       ha_messagex(LOG_ERR, "invalid value for '%s' (must be 'sub', 'base' or 'one')", name);
-      return HA_ERROR;
+      return HA_FAILED;
     }
 
     return HA_OK;
@@ -1232,24 +1256,29 @@ int ldap_inithand(ha_context_t* context)
     {
       ha_messagex(LOG_ERR, "LDAP module configured, but does not implement any "
                            "configured authentication type.");
-      return HA_ERROR;
+      return HA_FAILED;
     }
 
     /* Check for mandatory configuration */
-    if(!ctx->servers || !(ctx->dnmap || ctx->filter))
+    if(!ctx->servers)
     {
-      ha_messagex(LOG_ERR, "Digest LDAP configuration incomplete. "
-                           "Must have LDAPServers and either LDAPFilter or LDAPDNMap.");
-      return HA_ERROR;
+      ha_messagex(LOG_ERR, "LDAP configuration incomplete. "
+                           "Must have LDAPServers.");
+      return HA_FAILED;
     }
 
-    ASSERT(!ctx->cache);
+    if(!ctx->dnmap && (!ctx->filter || !ctx->base))
+    {
+      ha_messagex(LOG_ERR, "LDAP configuration incomplete. "
+                           "When not using LDAPDNMap must specify LDAPBase and LDAPFilter.");
+      return HA_FAILED;
+    }
 
     /* The cache for digest records and basic */
     if(!(ctx->cache = hash_create(MD5_LEN, free_hash_object, NULL)))
     {
       ha_messagex(LOG_CRIT, "out of memory");
-      return HA_ERROR;
+      return HA_CRITERROR;
     }
 
     ASSERT(!ctx->pool);
@@ -1264,7 +1293,7 @@ int ldap_inithand(ha_context_t* context)
     if(!ctx->pool)
     {
       ha_messagex(LOG_CRIT, "out of memory");
-      return HA_ERROR;
+      return HA_CRITERROR;
     }
 
     memset(ctx->pool, 0, sizeof(LDAP*) * ctx->ldap_max);
@@ -1338,7 +1367,7 @@ int ldap_process(ha_context_t* context, ha_request_t* req,
     {
       ret = digest_ldap_response(ctx, header, req->args[AUTH_ARG_METHOD],
                                  req->args[AUTH_ARG_URI], resp, buf);
-      if(ret == HA_ERROR)
+      if(ret < 0)
         return ret;
     }
   }
@@ -1350,7 +1379,7 @@ int ldap_process(ha_context_t* context, ha_request_t* req,
     if(header)
     {
       ret = basic_ldap_response(ctx, header, resp, buf);
-      if(ret == HA_ERROR)
+      if(ret < 0)
         return ret;
     }
   }
@@ -1364,7 +1393,7 @@ int ldap_process(ha_context_t* context, ha_request_t* req,
     if(context->types & HA_TYPE_DIGEST)
     {
       ret = digest_ldap_challenge(ctx, resp, buf, 0);
-      if(ret == HA_ERROR)
+      if(ret < 0)
         return ret;
     }
 
@@ -1373,7 +1402,7 @@ int ldap_process(ha_context_t* context, ha_request_t* req,
       ha_bufmcat(buf, "BASIC realm=\"", ctx->realm , "\"", NULL);
 
       if(ha_buferr(buf))
-        return HA_ERROR;
+        return HA_CRITERROR;
 
       ha_addheader(resp, "WWW-Authenticate", ha_bufdata(buf));
     }