| Message ID | 20241209-b4-ovpn-v14-17-ea243cf16417@openvpn.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introducing OpenVPN Data Channel Offload | expand |
On Mon, Dec 9, 2024 at 6:48 PM Antonio Quartulli <antonio@openvpn.net> wrote: [...] > +/** > + * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg > + * @peer: the peer to modify > + * @info: generic netlink info from the user request > + * @attrs: the attributes from the user request > + * > + * Return: a negative error code in case of failure, 0 on success or 1 on > + * success and the VPN IPs have been modified (requires rehashing in MP > + * mode) > + */ > +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info, > + struct nlattr **attrs) > +{ > + struct sockaddr_storage ss = {}; > + struct ovpn_socket *ovpn_sock; > + u32 sockfd, interv, timeout; > + struct socket *sock = NULL; > + u8 *local_ip = NULL; > + bool rehash = false; > + int ret; > + > + if (attrs[OVPN_A_PEER_SOCKET]) { Similar to link attributes in other tunnel drivers (e.g. IFLA_GRE_LINK, IFLA_GRE_FWMARK), user-supplied sockets could have sockopts (e.g. oif, fwmark, TOS). Since some of them may affect encapsulation and routing decision, which are supported in datapath? And do we need some validation here? [...] > +static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, > + const struct ovpn_peer *peer, u32 portid, u32 seq, > + int flags) > +{ > + const struct ovpn_bind *bind; > + struct nlattr *attr; > + void *hdr; > + > + hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags, > + OVPN_CMD_PEER_GET); > + if (!hdr) > + return -ENOBUFS; > + > + attr = nla_nest_start(skb, OVPN_A_PEER); > + if (!attr) > + goto err; > + > + if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id)) > + goto err; > + I think it would be helpful to include the netns ID and supported sockopts of the peer socket in peer info message.
Hi Xiao and thanks for chiming in, On 11/12/2024 04:08, Xiao Liang wrote: > On Mon, Dec 9, 2024 at 6:48 PM Antonio Quartulli <antonio@openvpn.net> wrote: > [...] >> +/** >> + * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg >> + * @peer: the peer to modify >> + * @info: generic netlink info from the user request >> + * @attrs: the attributes from the user request >> + * >> + * Return: a negative error code in case of failure, 0 on success or 1 on >> + * success and the VPN IPs have been modified (requires rehashing in MP >> + * mode) >> + */ >> +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info, >> + struct nlattr **attrs) >> +{ >> + struct sockaddr_storage ss = {}; >> + struct ovpn_socket *ovpn_sock; >> + u32 sockfd, interv, timeout; >> + struct socket *sock = NULL; >> + u8 *local_ip = NULL; >> + bool rehash = false; >> + int ret; >> + >> + if (attrs[OVPN_A_PEER_SOCKET]) { > > Similar to link attributes in other tunnel drivers (e.g. IFLA_GRE_LINK, > IFLA_GRE_FWMARK), user-supplied sockets could have sockopts > (e.g. oif, fwmark, TOS). Since some of them may affect encapsulation > and routing decision, which are supported in datapath? And do we need > some validation here? Thanks for pointing this out. At the moment ovpn doesn't expect any specific socket option. I haven't investigated how they could be used and what effect they would have on the packet processing. This is something we may consider later. At this point, do you still think I should add a check here of some sort? > > [...] >> +static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, >> + const struct ovpn_peer *peer, u32 portid, u32 seq, >> + int flags) >> +{ >> + const struct ovpn_bind *bind; >> + struct nlattr *attr; >> + void *hdr; >> + >> + hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags, >> + OVPN_CMD_PEER_GET); >> + if (!hdr) >> + return -ENOBUFS; >> + >> + attr = nla_nest_start(skb, OVPN_A_PEER); >> + if (!attr) >> + goto err; >> + >> + if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id)) >> + goto err; >> + > > I think it would be helpful to include the netns ID and supported sockopts > of the peer socket in peer info message. Technically the netns is the same as where the openvpn process in userspace is running, because it'll be it to open the socket and pass it down to ovpn. Therefore I am not sure there is any value in echoing back the netns ID. Wouldn't you agree? Regarding sockopts, as mentioned above, this is somewhat unsupported for now, so I Am not sure we have anything to send back. Regards,
On Wed, Dec 11, 2024 at 7:30 PM Antonio Quartulli <antonio@openvpn.net> wrote: > > Hi Xiao and thanks for chiming in, > > On 11/12/2024 04:08, Xiao Liang wrote: > > On Mon, Dec 9, 2024 at 6:48 PM Antonio Quartulli <antonio@openvpn.net> wrote: > > [...] > >> +/** > >> + * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg > >> + * @peer: the peer to modify > >> + * @info: generic netlink info from the user request > >> + * @attrs: the attributes from the user request > >> + * > >> + * Return: a negative error code in case of failure, 0 on success or 1 on > >> + * success and the VPN IPs have been modified (requires rehashing in MP > >> + * mode) > >> + */ > >> +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info, > >> + struct nlattr **attrs) > >> +{ > >> + struct sockaddr_storage ss = {}; > >> + struct ovpn_socket *ovpn_sock; > >> + u32 sockfd, interv, timeout; > >> + struct socket *sock = NULL; > >> + u8 *local_ip = NULL; > >> + bool rehash = false; > >> + int ret; > >> + > >> + if (attrs[OVPN_A_PEER_SOCKET]) { > > > > Similar to link attributes in other tunnel drivers (e.g. IFLA_GRE_LINK, > > IFLA_GRE_FWMARK), user-supplied sockets could have sockopts > > (e.g. oif, fwmark, TOS). Since some of them may affect encapsulation > > and routing decision, which are supported in datapath? And do we need > > some validation here? > > Thanks for pointing this out. > At the moment ovpn doesn't expect any specific socket option. > I haven't investigated how they could be used and what effect they would > have on the packet processing. > This is something we may consider later. > > At this point, do you still think I should add a check here of some sort? > I think some sockopts are important. Especially when oif is a VRF, the destination can be totally different than using the default routing table. If we don't support them now, it would be good to deny sockets with non-default values. > > > > [...] > >> +static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, > >> + const struct ovpn_peer *peer, u32 portid, u32 seq, > >> + int flags) > >> +{ > >> + const struct ovpn_bind *bind; > >> + struct nlattr *attr; > >> + void *hdr; > >> + > >> + hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags, > >> + OVPN_CMD_PEER_GET); > >> + if (!hdr) > >> + return -ENOBUFS; > >> + > >> + attr = nla_nest_start(skb, OVPN_A_PEER); > >> + if (!attr) > >> + goto err; > >> + > >> + if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id)) > >> + goto err; > >> + > > > > I think it would be helpful to include the netns ID and supported sockopts > > of the peer socket in peer info message. > > Technically the netns is the same as where the openvpn process in > userspace is running, because it'll be it to open the socket and pass it > down to ovpn. A userspace process could open UDP sockets in one namespace and the netlink socket in another. And the ovpn link could also be moved around. At this moment, we can remember the initial netns, or perhaps link-netns, of the ovpn link, and validate if the socket is in the same one. Thanks. > Therefore I am not sure there is any value in echoing back the netns ID. > Wouldn't you agree? > > Regarding sockopts, as mentioned above, this is somewhat unsupported for > now, so I Am not sure we have anything to send back. > > > Regards, > > -- > Antonio Quartulli > OpenVPN Inc. >
On 11/12/2024 13:35, Xiao Liang wrote: > On Wed, Dec 11, 2024 at 7:30 PM Antonio Quartulli <antonio@openvpn.net> wrote: >> >> Hi Xiao and thanks for chiming in, >> >> On 11/12/2024 04:08, Xiao Liang wrote: >>> On Mon, Dec 9, 2024 at 6:48 PM Antonio Quartulli <antonio@openvpn.net> wrote: >>> [...] >>>> +/** >>>> + * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg >>>> + * @peer: the peer to modify >>>> + * @info: generic netlink info from the user request >>>> + * @attrs: the attributes from the user request >>>> + * >>>> + * Return: a negative error code in case of failure, 0 on success or 1 on >>>> + * success and the VPN IPs have been modified (requires rehashing in MP >>>> + * mode) >>>> + */ >>>> +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info, >>>> + struct nlattr **attrs) >>>> +{ >>>> + struct sockaddr_storage ss = {}; >>>> + struct ovpn_socket *ovpn_sock; >>>> + u32 sockfd, interv, timeout; >>>> + struct socket *sock = NULL; >>>> + u8 *local_ip = NULL; >>>> + bool rehash = false; >>>> + int ret; >>>> + >>>> + if (attrs[OVPN_A_PEER_SOCKET]) { >>> >>> Similar to link attributes in other tunnel drivers (e.g. IFLA_GRE_LINK, >>> IFLA_GRE_FWMARK), user-supplied sockets could have sockopts >>> (e.g. oif, fwmark, TOS). Since some of them may affect encapsulation >>> and routing decision, which are supported in datapath? And do we need >>> some validation here? >> >> Thanks for pointing this out. >> At the moment ovpn doesn't expect any specific socket option. >> I haven't investigated how they could be used and what effect they would >> have on the packet processing. >> This is something we may consider later. >> >> At this point, do you still think I should add a check here of some sort? >> > > I think some sockopts are important. Especially when oif is a VRF, > the destination can be totally different than using the default routing > table. If we don't support them now, it would be good to deny sockets > with non-default values. I see - openvpn in userspace doesn't set any specific oif for the socket, but I understand ovpn should at least claim that those options are not supported. I am a bit lost regarding this aspect. Do you have a pointer for me where I can see how other modules are doing similar checks? > >>> >>> [...] >>>> +static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, >>>> + const struct ovpn_peer *peer, u32 portid, u32 seq, >>>> + int flags) >>>> +{ >>>> + const struct ovpn_bind *bind; >>>> + struct nlattr *attr; >>>> + void *hdr; >>>> + >>>> + hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags, >>>> + OVPN_CMD_PEER_GET); >>>> + if (!hdr) >>>> + return -ENOBUFS; >>>> + >>>> + attr = nla_nest_start(skb, OVPN_A_PEER); >>>> + if (!attr) >>>> + goto err; >>>> + >>>> + if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id)) >>>> + goto err; >>>> + >>> >>> I think it would be helpful to include the netns ID and supported sockopts >>> of the peer socket in peer info message. >> >> Technically the netns is the same as where the openvpn process in >> userspace is running, because it'll be it to open the socket and pass it >> down to ovpn. > > A userspace process could open UDP sockets in one namespace > and the netlink socket in another. And the ovpn link could also be > moved around. At this moment, we can remember the initial netns, > or perhaps link-netns, of the ovpn link, and validate if the socket > is in the same one. > You are correct, but we don't want to force sockets and link to be in the same netns. Openvpn in userspace may have been started in the global netns, where all sockets are expected to live (transport layer), but then the link/device is moved - or maybe created - somewhere else (tunnel layer). This is not an issue. Does it clarify? Thanks
On Wed, Dec 11, 2024 at 8:51 PM Antonio Quartulli <antonio@openvpn.net> wrote: > > On 11/12/2024 13:35, Xiao Liang wrote: > > On Wed, Dec 11, 2024 at 7:30 PM Antonio Quartulli <antonio@openvpn.net> wrote: > >> > >> Hi Xiao and thanks for chiming in, > >> > >> On 11/12/2024 04:08, Xiao Liang wrote: > >>> On Mon, Dec 9, 2024 at 6:48 PM Antonio Quartulli <antonio@openvpn.net> wrote: > >>> [...] > >>>> +/** > >>>> + * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg > >>>> + * @peer: the peer to modify > >>>> + * @info: generic netlink info from the user request > >>>> + * @attrs: the attributes from the user request > >>>> + * > >>>> + * Return: a negative error code in case of failure, 0 on success or 1 on > >>>> + * success and the VPN IPs have been modified (requires rehashing in MP > >>>> + * mode) > >>>> + */ > >>>> +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info, > >>>> + struct nlattr **attrs) > >>>> +{ > >>>> + struct sockaddr_storage ss = {}; > >>>> + struct ovpn_socket *ovpn_sock; > >>>> + u32 sockfd, interv, timeout; > >>>> + struct socket *sock = NULL; > >>>> + u8 *local_ip = NULL; > >>>> + bool rehash = false; > >>>> + int ret; > >>>> + > >>>> + if (attrs[OVPN_A_PEER_SOCKET]) { > >>> > >>> Similar to link attributes in other tunnel drivers (e.g. IFLA_GRE_LINK, > >>> IFLA_GRE_FWMARK), user-supplied sockets could have sockopts > >>> (e.g. oif, fwmark, TOS). Since some of them may affect encapsulation > >>> and routing decision, which are supported in datapath? And do we need > >>> some validation here? > >> > >> Thanks for pointing this out. > >> At the moment ovpn doesn't expect any specific socket option. > >> I haven't investigated how they could be used and what effect they would > >> have on the packet processing. > >> This is something we may consider later. > >> > >> At this point, do you still think I should add a check here of some sort? > >> > > > > I think some sockopts are important. Especially when oif is a VRF, > > the destination can be totally different than using the default routing > > table. If we don't support them now, it would be good to deny sockets > > with non-default values. > > I see - openvpn in userspace doesn't set any specific oif for the > socket, but I understand ovpn should at least claim that those options > are not supported. > > I am a bit lost regarding this aspect. Do you have a pointer for me > where I can see how other modules are doing similar checks? > The closest thing I can find is L2TP, which has some checks in l2tp_validate_socket(). However, it uses ip_queue_xmit() / inet6_csk_xmit() to send packets, where many sockopts are handled. Maybe someone else can give a more suitable example. I guess we can start with sockopts relevant to fields in struct flowi{4,6} and encap headers? Or at least add some documentation about this. > > > >>> > >>> [...] > >>>> +static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, > >>>> + const struct ovpn_peer *peer, u32 portid, u32 seq, > >>>> + int flags) > >>>> +{ > >>>> + const struct ovpn_bind *bind; > >>>> + struct nlattr *attr; > >>>> + void *hdr; > >>>> + > >>>> + hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags, > >>>> + OVPN_CMD_PEER_GET); > >>>> + if (!hdr) > >>>> + return -ENOBUFS; > >>>> + > >>>> + attr = nla_nest_start(skb, OVPN_A_PEER); > >>>> + if (!attr) > >>>> + goto err; > >>>> + > >>>> + if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id)) > >>>> + goto err; > >>>> + > >>> > >>> I think it would be helpful to include the netns ID and supported sockopts > >>> of the peer socket in peer info message. > >> > >> Technically the netns is the same as where the openvpn process in > >> userspace is running, because it'll be it to open the socket and pass it > >> down to ovpn. > > > > A userspace process could open UDP sockets in one namespace > > and the netlink socket in another. And the ovpn link could also be > > moved around. At this moment, we can remember the initial netns, > > or perhaps link-netns, of the ovpn link, and validate if the socket > > is in the same one. > > > > You are correct, but we don't want to force sockets and link to be in > the same netns. > > Openvpn in userspace may have been started in the global netns, where > all sockets are expected to live (transport layer), but then the > link/device is moved - or maybe created - somewhere else (tunnel layer). > This is not an issue. > > Does it clarify? If netns id is not included, then when the link has been moved, we can't infer which netns the socket is in from peer info message, thus can not figure out how packets are routed. Other tunnel drivers usually use IFLA_LINK_NETNSID for this. Probably have a look at rtnl_fill_link_netnsid()? Thanks.
On 11/12/2024 14:53, Xiao Liang wrote: > On Wed, Dec 11, 2024 at 8:51 PM Antonio Quartulli <antonio@openvpn.net> wrote: >> >> On 11/12/2024 13:35, Xiao Liang wrote: >>> On Wed, Dec 11, 2024 at 7:30 PM Antonio Quartulli <antonio@openvpn.net> wrote: >>>> >>>> Hi Xiao and thanks for chiming in, >>>> >>>> On 11/12/2024 04:08, Xiao Liang wrote: >>>>> On Mon, Dec 9, 2024 at 6:48 PM Antonio Quartulli <antonio@openvpn.net> wrote: >>>>> [...] >>>>>> +/** >>>>>> + * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg >>>>>> + * @peer: the peer to modify >>>>>> + * @info: generic netlink info from the user request >>>>>> + * @attrs: the attributes from the user request >>>>>> + * >>>>>> + * Return: a negative error code in case of failure, 0 on success or 1 on >>>>>> + * success and the VPN IPs have been modified (requires rehashing in MP >>>>>> + * mode) >>>>>> + */ >>>>>> +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info, >>>>>> + struct nlattr **attrs) >>>>>> +{ >>>>>> + struct sockaddr_storage ss = {}; >>>>>> + struct ovpn_socket *ovpn_sock; >>>>>> + u32 sockfd, interv, timeout; >>>>>> + struct socket *sock = NULL; >>>>>> + u8 *local_ip = NULL; >>>>>> + bool rehash = false; >>>>>> + int ret; >>>>>> + >>>>>> + if (attrs[OVPN_A_PEER_SOCKET]) { >>>>> >>>>> Similar to link attributes in other tunnel drivers (e.g. IFLA_GRE_LINK, >>>>> IFLA_GRE_FWMARK), user-supplied sockets could have sockopts >>>>> (e.g. oif, fwmark, TOS). Since some of them may affect encapsulation >>>>> and routing decision, which are supported in datapath? And do we need >>>>> some validation here? >>>> >>>> Thanks for pointing this out. >>>> At the moment ovpn doesn't expect any specific socket option. >>>> I haven't investigated how they could be used and what effect they would >>>> have on the packet processing. >>>> This is something we may consider later. >>>> >>>> At this point, do you still think I should add a check here of some sort? >>>> >>> >>> I think some sockopts are important. Especially when oif is a VRF, >>> the destination can be totally different than using the default routing >>> table. If we don't support them now, it would be good to deny sockets >>> with non-default values. >> >> I see - openvpn in userspace doesn't set any specific oif for the >> socket, but I understand ovpn should at least claim that those options >> are not supported. >> >> I am a bit lost regarding this aspect. Do you have a pointer for me >> where I can see how other modules are doing similar checks? >> > > The closest thing I can find is L2TP, which has some checks in > l2tp_validate_socket(). However, it uses ip_queue_xmit() / > inet6_csk_xmit() to send packets, where many sockopts are handled. mhh l2tp_sk_to_tunnel() doesn't have more checks than what we already have. > Maybe someone else can give a more suitable example. I guess we > can start with sockopts relevant to fields in struct flowi{4,6} and encap > headers? Since I have little experience with sockopts in general, and this is not truly mission critical, how would you feel about sending a patch for this once ovpn has been merged? I'd truly appreciate it. > >>> >>>>> >>>>> [...] >>>>>> +static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, >>>>>> + const struct ovpn_peer *peer, u32 portid, u32 seq, >>>>>> + int flags) >>>>>> +{ >>>>>> + const struct ovpn_bind *bind; >>>>>> + struct nlattr *attr; >>>>>> + void *hdr; >>>>>> + >>>>>> + hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags, >>>>>> + OVPN_CMD_PEER_GET); >>>>>> + if (!hdr) >>>>>> + return -ENOBUFS; >>>>>> + >>>>>> + attr = nla_nest_start(skb, OVPN_A_PEER); >>>>>> + if (!attr) >>>>>> + goto err; >>>>>> + >>>>>> + if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id)) >>>>>> + goto err; >>>>>> + >>>>> >>>>> I think it would be helpful to include the netns ID and supported sockopts >>>>> of the peer socket in peer info message. >>>> >>>> Technically the netns is the same as where the openvpn process in >>>> userspace is running, because it'll be it to open the socket and pass it >>>> down to ovpn. >>> >>> A userspace process could open UDP sockets in one namespace >>> and the netlink socket in another. And the ovpn link could also be >>> moved around. At this moment, we can remember the initial netns, >>> or perhaps link-netns, of the ovpn link, and validate if the socket >>> is in the same one. >>> >> >> You are correct, but we don't want to force sockets and link to be in >> the same netns. >> >> Openvpn in userspace may have been started in the global netns, where >> all sockets are expected to live (transport layer), but then the >> link/device is moved - or maybe created - somewhere else (tunnel layer). >> This is not an issue. >> >> Does it clarify? > > If netns id is not included, then when the link has been moved, > we can't infer which netns the socket is in from peer info message, > thus can not figure out how packets are routed. Other tunnel drivers > usually use IFLA_LINK_NETNSID for this. Probably have a look at > rtnl_fill_link_netnsid()? > Ok, I see what you mean. I was assuming this was not needed, because we'd always have a running openvpn process and it would live in the socket netns. But it still makes sense to report it with the peer info. I'll add this new attribute and fill it on PEER_GET. Thanks a lot. > Thanks.
On Wed, Dec 11, 2024 at 10:07 PM Antonio Quartulli <antonio@openvpn.net> wrote: > > On 11/12/2024 14:53, Xiao Liang wrote: > > On Wed, Dec 11, 2024 at 8:51 PM Antonio Quartulli <antonio@openvpn.net> wrote: > >> > >> On 11/12/2024 13:35, Xiao Liang wrote: > >>> On Wed, Dec 11, 2024 at 7:30 PM Antonio Quartulli <antonio@openvpn.net> wrote: > >>>> > >>>> Hi Xiao and thanks for chiming in, > >>>> > >>>> On 11/12/2024 04:08, Xiao Liang wrote: > >>>>> On Mon, Dec 9, 2024 at 6:48 PM Antonio Quartulli <antonio@openvpn.net> wrote: > >>>>> [...] > >>>>>> +/** > >>>>>> + * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg > >>>>>> + * @peer: the peer to modify > >>>>>> + * @info: generic netlink info from the user request > >>>>>> + * @attrs: the attributes from the user request > >>>>>> + * > >>>>>> + * Return: a negative error code in case of failure, 0 on success or 1 on > >>>>>> + * success and the VPN IPs have been modified (requires rehashing in MP > >>>>>> + * mode) > >>>>>> + */ > >>>>>> +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info, > >>>>>> + struct nlattr **attrs) > >>>>>> +{ > >>>>>> + struct sockaddr_storage ss = {}; > >>>>>> + struct ovpn_socket *ovpn_sock; > >>>>>> + u32 sockfd, interv, timeout; > >>>>>> + struct socket *sock = NULL; > >>>>>> + u8 *local_ip = NULL; > >>>>>> + bool rehash = false; > >>>>>> + int ret; > >>>>>> + > >>>>>> + if (attrs[OVPN_A_PEER_SOCKET]) { > >>>>> > >>>>> Similar to link attributes in other tunnel drivers (e.g. IFLA_GRE_LINK, > >>>>> IFLA_GRE_FWMARK), user-supplied sockets could have sockopts > >>>>> (e.g. oif, fwmark, TOS). Since some of them may affect encapsulation > >>>>> and routing decision, which are supported in datapath? And do we need > >>>>> some validation here? > >>>> > >>>> Thanks for pointing this out. > >>>> At the moment ovpn doesn't expect any specific socket option. > >>>> I haven't investigated how they could be used and what effect they would > >>>> have on the packet processing. > >>>> This is something we may consider later. > >>>> > >>>> At this point, do you still think I should add a check here of some sort? > >>>> > >>> > >>> I think some sockopts are important. Especially when oif is a VRF, > >>> the destination can be totally different than using the default routing > >>> table. If we don't support them now, it would be good to deny sockets > >>> with non-default values. > >> > >> I see - openvpn in userspace doesn't set any specific oif for the > >> socket, but I understand ovpn should at least claim that those options > >> are not supported. > >> > >> I am a bit lost regarding this aspect. Do you have a pointer for me > >> where I can see how other modules are doing similar checks? > >> > > > > The closest thing I can find is L2TP, which has some checks in > > l2tp_validate_socket(). However, it uses ip_queue_xmit() / > > inet6_csk_xmit() to send packets, where many sockopts are handled. > > mhh l2tp_sk_to_tunnel() doesn't have more checks than what we already have. > > > Maybe someone else can give a more suitable example. I guess we > > can start with sockopts relevant to fields in struct flowi{4,6} and encap > > headers? > > Since I have little experience with sockopts in general, and this is not > truly mission critical, how would you feel about sending a patch for > this once ovpn has been merged? > I'd truly appreciate it. Honestly I'm not an expert on this. Will see if I can. > > > > >>> > >>>>> > >>>>> [...] > >>>>>> +static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, > >>>>>> + const struct ovpn_peer *peer, u32 portid, u32 seq, > >>>>>> + int flags) > >>>>>> +{ > >>>>>> + const struct ovpn_bind *bind; > >>>>>> + struct nlattr *attr; > >>>>>> + void *hdr; > >>>>>> + > >>>>>> + hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags, > >>>>>> + OVPN_CMD_PEER_GET); > >>>>>> + if (!hdr) > >>>>>> + return -ENOBUFS; > >>>>>> + > >>>>>> + attr = nla_nest_start(skb, OVPN_A_PEER); > >>>>>> + if (!attr) > >>>>>> + goto err; > >>>>>> + > >>>>>> + if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id)) > >>>>>> + goto err; > >>>>>> + > >>>>> > >>>>> I think it would be helpful to include the netns ID and supported sockopts > >>>>> of the peer socket in peer info message. > >>>> > >>>> Technically the netns is the same as where the openvpn process in > >>>> userspace is running, because it'll be it to open the socket and pass it > >>>> down to ovpn. > >>> > >>> A userspace process could open UDP sockets in one namespace > >>> and the netlink socket in another. And the ovpn link could also be > >>> moved around. At this moment, we can remember the initial netns, > >>> or perhaps link-netns, of the ovpn link, and validate if the socket > >>> is in the same one. > >>> > >> > >> You are correct, but we don't want to force sockets and link to be in > >> the same netns. > >> > >> Openvpn in userspace may have been started in the global netns, where > >> all sockets are expected to live (transport layer), but then the > >> link/device is moved - or maybe created - somewhere else (tunnel layer). > >> This is not an issue. > >> > >> Does it clarify? > > > > If netns id is not included, then when the link has been moved, > > we can't infer which netns the socket is in from peer info message, > > thus can not figure out how packets are routed. Other tunnel drivers > > usually use IFLA_LINK_NETNSID for this. Probably have a look at > > rtnl_fill_link_netnsid()? > > > > Ok, I see what you mean. > I was assuming this was not needed, because we'd always have a running > openvpn process and it would live in the socket netns. > But it still makes sense to report it with the peer info. > > I'll add this new attribute and fill it on PEER_GET. > That would be nice. Thanks!
diff --git a/drivers/net/ovpn/netlink.c b/drivers/net/ovpn/netlink.c index c91368408b805d2bf4f12d64d5c55f4ed6d81343..6b78137a8a43e7e030308a61e73739454009aa03 100644 --- a/drivers/net/ovpn/netlink.c +++ b/drivers/net/ovpn/netlink.c @@ -7,6 +7,7 @@ */ #include <linux/netdevice.h> +#include <linux/types.h> #include <net/genetlink.h> #include <uapi/linux/ovpn.h> @@ -15,6 +16,9 @@ #include "main.h" #include "netlink.h" #include "netlink-gen.h" +#include "bind.h" +#include "peer.h" +#include "socket.h" MODULE_ALIAS_GENL_FAMILY(OVPN_FAMILY_NAME); @@ -85,29 +89,646 @@ void ovpn_nl_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb, netdev_put(ovpn->dev, &ovpn->dev_tracker); } +static int ovpn_nl_attr_sockaddr_remote(struct nlattr **attrs, + struct sockaddr_storage *ss) +{ + struct sockaddr_in6 *sin6; + struct sockaddr_in *sin; + struct in6_addr *in6; + __be16 port = 0; + __be32 *in; + int af; + + ss->ss_family = AF_UNSPEC; + + if (attrs[OVPN_A_PEER_REMOTE_PORT]) + port = nla_get_be16(attrs[OVPN_A_PEER_REMOTE_PORT]); + + if (attrs[OVPN_A_PEER_REMOTE_IPV4]) { + af = AF_INET; + ss->ss_family = AF_INET; + in = nla_data(attrs[OVPN_A_PEER_REMOTE_IPV4]); + } else if (attrs[OVPN_A_PEER_REMOTE_IPV6]) { + af = AF_INET6; + ss->ss_family = AF_INET6; + in6 = nla_data(attrs[OVPN_A_PEER_REMOTE_IPV6]); + } else { + return AF_UNSPEC; + } + + switch (ss->ss_family) { + case AF_INET6: + /* If this is a regular IPv6 just break and move on, + * otherwise switch to AF_INET and extract the IPv4 accordingly + */ + if (!ipv6_addr_v4mapped(in6)) { + sin6 = (struct sockaddr_in6 *)ss; + sin6->sin6_port = port; + memcpy(&sin6->sin6_addr, in6, sizeof(*in6)); + break; + } + + /* v4-mapped-v6 address */ + ss->ss_family = AF_INET; + in = &in6->s6_addr32[3]; + fallthrough; + case AF_INET: + sin = (struct sockaddr_in *)ss; + sin->sin_port = port; + sin->sin_addr.s_addr = *in; + break; + } + + /* don't return ss->ss_family as it may have changed in case of + * v4-mapped-v6 address + */ + return af; +} + +static u8 *ovpn_nl_attr_local_ip(struct nlattr **attrs) +{ + u8 *addr6; + + if (!attrs[OVPN_A_PEER_LOCAL_IPV4] && !attrs[OVPN_A_PEER_LOCAL_IPV6]) + return NULL; + + if (attrs[OVPN_A_PEER_LOCAL_IPV4]) + return nla_data(attrs[OVPN_A_PEER_LOCAL_IPV4]); + + addr6 = nla_data(attrs[OVPN_A_PEER_LOCAL_IPV6]); + /* this is an IPv4-mapped IPv6 address, therefore extract the actual + * v4 address from the last 4 bytes + */ + if (ipv6_addr_v4mapped((struct in6_addr *)addr6)) + return addr6 + 12; + + return addr6; +} + +static sa_family_t ovpn_nl_family_get(struct nlattr *addr4, + struct nlattr *addr6) +{ + if (addr4) + return AF_INET; + + if (addr6) { + if (ipv6_addr_v4mapped((struct in6_addr *)nla_data(addr6))) + return AF_INET; + return AF_INET6; + } + + return AF_UNSPEC; +} + +static int ovpn_nl_peer_precheck(struct ovpn_priv *ovpn, + struct genl_info *info, + struct nlattr **attrs) +{ + sa_family_t local_fam, remote_fam; + + if (NL_REQ_ATTR_CHECK(info->extack, info->attrs[OVPN_A_PEER], attrs, + OVPN_A_PEER_ID)) + return -EINVAL; + + if (attrs[OVPN_A_PEER_REMOTE_IPV4] && attrs[OVPN_A_PEER_REMOTE_IPV6]) { + NL_SET_ERR_MSG_MOD(info->extack, + "cannot specify both remote IPv4 or IPv6 address"); + return -EINVAL; + } + + if (!attrs[OVPN_A_PEER_REMOTE_IPV4] && + !attrs[OVPN_A_PEER_REMOTE_IPV6] && attrs[OVPN_A_PEER_REMOTE_PORT]) { + NL_SET_ERR_MSG_MOD(info->extack, + "cannot specify remote port without IP address"); + return -EINVAL; + } + + if (!attrs[OVPN_A_PEER_REMOTE_IPV4] && + attrs[OVPN_A_PEER_LOCAL_IPV4]) { + NL_SET_ERR_MSG_MOD(info->extack, + "cannot specify local IPv4 address without remote"); + return -EINVAL; + } + + if (!attrs[OVPN_A_PEER_REMOTE_IPV6] && + attrs[OVPN_A_PEER_LOCAL_IPV6]) { + NL_SET_ERR_MSG_MOD(info->extack, + "cannot specify local IPV6 address without remote"); + return -EINVAL; + } + + /* check that local and remote address families are the same even + * after parsing v4mapped IPv6 addresses. + * (if addresses are not provided, family will be AF_UNSPEC and + * the check is skipped) + */ + local_fam = ovpn_nl_family_get(attrs[OVPN_A_PEER_LOCAL_IPV4], + attrs[OVPN_A_PEER_LOCAL_IPV6]); + remote_fam = ovpn_nl_family_get(attrs[OVPN_A_PEER_REMOTE_IPV4], + attrs[OVPN_A_PEER_REMOTE_IPV6]); + if (local_fam != AF_UNSPEC && remote_fam != AF_UNSPEC && + local_fam != remote_fam) { + NL_SET_ERR_MSG_MOD(info->extack, + "mismatching local and remote address families"); + return -EINVAL; + } + + if (remote_fam != AF_INET6 && attrs[OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID]) { + NL_SET_ERR_MSG_MOD(info->extack, + "cannot specify scope id without remote IPv6 address"); + return -EINVAL; + } + + /* VPN IPs are needed only in MP mode for selecting the right peer */ + if (ovpn->mode == OVPN_MODE_P2P && (attrs[OVPN_A_PEER_VPN_IPV4] || + attrs[OVPN_A_PEER_VPN_IPV6])) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "VPN IP unexpected in P2P mode"); + return -EINVAL; + } + + if ((attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL] && + !attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]) || + (!attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL] && + attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT])) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "keepalive interval and timeout are required together"); + return -EINVAL; + } + + return 0; +} + +/** + * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg + * @peer: the peer to modify + * @info: generic netlink info from the user request + * @attrs: the attributes from the user request + * + * Return: a negative error code in case of failure, 0 on success or 1 on + * success and the VPN IPs have been modified (requires rehashing in MP + * mode) + */ +static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info, + struct nlattr **attrs) +{ + struct sockaddr_storage ss = {}; + struct ovpn_socket *ovpn_sock; + u32 sockfd, interv, timeout; + struct socket *sock = NULL; + u8 *local_ip = NULL; + bool rehash = false; + int ret; + + if (attrs[OVPN_A_PEER_SOCKET]) { + if (peer->sock) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "peer socket can't be modified"); + return -EINVAL; + } + + /* lookup the fd in the kernel table and extract the socket + * object + */ + sockfd = nla_get_u32(attrs[OVPN_A_PEER_SOCKET]); + /* sockfd_lookup() increases sock's refcounter */ + sock = sockfd_lookup(sockfd, &ret); + if (!sock) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "cannot lookup peer socket (fd=%u): %d", + sockfd, ret); + return -ENOTSOCK; + } + + /* Only when using UDP as transport protocol the remote endpoint + * can be configured so that ovpn knows where to send packets + * to. + * + * In case of TCP, the socket is connected to the peer and ovpn + * will just send bytes over it, without the need to specify a + * destination. + */ + if (sock->sk->sk_protocol != IPPROTO_UDP && + (attrs[OVPN_A_PEER_REMOTE_IPV4] || + attrs[OVPN_A_PEER_REMOTE_IPV6])) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "unexpected remote IP address for non UDP socket"); + sockfd_put(sock); + return -EINVAL; + } + + ovpn_sock = ovpn_socket_new(sock, peer); + if (IS_ERR(ovpn_sock)) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "cannot encapsulate socket: %ld", + PTR_ERR(ovpn_sock)); + sockfd_put(sock); + return -ENOTSOCK; + } + + peer->sock = ovpn_sock; + } + + /* ovpn_socket_new() may sleep and cannot be invoked under lock. + * For this reason we acquire the lock only from this point on. + * However, this is not an issue because the socket cannot be + * updated on an existing peer, therefore there is no concurrency + * to protect from + */ + spin_lock_bh(&peer->lock); + + if (ovpn_nl_attr_sockaddr_remote(attrs, &ss) != AF_UNSPEC) { + /* we carry the local IP in a generic container. + * ovpn_peer_reset_sockaddr() will properly interpret it + * based on ss.ss_family + */ + local_ip = ovpn_nl_attr_local_ip(attrs); + + /* set peer sockaddr */ + ret = ovpn_peer_reset_sockaddr(peer, &ss, local_ip); + if (ret < 0) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "cannot set peer sockaddr: %d", + ret); + goto err_unlock; + } + } + + if (attrs[OVPN_A_PEER_VPN_IPV4]) { + rehash = true; + peer->vpn_addrs.ipv4.s_addr = + nla_get_in_addr(attrs[OVPN_A_PEER_VPN_IPV4]); + } + + if (attrs[OVPN_A_PEER_VPN_IPV6]) { + rehash = true; + peer->vpn_addrs.ipv6 = + nla_get_in6_addr(attrs[OVPN_A_PEER_VPN_IPV6]); + } + + /* when setting the keepalive, both parameters have to be configured */ + if (attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL] && + attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]) { + interv = nla_get_u32(attrs[OVPN_A_PEER_KEEPALIVE_INTERVAL]); + timeout = nla_get_u32(attrs[OVPN_A_PEER_KEEPALIVE_TIMEOUT]); + ovpn_peer_keepalive_set(peer, interv, timeout); + } + + netdev_dbg(peer->ovpn->dev, + "modify peer id=%u endpoint=%pIScp/%s VPN-IPv4=%pI4 VPN-IPv6=%pI6c\n", + peer->id, &ss, peer->sock->sock->sk->sk_prot_creator->name, + &peer->vpn_addrs.ipv4.s_addr, &peer->vpn_addrs.ipv6); + + spin_unlock_bh(&peer->lock); + + return rehash ? 1 : 0; +err_unlock: + spin_unlock_bh(&peer->lock); + return ret; +} + int ovpn_nl_peer_new_doit(struct sk_buff *skb, struct genl_info *info) { - return -EOPNOTSUPP; + struct nlattr *attrs[OVPN_A_PEER_MAX + 1]; + struct ovpn_priv *ovpn = info->user_ptr[0]; + struct ovpn_peer *peer; + u32 peer_id; + int ret; + + /* peers can only be added when the interface is up and running */ + if (!netif_running(ovpn->dev)) + return -ENETDOWN; + + if (GENL_REQ_ATTR_CHECK(info, OVPN_A_PEER)) + return -EINVAL; + + ret = nla_parse_nested(attrs, OVPN_A_PEER_MAX, info->attrs[OVPN_A_PEER], + ovpn_peer_nl_policy, info->extack); + if (ret) + return ret; + + ret = ovpn_nl_peer_precheck(ovpn, info, attrs); + if (ret < 0) + return ret; + + if (NL_REQ_ATTR_CHECK(info->extack, info->attrs[OVPN_A_PEER], attrs, + OVPN_A_PEER_SOCKET)) + return -EINVAL; + + peer_id = nla_get_u32(attrs[OVPN_A_PEER_ID]); + peer = ovpn_peer_new(ovpn, peer_id); + if (IS_ERR(peer)) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "cannot create new peer object for peer %u: %ld", + peer_id, PTR_ERR(peer)); + return PTR_ERR(peer); + } + + ret = ovpn_nl_peer_modify(peer, info, attrs); + if (ret < 0) + goto peer_release; + + ret = ovpn_peer_add(ovpn, peer); + if (ret < 0) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "cannot add new peer (id=%u) to hashtable: %d\n", + peer->id, ret); + goto peer_release; + } + + return 0; + +peer_release: + /* release right away because peer is not used in any context */ + ovpn_peer_release(peer); + + return ret; } int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info) { - return -EOPNOTSUPP; + struct nlattr *attrs[OVPN_A_PEER_MAX + 1]; + struct ovpn_priv *ovpn = info->user_ptr[0]; + struct ovpn_peer *peer; + u32 peer_id; + int ret; + + if (GENL_REQ_ATTR_CHECK(info, OVPN_A_PEER)) + return -EINVAL; + + ret = nla_parse_nested(attrs, OVPN_A_PEER_MAX, info->attrs[OVPN_A_PEER], + ovpn_peer_nl_policy, info->extack); + if (ret) + return ret; + + ret = ovpn_nl_peer_precheck(ovpn, info, attrs); + if (ret < 0) + return ret; + + peer_id = nla_get_u32(attrs[OVPN_A_PEER_ID]); + peer = ovpn_peer_get_by_id(ovpn, peer_id); + if (!peer) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "cannot find peer with id %u", peer_id); + return -ENOENT; + } + + spin_lock_bh(&ovpn->lock); + ret = ovpn_nl_peer_modify(peer, info, attrs); + if (ret < 0) { + spin_unlock_bh(&ovpn->lock); + ovpn_peer_put(peer); + return ret; + } + + /* ret == 1 means that VPN IPv4/6 has been modified and rehashing + * is required + */ + if (ret > 0) + ovpn_peer_hash_vpn_ip(peer); + spin_unlock_bh(&ovpn->lock); + ovpn_peer_put(peer); + + return 0; +} + +static int ovpn_nl_send_peer(struct sk_buff *skb, const struct genl_info *info, + const struct ovpn_peer *peer, u32 portid, u32 seq, + int flags) +{ + const struct ovpn_bind *bind; + struct nlattr *attr; + void *hdr; + + hdr = genlmsg_put(skb, portid, seq, &ovpn_nl_family, flags, + OVPN_CMD_PEER_GET); + if (!hdr) + return -ENOBUFS; + + attr = nla_nest_start(skb, OVPN_A_PEER); + if (!attr) + goto err; + + if (nla_put_u32(skb, OVPN_A_PEER_ID, peer->id)) + goto err; + + if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY)) + if (nla_put_in_addr(skb, OVPN_A_PEER_VPN_IPV4, + peer->vpn_addrs.ipv4.s_addr)) + goto err; + + if (!ipv6_addr_equal(&peer->vpn_addrs.ipv6, &in6addr_any)) + if (nla_put_in6_addr(skb, OVPN_A_PEER_VPN_IPV6, + &peer->vpn_addrs.ipv6)) + goto err; + + if (nla_put_u32(skb, OVPN_A_PEER_KEEPALIVE_INTERVAL, + peer->keepalive_interval) || + nla_put_u32(skb, OVPN_A_PEER_KEEPALIVE_TIMEOUT, + peer->keepalive_timeout)) + goto err; + + rcu_read_lock(); + bind = rcu_dereference(peer->bind); + if (bind) { + if (bind->remote.in4.sin_family == AF_INET) { + if (nla_put_in_addr(skb, OVPN_A_PEER_REMOTE_IPV4, + bind->remote.in4.sin_addr.s_addr) || + nla_put_net16(skb, OVPN_A_PEER_REMOTE_PORT, + bind->remote.in4.sin_port) || + nla_put_in_addr(skb, OVPN_A_PEER_LOCAL_IPV4, + bind->local.ipv4.s_addr)) + goto err_unlock; + } else if (bind->remote.in4.sin_family == AF_INET6) { + if (nla_put_in6_addr(skb, OVPN_A_PEER_REMOTE_IPV6, + &bind->remote.in6.sin6_addr) || + nla_put_u32(skb, OVPN_A_PEER_REMOTE_IPV6_SCOPE_ID, + bind->remote.in6.sin6_scope_id) || + nla_put_net16(skb, OVPN_A_PEER_REMOTE_PORT, + bind->remote.in6.sin6_port) || + nla_put_in6_addr(skb, OVPN_A_PEER_LOCAL_IPV6, + &bind->local.ipv6)) + goto err_unlock; + } + } + rcu_read_unlock(); + + if (nla_put_net16(skb, OVPN_A_PEER_LOCAL_PORT, + inet_sk(peer->sock->sock->sk)->inet_sport) || + /* VPN RX stats */ + nla_put_uint(skb, OVPN_A_PEER_VPN_RX_BYTES, + atomic64_read(&peer->vpn_stats.rx.bytes)) || + nla_put_uint(skb, OVPN_A_PEER_VPN_RX_PACKETS, + atomic64_read(&peer->vpn_stats.rx.packets)) || + /* VPN TX stats */ + nla_put_uint(skb, OVPN_A_PEER_VPN_TX_BYTES, + atomic64_read(&peer->vpn_stats.tx.bytes)) || + nla_put_uint(skb, OVPN_A_PEER_VPN_TX_PACKETS, + atomic64_read(&peer->vpn_stats.tx.packets)) || + /* link RX stats */ + nla_put_uint(skb, OVPN_A_PEER_LINK_RX_BYTES, + atomic64_read(&peer->link_stats.rx.bytes)) || + nla_put_uint(skb, OVPN_A_PEER_LINK_RX_PACKETS, + atomic64_read(&peer->link_stats.rx.packets)) || + /* link TX stats */ + nla_put_uint(skb, OVPN_A_PEER_LINK_TX_BYTES, + atomic64_read(&peer->link_stats.tx.bytes)) || + nla_put_uint(skb, OVPN_A_PEER_LINK_TX_PACKETS, + atomic64_read(&peer->link_stats.tx.packets))) + goto err; + + nla_nest_end(skb, attr); + genlmsg_end(skb, hdr); + + return 0; +err_unlock: + rcu_read_unlock(); +err: + genlmsg_cancel(skb, hdr); + return -EMSGSIZE; } int ovpn_nl_peer_get_doit(struct sk_buff *skb, struct genl_info *info) { - return -EOPNOTSUPP; + struct nlattr *attrs[OVPN_A_PEER_MAX + 1]; + struct ovpn_priv *ovpn = info->user_ptr[0]; + struct ovpn_peer *peer; + struct sk_buff *msg; + u32 peer_id; + int ret; + + if (GENL_REQ_ATTR_CHECK(info, OVPN_A_PEER)) + return -EINVAL; + + ret = nla_parse_nested(attrs, OVPN_A_PEER_MAX, info->attrs[OVPN_A_PEER], + ovpn_peer_nl_policy, info->extack); + if (ret) + return ret; + + if (NL_REQ_ATTR_CHECK(info->extack, info->attrs[OVPN_A_PEER], attrs, + OVPN_A_PEER_ID)) + return -EINVAL; + + peer_id = nla_get_u32(attrs[OVPN_A_PEER_ID]); + peer = ovpn_peer_get_by_id(ovpn, peer_id); + if (!peer) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "cannot find peer with id %u", peer_id); + return -ENOENT; + } + + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); + if (!msg) { + ret = -ENOMEM; + goto err; + } + + ret = ovpn_nl_send_peer(msg, info, peer, info->snd_portid, + info->snd_seq, 0); + if (ret < 0) { + nlmsg_free(msg); + goto err; + } + + ret = genlmsg_reply(msg, info); +err: + ovpn_peer_put(peer); + return ret; } int ovpn_nl_peer_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) { - return -EOPNOTSUPP; + const struct genl_info *info = genl_info_dump(cb); + int bkt, last_idx = cb->args[1], dumped = 0; + struct ovpn_priv *ovpn; + struct ovpn_peer *peer; + + ovpn = ovpn_get_dev_from_attrs(sock_net(cb->skb->sk), info); + if (IS_ERR(ovpn)) + return PTR_ERR(ovpn); + + if (ovpn->mode == OVPN_MODE_P2P) { + /* if we already dumped a peer it means we are done */ + if (last_idx) + goto out; + + rcu_read_lock(); + peer = rcu_dereference(ovpn->peer); + if (peer) { + if (ovpn_nl_send_peer(skb, info, peer, + NETLINK_CB(cb->skb).portid, + cb->nlh->nlmsg_seq, + NLM_F_MULTI) == 0) + dumped++; + } + rcu_read_unlock(); + } else { + rcu_read_lock(); + hash_for_each_rcu(ovpn->peers->by_id, bkt, peer, + hash_entry_id) { + /* skip already dumped peers that were dumped by + * previous invocations + */ + if (last_idx > 0) { + last_idx--; + continue; + } + + if (ovpn_nl_send_peer(skb, info, peer, + NETLINK_CB(cb->skb).portid, + cb->nlh->nlmsg_seq, + NLM_F_MULTI) < 0) + break; + + /* count peers being dumped during this invocation */ + dumped++; + } + rcu_read_unlock(); + } + +out: + netdev_put(ovpn->dev, &ovpn->dev_tracker); + + /* sum up peers dumped in this message, so that at the next invocation + * we can continue from where we left + */ + cb->args[1] += dumped; + return skb->len; } int ovpn_nl_peer_del_doit(struct sk_buff *skb, struct genl_info *info) { - return -EOPNOTSUPP; + struct nlattr *attrs[OVPN_A_PEER_MAX + 1]; + struct ovpn_priv *ovpn = info->user_ptr[0]; + struct ovpn_peer *peer; + u32 peer_id; + int ret; + + if (GENL_REQ_ATTR_CHECK(info, OVPN_A_PEER)) + return -EINVAL; + + ret = nla_parse_nested(attrs, OVPN_A_PEER_MAX, info->attrs[OVPN_A_PEER], + ovpn_peer_nl_policy, info->extack); + if (ret) + return ret; + + if (NL_REQ_ATTR_CHECK(info->extack, info->attrs[OVPN_A_PEER], attrs, + OVPN_A_PEER_ID)) + return -EINVAL; + + peer_id = nla_get_u32(attrs[OVPN_A_PEER_ID]); + peer = ovpn_peer_get_by_id(ovpn, peer_id); + if (!peer) { + NL_SET_ERR_MSG_FMT_MOD(info->extack, + "cannot find peer with id %u", peer_id); + return -ENOENT; + } + + netdev_dbg(ovpn->dev, "del peer %u\n", peer->id); + ret = ovpn_peer_del(peer, OVPN_DEL_PEER_REASON_USERSPACE); + ovpn_peer_put(peer); + + return ret; } int ovpn_nl_key_new_doit(struct sk_buff *skb, struct genl_info *info) diff --git a/drivers/net/ovpn/peer.c b/drivers/net/ovpn/peer.c index 2711ecdfdc767fd252e1cce9f6d2f37fa1535646..86caac747c6173672e4d3294639f60cd4138ac43 100644 --- a/drivers/net/ovpn/peer.c +++ b/drivers/net/ovpn/peer.c @@ -115,9 +115,9 @@ struct ovpn_peer *ovpn_peer_new(struct ovpn_priv *ovpn, u32 id) * * Return: 0 on success or a negative error code otherwise */ -static int ovpn_peer_reset_sockaddr(struct ovpn_peer *peer, - const struct sockaddr_storage *ss, - const u8 *local_ip) +int ovpn_peer_reset_sockaddr(struct ovpn_peer *peer, + const struct sockaddr_storage *ss, + const u8 *local_ip) { struct ovpn_bind *bind; size_t ip_len; @@ -311,7 +311,7 @@ static void ovpn_peer_release_rcu(struct rcu_head *head) * ovpn_peer_release - release peer private members * @peer: the peer to release */ -static void ovpn_peer_release(struct ovpn_peer *peer) +void ovpn_peer_release(struct ovpn_peer *peer) { ovpn_crypto_state_release(&peer->crypto); spin_lock_bh(&peer->lock); @@ -816,6 +816,37 @@ bool ovpn_peer_check_by_src(struct ovpn_priv *ovpn, struct sk_buff *skb, return match; } +void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer) +{ + struct hlist_nulls_head *nhead; + + lockdep_assert_held(&peer->ovpn->lock); + + /* rehashing makes sense only in multipeer mode */ + if (peer->ovpn->mode != OVPN_MODE_MP) + return; + + if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY)) { + /* remove potential old hashing */ + hlist_nulls_del_init_rcu(&peer->hash_entry_addr4); + + nhead = ovpn_get_hash_head(peer->ovpn->peers->by_vpn_addr, + &peer->vpn_addrs.ipv4, + sizeof(peer->vpn_addrs.ipv4)); + hlist_nulls_add_head_rcu(&peer->hash_entry_addr4, nhead); + } + + if (!ipv6_addr_any(&peer->vpn_addrs.ipv6)) { + /* remove potential old hashing */ + hlist_nulls_del_init_rcu(&peer->hash_entry_addr6); + + nhead = ovpn_get_hash_head(peer->ovpn->peers->by_vpn_addr, + &peer->vpn_addrs.ipv6, + sizeof(peer->vpn_addrs.ipv6)); + hlist_nulls_add_head_rcu(&peer->hash_entry_addr6, nhead); + } +} + /** * ovpn_peer_add_mp - add peer to related tables in a MP instance * @ovpn: the instance to add the peer to @@ -877,19 +908,7 @@ static int ovpn_peer_add_mp(struct ovpn_priv *ovpn, struct ovpn_peer *peer) ovpn_get_hash_head(ovpn->peers->by_id, &peer->id, sizeof(peer->id))); - if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY)) { - nhead = ovpn_get_hash_head(ovpn->peers->by_vpn_addr, - &peer->vpn_addrs.ipv4, - sizeof(peer->vpn_addrs.ipv4)); - hlist_nulls_add_head_rcu(&peer->hash_entry_addr4, nhead); - } - - if (!ipv6_addr_any(&peer->vpn_addrs.ipv6)) { - nhead = ovpn_get_hash_head(ovpn->peers->by_vpn_addr, - &peer->vpn_addrs.ipv6, - sizeof(peer->vpn_addrs.ipv6)); - hlist_nulls_add_head_rcu(&peer->hash_entry_addr6, nhead); - } + ovpn_peer_hash_vpn_ip(peer); out: spin_unlock_bh(&ovpn->lock); return ret; diff --git a/drivers/net/ovpn/peer.h b/drivers/net/ovpn/peer.h index 361068a0a40fd11cd4d3d347f83352ed3d46048e..b3354d6ee8f5d9527b01565d98fe824d5b28d22d 100644 --- a/drivers/net/ovpn/peer.h +++ b/drivers/net/ovpn/peer.h @@ -124,6 +124,7 @@ static inline bool ovpn_peer_hold(struct ovpn_peer *peer) return kref_get_unless_zero(&peer->refcount); } +void ovpn_peer_release(struct ovpn_peer *peer); void ovpn_peer_release_kref(struct kref *kref); /** @@ -147,6 +148,7 @@ struct ovpn_peer *ovpn_peer_get_by_transp_addr(struct ovpn_priv *ovpn, struct ovpn_peer *ovpn_peer_get_by_id(struct ovpn_priv *ovpn, u32 peer_id); struct ovpn_peer *ovpn_peer_get_by_dst(struct ovpn_priv *ovpn, struct sk_buff *skb); +void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer); bool ovpn_peer_check_by_src(struct ovpn_priv *ovpn, struct sk_buff *skb, struct ovpn_peer *peer); @@ -154,5 +156,8 @@ void ovpn_peer_keepalive_set(struct ovpn_peer *peer, u32 interval, u32 timeout); void ovpn_peer_keepalive_work(struct work_struct *work); void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb); +int ovpn_peer_reset_sockaddr(struct ovpn_peer *peer, + const struct sockaddr_storage *ss, + const u8 *local_ip); #endif /* _NET_OVPN_OVPNPEER_H_ */
This change introduces the netlink command needed to add, delete and retrieve/dump known peers. Userspace is expected to use these commands to handle known peer lifecycles. Signed-off-by: Antonio Quartulli <antonio@openvpn.net> --- drivers/net/ovpn/netlink.c | 631 ++++++++++++++++++++++++++++++++++++++++++++- drivers/net/ovpn/peer.c | 53 ++-- drivers/net/ovpn/peer.h | 5 + 3 files changed, 667 insertions(+), 22 deletions(-)