| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <13efb9e14d378cf6ed81650f52fce21ce6faafe1.camel@redhat.com>
Date: Tue, 13 Feb 2024 11:13:39 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: dsahern@...nel.org
Cc: edumazet@...gle.com, kuba@...nel.org, jikos@...nel.org, Alex Henrie
<alexhenrie24@...il.com>, netdev@...r.kernel.org, dan@...m.net,
bagasdotme@...il.com, davem@...emloft.net
Subject: Re: [PATCH net-next 3/3] net: ipv6/addrconf: clamp preferred_lft to
the minimum required
On Thu, 2024-02-08 at 23:10 -0700, Alex Henrie wrote:
> If the preferred lifetime was less than the minimum required lifetime,
> ipv6_create_tempaddr would error out without creating any new address.
> On my machine and network, this error happened immediately with the
> preferred lifetime set to 5 seconds or less, after a few minutes with
> the preferred lifetime set to 6 seconds, and not at all with the
> preferred lifetime set to 7 seconds. During my investigation, I found a
> Stack Exchange post from another person who seems to have had the same
> problem: They stopped getting new addresses if they lowered the
> preferred lifetime below 3 seconds, and they didn't really know why.
>
> The preferred lifetime is a preference, not a hard requirement. The
> kernel does not strictly forbid new connections on a deprecated address,
> nor does it guarantee that the address will be disposed of the instant
> its total valid lifetime expires. So rather than disable IPv6 privacy
> extensions altogether if the minimum required lifetime swells above the
> preferred lifetime, it is more in keeping with the user's intent to
> increase the temporary address's lifetime to the minimum necessary for
> the current network conditions.
>
> With these fixes, setting the preferred lifetime to 5 or 6 seconds "just
> works" because the extra fraction of a second is practically
> unnoticeable. It's even possible to reduce the time before deprecation
> to 1 or 2 seconds by setting /proc/sys/net/ipv6/conf/*/regen_min_advance
> and /proc/sys/net/ipv6/conf/*/dad_transmits to 0. I realize that that is
> a pretty niche use case, but I know at least one person who would gladly
> sacrifice performance and convenience to be sure that they are getting
> the maximum possible level of privacy.
>
> Link: https://serverfault.com/a/1031168/310447
> Signed-off-by: Alex Henrie <alexhenrie24@...il.com>
> ---
> net/ipv6/addrconf.c | 43 ++++++++++++++++++++++++++++++++++---------
> 1 file changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 0b78ffc101ef..8d3023e54822 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1347,6 +1347,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
> unsigned long regen_advance;
> unsigned long now = jiffies;
> s32 cnf_temp_preferred_lft;
> + u32 if_public_preferred_lft;
[only if a repost is needed for some other reason] please respect the
reverse x-mas tree above.
> struct inet6_ifaddr *ift;
> struct ifa6_config cfg;
> long max_desync_factor;
> @@ -1401,11 +1402,13 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
> }
> }
>
> + if_public_preferred_lft = ifp->prefered_lft;
> +
> memset(&cfg, 0, sizeof(cfg));
> cfg.valid_lft = min_t(__u32, ifp->valid_lft,
> idev->cnf.temp_valid_lft + age);
> cfg.preferred_lft = cnf_temp_preferred_lft + age - idev->desync_factor;
> - cfg.preferred_lft = min_t(__u32, ifp->prefered_lft, cfg.preferred_lft);
> + cfg.preferred_lft = min_t(__u32, if_public_preferred_lft, cfg.preferred_lft);
> cfg.preferred_lft = min_t(__u32, cfg.valid_lft, cfg.preferred_lft);
>
> cfg.plen = ifp->prefix_len;
> @@ -1414,19 +1417,41 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
>
> write_unlock_bh(&idev->lock);
>
> - /* A temporary address is created only if this calculated Preferred
> - * Lifetime is greater than REGEN_ADVANCE time units. In particular,
> - * an implementation must not create a temporary address with a zero
> - * Preferred Lifetime.
> + /* From RFC 4941:
> + *
> + * A temporary address is created only if this calculated Preferred
> + * Lifetime is greater than REGEN_ADVANCE time units. In
> + * particular, an implementation must not create a temporary address
> + * with a zero Preferred Lifetime.
> + *
> + * ...
> + *
> + * When creating a temporary address, the lifetime values MUST be
> + * derived from the corresponding prefix as follows:
> + *
> + * ...
> + *
> + * * Its Preferred Lifetime is the lower of the Preferred Lifetime
> + * of the public address or TEMP_PREFERRED_LIFETIME -
> + * DESYNC_FACTOR.
> + *
> + * To comply with the RFC's requirements, clamp the preferred lifetime
> + * to a minimum of regen_advance, unless that would exceed valid_lft or
> + * ifp->prefered_lft.
> + *
> * Use age calculation as in addrconf_verify to avoid unnecessary
> * temporary addresses being generated.
> */
> age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ;
> if (cfg.preferred_lft <= regen_advance + age) {
> - in6_ifa_put(ifp);
> - in6_dev_put(idev);
> - ret = -1;
> - goto out;
> + cfg.preferred_lft = regen_advance + age + 1;
> + if (cfg.preferred_lft > cfg.valid_lft ||
> + cfg.preferred_lft > if_public_preferred_lft) {
> + in6_ifa_put(ifp);
> + in6_dev_put(idev);
> + ret = -1;
> + goto out;
> + }
> }
>
> cfg.ifa_flags = IFA_F_TEMPORARY;
The above sounds reasonable to me, but I would appreciate a couple of
additional eyeballs on it. @David, could you please have a look?
Thanks,
Paolo
Powered by blists - more mailing lists