[PATCH 1/4] eap-tls-common: Address PEAPv0 interoperability with Windows

[PATCH 1/4] eap-tls-common: Address PEAPv0 interoperability with Windows

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 3515 bytes --]

Windows Server 2008 - Network Policy Server (NPS) generates an invalid
Compound MAC for Cryptobinding TLV when is used within PEAPv0 due to
incorrect parsing of the message containing TLS Client Hello.
Setting L bit and including TLS Message Length field, even for the
packets that do not require fragmentation, corrects the issue. The
redundant TLS Message Length field in unfragmented packets doesn't
seem to effect the other server implementations.
---
 src/eap-tls-common.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/src/eap-tls-common.c b/src/eap-tls-common.c
index 080e4f27..d62d7ca5 100644
--- a/src/eap-tls-common.c
+++ b/src/eap-tls-common.c
@@ -112,6 +112,7 @@ struct eap_tls_state {
 	size_t tx_frag_last_len;
 
 	bool expecting_frag_ack:1;
+	bool tunnel_ready:1;
 
 	struct l_queue *ca_cert;
 	struct l_certchain *client_cert;
@@ -128,6 +129,7 @@ static void __eap_tls_common_state_reset(struct eap_tls_state *eap_tls)
 	eap_tls->method_completed = false;
 	eap_tls->phase2_failed = false;
 	eap_tls->expecting_frag_ack = false;
+	eap_tls->tunnel_ready = false;
 
 	if (eap_tls->tunnel) {
 		l_tls_free(eap_tls->tunnel);
@@ -244,6 +246,8 @@ static void eap_tls_tunnel_ready(const char *peer_identity, void *user_data)
 	 */
 	eap_start_complete_timeout(eap);
 
+	eap_tls->tunnel_ready = true;
+
 	if (!eap_tls->variant_ops->tunnel_ready)
 		return;
 
@@ -261,6 +265,7 @@ static void eap_tls_tunnel_disconnected(enum l_tls_alert_desc reason,
 			eap_get_method_name(eap), l_tls_alert_to_str(reason));
 
 	eap_tls->method_completed = true;
+	eap_tls->tunnel_ready = false;
 }
 
 static bool eap_tls_validate_version(struct eap_state *eap,
@@ -325,11 +330,32 @@ static void eap_tls_send_fragment(struct eap_state *eap)
 	eap_tls->tx_frag_last_len = len;
 }
 
+static bool needs_workaround(struct eap_state *eap)
+{
+	struct eap_tls_state *eap_tls = eap_get_data(eap);
+
+	/*
+	 * Windows Server 2008 - Network Policy Server (NPS) generates an
+	 * invalid Compound MAC for Cryptobinding TLV when is used within PEAPv0
+	 * due to incorrect parsing of the message containing TLS Client Hello.
+	 * Setting L bit and including TLS Message Length field, even for the
+	 * packets that do not require fragmentation, corrects the issue. The
+	 * redundant TLS Message Length field in unfragmented packets doesn't
+	 * seem to effect the other server implementations.
+	 */
+	return eap_get_method_type(eap) == EAP_TYPE_PEAP &&
+			eap_tls->version_negotiated == EAP_TLS_VERSION_0 &&
+			!eap_tls->tunnel_ready;
+}
+
 static void eap_tls_send_response(struct eap_state *eap,
 					const uint8_t *pdu, size_t pdu_len)
 {
 	struct eap_tls_state *eap_tls = eap_get_data(eap);
 	size_t msg_len = EAP_TLS_HEADER_LEN + pdu_len;
+	bool set_tls_msg_len = needs_workaround(eap);
+
+	msg_len += set_tls_msg_len ? 4 : 0;
 
 	if (msg_len <= eap_get_mtu(eap)) {
 		uint8_t *buf;
@@ -345,6 +371,15 @@ static void eap_tls_send_response(struct eap_state *eap,
 		buf[EAP_TLS_HEADER_OCTET_FLAGS + extra] =
 						eap_tls->version_negotiated;
 
+		if (set_tls_msg_len) {
+			buf[extra + EAP_TLS_HEADER_OCTET_FLAGS] |=
+								EAP_TLS_FLAG_L;
+			l_put_be32(pdu_len,
+				   &buf[extra + EAP_TLS_HEADER_OCTET_FRAG_LEN]);
+
+			extra += 4;
+		}
+
 		memcpy(buf + EAP_TLS_HEADER_LEN + extra, pdu, pdu_len);
 
 		eap_send_response(eap, eap_get_method_type(eap), buf, msg_len);
-- 
2.13.6

[PATCH 2/4] peap: Add inner EAP key material into imsk calculation

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 3154 bytes --]

For the inner EAP methods that support generation of the key material
include it into imck generation. This allows to cryptographically
bind the inner method with the tunnel.
---
 src/eap-peap.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/eap-peap.c b/src/eap-peap.c
index 005eacc4..6decbfb3 100644
--- a/src/eap-peap.c
+++ b/src/eap-peap.c
@@ -46,8 +46,23 @@ struct peap_state {
 	struct eap_state *phase2;
 
 	uint8_t key[128];
+	uint8_t isk[32];
 };
 
+static void eap_peap_phase2_key_ready(const uint8_t *msk_data, size_t msk_len,
+				const uint8_t *emsk_data, size_t emsk_len,
+				const uint8_t *iv, size_t iv_len,
+				const uint8_t *session_id, size_t session_len,
+				void *user_data)
+{
+	struct peap_state *peap_state =
+				eap_tls_common_get_variant_data(user_data);
+
+	l_debug("PEAP: New ISK received");
+
+	memcpy(peap_state->isk, msk_data, sizeof(peap_state->isk));
+}
+
 static void eap_peap_phase2_send_response(const uint8_t *pdu, size_t pdu_len,
 								void *user_data)
 {
@@ -103,6 +118,7 @@ static void eap_peap_phase2_complete(enum eap_result result, void *user_data)
 	eap_set_key_material(eap, peap_state->key + 0, 64, NULL, 0, NULL,
 								0, NULL, 0);
 	explicit_bzero(peap_state->key, sizeof(peap_state->key));
+	explicit_bzero(peap_state->isk, sizeof(peap_state->isk));
 
 	eap_method_success(eap);
 }
@@ -144,12 +160,9 @@ static bool cryptobinding_tlv_generate_imck(struct eap_state *eap,
 {
 	struct peap_state *peap_state = eap_tls_common_get_variant_data(eap);
 	static const char *label = "Inner Methods Compound Keys";
-	uint8_t isk[32];
-
-	memset(isk, 0, sizeof(isk));
 
 	if (!prf_plus_sha1(peap_state->key, 40, label, strlen(label),
-					isk, sizeof(isk), imck_out, 60))
+					peap_state->isk, 32, imck_out, 60))
 		return false;
 
 	return true;
@@ -441,6 +454,7 @@ static void eap_extensions_handle_request(struct eap_state *eap,
 	eap_set_key_material(eap, peap_state->key + 0, 64, NULL, 0, NULL,
 								0, NULL, 0);
 	explicit_bzero(peap_state->key, sizeof(peap_state->key));
+	explicit_bzero(peap_state->isk, sizeof(peap_state->isk));
 
 	eap_method_success(eap);
 }
@@ -528,6 +542,7 @@ static void eap_peap_state_reset(void *variant_data)
 	eap_reset(peap_state->phase2);
 
 	explicit_bzero(peap_state->key, sizeof(peap_state->key));
+	explicit_bzero(peap_state->isk, sizeof(peap_state->isk));
 }
 
 static void eap_peap_state_destroy(void *variant_data)
@@ -541,6 +556,7 @@ static void eap_peap_state_destroy(void *variant_data)
 	eap_free(peap_state->phase2);
 
 	explicit_bzero(peap_state->key, sizeof(peap_state->key));
+	explicit_bzero(peap_state->isk, sizeof(peap_state->isk));
 
 	l_free(peap_state);
 }
@@ -604,6 +620,8 @@ static bool eap_peap_settings_load(struct eap_state *eap,
 
 	peap_state = l_new(struct peap_state, 1);
 	peap_state->phase2 = phase2;
+	eap_set_key_material_func(peap_state->phase2,
+						eap_peap_phase2_key_ready);
 
 	snprintf(setting_key_prefix, sizeof(setting_key_prefix), "%sPEAP-",
 									prefix);
-- 
2.13.6

[PATCH 3/4] auto-t: Test PEAPv0 cryptobinding

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 9378 bytes --]

---
 .../misc/secrets/eap-user-peap-v0-mschapv2.text    |  4 ++
 autotests/testEAP-PEAPv0-CryptoBinding/ISK_test.py | 63 ++++++++++++++++
 .../testEAP-PEAPv0-CryptoBinding/NoISK_test.py     | 83 ++++++++++++++++++++++
 autotests/testEAP-PEAPv0-CryptoBinding/hw.conf     |  9 +++
 autotests/testEAP-PEAPv0-CryptoBinding/main.conf   |  2 +
 .../ssidEAP-PEAPv0-ISK.8021x                       | 12 ++++
 .../ssidEAP-PEAPv0-ISK.conf                        | 12 ++++
 .../ssidEAP-PEAPv0-NoISK.8021x                     | 12 ++++
 .../ssidEAP-PEAPv0-NoISK.conf                      | 12 ++++
 9 files changed, 209 insertions(+)
 create mode 100644 autotests/misc/secrets/eap-user-peap-v0-mschapv2.text
 create mode 100644 autotests/testEAP-PEAPv0-CryptoBinding/ISK_test.py
 create mode 100644 autotests/testEAP-PEAPv0-CryptoBinding/NoISK_test.py
 create mode 100644 autotests/testEAP-PEAPv0-CryptoBinding/hw.conf
 create mode 100644 autotests/testEAP-PEAPv0-CryptoBinding/main.conf
 create mode 100644 autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-ISK.8021x
 create mode 100644 autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-ISK.conf
 create mode 100644 autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-NoISK.8021x
 create mode 100644 autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-NoISK.conf

diff --git a/autotests/misc/secrets/eap-user-peap-v0-mschapv2.text b/autotests/misc/secrets/eap-user-peap-v0-mschapv2.text
new file mode 100644
index 00000000..c91693c4
--- /dev/null
+++ b/autotests/misc/secrets/eap-user-peap-v0-mschapv2.text
@@ -0,0 +1,4 @@
+# Phase 1 users
+* PEAP [ver=0]
+# Phase 2
+"secure(a)identity.com" MSCHAPV2 "testpasswd" [2]
diff --git a/autotests/testEAP-PEAPv0-CryptoBinding/ISK_test.py b/autotests/testEAP-PEAPv0-CryptoBinding/ISK_test.py
new file mode 100644
index 00000000..7ba49384
--- /dev/null
+++ b/autotests/testEAP-PEAPv0-CryptoBinding/ISK_test.py
@@ -0,0 +1,63 @@
+#!/usr/bin/python3
+
+import unittest
+import sys
+import time
+
+sys.path.append('../util')
+import iwd
+from iwd import IWD
+from iwd import NetworkType
+import testutil
+
+from hostapd import HostapdCLI
+from hostapd import hostapd_map
+
+class Test(unittest.TestCase):
+
+    def validate_connection(self, wd):
+        devices = wd.list_devices(1)
+        self.assertIsNotNone(devices)
+        device = devices[0]
+
+        condition = 'not obj.scanning'
+        wd.wait_for_object_condition(device, condition)
+
+        device.scan()
+
+        condition = 'not obj.scanning'
+        wd.wait_for_object_condition(device, condition)
+
+        ordered_network = device.get_ordered_network('ssidEAP-PEAPv0-ISK')
+
+        self.assertEqual(ordered_network.type, NetworkType.eap)
+
+        condition = 'not obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+        ordered_network.network_object.connect()
+
+        condition = 'obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+        device.disconnect()
+
+        condition = 'not obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+
+    def test_connection_success(self):
+        wd = IWD(True)
+
+        self.validate_connection(wd)
+
+    @classmethod
+    def setUpClass(cls):
+        IWD.copy_to_storage('ssidEAP-PEAPv0-ISK.8021x')
+
+    @classmethod
+    def tearDownClass(cls):
+        IWD.clear_storage()
+
+if __name__ == '__main__':
+    unittest.main(exit=True)
diff --git a/autotests/testEAP-PEAPv0-CryptoBinding/NoISK_test.py b/autotests/testEAP-PEAPv0-CryptoBinding/NoISK_test.py
new file mode 100644
index 00000000..fde8bfc2
--- /dev/null
+++ b/autotests/testEAP-PEAPv0-CryptoBinding/NoISK_test.py
@@ -0,0 +1,83 @@
+#!/usr/bin/python3
+
+import unittest
+import sys
+import time
+
+sys.path.append('../util')
+import iwd
+from iwd import IWD
+from iwd import NetworkType
+import testutil
+
+from hostapd import HostapdCLI
+from hostapd import hostapd_map
+
+class Test(unittest.TestCase):
+
+    def validate_connection(self, wd):
+        hostapd = None
+
+        for hostapd_if in list(hostapd_map.values()):
+            hpd = HostapdCLI(hostapd_if)
+            if hpd.get_config_value('ssid') == 'ssidEAP-PEAPv0-NoISK':
+                hostapd = hpd
+                break
+
+        self.assertIsNotNone(hostapd)
+
+        devices = wd.list_devices(1)
+        self.assertIsNotNone(devices)
+        device = devices[0]
+
+        condition = 'not obj.scanning'
+        wd.wait_for_object_condition(device, condition)
+
+        device.scan()
+
+        condition = 'not obj.scanning'
+        wd.wait_for_object_condition(device, condition)
+
+        ordered_network = device.get_ordered_network('ssidEAP-PEAPv0-NoISK')
+
+        self.assertEqual(ordered_network.type, NetworkType.eap)
+
+        condition = 'not obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+        ordered_network.network_object.connect()
+
+        condition = 'obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+        hostapd.eapol_reauth(device.address)
+
+        wd.wait(10)
+
+        condition = 'obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+        testutil.test_iface_operstate()
+        testutil.test_ifaces_connected()
+
+        device.disconnect()
+
+        condition = 'not obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+
+    def test_connection_success(self):
+        wd = IWD(True)
+
+        self.validate_connection(wd)
+
+    @classmethod
+    def setUpClass(cls):
+        IWD.copy_to_storage('ssidEAP-PEAPv0-NoISK.8021x')
+
+    @classmethod
+    def tearDownClass(cls):
+        IWD.clear_storage()
+
+if __name__ == '__main__':
+    unittest.main(exit=True)
diff --git a/autotests/testEAP-PEAPv0-CryptoBinding/hw.conf b/autotests/testEAP-PEAPv0-CryptoBinding/hw.conf
new file mode 100644
index 00000000..cceb79fb
--- /dev/null
+++ b/autotests/testEAP-PEAPv0-CryptoBinding/hw.conf
@@ -0,0 +1,9 @@
+[SETUP]
+num_radios=3
+start_iwd=0
+max_test_exec_interval_sec=60
+tmpfs_extra_stuff=../misc/certs:../misc/secrets:main.conf
+
+[HOSTAPD]
+rad0=ssidEAP-PEAPv0-NoISK.conf
+rad1=ssidEAP-PEAPv0-ISK.conf
diff --git a/autotests/testEAP-PEAPv0-CryptoBinding/main.conf b/autotests/testEAP-PEAPv0-CryptoBinding/main.conf
new file mode 100644
index 00000000..55a5543e
--- /dev/null
+++ b/autotests/testEAP-PEAPv0-CryptoBinding/main.conf
@@ -0,0 +1,2 @@
+[General]
+UseDefaultInterface=true
diff --git a/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-ISK.8021x b/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-ISK.8021x
new file mode 100644
index 00000000..56eed087
--- /dev/null
+++ b/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-ISK.8021x
@@ -0,0 +1,12 @@
+[Security]
+EAP-Method=PEAP
+EAP-Identity=open(a)identity.com
+EAP-PEAP-CACert=/tmp/certs/cert-ca.pem
+EAP-PEAP-ClientCert=/tmp/certs/cert-client.pem
+EAP-PEAP-ClientKey=/tmp/certs/cert-client-key-pkcs8.pem
+EAP-PEAP-Phase2-Method=MSCHAPV2
+EAP-PEAP-Phase2-Identity=secure(a)identity.com
+EAP-PEAP-Phase2-Password=testpasswd
+
+[Settings]
+AutoConnect=false
diff --git a/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-ISK.conf b/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-ISK.conf
new file mode 100644
index 00000000..c9980bd9
--- /dev/null
+++ b/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-ISK.conf
@@ -0,0 +1,12 @@
+hw_mode=g
+channel=1
+ssid=ssidEAP-PEAPv0-ISK
+
+wpa=3
+wpa_key_mgmt=WPA-EAP
+ieee8021x=1
+eap_server=1
+eap_user_file=/tmp/secrets/eap-user-peap-v0-mschapv2.text
+ca_cert=/tmp/certs/cert-ca.pem
+server_cert=/tmp/certs/cert-server.pem
+private_key=/tmp/certs/cert-server-key.pem
diff --git a/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-NoISK.8021x b/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-NoISK.8021x
new file mode 100644
index 00000000..382f86d0
--- /dev/null
+++ b/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-NoISK.8021x
@@ -0,0 +1,12 @@
+[Security]
+EAP-Method=PEAP
+EAP-Identity=open(a)identity.com
+EAP-PEAP-CACert=/tmp/certs/cert-ca.pem
+EAP-PEAP-ClientCert=/tmp/certs/cert-client.pem
+EAP-PEAP-ClientKey=/tmp/certs/cert-client-key-pkcs8.pem
+EAP-PEAP-Phase2-Method=MD5
+EAP-PEAP-Phase2-Identity=secure(a)identity.com
+EAP-PEAP-Phase2-Password=testpasswd
+
+[Settings]
+AutoConnect=false
diff --git a/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-NoISK.conf b/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-NoISK.conf
new file mode 100644
index 00000000..b92bb1ae
--- /dev/null
+++ b/autotests/testEAP-PEAPv0-CryptoBinding/ssidEAP-PEAPv0-NoISK.conf
@@ -0,0 +1,12 @@
+hw_mode=g
+channel=1
+ssid=ssidEAP-PEAPv0-NoISK
+
+wpa=3
+wpa_key_mgmt=WPA-EAP
+ieee8021x=1
+eap_server=1
+eap_user_file=/tmp/secrets/eap-user-peap-v0.text
+ca_cert=/tmp/certs/cert-ca.pem
+server_cert=/tmp/certs/cert-server.pem
+private_key=/tmp/certs/cert-server-key.pem
-- 
2.13.6

[PATCH 4/4] peap: Fail auth. if invalid compound MAC is received

[Tim Kourt]

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

Since the interoperability with the Windows server has been achieved,
witch back to failing authentication if invalid compound MAC is
received.
---
 src/eap-peap.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/eap-peap.c b/src/eap-peap.c
index 6decbfb3..8c3ba2f3 100644
--- a/src/eap-peap.c
+++ b/src/eap-peap.c
@@ -244,11 +244,7 @@ static int eap_extensions_handle_cryptobinding_tlv(struct eap_state *eap,
 					cryptobinding_compound_mac_len)) {
 		l_error("PEAP: Generated compound MAC and server compound MAC "
 							"don't match.");
-		/*
-		 * Ignore the Crypto-Binding TLV in the case of unmatched
-		 * compound MACs.
-		 */
-		return 0;
+		return -EIO;
 	}
 
 	/* Build response Crypto-Binding TLV */
-- 
2.13.6

Re: [PATCH 1/4] eap-tls-common: Address PEAPv0 interoperability with Windows

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

Hi Tim,

On 1/31/20 4:25 PM, Tim Kourt wrote:
> Windows Server 2008 - Network Policy Server (NPS) generates an invalid
> Compound MAC for Cryptobinding TLV when is used within PEAPv0 due to
> incorrect parsing of the message containing TLS Client Hello.
> Setting L bit and including TLS Message Length field, even for the
> packets that do not require fragmentation, corrects the issue. The
> redundant TLS Message Length field in unfragmented packets doesn't
> seem to effect the other server implementations.
> ---
>   src/eap-tls-common.c | 35 +++++++++++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
> 

patches 1-3 applied, thanks.

Regards,
-Denis

Re: [PATCH 4/4] peap: Fail auth. if invalid compound MAC is received

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 336 bytes --]

Hi Tim,

On 1/31/20 4:25 PM, Tim Kourt wrote:
> Since the interoperability with the Windows server has been achieved,
> witch back to failing authentication if invalid compound MAC is
> received.
> ---
>   src/eap-peap.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 

Applied, thanks.

Regards,
-Denis