(RADIATOR) Re: Only one Vendor attribute logged?

[Roland Rosenfeld]

Hi!

On Tue, 31 Jul 2001, I wrote:

> > I have an ugly problem with Radiator (currently 2.18.2): It only
> > writes the first (of 7) vendor attributes and their values to the
> > log file.
> 
> Okay after searching in the code (Radiator.pm) I found the problem
> documented in sub unpack:
> 
> 		    # Other vendor-specific
> 		    # REVISIT: RFC 2865 permits multiple attributes
> 		    # in a single vendor-specific attribute
> 		    $value = substr($attrdat, 8, $vlength - 2);
> 
> But it seems, that nobody revisited the code :-(
> 
> Did someone write a parser for multiple vendor attributes, that
> corrects this problem, or do I have to write it myself?

Okay, I just wrote a patch to fix this problem, maybe not completely
elegant, but it seems to work correct now.

Tschoeeee

        Roland
-------------- next part --------------
--- Radius.pm	2001/06/07 04:32:01	1.61
+++ Radius.pm	2001/07/31 10:25:37
@@ -624,12 +624,13 @@
     $self->set_authenticator($auth);
     
     my ($type, $length, $value, $vendor, $dummy, 
-	$aname, $anum, $atype, $avendor);
+	$aname, $anum, $atype, $avendor, $subvendor);
     # Unpack the attributes
     while (length $attrdat) 
     {
 	($type, $length) = unpack 'C C', $attrdat;
 	$vendor = undef;
+        $subvendor = 0;
 
 	# Look for bogus or malformed packets according to RFC2138
 	if ($length < 2)
@@ -648,7 +649,7 @@
 	    else
 	    {
 		# Its a vendor specific, length is OK, decode it
-		my $vlength;
+		my ($vlength, $vtype);
 		($vendor, $type, $vlength) = unpack 'x x N C C', $attrdat;
 		
 		if ($vendor == 429)
@@ -675,9 +676,27 @@
 		else
 		{
 		    # Other vendor-specific
-		    # REVISIT: RFC 2865 permits multiple attributes
-		    # in a single vendor-specific attribute
-		    $value = substr($attrdat, 8, $vlength - 2);
+                    # first strip of vendor header:
+                    my $vattrdat = substr($attrdat, 6, $length-6);
+                    while (length $vattrdat)
+                    {
+                        ($vtype, $vlength) = unpack 'C C', $vattrdat;
+                        if ($vlength < 2)
+                        {
+                            # Gasp a malformed packet, bomb out now, lest we stay forever
+                            &main::log($main::LOG_WARNING, "Malformed request packet: Vendor $vendor Attribute $vtype with length $vlength: ignored");
+                            return;
+                        }
+                        ($aname, $anum, $atype, $avendor)
+                            = $dict->attrByNum($vtype, $vendor);
+                        $value = substr($vattrdat, 2, $vlength - 2);
+                        $self->add_attr
+                            ($aname,
+                             &{$unpacker{$atype}}($value,$vtype,$vendor,$dict))
+                                if defined $atype;
+                        $vattrdat = substr($vattrdat, $vlength);
+                        $subvendor = 1;
+                    }
 		}
 	    }
 	}
@@ -691,12 +710,15 @@
 		if $type == $Radius::Radius::MESSAGE_AUTHENTICATOR;
 	}
 	# Dont try to unpack attributes we have never heard of
-	($aname, $anum, $atype, $avendor) 
-	    = $dict->attrByNum($type, $vendor);
-	$self->add_attr
-	    ($aname, 
-	     &{$unpacker{$atype}}($value, $type, $vendor, $dict))
+        if (! $subvendor)
+        {
+            ($aname, $anum, $atype, $avendor) 
+                = $dict->attrByNum($type, $vendor);
+            $self->add_attr
+                ($aname,
+                 &{$unpacker{$atype}}($value, $type, $vendor, $dict))
 		if defined $atype;
+        }
 
 	# Remove the attribute we just parsed
 	$attrdat = substr($attrdat, $length);