switch: allow datapath_id and maxflow ioctls for non-root

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

switch: allow datapath_id and maxflow ioctls for non-root

Klemens Nanni-2
ifconfig(8) detects switch(4) through its unique SIOCSWSDPID ioctl and
further does another switch specific ioctl for the default output
regardless of configuration and/or members:

        SIOCSWSDPID struct ifbrparam
                Set the datapath_id in the OpenFlow protocol of the switch named
                in ifbrp_name to the value in the ifbrpu_datapath field.
       
        SIOCSWGMAXFLOW struct ifbrparam
                Retrieve the maximum number of flows in the OpenFlow protocol of
                the switch named in ifbrp_name into the ifbrp_maxflow field.

This is how it should look like:

        # ifconfig switch0 create
        # ifconfig switch0
                switch0: flags=0<>
                index 29 llprio 3
                groups: switch
                datapath 0x5bea2b5b8e2456cf maxflow 10000 maxgroup 1000

But using ifconfig as unprivileged user makes it fail switch(4)
interfaces as such and thus interprets them as bridge(4) instead:

        $ ifconfig switch0
                switch0: flags=0<>
                index 29 llprio 3
                groups: switch
                priority 32768 hellotime 2 fwddelay 15 maxage 20 holdcnt 6 proto rstp
                designated: id 00:00:00:00:00:00 priority 0

This is because the above mentioned ioctls are listed together with all
other bridge and switch related ioctls that set or write things.
Getting datapath_id and maxflow values however is read-only and crucial
for ifconfig as demonstrated above, so I'd like to move them out of the
root check to fix ifconfig.

Feedback? OK?


Index: sys/net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.616
diff -u -p -r1.616 if.c
--- sys/net/if.c 24 Jul 2020 18:17:14 -0000 1.616
+++ sys/net/if.c 31 Jul 2020 04:13:40 -0000
@@ -2170,13 +2170,15 @@ ifioctl(struct socket *so, u_long cmd, c
  case SIOCBRDGSIFCOST:
  case SIOCBRDGSTXHC:
  case SIOCBRDGSPROTO:
- case SIOCSWGDPID:
  case SIOCSWSPORTNO:
- case SIOCSWGMAXFLOW:
 #endif
  if ((error = suser(p)) != 0)
  break;
  /* FALLTHROUGH */
+#if NBRIDGE > 0
+ case SIOCSWGDPID:
+ case SIOCSWGMAXFLOW:
+#endif
  default:
  error = ((*so->so_proto->pr_usrreq)(so, PRU_CONTROL,
  (struct mbuf *) cmd, (struct mbuf *) data,

Reply | Threaded
Open this post in threaded view
|

Re: switch: allow datapath_id and maxflow ioctls for non-root

Klemens Nanni-2
On Fri, Jul 31, 2020 at 06:28:32AM +0200, Klemens Nanni wrote:
> ifconfig(8) detects switch(4) through its unique SIOCSWSDPID ioctl and
> further does another switch specific ioctl for the default output
> regardless of configuration and/or members:
>
> SIOCSWSDPID struct ifbrparam
> Set the datapath_id in the OpenFlow protocol of the switch named
> in ifbrp_name to the value in the ifbrpu_datapath field.
Sorry, stupid copy/pasta mistake;  ifconfig uses the read-only "get"
ioctl of course for regular output which my diff addresses:

        SIOCSWGDPID
                Retrieve the datapath_id in the OpenFlow protocol of the switch
                named in ifbrp_name into the ifbrpu_datapath field.

Reply | Threaded
Open this post in threaded view
|

Re: switch: allow datapath_id and maxflow ioctls for non-root

David Gwynne-5
In reply to this post by Klemens Nanni-2


> On 31 Jul 2020, at 14:28, Klemens Nanni <[hidden email]> wrote:
>
> ifconfig(8) detects switch(4) through its unique SIOCSWSDPID ioctl and
> further does another switch specific ioctl for the default output
> regardless of configuration and/or members:
>
> SIOCSWSDPID struct ifbrparam
> Set the datapath_id in the OpenFlow protocol of the switch named
> in ifbrp_name to the value in the ifbrpu_datapath field.
>
> SIOCSWGMAXFLOW struct ifbrparam
> Retrieve the maximum number of flows in the OpenFlow protocol of
> the switch named in ifbrp_name into the ifbrp_maxflow field.
>
> This is how it should look like:
>
> # ifconfig switch0 create
> # ifconfig switch0
> switch0: flags=0<>
> index 29 llprio 3
> groups: switch
> datapath 0x5bea2b5b8e2456cf maxflow 10000 maxgroup 1000
>
> But using ifconfig as unprivileged user makes it fail switch(4)
> interfaces as such and thus interprets them as bridge(4) instead:
>
> $ ifconfig switch0
> switch0: flags=0<>
> index 29 llprio 3
> groups: switch
> priority 32768 hellotime 2 fwddelay 15 maxage 20 holdcnt 6 proto rstp
> designated: id 00:00:00:00:00:00 priority 0
>
> This is because the above mentioned ioctls are listed together with all
> other bridge and switch related ioctls that set or write things.
> Getting datapath_id and maxflow values however is read-only and crucial
> for ifconfig as demonstrated above, so I'd like to move them out of the
> root check to fix ifconfig.
>
> Feedback? OK?

can't they be caught by the default case now?

dlg

>
>
> Index: sys/net/if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.616
> diff -u -p -r1.616 if.c
> --- sys/net/if.c 24 Jul 2020 18:17:14 -0000 1.616
> +++ sys/net/if.c 31 Jul 2020 04:13:40 -0000
> @@ -2170,13 +2170,15 @@ ifioctl(struct socket *so, u_long cmd, c
> case SIOCBRDGSIFCOST:
> case SIOCBRDGSTXHC:
> case SIOCBRDGSPROTO:
> - case SIOCSWGDPID:
> case SIOCSWSPORTNO:
> - case SIOCSWGMAXFLOW:
> #endif
> if ((error = suser(p)) != 0)
> break;
> /* FALLTHROUGH */
> +#if NBRIDGE > 0
> + case SIOCSWGDPID:
> + case SIOCSWGMAXFLOW:
> +#endif
> default:
> error = ((*so->so_proto->pr_usrreq)(so, PRU_CONTROL,
> (struct mbuf *) cmd, (struct mbuf *) data,
>

Reply | Threaded
Open this post in threaded view
|

Re: switch: allow datapath_id and maxflow ioctls for non-root

Klemens Nanni-2
On Wed, Aug 05, 2020 at 11:00:00AM +1000, David Gwynne wrote:
> can't they be caught by the default case now?
Obviously...


Index: net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.617
diff -u -p -r1.617 if.c
--- net/if.c 4 Aug 2020 09:32:05 -0000 1.617
+++ net/if.c 5 Aug 2020 06:24:33 -0000
@@ -2163,9 +2163,7 @@ ifioctl(struct socket *so, u_long cmd, c
  case SIOCBRDGSIFCOST:
  case SIOCBRDGSTXHC:
  case SIOCBRDGSPROTO:
- case SIOCSWGDPID:
  case SIOCSWSPORTNO:
- case SIOCSWGMAXFLOW:
 #endif
  if ((error = suser(p)) != 0)
  break;