Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
ipsec: move ipsec-enc-alg, ipsec-auth-alg and ipsec-comp-alg sysctl variables out of netlock
To:
Alexander Bluhm <bluhm@openbsd.org>, tech@openbsd.org
Date:
Fri, 2 May 2025 20:45:49 +0300

Download raw body.

Thread
I propose to use integers instead of strings for in-kernel
representation. So we can update them atomically. Also we remove
strcmp() from the hot (?) path.

Note, this diff changes behavior for a little. Against previous it
denies to set incorrect values. In such case the EINVAL error will be
returned to userland.

Index: sys/net/pfkeyv2.c
===================================================================
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.264
diff -u -p -r1.264 pfkeyv2.c
--- sys/net/pfkeyv2.c	11 Mar 2025 15:31:03 -0000	1.264
+++ sys/net/pfkeyv2.c	2 May 2025 17:44:18 -0000
@@ -2157,6 +2157,7 @@ pfkeyv2_acquire(struct ipsec_policy *ipo
 	struct sadb_msg *smsg;
 	int rval = 0;
 	int i, j, registered;
+	int ipsec_def_enc_local, ipsec_def_comp_local, ipsec_def_auth_local;
 
 	mtx_enter(&pfkeyv2_mtx);
 	*seq = pfkeyv2_seq++;
@@ -2239,6 +2240,10 @@ pfkeyv2_acquire(struct ipsec_policy *ipo
 
 	sadb_comb = p;
 
+	ipsec_def_enc_local = atomic_load_int(&ipsec_def_enc);
+	ipsec_def_comp_local = atomic_load_int(&ipsec_def_comp);
+	ipsec_def_auth_local = atomic_load_int(&ipsec_def_auth);
+
 	/* XXX Should actually ask the crypto layer what's supported */
 	for (j = 0; j < sa_prop->sadb_prop_num; j++) {
 		sadb_comb->sadb_comb_flags = 0;
@@ -2246,75 +2251,88 @@ pfkeyv2_acquire(struct ipsec_policy *ipo
 		if (ipsec_require_pfs)
 			sadb_comb->sadb_comb_flags |= SADB_SAFLAGS_PFS;
 
-		/* Set the encryption algorithm */
 		if (ipo->ipo_sproto == IPPROTO_ESP) {
-			if (!strncasecmp(ipsec_def_enc, "aes",
-			    sizeof("aes"))) {
+			/* Set the encryption algorithm */
+			switch(ipsec_def_enc_local) {
+			case IPSEC_ENC_AES:
 				sadb_comb->sadb_comb_encrypt = SADB_X_EALG_AES;
 				sadb_comb->sadb_comb_encrypt_minbits = 128;
 				sadb_comb->sadb_comb_encrypt_maxbits = 256;
-			} else if (!strncasecmp(ipsec_def_enc, "aesctr",
-			    sizeof("aesctr"))) {
-				sadb_comb->sadb_comb_encrypt = SADB_X_EALG_AESCTR;
-				sadb_comb->sadb_comb_encrypt_minbits = 128+32;
-				sadb_comb->sadb_comb_encrypt_maxbits = 256+32;
-			} else if (!strncasecmp(ipsec_def_enc, "3des",
-			    sizeof("3des"))) {
-				sadb_comb->sadb_comb_encrypt = SADB_EALG_3DESCBC;
+				break;
+			case IPSEC_ENC_AESCTR:
+				sadb_comb->sadb_comb_encrypt =
+				    SADB_X_EALG_AESCTR;
+				sadb_comb->sadb_comb_encrypt_minbits = 128 + 32;
+				sadb_comb->sadb_comb_encrypt_maxbits = 256 + 32;
+				break;
+			case IPSEC_ENC_3DES:
+				sadb_comb->sadb_comb_encrypt =
+				    SADB_EALG_3DESCBC;
 				sadb_comb->sadb_comb_encrypt_minbits = 192;
 				sadb_comb->sadb_comb_encrypt_maxbits = 192;
-			} else if (!strncasecmp(ipsec_def_enc, "blowfish",
-			    sizeof("blowfish"))) {
+				break;
+			case IPSEC_ENC_BLOWFISH:
 				sadb_comb->sadb_comb_encrypt = SADB_X_EALG_BLF;
 				sadb_comb->sadb_comb_encrypt_minbits = 40;
-				sadb_comb->sadb_comb_encrypt_maxbits = BLF_MAXKEYLEN * 8;
-			} else if (!strncasecmp(ipsec_def_enc, "cast128",
-			    sizeof("cast128"))) {
+				sadb_comb->sadb_comb_encrypt_maxbits =
+				    BLF_MAXKEYLEN * 8;
+				break;
+			case IPSEC_ENC_CAST128:
 				sadb_comb->sadb_comb_encrypt = SADB_X_EALG_CAST;
 				sadb_comb->sadb_comb_encrypt_minbits = 40;
 				sadb_comb->sadb_comb_encrypt_maxbits = 128;
+				break;
+			default:
+				break;
 			}
 		} else if (ipo->ipo_sproto == IPPROTO_IPCOMP) {
 			/* Set the compression algorithm */
-			if (!strncasecmp(ipsec_def_comp, "deflate",
-			    sizeof("deflate"))) {
-				sadb_comb->sadb_comb_encrypt = SADB_X_CALG_DEFLATE;
+			switch(ipsec_def_comp_local) {
+			case IPSEC_COMP_DEFLATE:
+				sadb_comb->sadb_comb_encrypt =
+				    SADB_X_CALG_DEFLATE;
 				sadb_comb->sadb_comb_encrypt_minbits = 0;
 				sadb_comb->sadb_comb_encrypt_maxbits = 0;
+				break;
+			default:
+				break;
 			}
 		}
 
 		/* Set the authentication algorithm */
-		if (!strncasecmp(ipsec_def_auth, "hmac-sha1",
-		    sizeof("hmac-sha1"))) {
+		switch(ipsec_def_auth_local) {
+		case IPSEC_AUTH_HMAC_SHA1:
 			sadb_comb->sadb_comb_auth = SADB_AALG_SHA1HMAC;
 			sadb_comb->sadb_comb_auth_minbits = 160;
 			sadb_comb->sadb_comb_auth_maxbits = 160;
-		} else if (!strncasecmp(ipsec_def_auth, "hmac-ripemd160",
-		    sizeof("hmac_ripemd160"))) {
+			break;
+		case IPSEC_AUTH_HMAC_RIPEMD160:
 			sadb_comb->sadb_comb_auth = SADB_X_AALG_RIPEMD160HMAC;
 			sadb_comb->sadb_comb_auth_minbits = 160;
 			sadb_comb->sadb_comb_auth_maxbits = 160;
-		} else if (!strncasecmp(ipsec_def_auth, "hmac-md5",
-		    sizeof("hmac-md5"))) {
+			break;
+		case IPSEC_AUTH_MD5:
 			sadb_comb->sadb_comb_auth = SADB_AALG_MD5HMAC;
 			sadb_comb->sadb_comb_auth_minbits = 128;
 			sadb_comb->sadb_comb_auth_maxbits = 128;
-		} else if (!strncasecmp(ipsec_def_auth, "hmac-sha2-256",
-		    sizeof("hmac-sha2-256"))) {
+			break;
+		case IPSEC_AUTH_SHA2_256:
 			sadb_comb->sadb_comb_auth = SADB_X_AALG_SHA2_256;
 			sadb_comb->sadb_comb_auth_minbits = 256;
 			sadb_comb->sadb_comb_auth_maxbits = 256;
-		} else if (!strncasecmp(ipsec_def_auth, "hmac-sha2-384",
-		    sizeof("hmac-sha2-384"))) {
+			break;
+		case IPSEC_AUTH_SHA2_384:
 			sadb_comb->sadb_comb_auth = SADB_X_AALG_SHA2_384;
 			sadb_comb->sadb_comb_auth_minbits = 384;
 			sadb_comb->sadb_comb_auth_maxbits = 384;
-		} else if (!strncasecmp(ipsec_def_auth, "hmac-sha2-512",
-		    sizeof("hmac-sha2-512"))) {
+			break;
+		case IPSEC_AUTH_SHA2_512:
 			sadb_comb->sadb_comb_auth = SADB_X_AALG_SHA2_512;
 			sadb_comb->sadb_comb_auth_minbits = 512;
 			sadb_comb->sadb_comb_auth_maxbits = 512;
+			break;
+		default:
+			break;
 		}
 
 		sadb_comb->sadb_comb_soft_allocations = ipsec_soft_allocations;
Index: sys/netinet/ip_ipsp.h
===================================================================
RCS file: /cvs/src/sys/netinet/ip_ipsp.h,v
retrieving revision 1.248
diff -u -p -r1.248 ip_ipsp.h
--- sys/netinet/ip_ipsp.h	2 Mar 2025 21:28:32 -0000	1.248
+++ sys/netinet/ip_ipsp.h	2 May 2025 17:44:18 -0000
@@ -573,9 +573,30 @@ extern int ipsec_exp_first_use;		/* seco
 #define IPSEC_FIRSTUSE			IPCTL_IPSEC_FIRSTUSE		/* 24 */
 #define IPSEC_MAXID	25
 
-extern char ipsec_def_enc[];
-extern char ipsec_def_auth[];
-extern char ipsec_def_comp[];
+enum {
+	IPSEC_ENC_AES,
+	IPSEC_ENC_AESCTR,
+	IPSEC_ENC_3DES,
+	IPSEC_ENC_BLOWFISH,
+	IPSEC_ENC_CAST128,
+};
+
+enum {
+	IPSEC_AUTH_HMAC_SHA1,
+	IPSEC_AUTH_HMAC_RIPEMD160,
+	IPSEC_AUTH_MD5,
+	IPSEC_AUTH_SHA2_256,
+	IPSEC_AUTH_SHA2_384,
+	IPSEC_AUTH_SHA2_512,
+};
+
+enum {
+	IPSEC_COMP_DEFLATE,
+};
+
+extern int ipsec_def_enc;
+extern int ipsec_def_auth;
+extern int ipsec_def_comp;
 
 extern TAILQ_HEAD(ipsec_policy_head, ipsec_policy) ipsec_policy_head;
 
Index: sys/netinet/ipsec_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
retrieving revision 1.209
diff -u -p -r1.209 ipsec_input.c
--- sys/netinet/ipsec_input.c	4 Mar 2025 15:11:30 -0000	1.209
+++ sys/netinet/ipsec_input.c	2 May 2025 17:44:18 -0000
@@ -139,9 +139,44 @@ struct cpumem *ahcounters;
 struct cpumem *ipcompcounters;
 struct cpumem *ipseccounters;
 
-char ipsec_def_enc[20];
-char ipsec_def_auth[20];
-char ipsec_def_comp[20];
+struct ipsec_sysctl_algorithm {
+	const char *name;
+	int val;
+};
+
+const struct ipsec_sysctl_algorithm ipsec_sysctl_enc_algs[] = {
+	{"aes",			IPSEC_ENC_AES},
+	{"aesctr",		IPSEC_ENC_AESCTR},
+	{"3des",		IPSEC_ENC_3DES},
+	{"blowfish",		IPSEC_ENC_BLOWFISH},
+	{"cast128",		IPSEC_ENC_CAST128},
+};
+
+#define IPSEC_SYSCTL_ENC_ALGS_NUM \
+    (sizeof(ipsec_sysctl_enc_algs) / sizeof(ipsec_sysctl_enc_algs[0]))
+
+const struct ipsec_sysctl_algorithm ipsec_sysctl_auth_algs[] = {
+	{"hmac-sha1",		IPSEC_AUTH_HMAC_SHA1},
+	{"hmac-ripemd160",	IPSEC_AUTH_HMAC_RIPEMD160},
+	{"hmac-md5",		IPSEC_AUTH_MD5},
+	{"hmac-sha2-256",	IPSEC_AUTH_SHA2_256},
+	{"hmac-sha2-384",	IPSEC_AUTH_SHA2_384},
+	{"hmac-sha2-512",	IPSEC_AUTH_SHA2_512},
+};
+
+#define IPSEC_SYSCTL_AUTH_ALGS_NUM \
+    (sizeof(ipsec_sysctl_auth_algs) / sizeof(ipsec_sysctl_auth_algs[0]))
+
+const struct ipsec_sysctl_algorithm ipsec_sysctl_comp_algs[] = {
+	{"deflate",		IPSEC_COMP_DEFLATE},
+};
+
+#define IPSEC_SYSCTL_COMP_ALGS_NUM \
+    (sizeof(ipsec_sysctl_comp_algs) / sizeof(ipsec_sysctl_comp_algs[0]))
+
+int ipsec_def_enc = IPSEC_ENC_AES;		/* [a] */
+int ipsec_def_auth = IPSEC_AUTH_HMAC_SHA1;	/* [a] */
+int ipsec_def_comp = IPSEC_COMP_DEFLATE;	/* [a] */
 
 const struct sysctl_bounded_args ipsecctl_vars[] = {
 	{ IPSEC_ENCDEBUG, &encdebug, 0, 1 },
@@ -158,6 +193,7 @@ const struct sysctl_bounded_args ipsecct
 	{ IPSEC_FIRSTUSE, &ipsec_exp_first_use, 0, INT_MAX },
 };
 
+int ipsec_sysctl_algorythm(int, void *, size_t *, void *, size_t);
 int esp_sysctl_espstat(void *, size_t *, void *);
 int ah_sysctl_ahstat(void *, size_t *, void *);
 int ipcomp_sysctl_ipcompstat(void *, size_t *, void *);
@@ -171,10 +207,6 @@ ipsec_init(void)
 	ipcompcounters = counters_alloc(ipcomps_ncounters);
 	ipseccounters = counters_alloc(ipsec_ncounters);
 
-	strlcpy(ipsec_def_enc, IPSEC_DEFAULT_DEF_ENC, sizeof(ipsec_def_enc));
-	strlcpy(ipsec_def_auth, IPSEC_DEFAULT_DEF_AUTH, sizeof(ipsec_def_auth));
-	strlcpy(ipsec_def_comp, IPSEC_DEFAULT_DEF_COMP, sizeof(ipsec_def_comp));
-
 	ipsp_init();
 }
 
@@ -613,23 +645,10 @@ ipsec_sysctl(int *name, u_int namelen, v
 
 	switch (name[0]) {
 	case IPCTL_IPSEC_ENC_ALGORITHM:
-		NET_LOCK();
-		error = sysctl_tstring(oldp, oldlenp, newp, newlen,
-		    ipsec_def_enc, sizeof(ipsec_def_enc));
-		NET_UNLOCK();
-		return (error);
 	case IPCTL_IPSEC_AUTH_ALGORITHM:
-		NET_LOCK();
-		error = sysctl_tstring(oldp, oldlenp, newp, newlen,
-		    ipsec_def_auth, sizeof(ipsec_def_auth));
-		NET_UNLOCK();
-		return (error);
 	case IPCTL_IPSEC_IPCOMP_ALGORITHM:
-		NET_LOCK();
-		error = sysctl_tstring(oldp, oldlenp, newp, newlen,
-		    ipsec_def_comp, sizeof(ipsec_def_comp));
-		NET_UNLOCK();
-		return (error);
+		return (ipsec_sysctl_algorythm(name[0], oldp, oldlenp,
+		    newp, newlen));
 	case IPCTL_IPSEC_STATS:
 		return (ipsec_sysctl_ipsecstat(oldp, oldlenp, newp));
 	default:
@@ -639,6 +658,74 @@ ipsec_sysctl(int *name, u_int namelen, v
 		NET_UNLOCK();
 		return (error);
 	}
+}
+
+int
+ipsec_sysctl_algorythm(int name, void *oldp, size_t *oldlenp,
+    void *newp, size_t newlen)
+{
+	int *var, oldval, error;
+	size_t i;
+	char buf[20];
+
+	const struct ipsec_sysctl_algorithm *algs; 
+	size_t algs_num;
+
+	switch (name) {
+	case IPCTL_IPSEC_ENC_ALGORITHM:
+		algs = ipsec_sysctl_enc_algs;
+		algs_num = IPSEC_SYSCTL_ENC_ALGS_NUM;
+		var = &ipsec_def_enc;
+		break;
+	case IPCTL_IPSEC_AUTH_ALGORITHM:
+		algs = ipsec_sysctl_auth_algs;
+		algs_num = IPSEC_SYSCTL_AUTH_ALGS_NUM;
+		var = &ipsec_def_auth;
+		break;
+	case IPCTL_IPSEC_IPCOMP_ALGORITHM:
+		algs = ipsec_sysctl_comp_algs;
+		algs_num = IPSEC_SYSCTL_COMP_ALGS_NUM;
+		var = &ipsec_def_comp;
+		break;
+	default:
+		return (EOPNOTSUPP);
+	}
+
+	oldval = atomic_load_int(var);
+
+	for (i=0; i < algs_num; ++i) {
+		if (algs[i].val == oldval) {
+			strlcpy(buf, algs[i].name, sizeof(buf));
+			break;
+		}
+	}
+
+	KASSERT(i < algs_num);
+
+	error = sysctl_tstring(oldp, oldlenp, newp, newlen,
+	    buf, sizeof(buf));
+	if (error)
+		return (error);
+
+	if (newp) {
+		size_t buflen;
+
+		if ((buflen = strlen(buf)) == 0)
+			return (EINVAL);
+
+		for (i = 0; i < algs_num; ++i) {
+			if (strncasecmp(buf, algs[i].name, buflen) == 0)
+				break;
+		}
+
+		if (i >= algs_num)
+			return (EINVAL);
+
+		if (algs[i].val != oldval)
+			atomic_store_int(var, algs[i].val);
+	}
+
+	return (0);
 }
 
 int