On 18 Mar 2016, at 23:22, Matthew Newton <mcn4@leicester.ac.uk> wrote:
On Fri, Mar 18, 2016 at 12:02:40PM +0000, Matthew Newton wrote:
On Fri, Mar 18, 2016 at 09:23:20AM +0000, Scott Armitage wrote:
Alan Buxey and myself have spent some time and believe we have tracked down the commit which broke EAP:
commit 8a7f6e330f45439d333f61dde7ee0982ebcc2a29 Author: Arran Cudbard-Bell <a.cudbardb@freeradius.org> Date: Sun Dec 6 00:34:21 2015 -0500
Add additional debugging so we can track TLS fragments sent
Hmm. If that's the case, then the bug is probably to do with the length included flag and the length of the packet.
Close. Think I may have found it. Have found the differences, anyway.
What's your supplicant, just we can make sure we're using roughly the same methodology. Mine is Windows 10 with PEAP outer EAP-TLS inner.
First confusing thing is the FreeRADIUS is only printing out the first EAP-Message attribute in the debug output, hence the lengths before looked wrong (e.g. showing length 1000, but only having 247 or so bytes in the response). This is just because the other three EAP-Message attributes aren't being printed.
Fixed the debug. Still used a stack buffer of 256 bytes.
Testing here with eapol_test shows that they are being returned correctly and this is only a debug output issue.
Yes, it was.
The new code, for fragmented packets, is arguably more correct.
In 8a7f6e330f^, in my test here, I have
Challenge 1: length 1004, flags L+M Challenge 2: length 1000, flags M Challenge 3: length 726
Total length 2730, all verified OK (correct lengths in packets etc) as far as I can tell.
In 8a7f6e330f (with the patch) for the same request/config I get:
Challenge 1: length 1000, flags L+M Challenge 2: length 1000, flags M Challenge 3: length 730
So it's not going over the MTU, which is better.
Agreed :)
However...
The key difference seems to be that in packets that do *not* need fragmenting, the L flag and length are now included, whereas in 8a7f6e330f^ they are not.
Yes. I got that far too. In packets that don't need fragmenting, sent from the supplicant the L flag is set.
This seems fine in the PEAP negotiation stage, but fails on the first packet after PEAP goes into its inner tunnel.
If you don't fragment the inner tunnel data at all it succeeds. Arguably, PEAP, when proxying should set a more appropriate Framed-MTU. Having two layers of fragmentation is inefficient, both in terms of processing, and in terms of network utilisation. Internal attributes now are only limited by system memory. The entire certificate chain from the inner EAP module should be packed up and returned to the outer EAP module, where it can sit in the outbound memory buffer.
My hypothesis would be a supplicant bug that sees the Length flag set so erroneously expects more packets, but that should not be the case because M was not set. It certainly works fine in eapol_test both pre- and post-8a7f6e330f, so it seems down to the way the particular supplicant processes the reply.
No, something weirder. Even when fragmentation is required it hangs.
draft-josefsson-pppext-eap-tls-eap-02 is silent on this matter it seems. L and M MUST be set for the start of fragments, L to set the length of the whole amount of data. But there is no mention of whether L on its own (for a non-fragmented packet), without M, is banned. Certainly the supplicant seems to send data this way to the server,
Yes I saw that.
and other packets are sent like this back to the supplicant, hence guessing this change in behaviour is just hitting one particular Windows supplicant in the wrong places.
Yes. Have you got PEAP + TLS to run to completion before the patch? I suspect PEAP with MSCHAPv2 will, but PEAP with TLS (that still requires fragmentation), won't. -Arran Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS development team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2