rlm_perl - Asymmetric attribute encoding
I'm seeing some inconsistent handling of encoding/escaping of attributes in rlm_perl between calls to the module (i.e. when setting an attribute in one call and then reading it back in the next). I was expecting "something out, something in", but instead I'm getting "something out, something-modified in". In authorize, my script does an LDAP lookup and then sets the $RAD_CHECK{'Ldap-UserDn'} attribute for later: rlm_perl: Added pair Ldap-UserDn = CN=Ireland\, Scott,OU=[...] In authenticate, I go to read this back but find that the \ in the attribute is coming back to me escaped: rlm_perl: Added pair Ldap-UserDn = CN=Ireland\\, Scott,OU=[...] So, I end up having to strip off the escaping backslashes before using the value. I'm seeing something similar with the State attribute going out in an Access-Challenge and coming back in on the next Access-Request. In authorize, I set $RAD_REPLY{'State'} to the single byte value 0x01 that I send back in an Access-Challenge rlm_perl: Added pair State = ? Sending Access-Challenge of id 253 to [ip] port 63308 State = 0x01 When authorize next sees the follow-up request from the NAS, things look OK in the RADIUS packet: rad_recv: Access-Request packet from host [ip] port 14034, id=254, length=146 State = 0x01 rlm_perl: Added pair State = 0x01 But when I read it in Perl, $RAD_REQUEST{'State'} comes back as the string "0x01", so I have to turn around and pack() this to get back what I put in. The same thing happens for different values as well; for instance, if I were to use the literal 1 as a value, State goes out as 0x31 (the string "1") and comes back in as the string "0x31", which again needs a pack() to fix. I've been able to work around this by doing a bit of extra processing when I read any of these attributes back, but it doesn't really seem like intended behaviour.
Scott Ireland wrote:
In authorize, my script does an LDAP lookup and then sets the $RAD_CHECK{'Ldap-UserDn'} attribute for later:
rlm_perl: Added pair Ldap-UserDn = CN=Ireland\, Scott,OU=[...]
That string is a Perl string. It's allowed to have backslashes in it, and they mean nothing special. The server treats backslashes as something special.
In authenticate, I go to read this back but find that the \ in the attribute is coming back to me escaped:
rlm_perl: Added pair Ldap-UserDn = CN=Ireland\\, Scott,OU=[...]
Yes. The backslash is escaped.
So, I end up having to strip off the escaping backslashes before using the value.
Why are you escaping the comma? That doesn't make much sense. Commas are allowed in strings in FreeRADIUS, and have no special meaning. The solution would be to *not* escape the comma in the first place.
I'm seeing something similar with the State attribute going out in an Access-Challenge and coming back in on the next Access-Request. In authorize, I set $RAD_REPLY{'State'} to the single byte value 0x01 that I send back in an Access-Challenge
rlm_perl: Added pair State = ?
Because the State attribute is binary data. The debug message is trying to print that binary data, and not doing a good job of it.
Sending Access-Challenge of id 253 to [ip] port 63308 State = 0x01
When authorize next sees the follow-up request from the NAS, things look OK in the RADIUS packet:
rad_recv: Access-Request packet from host [ip] port 14034, id=254, length=146 State = 0x01 rlm_perl: Added pair State = 0x01 But when I read it in Perl, $RAD_REQUEST{'State'} comes back as the string "0x01",
That could be fixed. But you could then argue that IP addresses should be passed to Perl as a 4 byte binary blob. Well, the interface to the Perl code is text. Always text. If that's difficult for Perl, that's just the way it is right now.
so I have to turn around and pack() this to get back what I put in.
The same thing happens for different values as well; for instance, if I were to use the literal 1 as a value, State goes out as 0x31 (the string "1") and comes back in as the string "0x31", which again needs a pack() to fix.
Sad to say, yes.
I've been able to work around this by doing a bit of extra processing when I read any of these attributes back, but it doesn't really seem like intended behaviour.
It's awkward, but it's consistent. Everything is a string. For v3, it may be worth adding mapping functions like we now have in LDAP and other places. So you should be able to say "Yes, my Perl script expects State as a binary blob. That's fine". As always, patches are welcome. Alan DeKok.
On Mon, Mar 31, 2014 at 4:07 PM, Alan DeKok <aland@deployingradius.com> wrote:
Why are you escaping the comma? That doesn't make much sense. Commas are allowed in strings in FreeRADIUS, and have no special meaning.
The solution would be to *not* escape the comma in the first place.
This isn't actually anything I'm doing, it's verbatim from the LDAP server. There is actually a comma in the RDN, which the directory escapes when it returns the full DN (wihch seems reasonable, given that it could otherwise be seen as a separator). The choice of values here isn't mine either.
Because the State attribute is binary data. The debug message is trying to print that binary data, and not doing a good job of it.
As expected, but it does contrast with the later output for illustration.
That could be fixed. But you could then argue that IP addresses should be passed to Perl as a 4 byte binary blob.
Well, the interface to the Perl code is text. Always text. If that's difficult for Perl, that's just the way it is right now.
Right.. but still, no matter what kind of data I put in, I get something different back. If I put in binary data, I get back a string in hex notation. If I put in a string, I get that string back in hex notation. It isn't just weirdness with binary data or Perl arbitrarily treating numbers as strings. This goes out: rlm_perl: Added pair State = test Sending Access-Challenge of id 47 to [ip] port 39441 State = 0x74657374 This comes back: rad_recv: Access-Request packet from host [ip] port 34792, id=48, length=153 State = 0x74657374 rlm_perl: Added pair State = 0x74657374 I still have to pack() the State just to get the original plaintext.
It's awkward, but it's consistent. Everything is a string.
Except that it isn't.. even without arbitrary binary data, the same attribute is written one way and read a completely different way, which requires mangling on one side (and only one side) of the processing. The same goes for the escaped backslash.
Scott Ireland wrote:
This isn't actually anything I'm doing, it's verbatim from the LDAP server. There is actually a comma in the RDN, which the directory escapes when it returns the full DN (wihch seems reasonable, given that it could otherwise be seen as a separator). The choice of values here isn't mine either.
OK.
Right.. but still, no matter what kind of data I put in, I get something different back.
That just isn't true. Strings without backslashes work fine. Integers work fine, as do IP addresses, dates, IPv6 addresses, etc.
If I put in binary data, I get back a string in hex notation. If I put in a string, I get that string back in hex notation.
Yes. The parser is forgiving. If you give a hex string to an attribute of "octets" type, it parses the hex into binary data. If you give a printable string to an "octets" attribute, it assumes you're being lazy. The printable string is used as the value for the attribute.
Except that it isn't.. even without arbitrary binary data, the same attribute is written one way and read a completely different way, which requires mangling on one side (and only one side) of the processing.
No. The binary attributes are printed and parsed as hex strings: 0xabcdef. That is the normal (and expected) method of operation. If you pass binary data to the parser, it will assume it's a C string. It will parse the data as a C string. If you have a zero byte in the string, everything after that will be ignored. i.e. If your Perl code expects to see binary data, it MUST call pack() after it gets the attribute, and unpack() before it hands the attribute back. This is consistency. As I said, the interface to the Perl code is text. If you went with that assumption, you wouldn't see a probem. Instead, you hand random crap to the function, and are surprised when you get random crap back. Well... there's not much I can say to that.
The same goes for the escaped backslash.
That, I agree is inconsistent. It should either (a) treat the input string as needing escapes, and convert "\," to just ",", then there's no double backslash issue when outputting it or b) treat the input string as not needing escapes, in which case "\," is left as-is, and then the output string should not be escaped, either. The problem seems to be pairparsevalue(), which treats "\," as "\,", but DOES convert "\r" and friends to their one-byte sequence. The solution seems to be treating printable strings as special for FR to Perl conversions. The Perl string should just be copied to the FR string as-is, with no escaping. The FR string should be copied to the Perl string as-is, with no escaping. I can't do a patch now, but please check the v3.0.x branch tomorrow. And change your use of State to use pack() and unpack(). Mixing binary and printable forms of the attribute data is a bad idea. Alan DeKok.
On Mon, Mar 31, 2014 at 6:29 PM, Alan DeKok <aland@deployingradius.com>wrote:
Yes. The parser is forgiving. If you give a hex string to an attribute of "octets" type, it parses the hex into binary data. If you give a printable string to an "octets" attribute, it assumes you're being lazy. The printable string is used as the value for the attribute.
OK. So it's the "forgivingness" that gives the appearance of inconsistency here because of that extra conversion.
No. The binary attributes are printed and parsed as hex strings: 0xabcdef. That is the normal (and expected) method of operation.
Knowing that now makes a big difference, so yes, treating them as "0x"-prepended hex strings at both ends does give me consistent behaviour. Things seem to work in the general case with something like: $RAD_REPLY{'State'} = "0x".unpack("H*",$data); and my $data = pack("H*",substr($RAD_REPLY{'State'},2)); # remove leading '0x' before pack to move data in and out of the attribute.
Scott Ireland wrote:
OK. So it's the "forgivingness" that gives the appearance of inconsistency here because of that extra conversion.
Yes.
Knowing that now makes a big difference, so yes, treating them as "0x"-prepended hex strings at both ends does give me consistent behaviour.
Yes. See also the v3.0.x branch in git. I've fixed the module so it doesn't double-escape strings. I've added more documentation to raddb/mods-available/perl to make it clear what's going on. Alan DeKok.
participants (2)
-
Alan DeKok -
Scott Ireland