From 8b98482e041a7ee2a5c0327b16fcbec30d8d878b Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 16 Oct 2019 18:48:22 +0200 Subject: [PATCH 1/3] enum: Add compile-time check for missing strings If strings are missing (e.g. because the last value of a range changed unknowingly or adding a string was simply forgotten) compilation will now fail. This could be problematic if the upper limit is out of our control (e.g. from a system header like pfkeyv2.h), in which case patches might be required on certain platforms (enforcing at least, and not exactly, the required number of strings might also be an option to compile against older versions of such a header - for internal enums it's obviously better to enforce an exact match, though). --- src/libstrongswan/tests/suites/test_enum.c | 6 ----- src/libstrongswan/utils/enum.h | 28 +++++++++++++++------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/libstrongswan/tests/suites/test_enum.c b/src/libstrongswan/tests/suites/test_enum.c index dd6b86f8e..06b2c3ff3 100644 --- a/src/libstrongswan/tests/suites/test_enum.c +++ b/src/libstrongswan/tests/suites/test_enum.c @@ -28,9 +28,6 @@ enum { CONT5, } test_enum_cont; -/* can't be static */ -enum_name_t *test_enum_cont_names; - ENUM_BEGIN(test_enum_cont_names, CONT1, CONT5, "CONT1", "CONT2", "CONT3", "CONT4", "CONT5"); ENUM_END(test_enum_cont_names, CONT5); @@ -46,9 +43,6 @@ enum { SPLIT5 = 255, } test_enum_split; -/* can't be static */ -enum_name_t *test_enum_split_names; - ENUM_BEGIN(test_enum_split_names, SPLIT1, SPLIT2, "SPLIT1", "SPLIT2"); ENUM_NEXT(test_enum_split_names, SPLIT3, SPLIT4, SPLIT2, diff --git a/src/libstrongswan/utils/enum.h b/src/libstrongswan/utils/enum.h index 4312cb9a1..888dbe54f 100644 --- a/src/libstrongswan/utils/enum.h +++ b/src/libstrongswan/utils/enum.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009 Tobias Brunner + * Copyright (C) 2009-2019 Tobias Brunner * Copyright (C) 2006-2008 Martin Willi * HSR Hochschule fuer Technik Rapperswil * @@ -59,10 +59,11 @@ typedef struct enum_name_t enum_name_t; * by the numerical enum value. */ struct enum_name_t { - /** value of the first enum string */ - int first; + /** value of the first enum string, values are expected to be (u_)int, using + * int64_t here instead, however, avoids warnings for large unsigned ints */ + int64_t first; /** value of the last enum string */ - int last; + int64_t last; /** next enum_name_t in list, or ENUM_FLAG_MAGIC */ enum_name_t *next; /** array of strings containing names from first to last */ @@ -77,7 +78,10 @@ struct enum_name_t { * @param last enum value of the last enum string * @param ... a list of strings */ -#define ENUM_BEGIN(name, first, last, ...) static enum_name_t name##last = {first, last, NULL, { __VA_ARGS__ }} +#define ENUM_BEGIN(name, first, last, ...) \ + static enum_name_t name##last = {first, last + \ + BUILD_ASSERT(((last)-(first)+1) == countof(((char*[]){__VA_ARGS__}))), \ + NULL, { __VA_ARGS__ }} /** * Continue a enum name list startetd with ENUM_BEGIN. @@ -88,7 +92,10 @@ struct enum_name_t { * @param prev enum value of the "last" defined in ENUM_BEGIN/previous ENUM_NEXT * @param ... a list of strings */ -#define ENUM_NEXT(name, first, last, prev, ...) static enum_name_t name##last = {first, last, &name##prev, { __VA_ARGS__ }} +#define ENUM_NEXT(name, first, last, prev, ...) \ + static enum_name_t name##last = {first, last + \ + BUILD_ASSERT(((last)-(first)+1) == countof(((char*[]){__VA_ARGS__}))), \ + &name##prev, { __VA_ARGS__ }} /** * Complete enum name list started with ENUM_BEGIN. @@ -109,7 +116,8 @@ struct enum_name_t { * @param last enum value of the last enum string * @param ... a list of strings */ -#define ENUM(name, first, last, ...) ENUM_BEGIN(name, first, last, __VA_ARGS__); ENUM_END(name, last) +#define ENUM(name, first, last, ...) \ + ENUM_BEGIN(name, first, last, __VA_ARGS__); ENUM_END(name, last) /** * Define a enum name with only one range for flags. @@ -125,8 +133,10 @@ struct enum_name_t { * @param ... a list of strings */ #define ENUM_FLAGS(name, first, last, ...) \ - static enum_name_t name##last = {first, last, ENUM_FLAG_MAGIC, { __VA_ARGS__ }}; \ - ENUM_END(name, last) + static enum_name_t name##last = {first, last + \ + BUILD_ASSERT((__builtin_ffs(last)-__builtin_ffs(first)+1) == \ + countof(((char*[]){__VA_ARGS__}))), \ + ENUM_FLAG_MAGIC, { __VA_ARGS__ }}; ENUM_END(name, last) /** * Convert a enum value to its string representation. From 45c8399d783876543e59b815d61d449eb9ee67ef Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 22 Oct 2019 11:04:30 +0200 Subject: [PATCH 2/3] Add missing strings to several enum string definitions --- .../plugins/kernel_netlink/kernel_netlink_net.c | 5 +++-- src/libcharon/plugins/smp/smp.c | 2 ++ src/libstrongswan/plugins/pkcs11/pkcs11_library.c | 3 ++- .../tnccs_20/messages/ita/pb_mutual_capability_msg.c | 12 +++++++----- .../tnccs_20/messages/ita/pb_mutual_capability_msg.h | 2 +- src/libtpmtss/tpm_tss_tss2_names_v2.c | 1 + 6 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c b/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c index e6b3d10c3..f95ee3ce5 100644 --- a/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c +++ b/src/libcharon/plugins/kernel_netlink/kernel_netlink_net.c @@ -90,14 +90,15 @@ ENUM(rt_msg_names, RTM_NEWLINK, RTM_GETRULE, "RTM_NEWADDR", "RTM_DELADDR", "RTM_GETADDR", - "31", + "23", "RTM_NEWROUTE", "RTM_DELROUTE", "RTM_GETROUTE", - "35", + "27", "RTM_NEWNEIGH", "RTM_DELNEIGH", "RTM_GETNEIGH", + "31", "RTM_NEWRULE", "RTM_DELRULE", "RTM_GETRULE", diff --git a/src/libcharon/plugins/smp/smp.c b/src/libcharon/plugins/smp/smp.c index ad848182c..2953a603b 100644 --- a/src/libcharon/plugins/smp/smp.c +++ b/src/libcharon/plugins/smp/smp.c @@ -56,7 +56,9 @@ ENUM(ike_sa_state_lower_names, IKE_CREATED, IKE_DELETING, "created", "connecting", "established", + "passive", "rekeying", + "rekeyed", "deleting", ); diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_library.c b/src/libstrongswan/plugins/pkcs11/pkcs11_library.c index b42632fdb..6967ac253 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_library.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_library.c @@ -49,7 +49,7 @@ ENUM_NEXT(ck_rv_names, CKR_ATTRIBUTE_READ_ONLY, CKR_ATTRIBUTE_VALUE_INVALID, "ATTRIBUTE_VALUE_INVALID"); ENUM_NEXT(ck_rv_names, CKR_DATA_INVALID, CKR_DATA_LEN_RANGE, CKR_ATTRIBUTE_VALUE_INVALID, - "DATA_INVALID" + "DATA_INVALID", "DATA_LEN_RANGE"); ENUM_NEXT(ck_rv_names, CKR_DEVICE_ERROR, CKR_DEVICE_REMOVED, CKR_DATA_LEN_RANGE, @@ -540,6 +540,7 @@ ENUM_NEXT(ck_attr_names, CKA_HW_FEATURE_TYPE, CKA_HAS_RESET, "HAS_RESET"); ENUM_NEXT(ck_attr_names, CKA_PIXEL_X, CKA_BITS_PER_PIXEL, CKA_HAS_RESET, "PIXEL_X", + "PIXEL_Y", "RESOLUTION", "CHAR_ROWS", "CHAR_COLUMNS", diff --git a/src/libtnccs/plugins/tnccs_20/messages/ita/pb_mutual_capability_msg.c b/src/libtnccs/plugins/tnccs_20/messages/ita/pb_mutual_capability_msg.c index c31752019..f8b22c60b 100644 --- a/src/libtnccs/plugins/tnccs_20/messages/ita/pb_mutual_capability_msg.c +++ b/src/libtnccs/plugins/tnccs_20/messages/ita/pb_mutual_capability_msg.c @@ -19,11 +19,13 @@ #include #include -ENUM(pb_tnc_mutual_protocol_type_names, PB_MUTUAL_HALF_DUPLEX, - PB_MUTUAL_FULL_DUPLEX, - "half duplex", - "full duplex" -); +ENUM_BEGIN(pb_tnc_mutual_protocol_type_names, PB_MUTUAL_FULL_DUPLEX, + PB_MUTUAL_FULL_DUPLEX, + "full duplex"); +ENUM_NEXT(pb_tnc_mutual_protocol_type_names, PB_MUTUAL_HALF_DUPLEX, + PB_MUTUAL_HALF_DUPLEX, PB_MUTUAL_FULL_DUPLEX, + "half duplex"); +ENUM_END(pb_tnc_mutual_protocol_type_names, PB_MUTUAL_HALF_DUPLEX); typedef struct private_pb_mutual_capability_msg_t private_pb_mutual_capability_msg_t; diff --git a/src/libtnccs/plugins/tnccs_20/messages/ita/pb_mutual_capability_msg.h b/src/libtnccs/plugins/tnccs_20/messages/ita/pb_mutual_capability_msg.h index db810a012..c0f628e3a 100644 --- a/src/libtnccs/plugins/tnccs_20/messages/ita/pb_mutual_capability_msg.h +++ b/src/libtnccs/plugins/tnccs_20/messages/ita/pb_mutual_capability_msg.h @@ -30,8 +30,8 @@ typedef struct pb_mutual_capability_msg_t pb_mutual_capability_msg_t; * PB-TNC mutual protocol types */ enum pb_tnc_mutual_protocol_type_t { + PB_MUTUAL_FULL_DUPLEX = (1 << 30), PB_MUTUAL_HALF_DUPLEX = (1 << 31), - PB_MUTUAL_FULL_DUPLEX = (1 << 30) }; /** diff --git a/src/libtpmtss/tpm_tss_tss2_names_v2.c b/src/libtpmtss/tpm_tss_tss2_names_v2.c index c8d29e4e6..2b48408c4 100644 --- a/src/libtpmtss/tpm_tss_tss2_names_v2.c +++ b/src/libtpmtss/tpm_tss_tss2_names_v2.c @@ -51,6 +51,7 @@ ENUM_NEXT(tpm_alg_id_names, TPM2_ALG_SM3_256, TPM2_ALG_ECMQV, TPM2_ALG_NULL, "OAEP", "ECDSA", "ECDH", + "ECDAA", "SM2", "ECSCHNORR", "ECMQV" From f3d8179b4baaaccfab622afe09f97c7cb9cb41f3 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 16 Oct 2019 19:46:09 +0200 Subject: [PATCH 3/3] kernel-pfkey: Add additional strings for extensions on different platforms Don't define structs for macOS as we don't need them (that's true for most of the others too, though) and at least one is defined inside an extra ifdef. --- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index 0ae331424..a97daf4a9 100644 --- a/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -692,12 +692,27 @@ struct pfkey_msg_t struct sadb_x_kmprivate *x_kmprivate; /* SADB_X_EXT_KMPRIVATE */ struct sadb_x_policy *x_policy; /* SADB_X_EXT_POLICY */ struct sadb_x_sa2 *x_sa2; /* SADB_X_EXT_SA2 */ +#if defined(__linux__) || defined (__FreeBSD__) struct sadb_x_nat_t_type *x_natt_type; /* SADB_X_EXT_NAT_T_TYPE */ struct sadb_x_nat_t_port *x_natt_sport; /* SADB_X_EXT_NAT_T_SPORT */ struct sadb_x_nat_t_port *x_natt_dport; /* SADB_X_EXT_NAT_T_DPORT */ +#ifdef __linux__ struct sadb_address *x_natt_oa; /* SADB_X_EXT_NAT_T_OA */ struct sadb_x_sec_ctx *x_sec_ctx; /* SADB_X_EXT_SEC_CTX */ struct sadb_x_kmaddress *x_kmaddress; /* SADB_X_EXT_KMADDRESS */ +#else + struct sadb_address *x_natt_oai; /* SADB_X_EXT_NAT_T_OAI */ + struct sadb_address *x_natt_oar; /* SADB_X_EXT_NAT_T_OAR */ +#ifdef SADB_X_EXT_NAT_T_FRAG + struct sadb_x_nat_t_frag *x_natt_frag; /* SADB_X_EXT_NAT_T_FRAG */ +#ifdef SADB_X_EXT_SA_REPLAY + struct sadb_x_sa_replay *x_replay; /* SADB_X_EXT_SA_REPLAY */ + struct sadb_address *x_new_addr_src; /* SADB_X_EXT_NEW_ADDRESS_SRC */ + struct sadb_address *x_new_addr_dst; /* SADB_X_EXT_NEW_ADDRESS_DST */ +#endif +#endif +#endif /* __linux__ */ +#endif /* __linux__ || __FreeBSD__ */ } __attribute__((__packed__)); }; }; @@ -723,12 +738,34 @@ ENUM(sadb_ext_type_names, SADB_EXT_RESERVED, SADB_EXT_MAX, "SADB_X_EXT_KMPRIVATE", "SADB_X_EXT_POLICY", "SADB_X_EXT_SA2", +#ifdef __APPLE__ + "SADB_EXT_SESSION_ID", + "SADB_EXT_SASTAT", + "SADB_X_EXT_IPSECIF", + "SADB_X_EXT_ADDR_RANGE_SRC_START", + "SADB_X_EXT_ADDR_RANGE_SRC_END", + "SADB_X_EXT_ADDR_RANGE_DST_START", + "SADB_X_EXT_ADDR_RANGE_DST_END", + "SADB_EXT_MIGRATE_ADDRESS_SRC", + "SADB_EXT_MIGRATE_ADDRESS_DST", + "SADB_X_EXT_MIGRATE_IPSECIF", +#else "SADB_X_EXT_NAT_T_TYPE", "SADB_X_EXT_NAT_T_SPORT", "SADB_X_EXT_NAT_T_DPORT", +#ifdef __linux__ "SADB_X_EXT_NAT_T_OA", "SADB_X_EXT_SEC_CTX", - "SADB_X_EXT_KMADDRESS" + "SADB_X_EXT_KMADDRESS", +#else + "SADB_X_EXT_NAT_T_OAI", + "SADB_X_EXT_NAT_T_OAR", + "SADB_X_EXT_NAT_T_FRAG", + "SADB_X_EXT_SA_REPLAY", + "SADB_X_EXT_NEW_ADDRESS_SRC", + "SADB_X_EXT_NEW_ADDRESS_DST", +#endif /* __linux__ */ +#endif /* __APPLE__ */ ); /**