[PATCH net-next v2 0/4] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are in the valid range

[PATCH net-next v2 0/4] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are in the valid range

[Alex Henrie]

No changes from v2, but there are only four patches now because the
first patch has already been applied.

https://lore.kernel.org/all/20230829054623.104293-1-alexhenrie24@gmail.com/

Alex Henrie (4):
  net: ipv6/addrconf: clamp preferred_lft to the maximum allowed
  net: ipv6/addrconf: clamp preferred_lft to the minimum required
  Documentation: networking: explain what happens if temp_valid_lft is
    too small
  Documentation: networking: explain what happens if temp_prefered_lft
    is too small or too large

 Documentation/networking/ip-sysctl.rst | 10 ++++++++--
 net/ipv6/addrconf.c                    | 19 ++++++++++++++-----
 2 files changed, 22 insertions(+), 7 deletions(-)

-- 
2.42.0

[PATCH net-next v2 1/4] net: ipv6/addrconf: clamp preferred_lft to the maximum allowed

[Alex Henrie]

Without this patch, there is nothing to stop the preferred lifetime of a
temporary address from being greater than its valid lifetime. If that
was the case, the valid lifetime was effectively ignored.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 net/ipv6/addrconf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c2d471ad7922..26aedaab3647 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1399,6 +1399,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 			      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, cfg.valid_lft, cfg.preferred_lft);
 
 	cfg.plen = ifp->prefix_len;
 	tmp_tstamp = ifp->tstamp;
-- 
2.42.0

Re: [PATCH net-next v2 1/4] net: ipv6/addrconf: clamp preferred_lft to the maximum allowed

[Jiri Pirko]

Tue, Oct 24, 2023 at 11:23:07PM CEST, alexhenrie24@gmail.com wrote:
>Without this patch, there is nothing to stop the preferred lifetime of a
>temporary address from being greater than its valid lifetime. If that
>was the case, the valid lifetime was effectively ignored.
>

Sounds like a bugfix, correct? In that case, could you please
provide a proper Fixes tag and target -net tree?


>Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
>---
> net/ipv6/addrconf.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>index c2d471ad7922..26aedaab3647 100644
>--- a/net/ipv6/addrconf.c
>+++ b/net/ipv6/addrconf.c
>@@ -1399,6 +1399,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
> 			      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, cfg.valid_lft, cfg.preferred_lft);
> 
> 	cfg.plen = ifp->prefix_len;
> 	tmp_tstamp = ifp->tstamp;
>-- 
>2.42.0
>
>

Re: [PATCH net-next v2 1/4] net: ipv6/addrconf: clamp preferred_lft to the maximum allowed

[Alex Henrie]

On Wed, Oct 25, 2023 at 6:23 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, Oct 24, 2023 at 11:23:07PM CEST, alexhenrie24@gmail.com wrote:
> >Without this patch, there is nothing to stop the preferred lifetime of a
> >temporary address from being greater than its valid lifetime. If that
> >was the case, the valid lifetime was effectively ignored.
> >
>
> Sounds like a bugfix, correct? In that case, could you please
> provide a proper Fixes tag and target -net tree?

Paolo requested no Fixes tag, and Jakub seemed to agree:

https://lore.kernel.org/all/60d9d5f57fdb55a27748996d807712c680c4e7f9.camel@redhat.com/

https://lore.kernel.org/all/20230830182852.175e0ac2@kernel.org/

Which is fine by me. These changes are not important enough to backport.

-Alex

Re: [PATCH net-next v2 1/4] net: ipv6/addrconf: clamp preferred_lft to the maximum allowed

[Jiri Pirko]

Wed, Oct 25, 2023 at 06:28:00PM CEST, alexhenrie24@gmail.com wrote:
>On Wed, Oct 25, 2023 at 6:23 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Tue, Oct 24, 2023 at 11:23:07PM CEST, alexhenrie24@gmail.com wrote:
>> >Without this patch, there is nothing to stop the preferred lifetime of a
>> >temporary address from being greater than its valid lifetime. If that
>> >was the case, the valid lifetime was effectively ignored.
>> >
>>
>> Sounds like a bugfix, correct? In that case, could you please
>> provide a proper Fixes tag and target -net tree?
>
>Paolo requested no Fixes tag, and Jakub seemed to agree:
>
>https://lore.kernel.org/all/60d9d5f57fdb55a27748996d807712c680c4e7f9.camel@redhat.com/

Okay, makes sense.

Reviewed-by: Jiri Pirko <jiri@nvidia.com>

Re: [PATCH net-next v2 1/4] net: ipv6/addrconf: clamp preferred_lft to the maximum allowed

[David Ahern]

On 10/24/23 3:23 PM, Alex Henrie wrote:
> Without this patch, there is nothing to stop the preferred lifetime of a
> temporary address from being greater than its valid lifetime. If that
> was the case, the valid lifetime was effectively ignored.
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  net/ipv6/addrconf.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

[PATCH net-next v2 2/4] net: ipv6/addrconf: clamp preferred_lft to the minimum required

[Alex Henrie]

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 1 second, after a few minutes with the
preferred lifetime set to 4 seconds, and not at all with the preferred
lifetime set to 5 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 3 or 4 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 also disabling duplicate address detection (setting
/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@gmail.com>
---
 net/ipv6/addrconf.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 26aedaab3647..3aaea56b5166 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1407,15 +1407,23 @@ 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.
+	 *
+	 * Clamp the preferred lifetime to a minimum of regen_advance, unless
+	 * that would exceed valid_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) {
+	if (cfg.preferred_lft <= regen_advance + age)
+		cfg.preferred_lft = regen_advance + age + 1;
+	if (cfg.preferred_lft > cfg.valid_lft) {
 		in6_ifa_put(ifp);
 		in6_dev_put(idev);
 		ret = -1;
-- 
2.42.0

Re: [PATCH net-next v2 2/4] net: ipv6/addrconf: clamp preferred_lft to the minimum required

[Jiri Pirko]

Tue, Oct 24, 2023 at 11:23:08PM CEST, alexhenrie24@gmail.com 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 1 second, after a few minutes with the
>preferred lifetime set to 4 seconds, and not at all with the preferred
>lifetime set to 5 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 3 or 4 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 also disabling duplicate address detection (setting
>/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@gmail.com>

Again, Fixes tag and send to -net tree?

Re: [PATCH net-next v2 2/4] net: ipv6/addrconf: clamp preferred_lft to the minimum required

[David Ahern]

On 10/24/23 3:23 PM, 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 1 second, after a few minutes with the
> preferred lifetime set to 4 seconds, and not at all with the preferred
> lifetime set to 5 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 3 or 4 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 also disabling duplicate address detection (setting
> /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@gmail.com>
> ---
>  net/ipv6/addrconf.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

[PATCH net-next v2 3/4] Documentation: networking: explain what happens if temp_valid_lft is too small

[Alex Henrie]

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 Documentation/networking/ip-sysctl.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index e7ec9026e5db..6134ff4561e8 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2502,7 +2502,9 @@ use_tempaddr - INTEGER
 		* -1 (for point-to-point devices and loopback devices)
 
 temp_valid_lft - INTEGER
-	valid lifetime (in seconds) for temporary addresses.
+	valid lifetime (in seconds) for temporary addresses. If less than the
+	minimum required lifetime (typically 5 seconds), temporary addresses
+	will not be created.
 
 	Default: 172800 (2 days)
 
-- 
2.42.0

Re: [PATCH net-next v2 3/4] Documentation: networking: explain what happens if temp_valid_lft is too small

[David Ahern]

On 10/24/23 3:23 PM, Alex Henrie wrote:
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  Documentation/networking/ip-sysctl.rst | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

[PATCH net-next v2 4/4] Documentation: networking: explain what happens if temp_prefered_lft is too small or too large

[Alex Henrie]

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 Documentation/networking/ip-sysctl.rst | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 6134ff4561e8..4dfe0d9a57bb 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2509,7 +2509,11 @@ temp_valid_lft - INTEGER
 	Default: 172800 (2 days)
 
 temp_prefered_lft - INTEGER
-	Preferred lifetime (in seconds) for temporary addresses.
+	Preferred lifetime (in seconds) for temporary addresses. If
+	temp_prefered_lft is less than the minimum required lifetime (typically
+	5 seconds), the preferred lifetime is the minimum required. If
+	temp_prefered_lft is greater than temp_valid_lft, the preferred lifetime
+	is temp_valid_lft.
 
 	Default: 86400 (1 day)
 
-- 
2.42.0

Re: [PATCH net-next v2 4/4] Documentation: networking: explain what happens if temp_prefered_lft is too small or too large

[David Ahern]

On 10/24/23 3:23 PM, Alex Henrie wrote:
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  Documentation/networking/ip-sysctl.rst | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net-next v2 0/4] net: ipv6/addrconf: ensure that temporary addresses' preferred lifetimes are in the valid range

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 24 Oct 2023 15:23:06 -0600 you wrote:
> No changes from v2, but there are only four patches now because the
> first patch has already been applied.
> 
> https://lore.kernel.org/all/20230829054623.104293-1-alexhenrie24@gmail.com/
> 
> Alex Henrie (4):
>   net: ipv6/addrconf: clamp preferred_lft to the maximum allowed
>   net: ipv6/addrconf: clamp preferred_lft to the minimum required
>   Documentation: networking: explain what happens if temp_valid_lft is
>     too small
>   Documentation: networking: explain what happens if temp_prefered_lft
>     is too small or too large
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/4] net: ipv6/addrconf: clamp preferred_lft to the maximum allowed
    https://git.kernel.org/netdev/net-next/c/bfbf81b31093
  - [net-next,v2,2/4] net: ipv6/addrconf: clamp preferred_lft to the minimum required
    https://git.kernel.org/netdev/net-next/c/629df6701c8a
  - [net-next,v2,3/4] Documentation: networking: explain what happens if temp_valid_lft is too small
    https://git.kernel.org/netdev/net-next/c/433d6c8048cb
  - [net-next,v2,4/4] Documentation: networking: explain what happens if temp_prefered_lft is too small or too large
    https://git.kernel.org/netdev/net-next/c/ec575f885e3e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html