Rtl8812au: rtl8814au - Radiotap FCS flag is set but frame does not contain FCS

[bkiziuk]

I tried to use DWA192 USB dongle, based on 8814au chipset, with driver 5.1.5.
I seems that the driver sets FCS flag in radiotap header even though FCS is not present.
I also tried to capture on DWA182 (rtl8812au, same driver) and the problem doesn't appear, FCS tag is set and FCS is there.

[bkiziuk]

I've re-downloaded the driver and I can see this commit in the source code, but unfortunately it doesn't work for me.
frame example

[bkiziuk]

Yes, every frame is damaged like this. Waiting for good news :-)

[jcard0na]

The driver marks all frames as containing the Frame Check Sequence at the end. This confuses Wireshark because the FCS field is stripped before the frame is released on the monitor interface. You can avoid that by commenting out the following line in core/rtw_recv.c:

https://github.com/aircrack-ng/rtl8812au/blob/d0d20a3db0ef75f008aaf31b85c6f383e58fb451/core/rtw_recv.c#L3610-L3611

[kimocoder]

Great find. So what are pros vs cons by commenting out the line? how does it impact usages of the driver as per "normal" usage & penetration testing?

[jcard0na]

I have not found any problems with that change, but I have to admit that I've mostly used that the card as a sniffer, and even that has not been that useful due to #86.
In addition to that, all I can say is that with that change my device continues to associate and pass traffic in managed mode.

[kimocoder]

@bkiziuk can you try @jcard0na solution here and report back? would be great. don't wan't to make these changes without proper tests on it. thanks!

[bkiziuk]

@kimocoder
I tested it today. I took the newest master branch and applied @jcard0na solution.
For me, it works fine. I can access external networks and I can catch traffic with Wireshark without any issues.

[rofl0r]

+1 please merge the fix

this caused us from reaver team major annoying internal discussion because i removed the ignore-fcs-errors default, which is the only correct solution, but nobody would even had noticed wasn't it for this driver bug. :(

https://github.com/t6x/reaver-wps-fork-t6x/issues/193

[kcdtv]

Commenting the line as suggested by jcard0na (thanks!) fixed our issue with reaver and wash. It also fixed the issue with wireshark: captured packets do not appear anymore as "truncated" 😺
For me it is 👍 ❤️

[rofl0r]

right. i analyzed pcap's from before and after the change and can confirm that the patch is 100% correct. @jcard0na : could you open a proper PR with your change please ?

[kimocoder]

It's been added to my private fork. Merging changes.

[rofl0r]

thanks! imo it would have been better if the commit would have been made with @jcard0na as --author, plus a reference made to this issue report (e.g. fixes #28). the commit message is also a bit imprecise, since this is not about confusion of wireshark, but simply about generating a wrong packet.
this is meant as an improvement suggestion for future commits, not as critics. thank you!

[kimocoder]

I agree. I rushed a solution over here, cause this actually fixed injection & handshake capturing it seems. May actually capture handshake with 8814. Great. All creds to @jcard0na for sure, will do it properly next time.

[jcard0na]

Oh, no worries about credit here. I'm glad it worked for everyone.
Keep up the good work!

[j-forristal]

This commit appears to fix 8814 while breaking 8812 at the same time.

Digging around, the culprit is different logic when 8814 vs 8812 will strip the FCS (particularly in monitor mode). E.g.:

8814:

#ifdef CONFIG_RX_PACKET_APPEND_FCS
                if(pattrib->pkt_rpt_type == NORMAL_RX)
                        pattrib->pkt_len -= IEEE80211_FCS_LEN;
#endif

https://github.com/aircrack-ng/rtl8812au/blob/v5.1.5/hal/rtl8814a/usb/usb_ops_linux.c#L359

8812:

#ifdef CONFIG_RX_PACKET_APPEND_FCS
                if (check_fwstate(&padapter->mlmepriv, WIFI_MONITOR_STATE) == _FALSE)
                        if ((pattrib->pkt_rpt_type == NORMAL_RX) && (pHalData->ReceiveConfig & RCR_APPFCS))
                                pattrib->pkt_len -= IEEE80211_FCS_LEN;
#endif

https://github.com/aircrack-ng/rtl8812au/blob/v5.1.5/hal/rtl8812a/usb/usb_ops_linux.c#L170

8814 strips the FCS almost always, so it not being present (as a hardcoded decision, i.e. the prior commit change) works. But in 8812, note it won't strip the FCS in monitor mode, and other logic conditionals. The end effect is packets from 8812au devices now have an extra FCS on the end, and wireshark complains because radiotap FCS present bit isn't set.

As it turns out, that prior line of code (that just hardcoded the bit on/off) wasn't great anyways, since it didn't account for the CONFIG_RX_PACKET_APPEND_FCS define. But that define looks to be permanently defined (it's in include/autoconf.h, which doesn't seem to ever be regenerated in this build system). It also seems that, if CONFIG_RX_PACKET_APPEND_FCS is set, then ReceiveConfig RCR_APPFCS is always set too. So really the true end difference is 8814 always strips FCS, while 8812 only stripped it when not in monitor mode.

These three locations need to converge on the conditions FCS is stripped, or not, and make sure the radiotap header is updated accordingly. I don't have an 8814 to test with, but the following change to rtw_recv.c works for 8812 in monitor mode:

#ifdef CONFIG_RX_PACKET_APPEND_FCS
        // Start by always indicating FCS is there:
        hdr_buf[rt_len] |= IEEE80211_RADIOTAP_F_FCS;

        // Next, test for conditions of prior removed FCS, and update flag accordingly:
        if(check_fwstate(&padapter->mlmepriv,WIFI_MONITOR_STATE) == _FALSE)
                if((pattrib->pkt_rpt_type == NORMAL_RX) && (pHalData->ReceiveConfig & RCR_APPFCS))
                        hdr_buf[rt_len] &= ~IEEE80211_RADIOTAP_F_FCS;
#endif

I imagine updating usb_ops_linux.c of 8814 to match what's in usb_ops_linux.c of 8812 will converge everything. It seems preferable to leave FCS in for monitor mode, as that's consistent with other drivers and supports folks who desire doing FCS checksum experiments with packets.

(Edit: formatting)

[rofl0r]

@j-forristal
can you please post a diff/patch/pull request with your suggested change so others can easily test it ?

[j-forristal]

@rofl0r This change, and some other tweaks I'm working through, can be found in https://github.com/j-forristal/rtl8812au

I don't have an 8814 to test with, so the 8814 code is only a proposal at this point.

Thanks.

[kimocoder]

I got the AWUS1900 with 8814 onboard, so I may test that one.
Let me take a look the next few days and I may check.

As for now the injection on 8814 works fine, but we wan't them both to work of course.
Great work, let us see what this brings.

What changes did this do to the 8812 ? As I don't got that chipset, did the injection work before? Was it fixes in your repo (with the changes you made)?

[rofl0r]

direct link to the patch: https://github.com/j-forristal/rtl8812au/commit/14109ecfdbb4e02b2cb60cc045b17cdd108fee92

[kimocoder]

I will need someone to test changes for the 8812 om this one (and test), I'll grab the 8814 test.

[kimocoder]

Patch confirmed working on 8814au. Injection looks good. but I will need a confirmation on the 8812au, not only a "aireplay -9" test, it will print "Injection is working!" even if it's sluggish, so I suggest using e.g "wifite" and capture a handshake or something more.

[kcdtv]

Hi there! I have just tried the drivers from forristal branch with my rtl8812au (AWUS036ACH). So far so good! :smiley_cat:
uname -a Linux kalimuX0 4.14.0-kali3-amd64 #1 SMP Debian 4.14.17-1kali1 (2018-02-16) x86_64 GNU/Linux
Injection test gave good results; than i checked aireplay-ng DoS attacks and could obtain the handshake very fast:

I did not see anything weird and the suite aircrack-ng seems to work pretty well, monitoring and injecting.

[kimocoder]

@j-forristal looks like it's working on both 8812au and 8814au. Feel free to make the PR, it's absolutely accepted. great work!!

Tested 8814au on kernel v4.15.11 and passed with glance, nothing weird came out of the debug here either. awesome.

[rofl0r]

i think the right thing to test would be to look at wireshark and see whether the FCS is correct or packets marked as corrupted. also running wash would reveal FCS issues.

[kcdtv]

Packets aren't misformed/corrupted (checked in wireshark)

[kimocoder]

wash is reporting on bad fcs packets, I removed the support for pre-configured seq-num via RadioTap and it's working like a charm. reaver stopped yesterday under tests, but removing seq-num support seems to fix it.

the weird was, after hitting my test router yesterday it crashed the WPS function every single time (using reaver with pixiedust) and it reported FCS issues, but today after removing the SeqNum and with @j-forristal patches the pixiedust used 1 second to complete, without having this issues. wash doesn't report bad fcs longer either.

the changes has been pushed as they finally seems to be fixing my headache, at least for 8814au.

[rofl0r]

@kimocoder interesting. would you mind providing me with a pcap that exposes those issues with wash ? (i.e. done with the driver pre-seqnum removal commit)
a handful of packets would be sufficient.

[kimocoder]

Wireshark (tcpdump pcap format) with removed pre-configured SeqNum support.
you need a dump with SeqNum support also?

• captured with AWUS1900 on 8814au

captured-patched.zip

[kimocoder]

@rofl0r just tried re-producing everything (going back before patches and even mixing them up), but today everything seems to be working even before those patches (8814 at least). the routers WPS function doesn't crash or nothing.

no fcs messages and reaver (pixiedust) works before those patches provided and seqnum was removed.
wondering what the f* wrong (happened) over here, besides me of course.

just trash the "findings" above, the only question I'm stuck with is "stick with pre-configured SeqNum support" or not.

leaving everything as it is for now, it seems to be working great both for 8812 (kcdtv) and 8814 (me) for now. it captures, it cracks and no fcs messages.

[rofl0r]

thanks! i would suggest if nothing is wrong with the seqnum stuff, it'd be preferable to revert your commit that disabled it.

[kimocoder]

I'll need to do some more checks before it re-enters. Also noticed the developer who actually added the SeqNum PR removed it on his own repo, wonder why but he haven't answered the question yet.

Thank for feedback, looks like we at least got a 8812au + power saving fix out of this. Satisfied with that.

[j-forristal]

@kimocoder I would prefer if userspace could specify the sequence number. Otherwise, when injecting packets (presumably with spoofed source addresses), it becomes obvious it's the same radio when the sequence numbers are sequential across all the packets.

That said, I'll live with whatever works. :)

Thanks BTW to you and many others for continued work/maintenance on this driver.