Download raw body.
ipsec: move ipsec-enc-alg, ipsec-auth-alg and ipsec-comp-alg sysctl variables out of netlock
ipsec: move ipsec-enc-alg, ipsec-auth-alg and ipsec-comp-alg sysctl variables out of netlock
ipsec: move ipsec-enc-alg, ipsec-auth-alg and ipsec-comp-alg sysctl variables out of netlock
On Fri, May 09, 2025 at 12:48:37PM +0300, Vitaliy Makkoveev wrote:
> On Fri, May 09, 2025 at 05:59:50PM +0200, Alexander Bluhm wrote:
> > On Fri, May 02, 2025 at 08:45:49PM +0300, Vitaliy Makkoveev wrote:
> > > 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.
> >
> > I like it, a few comments inline.
> >
> > > 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;
> >
> > I know that you like the _local suffix. I usually remove the ipsec_
> > prefix. This results in much shorter names. But anyway, just a
> > suggestion.
> > int def_enc, def_comp, def_auth;
> >
>
> This makes me 100% sure that this is the cached value of global
> variable. I agree, the `ipsec_def_*_local' are long enough, so may be we
> will rethink the naming later.
>
> Also added missing "#ifdef IPSEC" around them.
>
> > Here I would use
> >
> > const struct ipsec_sysctl_algorithm *p;
> > for (p = algs; p->name != NULL; p++) {
> >
>
> No objections for this.
OK bluhm@
The
+ default:
+ break;
looks a bit useless.
> 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 9 May 2025 17:44:38 -0000
> @@ -2158,6 +2158,14 @@ pfkeyv2_acquire(struct ipsec_policy *ipo
> int rval = 0;
> int i, j, registered;
>
> +#ifdef IPSEC
> + int ipsec_def_enc_local, ipsec_def_comp_local, ipsec_def_auth_local;
> +
> + 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);
> +#endif
> +
> mtx_enter(&pfkeyv2_mtx);
> *seq = pfkeyv2_seq++;
>
> @@ -2246,75 +2254,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 9 May 2025 17:44:38 -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 9 May 2025 17:44:38 -0000
> @@ -139,9 +139,38 @@ 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},
> + {NULL, -1},
> +};
> +
> +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},
> + {NULL, -1},
> +};
> +
> +const struct ipsec_sysctl_algorithm ipsec_sysctl_comp_algs[] = {
> + {"deflate", IPSEC_COMP_DEFLATE},
> + {NULL, -1},
> +};
> +
> +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 +187,7 @@ const struct sysctl_bounded_args ipsecct
> { IPSEC_FIRSTUSE, &ipsec_exp_first_use, 0, INT_MAX },
> };
>
> +int ipsec_sysctl_algorithm(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 +201,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 +639,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_algorithm(name[0], oldp, oldlenp,
> + newp, newlen));
> case IPCTL_IPSEC_STATS:
> return (ipsec_sysctl_ipsecstat(oldp, oldlenp, newp));
> default:
> @@ -639,6 +652,68 @@ ipsec_sysctl(int *name, u_int namelen, v
> NET_UNLOCK();
> return (error);
> }
> +}
> +
> +int
> +ipsec_sysctl_algorithm(int name, void *oldp, size_t *oldlenp,
> + void *newp, size_t newlen)
> +{
> + const struct ipsec_sysctl_algorithm *algs, *p;
> + int *var, oldval, error;
> + char buf[20];
> +
> + switch (name) {
> + case IPCTL_IPSEC_ENC_ALGORITHM:
> + algs = ipsec_sysctl_enc_algs;
> + var = &ipsec_def_enc;
> + break;
> + case IPCTL_IPSEC_AUTH_ALGORITHM:
> + algs = ipsec_sysctl_auth_algs;
> + var = &ipsec_def_auth;
> + break;
> + case IPCTL_IPSEC_IPCOMP_ALGORITHM:
> + algs = ipsec_sysctl_comp_algs;
> + var = &ipsec_def_comp;
> + break;
> + default:
> + return (EOPNOTSUPP);
> + }
> +
> + oldval = atomic_load_int(var);
> +
> + for (p = algs; p->name != NULL; p++) {
> + if (p->val == oldval) {
> + strlcpy(buf, p->name, sizeof(buf));
> + break;
> + }
> + }
> +
> + KASSERT(p->name != NULL);
> +
> + 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 (p = algs; p->name != NULL; p++) {
> + if (strncasecmp(buf, p->name, buflen) == 0)
> + break;
> + }
> +
> + if (p->name == NULL)
> + return (EINVAL);
> +
> + if (p->val != oldval)
> + atomic_store_int(var, p->val);
> + }
> +
> + return (0);
> }
>
> int
ipsec: move ipsec-enc-alg, ipsec-auth-alg and ipsec-comp-alg sysctl variables out of netlock
ipsec: move ipsec-enc-alg, ipsec-auth-alg and ipsec-comp-alg sysctl variables out of netlock
ipsec: move ipsec-enc-alg, ipsec-auth-alg and ipsec-comp-alg sysctl variables out of netlock