3.0.x: user-password length decoding sometimes wrong?

[Alan DeKok]

On Sep 15, 2015, at 11:02 AM, Stefan Winter <stefan.winter at restena.lu> wrote:
> ugh. I just updated to 3.0.x from a few minutes ago to be prepared for
> the MS-CHAP key problem and iOS 9.
> 
> That seems to work - good. But I have a problem with good old PAP but
> only for requests originating from one of the clients - and consistently.
> 
> That client is insanely old and has ancient library versions. So it may
> well be doing something subtly wrong in the requests it sends; but
> still, this worked for a FreeRADIUS server versions since a decade. But
> not in current 3.0.x.

  I've ran into that, too.  With proftpd... I have no idea what they did, but they did something retarded.

> What I see coming in (decoded in -X) is:
> 
> (137) Received Access-Request Id 89 from 158.64.1.50:8010 to
> 158.64.2.220:1812 length 80
> [...]
> (137)   User-Password = "12345678\000\000\000\000\021"
> 
> That's an 8 char password (12345678 is the equivalent of correct; i.e.
> redacted in this mail but it otherwise fits).

  Yes.  The client is garbage.

  In 2.0, the password was searched from the beginning for the first zero character, and truncated there.

  In 3.0, the password  is searched from the end for the first non-zero character, and truncated there.

> On other clients, the same password comes in as
> 
> User-Password = "12345678"
> 
> as expected; a tcpdump between for both clients reveals that the
> on-the-wire length of User-Password is the same (16 chars net, to
> account for padding).

  That's the encrypted stuff.  The problem is that the *decrypted* password is garbage.

> I realise that all \0 is what RFC2865 chapter 5.2 mandates as padding
> text. So, yes, our client is at fault.

  Yup.  I have NO IDEA how people get random crap at the end of the password.  It's crazy.  How hard is it to pad the passwords with zeros?

> Still, is there any chance that the server-side parsing could be made a
> little more forgiving?

  Not really.  The change was made to be compatible with other systems which do sane things.  I don't want to fix one thing and break another.

  My fix for this in 3.0 is as follows.  A bit convoluted, but it works.

......
	# proftpd sends "password\000\000password"
	# we want to convert that to "password",
	# so we do a regex, which will stop at the
	# first zero.

	# copy the string with embedded binary zeros
	update request {
		Tmp-Octets-0 := &User-Password
	}

	# proftp sometimes sends "\000\000\000\000PASS"
	# which is fucking stupid.  Reject it outright.
	if (Tmp-Octets-0 =~ /^0x00/) {
		reject
	}

	# find the two binary zeros, and copy everything before them
	# regular expressions will NOT work on strings with embedded
	# zeros, because the regex libraries do strange things
	if (Tmp-Octets-0 =~ /^(0x(..)+)0000/) {

		# get the text BEFORE the zeros
		update request {
			Tmp-Octets-0 := "%{1}"
		}

		# cast the binary data to a printable string,
		# and now assign the printable string to the
		# User-Password attribute... which is a printable
		# string

		# if we just assign Tmp-Octets-0 to User-Password,
		# we get the ASCII version of Tmp-Octets-0, which
		# is "0xabcdef", and not what we want.
		#
		# if we just used the next line without editing
		# Tmp-Octets-0, then it's string representation
		# would be "password\000\000password", which is
		# not what we want.
		update request {
			User-Password := "%{string:Tmp-Octets-0}"
		}
	}
...