Hi,
On Thu, 14 Jan 2021 08:54:36 +0900
Yuichiro NAITO <[email protected]> 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 <[email protected]>のメール:
>> 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 <keyA> -wgpeer <keyB> -wgpeer <keyC>
>> ```
>>
>> Only <keyA> 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 = &peer_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 -0000 1.14
+++ sys/net/if_wg.c 14 Jan 2021 07:26:48 -0000
@@ -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->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 = &peer_p->p_aips[0];
+ aip_p += peer_o.p_aips_count;
peer_p = (struct wg_peer_io *)aip_p;
}
