Ready for 2.2.7?

[Jouni Malinen]

On Tue, Mar 31, 2015 at 8:13 PM, Alan DeKok <aland at deployingradius.com> wrote:
> On Mar 31, 2015, at 1:00 PM, Jouni Malinen <jkmalinen at gmail.com> wrote:
>> The error with 3.0.2 when eap_workaround=0 is used looked like this:
>> EAP-PEAP: TLS done, proceed to Phase 2
>> ...
>> EAP-PEAP: Selected Phase 2 EAP vendor 0 method 26
>> EAP-MSCHAPV2: Invalid header: len=71 ms_len=33
>
>   Ah... the extended EAP packets.  That came up on the list:
>
> http://lists.freeradius.org/pipermail/freeradius-users/2015-March/075995.html

This "EAP vendor 0 method 26" is not actually a case of an expanded
EAP header. wpa_supplicant prints out "vendor 0" for simplicity with
the same debug routines, but that IETF vendor code would never use an
expanded header.

The EAP-MSCHAPv2 header mismatch seems to be something that was fixed
somewhere between FreeRADIUS 3.0.2 and 3.0.3. There were number of
fixes in that area, so I did not confirm which one was the exact
change that addressed this part. Anyway, that's working fine since
3.0.3.

>   The supplicant was sending an entire EAP packet inside of an extended EAP packet.  i.e. the extended EAP packet didn't contain the data for EAP type 26.  Instead, it contain a complete EAP packet.
>
>   Is that really what's supposed to happen?  I don't see that in the RFCs.

I'm not sure what to say about PEAP v0 and various incarnations of it
from Microsoft, but PEAP v1 and other tunneling EAP methods should
allow expanded EAP header to be used within the tunnel. MS-PEAP is a
bit strange on this due to the lovely optimization of not included the
inner header in _some_ cases..

There is no point in using the expanded header with IETF vendor 0. I
think it is allowed (with the potential caveat of MS-PEAP having an
exception), though. As an example, if I configure wpa_supplicant to
use expanded EAP header for EAP-MSCHAPv2 within PEAP tunnel,
authentication does work against hostapd as the EAP server. Actually,
this seemed to work both with PEAPv0 (four bytes of the inner EAP
header removed) and PEAPv1 (full inner EAP header included). Not that
I would expect anyone to use such a setup with EAP-MSCHAPv2, but
anyway, yes, the expanded header can be used even within a PEAP
tunnel.

As far as that specific log in the "MACSEC on Cisco 3750-X and
FreeRADIUS 2.2.5" thread is concerned, that does look very similar to
the output I see in my test that forced expanded EAP header to be used
in the tunnel. I can reproduce the same result against FreeRADIUS
3.0.7 with my modified eapol_test version:

(8) eap_peap: Session established.  Decoding tunneled attributes
(8) eap_peap: PEAP state phase2
(8) eap_peap: EAP type unknown (254)
(8) eap_peap: Got tunneled request
(8) eap_peap:   EAP-Message =
0x0208004efe0000000000001a0208004231d9f55a7fcffb4ef8c1cf17df47726130000000000000000066e764105cf7c78c7e33f67d8aa85334cf6e71900dd9ee6a006a6b6d2d6d73636861707632
(8) eap_peap: Setting User-Name to jkm-mschapv2
(8) eap_peap: Sending tunneled request to inner-tunnel
(8) eap_peap:   EAP-Message =
0x0208004efe0000000000001a0208004231d9f55a7fcffb4ef8c1cf17df47726130000000000000000066e764105cf7c78c7e33f67d8aa85334cf6e71900dd9ee6a006a6b6d2d6d73636861707632

This is then failing here:

(8)   # Executing group from file
/home/jm/FreeRADIUS/307/etc/raddb/sites-enabled/inner-tunnel
(8)     authenticate {
(8) eap: Badly formatted EAP Message: Ignoring the packet
(8) eap: Failed in handler
(8)       [eap] = invalid
(8)     } # authenticate = invalid
(8)   Failed to authenticate the user


So the outer handling looks fine, but the inner tunnel handler is
clearly unwilling to accept expanded EAP header.

I would say that it would be reasonable to make FreeRADIUS support
expanded EAP header here. RFC 3748 has following to say about this:
"An implementation that supports the Expanded attribute MUST treat EAP
Types that are less than 256 equivalently, whether they appear as a
single octet or as the 32-bit Vendor-Type within an Expanded Type
where Vendor-Id is 0."

I guess it would be fair to say that FreeRADIUS doesn't currently
support the Expanded attribute, so in that sense the MUST requirement
does not apply. Anyway, it would not be that difficult to add support
for this if there are real EAP clients that do decide to use expanded
header with vendor=0 for some strange reason. eapol_test supports this
if you want to run a test yourself. It is not configurable, though, so
the following change would be needed for such a test:

diff --git a/src/eap_common/eap_common.c b/src/eap_common/eap_common.c
index 1de1328..f1f65c3 100644
--- a/src/eap_common/eap_common.c
+++ b/src/eap_common/eap_common.c
@@ -131,8 +131,11 @@ struct wpabuf * eap_msg_alloc(int vendor, EapType
type, size_t payload_len,
  struct wpabuf *buf;
  struct eap_hdr *hdr;
  size_t len;
+ int expanded = vendor != EAP_VENDOR_IETF;

- len = sizeof(struct eap_hdr) + (vendor == EAP_VENDOR_IETF ? 1 : 8) +
+ if (type == EAP_TYPE_MSCHAPV2 && code == EAP_CODE_RESPONSE)
+ expanded = 1; /* TESTING */
+ len = sizeof(struct eap_hdr) + (expanded ? 8 : 1) +
  payload_len;
  buf = wpabuf_alloc(len);
  if (buf == NULL)
@@ -143,7 +146,7 @@ struct wpabuf * eap_msg_alloc(int vendor, EapType
type, size_t payload_len,
  hdr->identifier = identifier;
  hdr->length = host_to_be16(len);

- if (vendor == EAP_VENDOR_IETF) {
+ if (!expanded) {
  wpabuf_put_u8(buf, type);
  } else {
  wpabuf_put_u8(buf, EAP_TYPE_EXPANDED);


(sorry about the lack of indentation.. not feeling like fixing this with gmail)

- Jouni