| Message ID | 1402424350-19261-4-git-send-email-ilan.peer@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Jun 10, 2014 at 09:19:07PM +0300, Ilan Peer wrote: > Some APs (Cisco) will tack on a weird IE to the end of the TDLS confirm > packet, which can fail negotiation. Could you please clarify what you mean with "a weird IE"? Is this the known issue of the TDLS Setup frame being shorter than the minimum Ethernet frame and the AP adding some arbitrary data to end to reach that minimum size even though the frame does not go through any real Ethernet interface? Or are you actually seeing some real IEs there? The a-bit-too-short-frame issue should really be worked around with a more complete change that pads the TDLS frame with an extra IE in the end (e.g., vendor specific IE defined just for this purpose) to avoid interop issues with an AP that behaves in this way. It is not enough to make this specific implementation ignore parsing errors since some other peer implementations may not do that.
On Mon, Jun 16, 2014 at 11:41 PM, Jouni Malinen <j@w1.fi> wrote: > On Tue, Jun 10, 2014 at 09:19:07PM +0300, Ilan Peer wrote: >> Some APs (Cisco) will tack on a weird IE to the end of the TDLS confirm >> packet, which can fail negotiation. > > Could you please clarify what you mean with "a weird IE"? Is this the > known issue of the TDLS Setup frame being shorter than the minimum > Ethernet frame and the AP adding some arbitrary data to end to reach > that minimum size even though the frame does not go through any real > Ethernet interface? Or are you actually seeing some real IEs there? > > The a-bit-too-short-frame issue should really be worked around with a > more complete change that pads the TDLS frame with an extra IE in the > end (e.g., vendor specific IE defined just for this purpose) to avoid > interop issues with an AP that behaves in this way. It is not enough to > make this specific implementation ignore parsing errors since some other > peer implementations may not do that. The frame is not too-short but too-long. We have a Cisco 1260 AP that adds some weird bits at the end of TDLS setup-confirm and TDLS discovery-response packets (fixed that in another patch). It's after the link-id and it's not an IE, so the wpa_s parser complains about buffer underflow and fails it. I think wireshark half-parsed it as related to CCX. Arik
On Tue, Jun 17, 2014 at 09:23:40AM +0300, Arik Nemtsov wrote: > The frame is not too-short but too-long. We have a Cisco 1260 AP that > adds some weird bits at the end of TDLS setup-confirm and TDLS > discovery-response packets (fixed that in another patch). > It's after the link-id and it's not an IE, so the wpa_s parser > complains about buffer underflow and fails it. I think wireshark > half-parsed it as related to CCX. OK, this is a different issue then than the one I've seen previously. I applied 4 and 7. I'm dropping 6 now. With that, none of these seven patches remain in my queue. Please resubmit an updated version of the couple of open patches once ready.
On Tue, Jun 17, 2014 at 6:44 PM, Jouni Malinen <j@w1.fi> wrote: > On Tue, Jun 17, 2014 at 09:23:40AM +0300, Arik Nemtsov wrote: >> The frame is not too-short but too-long. We have a Cisco 1260 AP that >> adds some weird bits at the end of TDLS setup-confirm and TDLS >> discovery-response packets (fixed that in another patch). >> It's after the link-id and it's not an IE, so the wpa_s parser >> complains about buffer underflow and fails it. I think wireshark >> half-parsed it as related to CCX. > > OK, this is a different issue then than the one I've seen previously. I > applied 4 and 7. I'm dropping 6 now. With that, none of these seven > patches remain in my queue. Please resubmit an updated version of the > couple of open patches once ready. Thanks. We'll also have a bit more stuff by then probably. Arik
diff --git a/src/rsn_supp/tdls.c b/src/rsn_supp/tdls.c index f0ef6c7..d8f7a47 100644 --- a/src/rsn_supp/tdls.c +++ b/src/rsn_supp/tdls.c @@ -2297,10 +2297,15 @@ static int wpa_tdls_process_tpk_m3(struct wpa_sm *sm, const u8 *src_addr, pos += 2 /* status code */ + 1 /* dialog token */; ielen = len - (pos - buf); /* start of IE in buf */ - if (wpa_supplicant_parse_ies((const u8 *) pos, ielen, &kde) < 0) { + + /* + * Don't fail if failing to parse IEs. The IEs we need are explicitly + * checked below. + * Some APs piggy-back broken IEs to the end of a TDLS confirm packet, + * which will fail the link if we don't ignore this error. + */ + if (wpa_supplicant_parse_ies((const u8 *)pos, ielen, &kde) < 0) wpa_printf(MSG_INFO, "TDLS: Failed to parse KDEs in TPK M3"); - goto error; - } if (kde.lnkid == NULL || kde.lnkid_len < 3 * ETH_ALEN) { wpa_printf(MSG_INFO, "TDLS: No Link Identifier IE in TPK M3");