Bogus default attrs file?

[Tomas Hoger]

Hi all!

Default attrs file used by rlm_attr_filter contains following DEFAULT
section:

DEFAULT
    Service-Type == Framed-User,
    Service-Type == Login-User,
    Login-Service == Telnet,
    Login-Service == Rlogin,
    Login-Service == TCP-Clear,
    [ ... ]
    Framed-Protocol == PPP,
    Framed-Protocol == SLIP,
    [ ... ]

This syntax, accompanied with tisp realm example, suggests that the attribute
e.g. Framed-Protocol will be preserved in proxy reply if it's value is
either PPP or SLIP.  However, it does not seem to be true.  Framed-Protocol
will not pass this filter at all.

Documentation quite clearly states: "an attribute must pass *all* the rules
which affect it in order to make it past the filter".  Hence using == rule
twice with different values for same attribute should be the same as using
!*.  Or am I missing anything?


Proposal for fix:

check_pair function after comparison always increments either fail or pass
counter for given attribute.  It may be modified to not increment fail for
'==' operator (and also '=~') when compare fails.  This should be consistent
with final " fail == 0 && pass > 0 " check and default attrs file should
work as expected.


Untested patch:

For released code:

--- rlm_attr_filter.c   2005-08-11 23:06:38.000000000 +0200
+++ rlm_attr_filter.c.new       2007-01-10 13:32:55.000000000 +0100
@@ -68,8 +68,6 @@
                case T_OP_CMP_EQ:
                    if (comp == 0) {
                        ++*(pa);
-                   } else {
-                       ++*(fa);
                    }
                    break;

@@ -120,8 +118,6 @@
                    regfree(&reg);
                    if (comp == 0) {
                        ++*(pa);
-                   } else {
-                       ++*(fa);
                    }
                    break;


For CVS code (some #ifdef HAVE_REGEX_H should probably appear here):

--- rlm_attr_filter.c   2006-11-22 22:44:19.000000000 +0100
+++ rlm_attr_filter.c.new       2007-01-10 13:37:05.000000000 +0100
@@ -68,7 +68,8 @@
        compare = paircmp(check_item, reply_item);
        if (compare == 1) {
                ++*(pass);
-       } else {
+       } else if (check_item->operator != T_OP_CMP_EQ  &&
+                  check_item->operator != T_OP_REG_EQ) {
                ++*(fail);
        }


Ideas?

th.