Problem with 3Com-User-Access-Level and rlm_perl
Hello, It seems there is something wrong with RAD_REPLY handling. With 3Com-User-Access-Level set to 3Com-Manager/3Com-Monitor freeradius finally sends 3Com-Administrator: My code looks like thos: $RAD_CHECK{'Auth-Type'} = 'Accept'; $RAD_REPLY{'3Com-User-Access-Level'} = '3Com-Manager'; # radiusd -X (...) rlm_perl: Added pair 3Com-User-Access-Level = 3Com-Manager rlm_perl: Added pair Auth-Type = Accept (...) Sending Access-Accept of id 68 to 192.168.139.254 port 2048 3Com-User-Access-Level = 3Com-Administrator (...) But if I set it to 2: $RAD_REPLY{'3Com-User-Access-Level'} = 2; then: # radiusd -X (...) rlm_perl: Added pair 3Com-User-Access-Level = 2 rlm_perl: Added pair Auth-Type = Accept (...) Sending Access-Accept of id 69 to 192.168.139.254 port 2048 3Com-User-Access-Level = 3Com-Manager BTW, there is no problem with other attributes like for example: $RAD_REPLY{'Tunnel-Medium-Type'} = 'IEEE-802'; Tested with freeradius-1.1.2 and 1.1.3. Best regards, Krzysztof Olędzki
Krzysztof Olędzki wrote:
Hello,
It seems there is something wrong with RAD_REPLY handling. With 3Com-User-Access-Level set to 3Com-Manager/3Com-Monitor freeradius finally sends 3Com-Administrator:
What happens when you put a similar config in the "users" file? Alan DeKok. -- http://deployingradius.com - The web site of the book http://deployingradius.com/blog/ - The blog
On 2006-11-26 16:38, Alan DeKok wrote:
Krzysztof Olędzki wrote:
Hello,
It seems there is something wrong with RAD_REPLY handling. With 3Com-User-Access-Level set to 3Com-Manager/3Com-Monitor freeradius finally sends 3Com-Administrator:
What happens when you put a similar config in the "users" file?
OK, it looks like this is not a problem with rlm_perl: DEFAULT Auth-Type := Accept 3Com-User-Access-Level = 3Com-Monitor # radiusd -X (...) Sending Access-Accept of id 76 to 192.168.139.254 port 2048 3Com-User-Access-Level = 3Com-Administrator (...) DEFAULT Auth-Type := Accept 3Com-User-Access-Level = 1 # radiusd -X (...) Sending Access-Accept of id 77 to 192.168.139.254 port 2048 3Com-User-Access-Level = 3Com-Monitor (...) Anyway, handling of 3Com-User-Access-Level is wrong. Best regards, Krzysztof Olędzki -- Krzysztof Olędzki Axel Springer Polska Sp. z o.o. tel: +48-22-2320969 fax: +48-22-2325530
Krzysztof Olędzki wrote:
OK, it looks like this is not a problem with rlm_perl:
Which version of the server are you running? 1.1.3 fixes a problem that *might* be contributing here. Alan DeKok. -- http://deployingradius.com - The web site of the book http://deployingradius.com/blog/ - The blog
On 2006-11-26 19:09, Alan DeKok wrote:
Krzysztof Olędzki wrote:
OK, it looks like this is not a problem with rlm_perl:
Which version of the server are you running? 1.1.3 fixes a problem that *might* be contributing here.
Like I said in my first mail: problem with rlm_perl was tested with both freeradius-1.1.2 and 1.1.3. The test with users file was done using version 1.1.3. I already know that before submitting bug report it is always good to check the most recent version. ;) Pozdrawiam, Krzysztof Olędzki -- Krzysztof Olędzki Axel Springer Polska Sp. z o.o. tel: +48-22-2320969 fax: +48-22-2325530
Krzysztof Olędzki wrote:
Like I said in my first mail: problem with rlm_perl was tested with both freeradius-1.1.2 and 1.1.3. The test with users file was done using version 1.1.3.
Ok... I get enough email that I don't keep track of previous messages in a thread. In any case, the issue is that the VALUE name starts with '3', so *any* of the '3Com-*' VALUES get parsed as '3'. The solution is to update the VALUE name so it doesn't start with '3'. Or, update the source code to be a little more intelligent about parsing VALUEs Alan DeKok. -- http://deployingradius.com - The web site of the book http://deployingradius.com/blog/ - The blog
On 2006-11-26 20:25, Alan DeKok wrote:
Krzysztof Olędzki wrote:
Like I said in my first mail: problem with rlm_perl was tested with both freeradius-1.1.2 and 1.1.3. The test with users file was done using version 1.1.3.
Ok... I get enough email that I don't keep track of previous messages in a thread.
In any case, the issue is that the VALUE name starts with '3', so *any* of the '3Com-*' VALUES get parsed as '3'. The solution is to update the VALUE name so it doesn't start with '3'.
Or, update the source code to be a little more intelligent about parsing VALUEs
OK, probably found it (src/lib/valuepair.c): case PW_TYPE_INTEGER: /* * If it starts with a digit, it must * be a number (or a range). * * Note that ALL integers are unsigned! */ if (isdigit((int) *value)) { ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vp->lvalue = (uint32_t) strtoul(value, NULL, 10); vp->length = 4; } /* * Look for the named value for the given * attribute. */ else if ((dval = dict_valbyname(vp->attribute, value)) == NULL){ librad_log("Unknown value %s for attribute %s", value, vp->name); return NULL; } else { vp->lvalue = dval->value; vp->length = 4; } break; I'll try to cook a patch. Pozdrawiam, Krzysztof Olędzki -- Krzysztof Olędzki Axel Springer Polska Sp. z o.o. tel: +48-22-2320969 fax: +48-22-2325530
On Sun, 26 Nov 2006, Krzysztof Olędzki wrote:
On 2006-11-26 20:25, Alan DeKok wrote:
Krzysztof Olędzki wrote:
Like I said in my first mail: problem with rlm_perl was tested with both freeradius-1.1.2 and 1.1.3. The test with users file was done using version 1.1.3.
Ok... I get enough email that I don't keep track of previous messages in a thread.
In any case, the issue is that the VALUE name starts with '3', so *any* of the '3Com-*' VALUES get parsed as '3'. The solution is to update the VALUE name so it doesn't start with '3'.
Or, update the source code to be a little more intelligent about parsing VALUEs
OK, probably found it (src/lib/valuepair.c):
How about attached (for patching) and inlined (for easy review) patch? I also fixed else/if abuse - this is a code in a switch/case statement so we can simply use a "break;". diff -Nur freeradius-1.1.3-orig/src/lib/valuepair.c freeradius-1.1.3/src/lib/valuepair.c --- freeradius-1.1.3-orig/src/lib/valuepair.c 2006-05-03 19:40:58.000000000 +0200 +++ freeradius-1.1.3/src/lib/valuepair.c 2006-11-26 21:26:05.000000000 +0100 @@ -699,29 +699,29 @@ if (s) free(s); vp->length = 4; break; + case PW_TYPE_INTEGER: /* - * If it starts with a digit, it must - * be a number (or a range). - * * Note that ALL integers are unsigned! */ - if (isdigit((int) *value)) { - vp->lvalue = (uint32_t) strtoul(value, NULL, 10); + vp->lvalue = (uint32_t) strtoul(value, &p, 10); + if (!*p) { vp->length = 4; + break; } + /* * Look for the named value for the given * attribute. */ - else if ((dval = dict_valbyname(vp->attribute, value)) == NULL) { + if ((dval = dict_valbyname(vp->attribute, value)) == NULL) { librad_log("Unknown value %s for attribute %s", value, vp->name); return NULL; - } else { - vp->lvalue = dval->value; - vp->length = 4; } + + vp->lvalue = dval->value; + vp->length = 4; break; case PW_TYPE_DATE: Best regards, Krzysztof Olędzki
Krzysztof Olędzki wrote:
OK, probably found it (src/lib/valuepair.c):
How about attached (for patching) and inlined (for easy review) patch?
Thanks, I committed a fix.
I also fixed else/if abuse - this is a code in a switch/case statement so we can simply use a "break;".
I'll take a look at that separately. I prefer to have different fixes in different commits. Alan DeKok. -- http://deployingradius.com - The web site of the book http://deployingradius.com/blog/ - The blog
participants (2)
-
Alan DeKok -
Krzysztof Olędzki