Re: Wireguard: can't remove multiple peers at once.
Hi,
> On 14/01/2021, at 8:33 PM, YASUOKA Masahiko wrote:
>
> Hi,
>
> On Thu, 14 Jan 2021 08:54:36 +0900
> Yuichiro NAITO wrote:
>> Does anybody please review my code?
>>
>> Yasuoka-san is my coleague of my work.
>> So, he is interested in this topic. That’s why I CCed this mail.
>> I don’t mean he is an reviewer.
>>
>>> 2021/01/12 11:27、Yuichiro NAITO のメール:
>>> I have set up multiple peers in a wg0 interface,
>>> and tried to remove more than one peers at once.
>>> Ifconfig(1) only removes the first peer.
>>>
>>> Command line was like following.
>>>
>>> ```
>>> # ifconfig wg0 -wgpeer -wgpeer -wgpeer
>>> ```
>>>
>>> Only was removed.
>>>
>>> I think next peer pointer isn't calculated in case of removing peer
>>> in sys/net/if_wg.c: wg_ioctl_set() function.
>>>
>>> I have tried following patch that can fix this problem.
>
> Yes, the diff seems good.
>
> I made the following whitespace change.
>
>> @@ -2333,6 +2333,11 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
>> *data)
>> }
>>
>> peer_p = (struct wg_peer_io *)aip_p;
>> +continue;
>> +next_peer:
>> +aip_p = _p->p_aips[0];
>> +aip_p += peer_o.p_aips_count;
>> +peer_p = (struct wg_peer_io *)aip_p;
>> }
>>
>> error:
>
> It seems we prefer putting goto labels at the beginning of the line.
>
>
> ok?
ok procter@
>
> Fix wg(4) ioctl to be able to handle multiple wgpeers.
> Diff from Yuichiro NAITO.
>
> Index: sys/net/if_wg.c
> ===
> RCS file: /cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 if_wg.c
> --- sys/net/if_wg.c 1 Sep 2020 19:06:59 - 1.14
> +++ sys/net/if_wg.c 14 Jan 2021 07:26:48 -
> @@ -2270,7 +2270,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
>
> /* Peer must have public key */
> if (!(peer_o.p_flags & WG_PEER_HAS_PUBLIC))
> - continue;
> + goto next_peer;
>
> /* 0 = latest protocol, 1 = this protocol */
> if (peer_o.p_protocol_version != 0) {
> @@ -2283,7 +2283,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
> /* Get local public and check that peer key doesn't match */
> if (noise_local_keys(>sc_local, public, NULL) == 0 &&
> bcmp(public, peer_o.p_public, WG_KEY_SIZE) == 0)
> - continue;
> + goto next_peer;
>
> /* Lookup peer, or create if it doesn't exist */
> if ((peer = wg_peer_lookup(sc, peer_o.p_public)) == NULL) {
> @@ -2291,7 +2291,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
>* Also, don't create a new one if we only want to
>* update. */
> if (peer_o.p_flags & (WG_PEER_REMOVE|WG_PEER_UPDATE))
> - continue;
> + goto next_peer;
>
> if ((peer = wg_peer_create(sc,
> peer_o.p_public)) == NULL) {
> @@ -2303,7 +2303,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
> /* Remove peer and continue if specified */
> if (peer_o.p_flags & WG_PEER_REMOVE) {
> wg_peer_destroy(peer);
> - continue;
> + goto next_peer;
> }
>
> if (peer_o.p_flags & WG_PEER_HAS_ENDPOINT)
> @@ -2332,6 +2332,11 @@ wg_ioctl_set(struct wg_softc *sc, struct
> aip_p++;
> }
>
> + peer_p = (struct wg_peer_io *)aip_p;
> + continue;
> +next_peer:
> + aip_p = _p->p_aips[0];
> + aip_p += peer_o.p_aips_count;
> peer_p = (struct wg_peer_io *)aip_p;
> }
>
>
Re: Wireguard: can't remove multiple peers at once.
Hi,
On Thu, 14 Jan 2021 08:54:36 +0900
Yuichiro NAITO wrote:
> Does anybody please review my code?
>
> Yasuoka-san is my coleague of my work.
> So, he is interested in this topic. That’s why I CCed this mail.
> I don’t mean he is an reviewer.
>
>> 2021/01/12 11:27、Yuichiro NAITO のメール:
>> I have set up multiple peers in a wg0 interface,
>> and tried to remove more than one peers at once.
>> Ifconfig(1) only removes the first peer.
>>
>> Command line was like following.
>>
>> ```
>> # ifconfig wg0 -wgpeer -wgpeer -wgpeer
>> ```
>>
>> Only was removed.
>>
>> I think next peer pointer isn't calculated in case of removing peer
>> in sys/net/if_wg.c: wg_ioctl_set() function.
>>
>> I have tried following patch that can fix this problem.
Yes, the diff seems good.
I made the following whitespace change.
> @@ -2333,6 +2333,11 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
> *data)
> }
>
> peer_p = (struct wg_peer_io *)aip_p;
> + continue;
> + next_peer:
> + aip_p = _p->p_aips[0];
> + aip_p += peer_o.p_aips_count;
> + peer_p = (struct wg_peer_io *)aip_p;
> }
>
> error:
It seems we prefer putting goto labels at the beginning of the line.
ok?
Fix wg(4) ioctl to be able to handle multiple wgpeers.
Diff from Yuichiro NAITO.
Index: sys/net/if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.14
diff -u -p -r1.14 if_wg.c
--- sys/net/if_wg.c 1 Sep 2020 19:06:59 - 1.14
+++ sys/net/if_wg.c 14 Jan 2021 07:26:48 -
@@ -2270,7 +2270,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
/* Peer must have public key */
if (!(peer_o.p_flags & WG_PEER_HAS_PUBLIC))
- continue;
+ goto next_peer;
/* 0 = latest protocol, 1 = this protocol */
if (peer_o.p_protocol_version != 0) {
@@ -2283,7 +2283,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
/* Get local public and check that peer key doesn't match */
if (noise_local_keys(>sc_local, public, NULL) == 0 &&
bcmp(public, peer_o.p_public, WG_KEY_SIZE) == 0)
- continue;
+ goto next_peer;
/* Lookup peer, or create if it doesn't exist */
if ((peer = wg_peer_lookup(sc, peer_o.p_public)) == NULL) {
@@ -2291,7 +2291,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
* Also, don't create a new one if we only want to
* update. */
if (peer_o.p_flags & (WG_PEER_REMOVE|WG_PEER_UPDATE))
- continue;
+ goto next_peer;
if ((peer = wg_peer_create(sc,
peer_o.p_public)) == NULL) {
@@ -2303,7 +2303,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
/* Remove peer and continue if specified */
if (peer_o.p_flags & WG_PEER_REMOVE) {
wg_peer_destroy(peer);
- continue;
+ goto next_peer;
}
if (peer_o.p_flags & WG_PEER_HAS_ENDPOINT)
@@ -2332,6 +2332,11 @@ wg_ioctl_set(struct wg_softc *sc, struct
aip_p++;
}
+ peer_p = (struct wg_peer_io *)aip_p;
+ continue;
+next_peer:
+ aip_p = _p->p_aips[0];
+ aip_p += peer_o.p_aips_count;
peer_p = (struct wg_peer_io *)aip_p;
}
Re: Wireguard: can't remove multiple peers at once.
Does anybody please review my code?
Yasuoka-san is my coleague of my work.
So, he is interested in this topic. That’s why I CCed this mail.
I don’t mean he is an reviewer.
> 2021/01/12 11:27、Yuichiro NAITO のメール:
>
>
> Hi.
>
> I have set up multiple peers in a wg0 interface,
> and tried to remove more than one peers at once.
> Ifconfig(1) only removes the first peer.
>
> Command line was like following.
>
> ```
> # ifconfig wg0 -wgpeer -wgpeer -wgpeer
> ```
>
> Only was removed.
>
> I think next peer pointer isn't calculated in case of removing peer
> in sys/net/if_wg.c: wg_ioctl_set() function.
>
> I have tried following patch that can fix this problem.
> Is it OK?
>
> diff --git a/sys/net/if_wg.c b/sys/net/if_wg.c
> index c534f966363..c21f883269f 100644
> --- a/sys/net/if_wg.c
> +++ b/sys/net/if_wg.c
> @@ -2270,7 +2270,7 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
> *data)
>
> /* Peer must have public key */
> if (!(peer_o.p_flags & WG_PEER_HAS_PUBLIC))
> - continue;
> + goto next_peer;
>
> /* 0 = latest protocol, 1 = this protocol */
> if (peer_o.p_protocol_version != 0) {
> @@ -2283,7 +2283,7 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
> *data)
> /* Get local public and check that peer key doesn't match */
> if (noise_local_keys(>sc_local, public, NULL) == 0 &&
> bcmp(public, peer_o.p_public, WG_KEY_SIZE) == 0)
> - continue;
> + goto next_peer;
>
> /* Lookup peer, or create if it doesn't exist */
> if ((peer = wg_peer_lookup(sc, peer_o.p_public)) == NULL) {
> @@ -2291,7 +2291,7 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
> *data)
>* Also, don't create a new one if we only want to
>* update. */
> if (peer_o.p_flags & (WG_PEER_REMOVE|WG_PEER_UPDATE))
> - continue;
> + goto next_peer;
>
> if ((peer = wg_peer_create(sc,
> peer_o.p_public)) == NULL) {
> @@ -2303,7 +2303,7 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
> *data)
> /* Remove peer and continue if specified */
> if (peer_o.p_flags & WG_PEER_REMOVE) {
> wg_peer_destroy(peer);
> - continue;
> + goto next_peer;
> }
>
> if (peer_o.p_flags & WG_PEER_HAS_ENDPOINT)
> @@ -2333,6 +2333,11 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
> *data)
> }
>
> peer_p = (struct wg_peer_io *)aip_p;
> + continue;
> + next_peer:
> + aip_p = _p->p_aips[0];
> + aip_p += peer_o.p_aips_count;
> + peer_p = (struct wg_peer_io *)aip_p;
> }
>
> error:
>
> —
> Yuichiro NAITO
> naito.yuich...@gmail.com
>
—
Yuichiro NAITO
naito.yuich...@gmail.com
Wireguard: can't remove multiple peers at once.
Hi.
I have set up multiple peers in a wg0 interface,
and tried to remove more than one peers at once.
Ifconfig(1) only removes the first peer.
Command line was like following.
```
# ifconfig wg0 -wgpeer -wgpeer -wgpeer
```
Only was removed.
I think next peer pointer isn't calculated in case of removing peer
in sys/net/if_wg.c: wg_ioctl_set() function.
I have tried following patch that can fix this problem.
Is it OK?
diff --git a/sys/net/if_wg.c b/sys/net/if_wg.c
index c534f966363..c21f883269f 100644
--- a/sys/net/if_wg.c
+++ b/sys/net/if_wg.c
@@ -2270,7 +2270,7 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io *data)
/* Peer must have public key */
if (!(peer_o.p_flags & WG_PEER_HAS_PUBLIC))
- continue;
+ goto next_peer;
/* 0 = latest protocol, 1 = this protocol */
if (peer_o.p_protocol_version != 0) {
@@ -2283,7 +2283,7 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io *data)
/* Get local public and check that peer key doesn't match */
if (noise_local_keys(>sc_local, public, NULL) == 0 &&
bcmp(public, peer_o.p_public, WG_KEY_SIZE) == 0)
- continue;
+ goto next_peer;
/* Lookup peer, or create if it doesn't exist */
if ((peer = wg_peer_lookup(sc, peer_o.p_public)) == NULL) {
@@ -2291,7 +2291,7 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io *data)
* Also, don't create a new one if we only want to
* update. */
if (peer_o.p_flags & (WG_PEER_REMOVE|WG_PEER_UPDATE))
- continue;
+ goto next_peer;
if ((peer = wg_peer_create(sc,
peer_o.p_public)) == NULL) {
@@ -2303,7 +2303,7 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io *data)
/* Remove peer and continue if specified */
if (peer_o.p_flags & WG_PEER_REMOVE) {
wg_peer_destroy(peer);
- continue;
+ goto next_peer;
}
if (peer_o.p_flags & WG_PEER_HAS_ENDPOINT)
@@ -2333,6 +2333,11 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
*data)
}
peer_p = (struct wg_peer_io *)aip_p;
+ continue;
+ next_peer:
+ aip_p = _p->p_aips[0];
+ aip_p += peer_o.p_aips_count;
+ peer_p = (struct wg_peer_io *)aip_p;
}
error:
—
Yuichiro NAITO
naito.yuich...@gmail.com
