Re: separate storage for RFC931 and proxy-auth user names

From: Robert Collins <robert.collins@dont-contact.us>
Date: Thu, 9 Nov 2000 19:40:08 +1100

I've put the equivalent changes into auth_rewrite.Attached is a diff against
auth_rewrite before and after the change.
There's one thing that I've done differently: please speak up if that's
wrong...

In acl.c, the check against overwriting ident should be

     if (!checklist->rfc931[0])
         xstrncpy(checklist->request->authuser,
auth_user->auth_data.basic_auth.username, USER_IDENT_SZ);

instead of
     if (!checklist->request->authuser[0])
 xstrncpy(checklist->request->authuser, user, USER_IDENT_SZ);

because we only have one username field in the standard access.log. Can we
change this to have two fields without losing any data?

Rob

----- Original Message -----
From: "Duane Wessels" <wessels@squid-cache.org>
To: <squid-dev@squid-cache.org>
Sent: Wednesday, November 08, 2000 8:43 AM
Subject: separate storage for RFC931 and proxy-auth user names

> Is it safe to separate these two user names? It seems to me that
> request->user_ident may store either one. I think proxy-auth user
> names should not be used in 'ident' ACLs but maybe they are.
>
> A lot of this patch below is just to rename structure members to
> clarify where the user name came from.
>
> Index: access_log.c
> ===================================================================
> RCS file: /squid/squid/src/access_log.c,v
> retrieving revision 1.61
> diff -u -r1.61 access_log.c
> --- access_log.c 2000/11/01 04:50:25 1.61
> +++ access_log.c 2000/11/07 21:35:43
> @@ -182,6 +182,14 @@
> return buf;
> }
>
> +static const char *
> +accessLogFormatName(const char *name)
> +{
> + if (NULL == name)
> + return dash_str;
> + return rfc1738_escape(name);
> +}
> +
> static void
> accessLogSquid(AccessLogEntry * al)
> {
> @@ -200,7 +208,9 @@
> al->cache.size,
> al->private.method_str,
> al->url,
> - al->cache.ident,
> + al->cache.authuser ?
> + accessLogFormatName(al->cache.authuser) :
> + accessLogFormatName(al->cache.rfc931),
> al->hier.ping.timedout ? "TIMEOUT_" : "",
> hier_strings[al->hier.code],
> al->hier.host,
> @@ -215,9 +225,10 @@
> client = fqdncache_gethostbyaddr(al->cache.caddr, 0);
> if (client == NULL)
> client = inet_ntoa(al->cache.caddr);
> - logfilePrintf(logfile, "%s %s - [%s] \"%s %s HTTP/%.1f\" %d %d
%s:%s",
> + logfilePrintf(logfile, "%s %s %s [%s] \"%s %s HTTP/%.1f\" %d %d
%s:%s",
> client,
> - al->cache.ident,
> + accessLogFormatName(al->cache.rfc931),
> + accessLogFormatName(al->cache.authuser),
> mkhttpdlogtime(&squid_curtime),
> al->private.method_str,
> al->url,
> @@ -231,20 +242,12 @@
> void
> accessLogLog(AccessLogEntry * al)
> {
> - LOCAL_ARRAY(char, ident_buf, USER_IDENT_SZ);
> -
> if (LogfileStatus != LOG_ENABLE)
> return;
> if (al->url == NULL)
> al->url = dash_str;
> if (!al->http.content_type || *al->http.content_type == '\0')
> al->http.content_type = dash_str;
> - if (!al->cache.ident || *al->cache.ident == '\0') {
> - al->cache.ident = dash_str;
> - } else {
> - xstrncpy(ident_buf, rfc1738_escape(al->cache.ident), USER_IDENT_SZ);
> - al->cache.ident = ident_buf;
> - }
> if (al->icp.opcode)
> al->private.method_str = icp_opcode_str[al->icp.opcode];
> else
> Index: acl.c
> ===================================================================
> RCS file: /squid/squid/src/acl.c,v
> retrieving revision 1.225
> diff -u -r1.225 acl.c
> --- acl.c 2000/10/31 23:48:13 1.225
> +++ acl.c 2000/11/07 21:35:43
> @@ -1014,7 +1014,7 @@
> debug(28, 3) ("aclMatchUser: checking '%s'\n", user);
> while (data) {
> debug(28, 3) ("aclMatchUser: looking for '%s'\n", data->key);
> - if (strcmp(data->key, "REQUIRED") == 0 && *user != '\0' && strcmp(user,
"-") != 0)
> + if (strcmp(data->key, "REQUIRED") == 0 && *user != '\0' && strcmp(user,
dash_str) != 0)
> return 1;
> if (strcmp(data->key, user) == 0)
> return 1;
> @@ -1109,14 +1109,14 @@
> * unless ident is known (do not override ident with
> * false proxy auth names)
> */
> - if (!*checklist->request->user_ident)
> - xstrncpy(checklist->request->user_ident, user, USER_IDENT_SZ);
> + if (!checklist->request->authuser[0])
> + xstrncpy(checklist->request->authuser, user, USER_IDENT_SZ);
> return -2;
> } else {
> /* password was checked and did match */
> debug(28, 4) ("aclMatchProxyAuth: user '%s' validated OK\n", user);
> /* store validated user in hash, after filling in expiretime */
> - xstrncpy(checklist->request->user_ident, user, USER_IDENT_SZ);
> + xstrncpy(checklist->request->authuser, user, USER_IDENT_SZ);
> auth_user->expiretime = current_time.tv_sec +
Config.authenticateTTL;
> auth_user->ip_expiretime = squid_curtime + Config.authenticateIpTTL;
> auth_user->ipaddr = checklist->src_addr;
> @@ -1142,7 +1142,7 @@
> auth_user->ip_expiretime = squid_curtime + Config.authenticateIpTTL;
> auth_user->ipaddr = checklist->src_addr;
> /* copy username to request for logging on client-side */
> - xstrncpy(checklist->request->user_ident, user, USER_IDENT_SZ);
> + xstrncpy(checklist->request->authuser, user, USER_IDENT_SZ);
> switch (acltype) {
> case ACL_PROXY_AUTH:
> return aclMatchUser(data, user);
> @@ -1425,16 +1425,16 @@
> /* NOTREACHED */
> #if USE_IDENT
> case ACL_IDENT:
> - if (checklist->ident[0]) {
> - return aclMatchUser(ae->data, checklist->ident);
> + if (checklist->rfc931[0]) {
> + return aclMatchUser(ae->data, checklist->rfc931);
> } else {
> checklist->state[ACL_IDENT] = ACL_LOOKUP_NEEDED;
> return 0;
> }
> /* NOTREACHED */
> case ACL_IDENT_REGEX:
> - if (checklist->ident[0]) {
> - return aclMatchRegex(ae->data, checklist->ident);
> + if (checklist->rfc931[0]) {
> + return aclMatchRegex(ae->data, checklist->rfc931);
> } else {
> checklist->state[ACL_IDENT] = ACL_LOOKUP_NEEDED;
> return 0;
> @@ -1716,17 +1716,19 @@
> {
> aclCheck_t *checklist = data;
> if (ident) {
> - xstrncpy(checklist->ident, ident, sizeof(checklist->ident));
> - xstrncpy(checklist->request->user_ident, ident,
sizeof(checklist->request->user_ident));
> + xstrncpy(checklist->rfc931, ident, USER_IDENT_SZ);
> +#if DONT
> + xstrncpy(checklist->request->authuser, ident, USER_IDENT_SZ);
> +#endif
> } else {
> - xstrncpy(checklist->ident, "-", sizeof(checklist->ident));
> + xstrncpy(checklist->rfc931, dash_str, USER_IDENT_SZ);
> }
> /*
> * Cache the ident result in the connection, to avoid redoing ident
lookup
> * over and over on persistent connections
> */
> - if (cbdataValid(checklist->conn) && !checklist->conn->ident[0])
> - xstrncpy(checklist->conn->ident, checklist->ident,
sizeof(checklist->conn->ident));
> + if (cbdataValid(checklist->conn) && !checklist->conn->rfc931[0])
> + xstrncpy(checklist->conn->rfc931, checklist->rfc931, USER_IDENT_SZ);
> aclCheck(checklist);
> }
> #endif
> @@ -1801,7 +1803,7 @@
> checklist->state[i] = ACL_LOOKUP_NONE;
> #if USE_IDENT
> if (ident)
> - xstrncpy(checklist->ident, ident, USER_IDENT_SZ);
> + xstrncpy(checklist->rfc931, ident, USER_IDENT_SZ);
> #endif
> checklist->auth_user = NULL; /* init to NULL */
> return checklist;
> Index: client_side.c
> ===================================================================
> RCS file: /squid/squid/src/client_side.c,v
> retrieving revision 1.510
> diff -u -r1.510 client_side.c
> --- client_side.c 2000/11/01 09:36:05 1.510
> +++ client_side.c 2000/11/07 21:35:44
> @@ -135,9 +135,9 @@
> {
> ConnStateData *conn = data;
> if (ident)
> - xstrncpy(conn->ident, ident, sizeof(conn->ident));
> + xstrncpy(conn->rfc931, ident, USER_IDENT_SZ);
> else
> - xstrncpy(conn->ident, "-", sizeof(conn->ident));
> + xstrncpy(conn->rfc931, dash_str, USER_IDENT_SZ);
> }
> #endif
>
> @@ -148,7 +148,7 @@
> ConnStateData *conn = http->conn;
> ch = aclChecklistCreate(acl,
> http->request,
> - conn->ident);
> + conn->rfc931);
> #if USE_IDENT
> /*
> * hack for ident ACL. It needs to get full addresses, and a
> @@ -294,8 +294,8 @@
> new_request->my_addr = old_request->my_addr;
> new_request->my_port = old_request->my_port;
> new_request->flags.redirected = 1;
> - if (old_request->user_ident[0])
> - xstrncpy(new_request->user_ident, old_request->user_ident,
> + if (old_request->authuser[0])
> + xstrncpy(new_request->authuser, old_request->authuser,
> USER_IDENT_SZ);
> if (old_request->body) {
> new_request->body = xmalloc(old_request->body_sz);
> @@ -733,10 +733,10 @@
> http->al.http.version = request->http_ver;
> http->al.headers.request = xstrdup(mb.buf);
> http->al.hier = request->hier;
> - if (request->user_ident[0])
> - http->al.cache.ident = request->user_ident;
> - else
> - http->al.cache.ident = conn->ident;
> + if (request->authuser[0])
> + http->al.cache.authuser = request->authuser;
> + if (conn->rfc931[0])
> + http->al.cache.rfc931 = conn->rfc931;
> packerClean(&p);
> memBufClean(&mb);
> }
> Index: redirect.c
> ===================================================================
> RCS file: /squid/squid/src/redirect.c,v
> retrieving revision 1.84
> diff -u -r1.84 redirect.c
> --- redirect.c 2000/03/06 16:23:34 1.84
> +++ redirect.c 2000/11/07 21:35:44
> @@ -126,12 +126,12 @@
> cbdataAdd(r, cbdataXfree, 0);
> r->orig_url = xstrdup(http->uri);
> r->client_addr = conn->log_addr;
> - if (http->request->user_ident[0])
> - r->client_ident = http->request->user_ident;
> - else if (conn->ident == NULL || *conn->ident == '\0') {
> - r->client_ident = dash_str;
> + if (http->request->authuser[0])
> + r->client_ident = http->request->authuser;
> + else if (conn->rfc931[0]) {
> + r->client_ident = conn->rfc931;
> } else {
> - r->client_ident = conn->ident;
> + r->client_ident = dash_str;
> }
> r->method_s = RequestMethodStr[http->request->method];
> r->handler = handler;
> Index: structs.h
> ===================================================================
> RCS file: /squid/squid/src/structs.h,v
> retrieving revision 1.358
> diff -u -r1.358 structs.h
> --- structs.h 2000/11/01 04:03:15 1.358
> +++ structs.h 2000/11/07 21:35:44
> @@ -140,7 +140,7 @@
> request_t *request;
> #if USE_IDENT
> ConnStateData *conn; /* hack for ident */
> - char ident[USER_IDENT_SZ];
> + char rfc931[USER_IDENT_SZ];
> #endif
> acl_proxy_auth_user *auth_user;
> acl_lookup_state state[ACL_ENUM_MAX];
> @@ -847,7 +847,8 @@
> size_t size;
> log_type code;
> int msec;
> - const char *ident;
> + const char *rfc931;
> + const char *authuser;
> } cache;
> struct {
> char *request;
> @@ -907,7 +908,7 @@
> struct sockaddr_in peer;
> struct sockaddr_in me;
> struct in_addr log_addr;
> - char ident[USER_IDENT_SZ];
> + char rfc931[USER_IDENT_SZ];
> int nrequests;
> int persistent;
> struct {
> @@ -1427,7 +1428,7 @@
> protocol_t protocol;
> char login[MAX_LOGIN_SZ];
> char host[SQUIDHOSTNAMELEN + 1];
> - char user_ident[USER_IDENT_SZ]; /* from proxy auth or ident server */
> + char authuser[USER_IDENT_SZ]; /* from proxy auth only */
> u_short port;
> String urlpath;
> char *canonical;
>
>

Received on Thu Nov 09 2000 - 01:34:23 MST

This archive was generated by hypermail pre-2.1.9 : Tue Dec 09 2003 - 16:12:57 MST