[Chilli] [Patch] Fix access_request() to conform with radius pack ID semantics

[IT-Systemmanagement Pieter Hollants]

Short:
Coova-Chilli 1.2.2 and current SVN have a bug in the radius packet ID
semantics which (at least for me) prevented WPA-EAP from working.

Long:
Stemming from ChilliSpot are the structures radius_t, radius_queue_t and
radius_packet_t for communication with the "upstream" proxy server.

(struct radius_t)->queue is an array of radius_queue_t structures, each
of which in turn contains in "p" a struct radius_packet_id, representing
a single RADIUS packet that is supposed to be sent to the proxy server.
radius_queue_t contains a timeout and the number of retransmissions so
far, radius_packet_id contains an "id" field which is of importance here.

In ChilliSpot, the array was of a fixed size, RADIUS_QUEUESIZE which was
defined as 256. The comment "/* Same size as id address space */"
already hinted at a simple indexing scheme used for the "queue" array.

Any new packet that was to be stored was prepared with
radius_default_pack():

 pack->id = 0;

When a packet was to be sent off with radius_req(), that function called
radius_queue_in() which did this:

 pack->id = this->next;
 this->next;

where this is the struct radius_t and "next" the next ID to be assigned.
The IDs served as indexes into "queue" and since "next" was defined as
an "int", it was supposed to magically wrap around to 0, in the event
that all RADIUS_QUEUESIZE radius_queue_t slots had been used.

Now in CoovaChilli, (struct radius_t)->queue was instead implemented to
be optionally of a dynamic size below RADIUS_QUEUESIZE. Modulo operators
were used to map IDs to actual indexes.

In the process of this implementation, pack->id assignment was changed.
radius_default_pack() now does (this->next was renamed to this->qnext):

 if (this->qsize == 0)
   pack->id = this->qnext;
 else
   pack->id = /* special magic */

and radius_queue_in() no longer set pack->id itself.

This breaks the access_request() function in chilli.c: there, a
remainder of ChilliSpot origin, we read

 radius_pack.id = pack->id;

At the end radius_req() is called, which calls radius_queue_in(), which
does not set radius_pack.id itself because it expects that the value
chosen by radius_default_pack() is still there. Boom :) This gave nice
error messages "No such id in radius queue" resp. "Matching request was
not found in queue".

Attached patch removes the offending line from access_request() and
applies to both 1.2.2 and SVN. Actually, the line seems to have served
no purpose anyway, neither in ChilliSpot nor in CoovaChilli. In
ChilliSpot, as described, radius_queue_in() overwrote pack->id anyway.
In CoovaChilli it caused the erroneous behaviour described above.

The offending line was probably copied from other functions such as
radius_access_reject(), where it is correct since radius_resp() is
subsequently called, not radius_req().

Please verify and have a nice Sunday :)

-- 
Dipl.-Wirtsch.-Inform. Pieter Hollants
IT-Systemmanagement Pieter Hollants          Tel. : (+49) (0)6192-910717
Rossertstraße 80                             Fax  : (+49) (0)6192-910713
65830 Kriftel                                eMail: pieter at hollants.com

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: coova-chilli-1.2.2-access_request_radius_pack_id.diff
URL: <http://lists.coova.org/pipermail/chilli/attachments/20100516/2f5a37a5/attachment.txt>