diff mbox series

[net-next] net: add sysctl to disable rfc4862 5.5.3e lifetime handling

Message ID 20230831172322.668507-1-prohr@google.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: add sysctl to disable rfc4862 5.5.3e lifetime handling | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3177 this patch: 3177
netdev/cc_maintainers warning 6 maintainers not CCed: kuba@kernel.org dsahern@kernel.org corbet@lwn.net pabeni@redhat.com edumazet@google.com linux-doc@vger.kernel.org
netdev/build_clang success Errors and warnings before: 1839 this patch: 1839
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 3344 this patch: 3344
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 89 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Patrick Rohr Aug. 31, 2023, 5:23 p.m. UTC
This change adds a sysctl to opt-out of RFC4862 section 5.5.3e's valid
lifetime derivation mechanism.

RFC4862 section 5.5.3e prescribes that the valid lifetime in a Router
Advertisement PIO shall be ignored if it less than 2 hours and to reset
the lifetime of the corresponding address to 2 hours. An in-progress
6man draft (see draft-ietf-6man-slaac-renum-07 section 4.2) is currently
looking to remove this mechanism. While this draft has not been moving
particularly quickly for other reasons, there is widespread consensus on
section 4.2 which updates RFC4862 section 5.5.3e.

Cc: Maciej Żenczykowski <maze@google.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: Jen Linkova <furry@google.com>
Signed-off-by: Patrick Rohr <prohr@google.com>
---
 Documentation/networking/ip-sysctl.rst | 11 ++++++++
 include/linux/ipv6.h                   |  1 +
 net/ipv6/addrconf.c                    | 38 +++++++++++++++++---------
 3 files changed, 37 insertions(+), 13 deletions(-)

Comments

Maciej Żenczykowski Aug. 31, 2023, 5:35 p.m. UTC | #1
On Thu, Aug 31, 2023 at 10:23 AM Patrick Rohr <prohr@google.com> wrote:
>
> This change adds a sysctl to opt-out of RFC4862 section 5.5.3e's valid
> lifetime derivation mechanism.
>
> RFC4862 section 5.5.3e prescribes that the valid lifetime in a Router
> Advertisement PIO shall be ignored if it less than 2 hours and to reset
> the lifetime of the corresponding address to 2 hours. An in-progress
> 6man draft (see draft-ietf-6man-slaac-renum-07 section 4.2) is currently
> looking to remove this mechanism. While this draft has not been moving
> particularly quickly for other reasons, there is widespread consensus on
> section 4.2 which updates RFC4862 section 5.5.3e.
>
> Cc: Maciej Żenczykowski <maze@google.com>
> Cc: Lorenzo Colitti <lorenzo@google.com>
> Cc: Jen Linkova <furry@google.com>
> Signed-off-by: Patrick Rohr <prohr@google.com>
> ---
>  Documentation/networking/ip-sysctl.rst | 11 ++++++++
>  include/linux/ipv6.h                   |  1 +
>  net/ipv6/addrconf.c                    | 38 +++++++++++++++++---------
>  3 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index a66054d0763a..7f21877e3f78 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -2304,6 +2304,17 @@ accept_ra_pinfo - BOOLEAN
>                 - enabled if accept_ra is enabled.
>                 - disabled if accept_ra is disabled.
>
> +ra_pinfo_rfc4862_5_5_3e - BOOLEAN
> +       Use RFC4862 Section 5.5.3e to determine the valid lifetime of
> +       an address matching a prefix sent in a Router Advertisement
> +       Prefix Information Option.
> +
> +       - If enabled, RFC4862 section 5.5.3e is used to determine
> +         the valid lifetime of the address.
> +       - If disabled, the PIO valid lifetime will always be honored.
> +
> +       Default: 1
> +
>  accept_ra_rt_info_min_plen - INTEGER
>         Minimum prefix length of Route Information in RA.
>
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index 5883551b1ee8..f90cf8835ed4 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -35,6 +35,7 @@ struct ipv6_devconf {
>         __s32           accept_ra_min_hop_limit;
>         __s32           accept_ra_min_lft;
>         __s32           accept_ra_pinfo;
> +       __s32           ra_pinfo_rfc4862_5_5_3e;
>         __s32           ignore_routes_with_linkdown;
>  #ifdef CONFIG_IPV6_ROUTER_PREF
>         __s32           accept_ra_rtr_pref;
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 47d1dd8501b7..1ac23a37e8eb 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -204,6 +204,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
>         .accept_ra_min_hop_limit= 1,
>         .accept_ra_min_lft      = 0,
>         .accept_ra_pinfo        = 1,
> +       .ra_pinfo_rfc4862_5_5_3e = 1,
>  #ifdef CONFIG_IPV6_ROUTER_PREF
>         .accept_ra_rtr_pref     = 1,
>         .rtr_probe_interval     = 60 * HZ,
> @@ -265,6 +266,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
>         .accept_ra_min_hop_limit= 1,
>         .accept_ra_min_lft      = 0,
>         .accept_ra_pinfo        = 1,
> +       .ra_pinfo_rfc4862_5_5_3e = 1,
>  #ifdef CONFIG_IPV6_ROUTER_PREF
>         .accept_ra_rtr_pref     = 1,
>         .rtr_probe_interval     = 60 * HZ,
> @@ -2657,22 +2659,23 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
>                         stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
>                 else
>                         stored_lft = 0;
> -               if (!create && stored_lft) {
> +
> +               /* RFC4862 Section 5.5.3e:
> +                * "Note that the preferred lifetime of the
> +                *  corresponding address is always reset to
> +                *  the Preferred Lifetime in the received
> +                *  Prefix Information option, regardless of
> +                *  whether the valid lifetime is also reset or
> +                *  ignored."
> +                *
> +                * So we should always update prefered_lft here.
> +                */
> +               update_lft = !create && stored_lft;
> +
> +               if (update_lft && in6_dev->cnf.ra_pinfo_rfc4862_5_5_3e) {
>                         const u32 minimum_lft = min_t(u32,
>                                 stored_lft, MIN_VALID_LIFETIME);
>                         valid_lft = max(valid_lft, minimum_lft);
> -
> -                       /* RFC4862 Section 5.5.3e:
> -                        * "Note that the preferred lifetime of the
> -                        *  corresponding address is always reset to
> -                        *  the Preferred Lifetime in the received
> -                        *  Prefix Information option, regardless of
> -                        *  whether the valid lifetime is also reset or
> -                        *  ignored."
> -                        *
> -                        * So we should always update prefered_lft here.
> -                        */
> -                       update_lft = 1;
>                 }
>
>                 if (update_lft) {
> @@ -6846,6 +6849,15 @@ static const struct ctl_table addrconf_sysctl[] = {
>                 .mode           = 0644,
>                 .proc_handler   = proc_dointvec,
>         },
> +       {
> +               .procname       = "ra_pinfo_rfc4862_5_5_3e",
> +               .data           = &ipv6_devconf.ra_pinfo_rfc4862_5_5_3e,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0644,
> +               .proc_handler   = proc_dointvec_minmax,
> +               .extra1         = SYSCTL_ZERO,
> +               .extra2         = SYSCTL_ONE,
> +       },
>  #ifdef CONFIG_IPV6_ROUTER_PREF
>         {
>                 .procname       = "accept_ra_rtr_pref",
> --
> 2.42.0.283.g2d96d420d3-goog

Reviewed-by: Maciej Żenczykowski <maze@google.com>

Obviously 'ra_pinfo_rfc4862_5_5_3e' isn't a particularly pretty name...
But we couldn't come up with anything that was better.
In particular it shouldn't start with 'accept_ra_' since it's not
relevant to actually accepting it.
Similarly it shouldn't end in _lft since it's a boolean and not a
length of time...

As for why we want to be able to disable it:
The existing 2hours is extremely arbitrary.
It was added to prevent security attacks from rogue RAs expiring things.
However, any network without RA guard (which would prevent this attack)
is already susceptible to so many other possible RA attacks, that it
just doesn't matter.
What it does do however, is prevent expiring client devices ip/route information
when the upstream configuration of a router changes (cable modem
reconnects, etc).

Unfortunately the old behaviour has to stay the default, to pass some
certification tests...
(until the RFC actually gets updated)

Maciej Żenczykowski, Kernel Networking Developer @ Google
Maciej Żenczykowski Aug. 31, 2023, 5:51 p.m. UTC | #2
On Thu, Aug 31, 2023 at 10:35 AM Maciej Żenczykowski <maze@google.com> wrote:
>
> On Thu, Aug 31, 2023 at 10:23 AM Patrick Rohr <prohr@google.com> wrote:
> >
> > This change adds a sysctl to opt-out of RFC4862 section 5.5.3e's valid
> > lifetime derivation mechanism.
> >
> > RFC4862 section 5.5.3e prescribes that the valid lifetime in a Router
> > Advertisement PIO shall be ignored if it less than 2 hours and to reset
> > the lifetime of the corresponding address to 2 hours. An in-progress
> > 6man draft (see draft-ietf-6man-slaac-renum-07 section 4.2) is currently
> > looking to remove this mechanism. While this draft has not been moving
> > particularly quickly for other reasons, there is widespread consensus on
> > section 4.2 which updates RFC4862 section 5.5.3e.
> >
> > Cc: Maciej Żenczykowski <maze@google.com>
> > Cc: Lorenzo Colitti <lorenzo@google.com>
> > Cc: Jen Linkova <furry@google.com>
> > Signed-off-by: Patrick Rohr <prohr@google.com>
> > ---
> >  Documentation/networking/ip-sysctl.rst | 11 ++++++++
> >  include/linux/ipv6.h                   |  1 +
> >  net/ipv6/addrconf.c                    | 38 +++++++++++++++++---------
> >  3 files changed, 37 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> > index a66054d0763a..7f21877e3f78 100644
> > --- a/Documentation/networking/ip-sysctl.rst
> > +++ b/Documentation/networking/ip-sysctl.rst
> > @@ -2304,6 +2304,17 @@ accept_ra_pinfo - BOOLEAN
> >                 - enabled if accept_ra is enabled.
> >                 - disabled if accept_ra is disabled.
> >
> > +ra_pinfo_rfc4862_5_5_3e - BOOLEAN
> > +       Use RFC4862 Section 5.5.3e to determine the valid lifetime of
> > +       an address matching a prefix sent in a Router Advertisement
> > +       Prefix Information Option.
> > +
> > +       - If enabled, RFC4862 section 5.5.3e is used to determine
> > +         the valid lifetime of the address.
> > +       - If disabled, the PIO valid lifetime will always be honored.
> > +
> > +       Default: 1
> > +
> >  accept_ra_rt_info_min_plen - INTEGER
> >         Minimum prefix length of Route Information in RA.
> >
> > diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> > index 5883551b1ee8..f90cf8835ed4 100644
> > --- a/include/linux/ipv6.h
> > +++ b/include/linux/ipv6.h
> > @@ -35,6 +35,7 @@ struct ipv6_devconf {
> >         __s32           accept_ra_min_hop_limit;
> >         __s32           accept_ra_min_lft;
> >         __s32           accept_ra_pinfo;
> > +       __s32           ra_pinfo_rfc4862_5_5_3e;
> >         __s32           ignore_routes_with_linkdown;
> >  #ifdef CONFIG_IPV6_ROUTER_PREF
> >         __s32           accept_ra_rtr_pref;
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index 47d1dd8501b7..1ac23a37e8eb 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -204,6 +204,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
> >         .accept_ra_min_hop_limit= 1,
> >         .accept_ra_min_lft      = 0,
> >         .accept_ra_pinfo        = 1,
> > +       .ra_pinfo_rfc4862_5_5_3e = 1,
> >  #ifdef CONFIG_IPV6_ROUTER_PREF
> >         .accept_ra_rtr_pref     = 1,
> >         .rtr_probe_interval     = 60 * HZ,
> > @@ -265,6 +266,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
> >         .accept_ra_min_hop_limit= 1,
> >         .accept_ra_min_lft      = 0,
> >         .accept_ra_pinfo        = 1,
> > +       .ra_pinfo_rfc4862_5_5_3e = 1,
> >  #ifdef CONFIG_IPV6_ROUTER_PREF
> >         .accept_ra_rtr_pref     = 1,
> >         .rtr_probe_interval     = 60 * HZ,
> > @@ -2657,22 +2659,23 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
> >                         stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
> >                 else
> >                         stored_lft = 0;
> > -               if (!create && stored_lft) {
> > +
> > +               /* RFC4862 Section 5.5.3e:
> > +                * "Note that the preferred lifetime of the
> > +                *  corresponding address is always reset to
> > +                *  the Preferred Lifetime in the received
> > +                *  Prefix Information option, regardless of
> > +                *  whether the valid lifetime is also reset or
> > +                *  ignored."
> > +                *
> > +                * So we should always update prefered_lft here.
> > +                */
> > +               update_lft = !create && stored_lft;
> > +
> > +               if (update_lft && in6_dev->cnf.ra_pinfo_rfc4862_5_5_3e) {
> >                         const u32 minimum_lft = min_t(u32,
> >                                 stored_lft, MIN_VALID_LIFETIME);
> >                         valid_lft = max(valid_lft, minimum_lft);
> > -
> > -                       /* RFC4862 Section 5.5.3e:
> > -                        * "Note that the preferred lifetime of the
> > -                        *  corresponding address is always reset to
> > -                        *  the Preferred Lifetime in the received
> > -                        *  Prefix Information option, regardless of
> > -                        *  whether the valid lifetime is also reset or
> > -                        *  ignored."
> > -                        *
> > -                        * So we should always update prefered_lft here.
> > -                        */
> > -                       update_lft = 1;
> >                 }
> >
> >                 if (update_lft) {
> > @@ -6846,6 +6849,15 @@ static const struct ctl_table addrconf_sysctl[] = {
> >                 .mode           = 0644,
> >                 .proc_handler   = proc_dointvec,
> >         },
> > +       {
> > +               .procname       = "ra_pinfo_rfc4862_5_5_3e",
> > +               .data           = &ipv6_devconf.ra_pinfo_rfc4862_5_5_3e,
> > +               .maxlen         = sizeof(int),
> > +               .mode           = 0644,
> > +               .proc_handler   = proc_dointvec_minmax,
> > +               .extra1         = SYSCTL_ZERO,
> > +               .extra2         = SYSCTL_ONE,
> > +       },
> >  #ifdef CONFIG_IPV6_ROUTER_PREF
> >         {
> >                 .procname       = "accept_ra_rtr_pref",
> > --
> > 2.42.0.283.g2d96d420d3-goog
>
> Reviewed-by: Maciej Żenczykowski <maze@google.com>
>
> Obviously 'ra_pinfo_rfc4862_5_5_3e' isn't a particularly pretty name...
> But we couldn't come up with anything that was better.
> In particular it shouldn't start with 'accept_ra_' since it's not
> relevant to actually accepting it.
> Similarly it shouldn't end in _lft since it's a boolean and not a
> length of time...
>
> As for why we want to be able to disable it:
> The existing 2hours is extremely arbitrary.
> It was added to prevent security attacks from rogue RAs expiring things.
> However, any network without RA guard (which would prevent this attack)
> is already susceptible to so many other possible RA attacks, that it
> just doesn't matter.
> What it does do however, is prevent expiring client devices ip/route information
> when the upstream configuration of a router changes (cable modem
> reconnects, etc).
>
> Unfortunately the old behaviour has to stay the default, to pass some
> certification tests...
> (until the RFC actually gets updated)

btw. net-next is currently closed (and this isn't a fix)
https://patchwork.hopto.org/net-next.html

You'll most likely be asked to resend in ~1.5 weeks.
diff mbox series

Patch

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index a66054d0763a..7f21877e3f78 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2304,6 +2304,17 @@  accept_ra_pinfo - BOOLEAN
 		- enabled if accept_ra is enabled.
 		- disabled if accept_ra is disabled.
 
+ra_pinfo_rfc4862_5_5_3e - BOOLEAN
+	Use RFC4862 Section 5.5.3e to determine the valid lifetime of
+	an address matching a prefix sent in a Router Advertisement
+	Prefix Information Option.
+
+	- If enabled, RFC4862 section 5.5.3e is used to determine
+	  the valid lifetime of the address.
+	- If disabled, the PIO valid lifetime will always be honored.
+
+	Default: 1
+
 accept_ra_rt_info_min_plen - INTEGER
 	Minimum prefix length of Route Information in RA.
 
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 5883551b1ee8..f90cf8835ed4 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -35,6 +35,7 @@  struct ipv6_devconf {
 	__s32		accept_ra_min_hop_limit;
 	__s32		accept_ra_min_lft;
 	__s32		accept_ra_pinfo;
+	__s32		ra_pinfo_rfc4862_5_5_3e;
 	__s32		ignore_routes_with_linkdown;
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	__s32		accept_ra_rtr_pref;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 47d1dd8501b7..1ac23a37e8eb 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -204,6 +204,7 @@  static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.accept_ra_min_hop_limit= 1,
 	.accept_ra_min_lft	= 0,
 	.accept_ra_pinfo	= 1,
+	.ra_pinfo_rfc4862_5_5_3e = 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
 	.rtr_probe_interval	= 60 * HZ,
@@ -265,6 +266,7 @@  static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.accept_ra_min_hop_limit= 1,
 	.accept_ra_min_lft	= 0,
 	.accept_ra_pinfo	= 1,
+	.ra_pinfo_rfc4862_5_5_3e = 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
 	.rtr_probe_interval	= 60 * HZ,
@@ -2657,22 +2659,23 @@  int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
 			stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
 		else
 			stored_lft = 0;
-		if (!create && stored_lft) {
+
+		/* RFC4862 Section 5.5.3e:
+		 * "Note that the preferred lifetime of the
+		 *  corresponding address is always reset to
+		 *  the Preferred Lifetime in the received
+		 *  Prefix Information option, regardless of
+		 *  whether the valid lifetime is also reset or
+		 *  ignored."
+		 *
+		 * So we should always update prefered_lft here.
+		 */
+		update_lft = !create && stored_lft;
+
+		if (update_lft && in6_dev->cnf.ra_pinfo_rfc4862_5_5_3e) {
 			const u32 minimum_lft = min_t(u32,
 				stored_lft, MIN_VALID_LIFETIME);
 			valid_lft = max(valid_lft, minimum_lft);
-
-			/* RFC4862 Section 5.5.3e:
-			 * "Note that the preferred lifetime of the
-			 *  corresponding address is always reset to
-			 *  the Preferred Lifetime in the received
-			 *  Prefix Information option, regardless of
-			 *  whether the valid lifetime is also reset or
-			 *  ignored."
-			 *
-			 * So we should always update prefered_lft here.
-			 */
-			update_lft = 1;
 		}
 
 		if (update_lft) {
@@ -6846,6 +6849,15 @@  static const struct ctl_table addrconf_sysctl[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "ra_pinfo_rfc4862_5_5_3e",
+		.data		= &ipv6_devconf.ra_pinfo_rfc4862_5_5_3e,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_ONE,
+	},
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	{
 		.procname	= "accept_ra_rtr_pref",