--------------------- PatchSet 629 Date: 2000/10/10 17:00:58 Author: kinkie Branch: ntlm Tag: (none) Log: Fixed DC load-balancing (it could infinite-loop due to a typo) Added Domain Controller status (don't reconnect to a dead DC) and resurrection checks. Added -s flags, to force strict protocol conformance (shouldn't be needed, but if somebody wishes to be extra-paranoid...) Moved fast-track-granting back to after figuring what's going on. The reason is that where I had formerly put it, it would have had a 50% guarranteed cache miss ratio. Since I believe that base64-decoding should be faster than half the cost of a missed cache lookup, I believe that it should be more performing this way. It's #ifdef'd for those who disagree with me. Fixed a few warning messages. There was a bug in caching (sizeof in pleace of strlen) which could cause heap overflows and authorization failures. Fixed. Members: ntlm_auth_modules/NTLMSSP/ntlm_auth.c:1.1.2.8->1.1.2.9 Index: squid/ntlm_auth_modules/NTLMSSP/ntlm_auth.c =================================================================== RCS file: /cvsroot/squid-sf//squid/ntlm_auth_modules/NTLMSSP/Attic/ntlm_auth.c,v retrieving revision 1.1.2.8 retrieving revision 1.1.2.9 diff -u -r1.1.2.8 -r1.1.2.9 --- squid/ntlm_auth_modules/NTLMSSP/ntlm_auth.c 10 Oct 2000 10:12:33 -0000 1.1.2.8 +++ squid/ntlm_auth_modules/NTLMSSP/ntlm_auth.c 10 Oct 2000 17:00:58 -0000 1.1.2.9 @@ -72,22 +72,22 @@ #endif time_t last_request; -char load_balance=0, failover_enabled=0; +char load_balance=0, failover_enabled=0, protocol_pedantic=0; char *domain = NULL, *domain_controller = NULL; /* cache-related hash and management functions */ -typedef struct _dc dc; -struct _dc { - char *domain; - char *controller; - dc *next; -}; - dc* controllers=NULL; int numcontrollers=0; dc *current_dc; +/* status in the current negotiation */ +int status=0; /* 0=waiting, 1=got neg, waiting for auth */ + +/* DC resurrection should be performed synchronously. */ +static unsigned char need_dc_resurrection=0; + + hash_table *cache; typedef struct _hash_entry { char *k; /* the entry's value */ @@ -111,6 +111,16 @@ } /* housekeeping cycle and periodic operations */ +static void resurrect_dead_dc () { + int j; + dc *c=controllers; + + need_dc_resurrection=0; + for (j=0;jstatus!=DC_OK && is_dc_ok(c->domain,c->controller)) + c->status=DC_OK; +} + /* cleans the auth-strings cache up. */ static void cleanup_cache(int allowed_age) { @@ -188,6 +198,7 @@ cleanup_cache(entry_expiry); refresh_challenge(); check_for_inactivity(); + need_dc_resurrection=1; /* maybe make this configurable would make sense */ } /* @@ -197,12 +208,13 @@ * -e positive user-auth caching period * -b try load-balancing the domain-controllers * -f fail-over to another DC if DC connection fails. + * -s be strict about protocol conformance * domain\controller ... */ void process_options (int argc, char *argv[]) { int opt,j,tmp,had_error=0; dc *new_dc=NULL, *last_dc=NULL; - while (-1!=(opt=getopt(argc,argv,"d:s:k:c:e:bf"))) { + while (-1!=(opt=getopt(argc,argv,"d:s:k:c:e:bfs"))) { switch(opt) { case 'b': load_balance=1; @@ -210,6 +222,9 @@ case 'f': failover_enabled=1; break; + case 's': + protocol_pedantic=1; + break; case 'k': tmp=atoi(optarg); if (tmp && tmp > 0) @@ -274,6 +289,7 @@ numcontrollers++; new_dc->domain=d; new_dc->controller=c; + new_dc->status=DC_OK; if (controllers==NULL) { /* first controller */ controllers=new_dc; last_dc=new_dc; @@ -296,17 +312,21 @@ int j=0; char *ch; for (j=0;jdomain,current_dc->controller); - if (ch) - return ch; - if (failover_enabled==0) - break; + if (current_dc->status == DC_OK) { + ch=make_challenge(current_dc->domain,current_dc->controller); + if (ch) + return ch; /* All went OK, returning */ + /* Huston, we've got a problem. Take this DC out of the loop */ + current_dc->status=DC_DEAD; + } + if (failover_enabled==0) /* No failover. Just return */ + return NULL; + /* Try with the next */ current_dc=current_dc->next; } return NULL; } -int status=0; /* 0=waiting, 1=got neg, waiting for auth */ void manage_request () { char buf[10240]; char *ch, *decoded, *cred; @@ -331,6 +351,7 @@ } last_request=time(NULL); +#if HASHTABLE_IS_AT_LEAST_TWICE_AS_FAST_AS_BASE64 /* ultra-fast-track credentials checking. We don't even try to understand what's going on. If it matches anything we got in cache, it's OK for us.*/ @@ -348,6 +369,7 @@ } else { debug("cache miss. taking the long route, stopping by Redmond.\n"); } +#endif /* figure out what we got */ decoded=base64_decode(buf); @@ -356,7 +378,7 @@ */ if (!decoded) { /* decoding failure, return error */ - ERR("WHAT? Packet format error, couldn't base64-decode"); + ERR("Packet format error, couldn't base64-decode"); status=0; return; } @@ -376,7 +398,12 @@ case NTLM_NEGOTIATE: debug("status %d\n",status); if (status==1) { - fprintf(stderr,"Huh? Got two negotiations in a row\n"); + if (protocol_pedantic) { + ERR("Got negotiation while expecting response"); + return; + } else { + fprintf(stderr,"NTLM warning. Got two negotiations in a row\n"); + } } status=1; /* Challenge refresh. It's done here so to be done synchronously */ @@ -401,30 +428,55 @@ break; /* useless */ case NTLM_AUTHENTICATE: if (status==0) { - fprintf(stderr,"Huh? Got two authentications in a row\n"); + if (protocol_pedantic) { + ERR("Got Authenticate-request while expecting Negotiate-request"); + return; + } else { + fprintf(stderr,"NTLM warning. Authenticate-request without " + "negotiation\n"); + } } +#if !HASHTABLE_IS_AT_LEAST_TWICE_AS_FAST_AS_BASE64 + /* look up the cache This used to be first thing, but I believe + that base64-decoding is cheaper than a failed hashtable lookup. + and we would fail one lookup out of two (when we received a + negotiate request) */ + debug("Trying the fast-track way\n"); + he.k=buf; + lnk=hash_lookup(cache,&he); + if (lnk) { + hash_entry *v=(hash_entry *)lnk->key; + debug("cache hit. Fast-track-granting.\n"); + OK(v->creds); + v->atime=time(NULL); + status=0; + return; + } else { + debug("cache miss. taking the long route, stopping by Redmond.\n"); + } +#endif + /* check against the DC */ plen=strlen(buf)*3/4; /* we only need it here. Optimization */ status=0; - do { - cred=ntlm_check_auth((struct ntlm_authenticate *)decoded, plen); - if (cred==NULL) { - switch (ntlm_errno) { - case NTLM_SERVER_ERROR: - case NTLM_PROTOCOL_ERROR: - case NTLM_NOT_CONNECTED: - /* reconnect */ - dc_disconnect(); - current_dc=current_dc->next; - obtain_challenge(); - ERR("domain controller failure"); - return; - break; /* not reached */ - case NTLM_LOGON_ERROR: - ERR("authentication failure"); - return; - } + cred=ntlm_check_auth((struct ntlm_authenticate *)decoded, plen); + if (cred==NULL) { + switch (ntlm_errno) { + case NTLM_LOGON_ERROR: + ERR("authentication failure"); + return; + case NTLM_SERVER_ERROR: + case NTLM_PROTOCOL_ERROR: + case NTLM_NOT_CONNECTED: + case NTLM_BAD_PROTOCOL: + /* reconnect */ + dc_disconnect(); + current_dc=current_dc->next; + obtain_challenge(); + ERR("domain controller failure"); + return; + break; /* not reached */ } - } while (cred==NULL); + } lc(cred); /* let's lowercase them for our convenience */ OK(cred); /* set the value in the cache */ @@ -432,12 +484,17 @@ h=malloc(sizeof(hash_entry)); h->k=malloc(strlen(buf)+1); strcpy(h->k,buf); - h->creds=malloc(sizeof(cred)+1); + h->creds=malloc(strlen(cred)+1); strcpy(h->creds,cred); h->atime=time(NULL); lnk=malloc(sizeof(hash_link)); lnk->key=(char *)h; hash_join(cache,lnk); + /* this looks like a good moment to perform dead dc resurrection */ + if (need_dc_resurrection != 0) { + resurrect_dead_dc(); + } + return; default: ERR("unknown authentication packet type"); status=0; @@ -453,22 +510,23 @@ debug("options processed OK\n"); + /* initialize FDescs */ + setbuf(stdout,NULL); + setbuf(stderr,NULL); + /* select the first domain controller we're going to use */ current_dc=controllers; if (load_balance!=0 && numcontrollers > 1) { + int n; pid_t pid=getpid(); - unsigned int n; n=pid%numcontrollers; debug("load balancing. Selected controller #%d\n",n); - while (--n>=0) { + while (n > 0) { current_dc=current_dc->next; + n--; } } - /* initialize FDescs */ - setbuf(stdout,NULL); - setbuf(stderr,NULL); - /* initialize cache hashtable */ debug("creating cache structure\n"); cache=hash_create(hashcmp,467,hashhash);