Unfortunately there is no way to determine the MIC length just from the
eapol frame. 802.11 defined AKMs define the MIC length, but non 802.11
AKMs (e.g. OWE) can define their own MIC length. For this reason it seem
infeasable to track these special AKM's data flow to determine the MIC
length.
To work around this we can just try different MIC lengths (since there
are only 3 after all). This allows us to get key data length and see if
the total packet size equals the frame length + key data length. If the
sizes don't match we can try the next MIC length.
The MIC length was hard coded to 16 bytes everywhere, and since several
AKMs require larger MIC's (24/32) this needed to change. The main issue
was that the MIC was hard coded to 16 bytes inside eapol_key. Instead
of doing this, the MIC, key_data_length, and key_data elements were all
bundled into key_data[0]. In order to retrieve the MIC, key_data_len,
or key_data several macros were introduced which account for the MIC
length provided.
A consequence of this is that all the verify functions inside eapol now
require the MIC length as a parameter because without it they cannot
determine the byte offset of key_data or key_data_length.
The MIC length for a given handshake is set inside the SM when starting
EAPoL. This length is determined by the AKM for the handshake.
Non-802.11 AKMs can define their own key lengths. Currently only OWE does
this, and the MIC/KEK/KCK lengths will be determined by the PMK length so
we need to save it.
Make sure we don't pass NULLs to memcmp or l_memdup when the prefix
buffer is NULL. There's no point having callers pass dummy buffers if
they need to watch frames independent of the frame data.
Start using l_key_generate_dh_private and l_key_validate_dh_payload to
check for the disallowed corner case values in the DH private/public
values generated/received.
Some of the EAP methods don't require a clear-text identity to
be sent with the Identity Response packet. The mandatory identity
filed has resulted in unnecessary transmission of the garbage
values. This patch makes the Identity field to be optional and
shift responsibility to ensure its existence to the individual
methods if the field is required. All necessary identity checks
have been previously propagated to individual methods.
If a network is being forgotten, then make sure to reset connected_time.
Otherwise the rank logic thinks that the network is known which can
result in network_find_rank_index returning -1.
Found by sanitizer:
src/network.c:1329:23: runtime error: index -1 out of bounds for type
'double [64]'
==25412==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000421ab0 at pc 0x000000402faf bp 0x7fffffffdb00 sp 0x7fffffffdaf0
READ of size 4 at 0x000000421ab0 thread T0
#0 0x402fae in validate_mgmt_ies src/mpdu.c:128
#1 0x403ce8 in validate_probe_request_mmpdu src/mpdu.c:370
#2 0x404ef2 in validate_mgmt_mpdu src/mpdu.c:662
#3 0x405166 in mpdu_validate src/mpdu.c:706
#4 0x402529 in ie_order_test unit/test-mpdu.c:156
#5 0x418f49 in l_test_run ell/test.c:83
#6 0x402715 in main unit/test-mpdu.c:171
#7 0x7ffff5d43ed9 in __libc_start_main (/lib64/libc.so.6+0x20ed9)
#8 0x4019a9 in _start (/home/denkenz/iwd-master/unit/test-mpdu+0x4019a9)
==24642==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe00450850 at pc 0x7f2043eef5e3 bp 0x7ffe00450660 sp 0x7ffe0044fdf0
WRITE of size 3 at 0x7ffe00450850 thread T0
#0 0x7f2043eef5e2 in __interceptor_vsprintf /var/tmp/portage/sys-devel/gcc-8.2.0-r2/work/gcc-8.2.0/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1522
#1 0x7f2043eef956 in __interceptor_sprintf /var/tmp/portage/sys-devel/gcc-8.2.0-r2/work/gcc-8.2.0/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1553
#2 0x4026f1 in prf_test unit/test-prf-sha1.c:64
#3 0x407478 in l_test_run ell/test.c:83
#4 0x4029e3 in main unit/test-prf-sha1.c:130
#5 0x7f2042dd9ed9 in __libc_start_main (/lib64/libc.so.6+0x20ed9)
#6 0x401f79 in _start (/home/denkenz/iwd-master/unit/test-prf-sha1+0x401f79)
This fixes the valgrind warning:
==14804== Conditional jump or move depends on uninitialised value(s)
==14804== at 0x402E56: sae_is_quadradic_residue (sae.c:218)
==14804== by 0x402E56: sae_compute_pwe (sae.c:272)
==14804== by 0x402E56: sae_build_commit (sae.c:333)
==14804== by 0x402E56: sae_send_commit (sae.c:591)
==14804== by 0x401CC3: test_confirm_after_accept (test-sae.c:454)
==14804== by 0x408A28: l_test_run (test.c:83)
==14804== by 0x401427: main (test-sae.c:566)
Now that the peer element is validated we must send valid point data.
This should have been done in the beginning anyways, but since the
point data was not previously validated this test still passed.
Since this particular unit test is really testing timeouts we can just
use the commit buffer for other tests since it contains a valid scalar
and point.
The return from l_ecc_point_from_data was not being checked for NULL,
which would cause a segfault if the peer sent an invalid point.
This adds a check and fails the protocol if p_element is NULL, as the
spec defines.
