From df999d1fca209767a235b5aaa6e3e7e5294746fc Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 15 Nov 2022 13:48:25 +0100 Subject: [PATCH 1/2] macsec: allow CKN shorter than 64 characters See wpa_supplicant commit [1]: macsec: Make pre-shared CKN variable length IEEE Std 802.1X-2010, 9.3.1 defines following restrictions for CKN: "MKA places no restriction on the format of the CKN, save that it comprise an integral number of octets, between 1 and 32 (inclusive), and that all potential members of the CA use the same CKN. No further constraints are placed on the CKNs used with PSKs, ..." Hence do not require a 32 octet long CKN but instead allow a shorter CKN to be configured. This fixes interoperability with some Aruba switches, that do not accept a 32 octet long CKN (only support shorter ones). [1] https://w1.fi/cgit/hostap/commit/?id=b678ed1efc50e8da4638d962f8eac13312a4048f --- src/core/supplicant/nm-supplicant-config.c | 14 +++++++++-- src/libnm-core-impl/nm-setting-macsec.c | 29 +++++++++++++++------- src/libnm-core-public/nm-setting-macsec.h | 2 ++ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/core/supplicant/nm-supplicant-config.c b/src/core/supplicant/nm-supplicant-config.c index 6d3ea10220..c63a00585a 100644 --- a/src/core/supplicant/nm-supplicant-config.c +++ b/src/core/supplicant/nm-supplicant-config.c @@ -403,6 +403,7 @@ nm_supplicant_config_add_setting_macsec(NMSupplicantConfig *self, const char *value; char buf[32]; int port; + gsize key_len; g_return_val_if_fail(NM_IS_SUPPLICANT_CONFIG(self), FALSE); g_return_val_if_fail(setting != NULL, FALSE); @@ -446,7 +447,16 @@ nm_supplicant_config_add_setting_macsec(NMSupplicantConfig *self, return FALSE; value = nm_setting_macsec_get_mka_ckn(setting); - if (!value || !nm_utils_hexstr2bin_buf(value, FALSE, FALSE, NULL, buffer_ckn)) { + if (!value + || !nm_utils_hexstr2bin_full(value, + FALSE, + FALSE, + FALSE, + NULL, + 0, + buffer_ckn, + G_N_ELEMENTS(buffer_ckn), + &key_len)) { g_set_error_literal(error, NM_SUPPLICANT_ERROR, NM_SUPPLICANT_ERROR_CONFIG, @@ -456,7 +466,7 @@ nm_supplicant_config_add_setting_macsec(NMSupplicantConfig *self, if (!nm_supplicant_config_add_option(self, "mka_ckn", (char *) buffer_ckn, - sizeof(buffer_ckn), + key_len, value, error)) return FALSE; diff --git a/src/libnm-core-impl/nm-setting-macsec.c b/src/libnm-core-impl/nm-setting-macsec.c index 3c928e0138..86949e1d2f 100644 --- a/src/libnm-core-impl/nm-setting-macsec.c +++ b/src/libnm-core-impl/nm-setting-macsec.c @@ -236,7 +236,7 @@ need_secrets(NMSetting *setting, gboolean check_rerequest) static gboolean verify_macsec_key(const char *key, gboolean cak, GError **error) { - int req_len; + size_t len; /* CAK is a connection secret and can be NULL for various * reasons (agent-owned, no permissions to get secrets, etc.) @@ -252,14 +252,25 @@ verify_macsec_key(const char *key, gboolean cak, GError **error) return FALSE; } - req_len = cak ? NM_SETTING_MACSEC_MKA_CAK_LENGTH : NM_SETTING_MACSEC_MKA_CKN_LENGTH; - if (strlen(key) != (gsize) req_len) { - g_set_error(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("the key must be %d characters"), - req_len); - return FALSE; + len = strlen(key); + if (cak) { + if (len != NM_SETTING_MACSEC_MKA_CAK_LENGTH) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("the key must be %d characters"), + NM_SETTING_MACSEC_MKA_CAK_LENGTH); + return FALSE; + } + } else { + if (len < 2 || len > 64 || len % 2 != 0) { + g_set_error_literal( + error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("the key must have an even number of characters between 2 and 64")); + return FALSE; + } } if (!NM_STRCHAR_ALL(key, ch, g_ascii_isxdigit(ch))) { diff --git a/src/libnm-core-public/nm-setting-macsec.h b/src/libnm-core-public/nm-setting-macsec.h index 52e4313d45..c2662b1f5e 100644 --- a/src/libnm-core-public/nm-setting-macsec.h +++ b/src/libnm-core-public/nm-setting-macsec.h @@ -73,6 +73,8 @@ typedef enum { } NMSettingMacsecValidation; #define NM_SETTING_MACSEC_MKA_CAK_LENGTH 32 + +/* Deprecated. The CKN can be between 2 and 64 characters. */ #define NM_SETTING_MACSEC_MKA_CKN_LENGTH 64 NM_AVAILABLE_IN_1_6 From dfe63d9eb3e11e67c0d4c5e1e64148e21a822d99 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 15 Nov 2022 13:58:49 +0100 Subject: [PATCH 2/2] macsec: document the format of CAK and CKN properties --- src/libnm-core-impl/nm-setting-macsec.c | 5 +++-- src/libnmc-setting/settings-docs.h.in | 4 ++-- src/nmcli/gen-metadata-nm-settings-nmcli.xml.in | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-macsec.c b/src/libnm-core-impl/nm-setting-macsec.c index 86949e1d2f..bf48f049df 100644 --- a/src/libnm-core-impl/nm-setting-macsec.c +++ b/src/libnm-core-impl/nm-setting-macsec.c @@ -501,7 +501,7 @@ nm_setting_macsec_class_init(NMSettingMacsecClass *klass) * NMSettingMacsec:mka-cak: * * The pre-shared CAK (Connectivity Association Key) for MACsec - * Key Agreement. + * Key Agreement. Must be a string of 32 hexadecimal characters. * * Since: 1.6 **/ @@ -532,7 +532,8 @@ nm_setting_macsec_class_init(NMSettingMacsecClass *klass) * NMSettingMacsec:mka-ckn: * * The pre-shared CKN (Connectivity-association Key Name) for - * MACsec Key Agreement. + * MACsec Key Agreement. Must be a string of hexadecimal characters + * with a even length between 2 and 64. * * Since: 1.6 **/ diff --git a/src/libnmc-setting/settings-docs.h.in b/src/libnmc-setting/settings-docs.h.in index 56b63760c4..2873abd127 100644 --- a/src/libnmc-setting/settings-docs.h.in +++ b/src/libnmc-setting/settings-docs.h.in @@ -226,9 +226,9 @@ #define DESCRIBE_DOC_NM_SETTING_IP_TUNNEL_TOS N_("The type of service (IPv4) or traffic class (IPv6) field to be set on tunneled packets.") #define DESCRIBE_DOC_NM_SETTING_IP_TUNNEL_TTL N_("The TTL to assign to tunneled packets. 0 is a special value meaning that packets inherit the TTL value.") #define DESCRIBE_DOC_NM_SETTING_MACSEC_ENCRYPT N_("Whether the transmitted traffic must be encrypted.") -#define DESCRIBE_DOC_NM_SETTING_MACSEC_MKA_CAK N_("The pre-shared CAK (Connectivity Association Key) for MACsec Key Agreement.") +#define DESCRIBE_DOC_NM_SETTING_MACSEC_MKA_CAK N_("The pre-shared CAK (Connectivity Association Key) for MACsec Key Agreement. Must be a string of 32 hexadecimal characters.") #define DESCRIBE_DOC_NM_SETTING_MACSEC_MKA_CAK_FLAGS N_("Flags indicating how to handle the \"mka-cak\" property.") -#define DESCRIBE_DOC_NM_SETTING_MACSEC_MKA_CKN N_("The pre-shared CKN (Connectivity-association Key Name) for MACsec Key Agreement.") +#define DESCRIBE_DOC_NM_SETTING_MACSEC_MKA_CKN N_("The pre-shared CKN (Connectivity-association Key Name) for MACsec Key Agreement. Must be a string of hexadecimal characters with a even length between 2 and 64.") #define DESCRIBE_DOC_NM_SETTING_MACSEC_MODE N_("Specifies how the CAK (Connectivity Association Key) for MKA (MACsec Key Agreement) is obtained.") #define DESCRIBE_DOC_NM_SETTING_MACSEC_PARENT N_("If given, specifies the parent interface name or parent connection UUID from which this MACSEC interface should be created. If this property is not specified, the connection must contain an \"802-3-ethernet\" setting with a \"mac-address\" property.") #define DESCRIBE_DOC_NM_SETTING_MACSEC_PORT N_("The port component of the SCI (Secure Channel Identifier), between 1 and 65534.") diff --git a/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in b/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in index 8ce403e94f..941b1e353e 100644 --- a/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in +++ b/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in @@ -774,12 +774,12 @@ description="Whether the transmitted traffic must be encrypted." /> + description="The pre-shared CAK (Connectivity Association Key) for MACsec Key Agreement. Must be a string of 32 hexadecimal characters." /> + description="The pre-shared CKN (Connectivity-association Key Name) for MACsec Key Agreement. Must be a string of hexadecimal characters with a even length between 2 and 64." />