hints processing for Accounting-On / Off packets?

[Phil Mayers]

Stefan Winter wrote:
> Hi!
> 
>> It seems you're right.
> 
> Glad that I'm not just plain stupid :-)
> 
>> The 1.1.0 source has in rlm_preprocess: 
>>
>> static int hints_setup(PAIR_LIST *hints, REQUEST *request)
>> {
>>          char            *name;
>>          VALUE_PAIR      *add;
>>          VALUE_PAIR      *tmp;
>>          PAIR_LIST       *i;
>>          VALUE_PAIR *request_pairs;
>>
>>          request_pairs = request->packet->vps;
>>
>>          if (hints == NULL || request_pairs == NULL)
>>                  return RLM_MODULE_NOOP;
>>
>> i.e. if there's no username, which there isn't in an Acct-On/Off, skip
>> processing.
> 
> Well, the accounting packet does contain several value pairs, so I would be 
> surprised that the request_pairs == NULL condition is fulfilled. I mean, if 
> you look at that packet I sent, it contains NAS-Identifier and lots of 
> others, and these _are_ VPs, no?

Sorry, I cut too early. Immediately following code says:

         /*
          *      Check for valid input, zero length names not permitted
          */
         if ((tmp = pairfind(request_pairs, PW_USER_NAME)) == NULL)
                 name = NULL;
         else
                 name = (char *)tmp->strvalue;

         if (name == NULL || name[0] == 0)
                 /*
                  *      No name, nothing to do.
                  */
                 return RLM_MODULE_NOOP;


> 
>> This makes sense of course, because the "hints" file refers 
>> to hint strings in the USERNAME, such as "username.ppp" meaning to start
>> PPP.
> 
> That's not how I read the comments in hints:
> 
> #       The hints file.   This file is used to match
> #       a request, and then add attributes to it.
> 
> There's some mention of some special rules "Prefix" and "Suffix", and _these_ 
> can only work on the User-Name. Anything else should be doable anyway.

That's definitely what that file does. The "hints" the filename refer to 
are hints that the USER submits in their authentication request, by 
prefixing, suffixing or otherwise formatting their username (the only 
value the user has total, sensible control over). It's an old method 
that the ancestors of FreeRadius used extensively. The examples in the 
default hints file make it pretty clear how it was originally intended 
to be used.

> 
>> Sadly you can't use an ordinary "users" file, because you want to add
>> the RESTENA-Service-Type to the INPUT AVPs. You could add it to the
>> config AVPs then do this:
>>
>> detailfile = path/%{config:RESTENA-Service-Type}-service/detail
>>
>> ...but sadly the "users" file can't add arbitrary things to config
>> items; just AVPs it "knows" are server AVPs.
>>
>> You could use a "passwd" module instance like this:
>>
>> modules {
>>    passwd accttype {
>>      filename = /path/to/file
>>      # lookup on *ed field; ~ed are added to request; unprefixed
>>      # are added to configure; =ed are added to reply
>>      format = "*Client-IP-Address:~RESTENA-Service-Type:Acct-Type"
>>      hashsize = 100
>>    }
>> }
>> authorize {
>>    preprocess
>>    accttype
>>    # other stuff
> 
> Thanks for those workarounds. But, I'm not sure I like them. hints would have 
> made this wonderfully easy... and the way I see the hints file described, I 

Certainly that's not what hints used to do. It may make sense to update 
the function of hints, since it is indeed a wonderfully flexible and 
easy way to alter / add to the incoming request. But it's definitely not 
a "bug" per se.

> tend to think it should work. Maybe this can be considered a bug? If yes, I 
> might look into the code and submit a patch. Just give me a go-ahead...

The patch looks relatively trivial. Simply delete the check for null 
usernames. In *fact*, I don't think the left hand side of the "users" 
entries in that file is used at all.