Re: ifconfig description for wireguard peers
On Wed, May 24, 2023 at 11:43:03PM +0200, Hrvoje Popovski wrote: > On 23.5.2023. 21:13, Klemens Nanni wrote: > > On Sat, Jan 14, 2023 at 02:28:27PM +, Stuart Henderson wrote: > >> On 2023/01/12 04:49, Mikolaj Kucharski wrote: > >>> Hi, > >>> > >>> Is there anything else which I can do, to help this diff reviwed and > >>> increase the chance of getting in? > >>> > >>> Thread at https://marc.info/?t=16347829861=1=2 > >>> > >>> Last version of the diff at > >>> https://marc.info/?l=openbsd-tech=167185582521873=mbox > >> Inlining that for a few comments, otherwise it's ok sthen > > wgdescr[iption] would be consistent with the existing descr[iption]. > > At least my keep typing the trailing "r"... > > > > Then '-wgdescr' and 'wgdescr ""' work and are implemented exactly like > > te inteface description equivalents. > > > > I could use this now in a new VPN setup, so here's a polished diff, > > with the above, missing ifconfig.8 bits written and other nits inline. > > > > As Theo suggested, I'd drop the wg.4 and leave it to ifconfig.8 proper. > > > > Feedback? > > > > Either way, net/wireguard-tools needs a bump/rebuild. > > > > Hi, > > this would be nice features when having wg server with lots of wgpeers. > > I've tried this diff ans it's working as expected. > > > Thank you Mikolaj and kn@ > I just rebased the diff. Thanks Klemens for pushing this forward. I would like to point out, that initial version was written by Noah Meier https://marc.info/?l=openbsd-tech=163478285129091=2 -- Regards, Mikolaj
Re: ifconfig description for wireguard peers
On Wed, May 31, 2023 at 02:07:17PM +, Klemens Nanni wrote:
> On Wed, May 31, 2023 at 10:27:13AM +0200, Claudio Jeker wrote:
> > On Tue, May 30, 2023 at 11:56:01PM +, Klemens Nanni wrote:
> > > On Tue, May 23, 2023 at 07:13:28PM +, Klemens Nanni wrote:
> > > > On Sat, Jan 14, 2023 at 02:28:27PM +, Stuart Henderson wrote:
> > > > > On 2023/01/12 04:49, Mikolaj Kucharski wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Is there anything else which I can do, to help this diff reviwed and
> > > > > > increase the chance of getting in?
> > > > > >
> > > > > > Thread at https://marc.info/?t=16347829861=1=2
> > > > > >
> > > > > > Last version of the diff at
> > > > > > https://marc.info/?l=openbsd-tech=167185582521873=mbox
> > > > >
> > > > > Inlining that for a few comments, otherwise it's ok sthen
> > > >
> > > > wgdescr[iption] would be consistent with the existing descr[iption].
> > > > At least my keep typing the trailing "r"...
> > > >
> > > > Then '-wgdescr' and 'wgdescr ""' work and are implemented exactly like
> > > > te inteface description equivalents.
> > > >
> > > > I could use this now in a new VPN setup, so here's a polished diff,
> > > > with the above, missing ifconfig.8 bits written and other nits inline.
> > > >
> > > > As Theo suggested, I'd drop the wg.4 and leave it to ifconfig.8 proper.
> > > >
> > > > Feedback?
> > > >
> > > > Either way, net/wireguard-tools needs a bump/rebuild.
> > >
> > > Updated diff at the end, grabbing the new per-description mutex also for
> > > reading, not just writing it.
> > >
> > > I did not run into an issue with the first two diffs, but other peer
> > > properties have their own mutex as well and they're consistently used
> > > for all accesses, as I'd expect, so protect new description properly.
> > >
> > > Also fixed ifconfig.8's wireguard synopsis bits.
> > >
> > > Anyone?
> >
> > This mutex makes very little sense to me.
> > Access to this field is already serialized by the sc->sc_lock rwlock
> > so there is no need for this mutex.
>
> Right, description is only read/written through the ioctl, whereas other
> stuff under its own mutex, e.g. peer rx/tx counters, need to serialise
> packet processing/actual counting with reading out from the ioctl layer.
>
> I did not look close enough, but lazily grabbed the original diff's mutex
> in all places.
>
> Hopefully the last diff.
Two comments below. With them fixed OK claudio@
> Index: sys/net/if_wg.c
> ===
> RCS file: /cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 if_wg.c
> --- sys/net/if_wg.c 30 May 2023 08:30:01 - 1.27
> +++ sys/net/if_wg.c 31 May 2023 12:41:50 -
> @@ -221,6 +221,8 @@ struct wg_peer {
>
> SLIST_ENTRY(wg_peer) p_start_list;
> int p_start_onlist;
> +
> + char p_description[IFDESCRSIZE];
> };
>
> struct wg_softc {
> @@ -407,6 +409,8 @@ wg_peer_create(struct wg_softc *sc, uint
> peer->p_counters_tx = 0;
> peer->p_counters_rx = 0;
>
> + strlcpy(peer->p_description, "", IFDESCRSIZE);
> +
> mtx_init(>p_endpoint_mtx, IPL_NET);
> bzero(>p_endpoint, sizeof(peer->p_endpoint));
>
> @@ -581,6 +585,11 @@ wg_peer_counters_add(struct wg_peer *pee
> mtx_leave(>p_counters_mtx);
> }
>
> +void
> +wg_peer_get_description(struct wg_peer *peer, char *description)
> +{
> +}
> +
Empty function does nothing.
> int
> wg_aip_add(struct wg_softc *sc, struct wg_peer *peer, struct wg_aip_io *d)
> {
> @@ -2320,6 +2329,10 @@ wg_ioctl_set(struct wg_softc *sc, struct
> }
> }
>
> + if (peer_o.p_flags & WG_PEER_SET_DESCRIPTION)
> + strlcpy(peer->p_description, peer_o.p_description,
> + IFDESCRSIZE);
> +
> aip_p = _p->p_aips[0];
> for (j = 0; j < peer_o.p_aips_count; j++) {
> if ((ret = copyin(aip_p, _o, sizeof(aip_o))) != 0)
> @@ -2429,6 +2442,8 @@ wg_ioctl_get(struct wg_softc *sc, struct
> aip_count++;
> }
> peer_o.p_aips_count = aip_count;
> +
> + strlcpy(peer_o.p_description, peer->p_description, IFDESCRSIZE);
>
> if ((ret = copyout(_o, peer_p, sizeof(peer_o))) != 0)
> goto unlock_and_ret_size;
> Index: sys/net/if_wg.h
> ===
> RCS file: /cvs/src/sys/net/if_wg.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 if_wg.h
> --- sys/net/if_wg.h 22 Jun 2020 12:20:44 - 1.4
> +++ sys/net/if_wg.h 29 May 2023 22:20:02 -
> @@ -61,6 +61,7 @@ struct wg_aip_io {
> #define WG_PEER_REPLACE_AIPS (1 << 4)
> #define WG_PEER_REMOVE (1 << 5)
> #define WG_PEER_UPDATE (1 << 6)
> +#define WG_PEER_SET_DESCRIPTION
Re: ifconfig description for wireguard peers
On Wed, May 31, 2023 at 10:27:13AM +0200, Claudio Jeker wrote:
> On Tue, May 30, 2023 at 11:56:01PM +, Klemens Nanni wrote:
> > On Tue, May 23, 2023 at 07:13:28PM +, Klemens Nanni wrote:
> > > On Sat, Jan 14, 2023 at 02:28:27PM +, Stuart Henderson wrote:
> > > > On 2023/01/12 04:49, Mikolaj Kucharski wrote:
> > > > > Hi,
> > > > >
> > > > > Is there anything else which I can do, to help this diff reviwed and
> > > > > increase the chance of getting in?
> > > > >
> > > > > Thread at https://marc.info/?t=16347829861=1=2
> > > > >
> > > > > Last version of the diff at
> > > > > https://marc.info/?l=openbsd-tech=167185582521873=mbox
> > > >
> > > > Inlining that for a few comments, otherwise it's ok sthen
> > >
> > > wgdescr[iption] would be consistent with the existing descr[iption].
> > > At least my keep typing the trailing "r"...
> > >
> > > Then '-wgdescr' and 'wgdescr ""' work and are implemented exactly like
> > > te inteface description equivalents.
> > >
> > > I could use this now in a new VPN setup, so here's a polished diff,
> > > with the above, missing ifconfig.8 bits written and other nits inline.
> > >
> > > As Theo suggested, I'd drop the wg.4 and leave it to ifconfig.8 proper.
> > >
> > > Feedback?
> > >
> > > Either way, net/wireguard-tools needs a bump/rebuild.
> >
> > Updated diff at the end, grabbing the new per-description mutex also for
> > reading, not just writing it.
> >
> > I did not run into an issue with the first two diffs, but other peer
> > properties have their own mutex as well and they're consistently used
> > for all accesses, as I'd expect, so protect new description properly.
> >
> > Also fixed ifconfig.8's wireguard synopsis bits.
> >
> > Anyone?
>
> This mutex makes very little sense to me.
> Access to this field is already serialized by the sc->sc_lock rwlock
> so there is no need for this mutex.
Right, description is only read/written through the ioctl, whereas other
stuff under its own mutex, e.g. peer rx/tx counters, need to serialise
packet processing/actual counting with reading out from the ioctl layer.
I did not look close enough, but lazily grabbed the original diff's mutex
in all places.
Hopefully the last diff.
Index: sys/net/if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.27
diff -u -p -r1.27 if_wg.c
--- sys/net/if_wg.c 30 May 2023 08:30:01 - 1.27
+++ sys/net/if_wg.c 31 May 2023 12:41:50 -
@@ -221,6 +221,8 @@ struct wg_peer {
SLIST_ENTRY(wg_peer) p_start_list;
int p_start_onlist;
+
+ char p_description[IFDESCRSIZE];
};
struct wg_softc {
@@ -407,6 +409,8 @@ wg_peer_create(struct wg_softc *sc, uint
peer->p_counters_tx = 0;
peer->p_counters_rx = 0;
+ strlcpy(peer->p_description, "", IFDESCRSIZE);
+
mtx_init(>p_endpoint_mtx, IPL_NET);
bzero(>p_endpoint, sizeof(peer->p_endpoint));
@@ -581,6 +585,11 @@ wg_peer_counters_add(struct wg_peer *pee
mtx_leave(>p_counters_mtx);
}
+void
+wg_peer_get_description(struct wg_peer *peer, char *description)
+{
+}
+
int
wg_aip_add(struct wg_softc *sc, struct wg_peer *peer, struct wg_aip_io *d)
{
@@ -2320,6 +2329,10 @@ wg_ioctl_set(struct wg_softc *sc, struct
}
}
+ if (peer_o.p_flags & WG_PEER_SET_DESCRIPTION)
+ strlcpy(peer->p_description, peer_o.p_description,
+ IFDESCRSIZE);
+
aip_p = _p->p_aips[0];
for (j = 0; j < peer_o.p_aips_count; j++) {
if ((ret = copyin(aip_p, _o, sizeof(aip_o))) != 0)
@@ -2429,6 +2442,8 @@ wg_ioctl_get(struct wg_softc *sc, struct
aip_count++;
}
peer_o.p_aips_count = aip_count;
+
+ strlcpy(peer_o.p_description, peer->p_description, IFDESCRSIZE);
if ((ret = copyout(_o, peer_p, sizeof(peer_o))) != 0)
goto unlock_and_ret_size;
Index: sys/net/if_wg.h
===
RCS file: /cvs/src/sys/net/if_wg.h,v
retrieving revision 1.4
diff -u -p -r1.4 if_wg.h
--- sys/net/if_wg.h 22 Jun 2020 12:20:44 - 1.4
+++ sys/net/if_wg.h 29 May 2023 22:20:02 -
@@ -61,6 +61,7 @@ struct wg_aip_io {
#define WG_PEER_REPLACE_AIPS (1 << 4)
#define WG_PEER_REMOVE (1 << 5)
#define WG_PEER_UPDATE (1 << 6)
+#define WG_PEER_SET_DESCRIPTION(1 << 7)
#define p_sa p_endpoint.sa_sa
#define p_sin p_endpoint.sa_sin
@@ -80,6 +81,7 @@ struct wg_peer_io {
uint64_tp_txbytes;
uint64_tp_rxbytes;
struct timespec p_last_handshake; /* nanotime */
+ char
Re: ifconfig description for wireguard peers
On Tue, May 30, 2023 at 11:56:01PM +, Klemens Nanni wrote:
> On Tue, May 23, 2023 at 07:13:28PM +, Klemens Nanni wrote:
> > On Sat, Jan 14, 2023 at 02:28:27PM +, Stuart Henderson wrote:
> > > On 2023/01/12 04:49, Mikolaj Kucharski wrote:
> > > > Hi,
> > > >
> > > > Is there anything else which I can do, to help this diff reviwed and
> > > > increase the chance of getting in?
> > > >
> > > > Thread at https://marc.info/?t=16347829861=1=2
> > > >
> > > > Last version of the diff at
> > > > https://marc.info/?l=openbsd-tech=167185582521873=mbox
> > >
> > > Inlining that for a few comments, otherwise it's ok sthen
> >
> > wgdescr[iption] would be consistent with the existing descr[iption].
> > At least my keep typing the trailing "r"...
> >
> > Then '-wgdescr' and 'wgdescr ""' work and are implemented exactly like
> > te inteface description equivalents.
> >
> > I could use this now in a new VPN setup, so here's a polished diff,
> > with the above, missing ifconfig.8 bits written and other nits inline.
> >
> > As Theo suggested, I'd drop the wg.4 and leave it to ifconfig.8 proper.
> >
> > Feedback?
> >
> > Either way, net/wireguard-tools needs a bump/rebuild.
>
> Updated diff at the end, grabbing the new per-description mutex also for
> reading, not just writing it.
>
> I did not run into an issue with the first two diffs, but other peer
> properties have their own mutex as well and they're consistently used
> for all accesses, as I'd expect, so protect new description properly.
>
> Also fixed ifconfig.8's wireguard synopsis bits.
>
> Anyone?
This mutex makes very little sense to me.
Access to this field is already serialized by the sc->sc_lock rwlock
so there is no need for this mutex.
> Index: sys/net/if_wg.c
> ===
> RCS file: /cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 if_wg.c
> --- sys/net/if_wg.c 30 May 2023 08:30:01 - 1.27
> +++ sys/net/if_wg.c 30 May 2023 15:37:41 -
> @@ -221,6 +221,9 @@ struct wg_peer {
>
> SLIST_ENTRY(wg_peer) p_start_list;
> int p_start_onlist;
> +
> + struct mutex p_description_mtx;
> + char p_description[IFDESCRSIZE];
> };
>
> struct wg_softc {
> @@ -275,6 +278,8 @@ int wg_peer_get_sockaddr(struct wg_peer
> void wg_peer_clear_src(struct wg_peer *);
> void wg_peer_get_endpoint(struct wg_peer *, struct wg_endpoint *);
> void wg_peer_counters_add(struct wg_peer *, uint64_t, uint64_t);
> +void wg_peer_set_description(struct wg_peer *, const char *);
> +void wg_peer_get_description(struct wg_peer *, char *);
>
> int wg_aip_add(struct wg_softc *, struct wg_peer *, struct wg_aip_io *);
> struct wg_peer *
> @@ -407,6 +412,9 @@ wg_peer_create(struct wg_softc *sc, uint
> peer->p_counters_tx = 0;
> peer->p_counters_rx = 0;
>
> + mtx_init(>p_description_mtx, IPL_NET);
> + strlcpy(peer->p_description, "", IFDESCRSIZE);
> +
> mtx_init(>p_endpoint_mtx, IPL_NET);
> bzero(>p_endpoint, sizeof(peer->p_endpoint));
>
> @@ -581,6 +589,22 @@ wg_peer_counters_add(struct wg_peer *pee
> mtx_leave(>p_counters_mtx);
> }
>
> +void
> +wg_peer_set_description(struct wg_peer *peer, const char *description)
> +{
> + mtx_enter(>p_description_mtx);
> + strlcpy(peer->p_description, description, IFDESCRSIZE);
> + mtx_leave(>p_description_mtx);
> +}
> +
> +void
> +wg_peer_get_description(struct wg_peer *peer, char *description)
> +{
> + mtx_enter(>p_description_mtx);
> + strlcpy(description, peer->p_description, IFDESCRSIZE);
> + mtx_leave(>p_description_mtx);
> +}
> +
> int
> wg_aip_add(struct wg_softc *sc, struct wg_peer *peer, struct wg_aip_io *d)
> {
> @@ -2320,6 +2344,9 @@ wg_ioctl_set(struct wg_softc *sc, struct
> }
> }
>
> + if (peer_o.p_flags & WG_PEER_SET_DESCRIPTION)
> + wg_peer_set_description(peer, peer_o.p_description);
> +
> aip_p = _p->p_aips[0];
> for (j = 0; j < peer_o.p_aips_count; j++) {
> if ((ret = copyin(aip_p, _o, sizeof(aip_o))) != 0)
> @@ -2429,6 +2456,8 @@ wg_ioctl_get(struct wg_softc *sc, struct
> aip_count++;
> }
> peer_o.p_aips_count = aip_count;
> +
> + wg_peer_get_description(peer, peer_o.p_description);
>
> if ((ret = copyout(_o, peer_p, sizeof(peer_o))) != 0)
> goto unlock_and_ret_size;
--
:wq Claudio
Re: ifconfig description for wireguard peers
On Tue, May 23, 2023 at 07:13:28PM +, Klemens Nanni wrote:
> On Sat, Jan 14, 2023 at 02:28:27PM +, Stuart Henderson wrote:
> > On 2023/01/12 04:49, Mikolaj Kucharski wrote:
> > > Hi,
> > >
> > > Is there anything else which I can do, to help this diff reviwed and
> > > increase the chance of getting in?
> > >
> > > Thread at https://marc.info/?t=16347829861=1=2
> > >
> > > Last version of the diff at
> > > https://marc.info/?l=openbsd-tech=167185582521873=mbox
> >
> > Inlining that for a few comments, otherwise it's ok sthen
>
> wgdescr[iption] would be consistent with the existing descr[iption].
> At least my keep typing the trailing "r"...
>
> Then '-wgdescr' and 'wgdescr ""' work and are implemented exactly like
> te inteface description equivalents.
>
> I could use this now in a new VPN setup, so here's a polished diff,
> with the above, missing ifconfig.8 bits written and other nits inline.
>
> As Theo suggested, I'd drop the wg.4 and leave it to ifconfig.8 proper.
>
> Feedback?
>
> Either way, net/wireguard-tools needs a bump/rebuild.
Updated diff at the end, grabbing the new per-description mutex also for
reading, not just writing it.
I did not run into an issue with the first two diffs, but other peer
properties have their own mutex as well and they're consistently used
for all accesses, as I'd expect, so protect new description properly.
Also fixed ifconfig.8's wireguard synopsis bits.
Anyone?
>
> >
> > : Index: sbin/ifconfig/ifconfig.c
> > : ===
> > : RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> > : retrieving revision 1.460
> > : diff -u -p -u -r1.460 ifconfig.c
> > : --- sbin/ifconfig/ifconfig.c 18 Dec 2022 18:56:38 - 1.460
> > : +++ sbin/ifconfig/ifconfig.c 24 Dec 2022 00:49:05 -
> > : @@ -355,12 +355,14 @@ voidsetwgpeerep(const char *, const cha
> > : void setwgpeeraip(const char *, int);
> > : void setwgpeerpsk(const char *, int);
> > : void setwgpeerpka(const char *, int);
> > : +void setwgpeerdesc(const char *, int);
> > : void setwgport(const char *, int);
> > : void setwgkey(const char *, int);
> > : void setwgrtable(const char *, int);
> > :
> > : void unsetwgpeer(const char *, int);
> > : void unsetwgpeerpsk(const char *, int);
> > : +void unsetwgpeerdesc(const char *, int);
> > : void unsetwgpeerall(const char *, int);
> > :
> > : void wg_status(int);
> > : @@ -623,11 +625,13 @@ const structcmd {
> > : { "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
> > : { "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
> > : { "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
> > : + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
> > : { "wgport", NEXTARG,A_WIREGUARD,setwgport},
> > : { "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
> > : { "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
> > : { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
> > : { "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
> > : + { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
> > : { "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
> > :
> > : #else /* SMALL */
> > : @@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
> > : }
> > :
> > : void
> > : +setwgpeerdesc(const char *wgdesc, int param)
> > : +{
> > : + if (wg_peer == NULL)
> > : + errx(1, "wgdesc: wgpeer not set");
> > : + if (strlen(wgdesc))
> > : + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
> > : + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> > : +}
> > : +
> > : +void
> > : setwgport(const char *port, int param)
> > : {
> > : const char *errmsg = NULL;
> > : @@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
> > : }
> > :
> > : void
> > : +unsetwgpeerdesc(const char *value, int param)
> > : +{
> > : + if (wg_peer == NULL)
> > : + errx(1, "wgdesc: wgpeer not set");
> > : + strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
> > : + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> >
> > I was a bit confused by this at first (wondering if it should use
> > "&= ~WG_PEER_SET_DESCRIPTION"). I understand it now but I think that
> > a different name would make it clearer. Maybe WG_PEER_UPDATE_DESCR?
>
> This matches the [-]descr[iption] implementation, which always sets it,
> either to the user's value or to the empty string.
>
> Set and update thus seem equivalent to me, I'm fine with flag name and
> handling as-is.
>
> >
> > : +}
> > : +
> > : +void
> > : unsetwgpeerall(const char *value, int param)
> > : {
> > : ensurewginterface();
> > : @@ -5961,6 +5984,9 @@ wg_status(int ifaliases)
> > : b64_ntop(wg_peer->p_public, WG_KEY_LEN,
> > : key, sizeof(key));
> > :
Re: ifconfig description for wireguard peers
On 23.5.2023. 21:13, Klemens Nanni wrote: > On Sat, Jan 14, 2023 at 02:28:27PM +, Stuart Henderson wrote: >> On 2023/01/12 04:49, Mikolaj Kucharski wrote: >>> Hi, >>> >>> Is there anything else which I can do, to help this diff reviwed and >>> increase the chance of getting in? >>> >>> Thread at https://marc.info/?t=16347829861=1=2 >>> >>> Last version of the diff at >>> https://marc.info/?l=openbsd-tech=167185582521873=mbox >> Inlining that for a few comments, otherwise it's ok sthen > wgdescr[iption] would be consistent with the existing descr[iption]. > At least my keep typing the trailing "r"... > > Then '-wgdescr' and 'wgdescr ""' work and are implemented exactly like > te inteface description equivalents. > > I could use this now in a new VPN setup, so here's a polished diff, > with the above, missing ifconfig.8 bits written and other nits inline. > > As Theo suggested, I'd drop the wg.4 and leave it to ifconfig.8 proper. > > Feedback? > > Either way, net/wireguard-tools needs a bump/rebuild. > Hi, this would be nice features when having wg server with lots of wgpeers. I've tried this diff ans it's working as expected. Thank you Mikolaj and kn@
Re: ifconfig description for wireguard peers
On Sat, Jan 14, 2023 at 02:28:27PM +, Stuart Henderson wrote:
> On 2023/01/12 04:49, Mikolaj Kucharski wrote:
> > Hi,
> >
> > Is there anything else which I can do, to help this diff reviwed and
> > increase the chance of getting in?
> >
> > Thread at https://marc.info/?t=16347829861=1=2
> >
> > Last version of the diff at
> > https://marc.info/?l=openbsd-tech=167185582521873=mbox
>
> Inlining that for a few comments, otherwise it's ok sthen
wgdescr[iption] would be consistent with the existing descr[iption].
At least my keep typing the trailing "r"...
Then '-wgdescr' and 'wgdescr ""' work and are implemented exactly like
te inteface description equivalents.
I could use this now in a new VPN setup, so here's a polished diff,
with the above, missing ifconfig.8 bits written and other nits inline.
As Theo suggested, I'd drop the wg.4 and leave it to ifconfig.8 proper.
Feedback?
Either way, net/wireguard-tools needs a bump/rebuild.
>
> : Index: sbin/ifconfig/ifconfig.c
> : ===
> : RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> : retrieving revision 1.460
> : diff -u -p -u -r1.460 ifconfig.c
> : --- sbin/ifconfig/ifconfig.c18 Dec 2022 18:56:38 - 1.460
> : +++ sbin/ifconfig/ifconfig.c24 Dec 2022 00:49:05 -
> : @@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
> : void setwgpeeraip(const char *, int);
> : void setwgpeerpsk(const char *, int);
> : void setwgpeerpka(const char *, int);
> : +void setwgpeerdesc(const char *, int);
> : void setwgport(const char *, int);
> : void setwgkey(const char *, int);
> : void setwgrtable(const char *, int);
> :
> : void unsetwgpeer(const char *, int);
> : void unsetwgpeerpsk(const char *, int);
> : +void unsetwgpeerdesc(const char *, int);
> : void unsetwgpeerall(const char *, int);
> :
> : void wg_status(int);
> : @@ -623,11 +625,13 @@ const struct cmd {
> : { "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
> : { "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
> : { "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
> : + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
> : { "wgport", NEXTARG,A_WIREGUARD,setwgport},
> : { "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
> : { "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
> : { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
> : { "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
> : + { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
> : { "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
> :
> : #else /* SMALL */
> : @@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
> : }
> :
> : void
> : +setwgpeerdesc(const char *wgdesc, int param)
> : +{
> : + if (wg_peer == NULL)
> : + errx(1, "wgdesc: wgpeer not set");
> : + if (strlen(wgdesc))
> : + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
> : + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> : +}
> : +
> : +void
> : setwgport(const char *port, int param)
> : {
> : const char *errmsg = NULL;
> : @@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
> : }
> :
> : void
> : +unsetwgpeerdesc(const char *value, int param)
> : +{
> : + if (wg_peer == NULL)
> : + errx(1, "wgdesc: wgpeer not set");
> : + strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
> : + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
>
> I was a bit confused by this at first (wondering if it should use
> "&= ~WG_PEER_SET_DESCRIPTION"). I understand it now but I think that
> a different name would make it clearer. Maybe WG_PEER_UPDATE_DESCR?
This matches the [-]descr[iption] implementation, which always sets it,
either to the user's value or to the empty string.
Set and update thus seem equivalent to me, I'm fine with flag name and
handling as-is.
>
> : +}
> : +
> : +void
> : unsetwgpeerall(const char *value, int param)
> : {
> : ensurewginterface();
> : @@ -5961,6 +5984,9 @@ wg_status(int ifaliases)
> : b64_ntop(wg_peer->p_public, WG_KEY_LEN,
> : key, sizeof(key));
> : printf("\twgpeer %s\n", key);
> : +
> : + if (strlen(wg_peer->p_description))
> : + printf("\t\twgdesc %s\n",
> wg_peer->p_description);
I made this a) print a double-colon and b) always say "wgdescr" without
"iption" such that any potential script parsing ifconfig output for
"description:" won't suddenly match this as well.
> :
> : if (wg_peer->p_flags & WG_PEER_HAS_PSK)
> : printf("\t\twgpsk (present)\n");
> : Index: share/man/man4/wg.4
... dropped.
> : Index: sys/net/if_wg.c
> :
Re: ifconfig description for wireguard peers
On 15.5.2023. 9:47, Hrvoje Popovski wrote: > On 12.1.2023. 5:49, Mikolaj Kucharski wrote: >> Hi, >> >> Is there anything else which I can do, to help this diff reviwed and >> increase the chance of getting in? >> >> Thread at https://marc.info/?t=16347829861=1=2 >> >> Last version of the diff at >> https://marc.info/?l=openbsd-tech=167185582521873=mbox > > Hi, > > I've applied this diff and it's works as expected. wgdesc would be nice > features to have when having remote access server with wireguard. Hi, With this diff when executing wg in shell or wg showconf I'm getting core dump. maybe I did something wrong? Mikolaj did you get core dump with diff? "wg show" without this diff r620-1# wg show interface: wg0 public key: 123= private key: (hidden) listening port: 12345 peer: 111= preshared key: (hidden) allowed ips: 10.123.123.2/32 "wg show" with this diff smc4# wg show Segmentation fault (core dumped) smc4# gdb wg wg.core GNU gdb 6.3 Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "amd64-unknown-openbsd7.3"...(no debugging symbols found) Core was generated by `wg'. Program terminated with signal 11, Segmentation fault. (no debugging symbols found) Loaded symbols for /usr/local/bin/wg Reading symbols from /usr/lib/libc.so.97.0...done. Loaded symbols for /usr/lib/libc.so.97.0 Reading symbols from /usr/libexec/ld.so...Error while reading shared library symbols: Dwarf Error: wrong version in compilation unit header (is 4, should be 2) [in module /usr/libexec/ld.so] #0 0x0ec9a681649d in ipc_get_device () from /usr/local/bin/wg (gdb) bt #0 0x0ec9a681649d in ipc_get_device () from /usr/local/bin/wg #1 0x0ec9a6817ac2 in show_main () from /usr/local/bin/wg #2 0x0ec9a6808822 in _start () from /usr/local/bin/wg #3 0x in ?? () (gdb)
Re: ifconfig description for wireguard peers
On 12.1.2023. 5:49, Mikolaj Kucharski wrote: > Hi, > > Is there anything else which I can do, to help this diff reviwed and > increase the chance of getting in? > > Thread at https://marc.info/?t=16347829861=1=2 > > Last version of the diff at > https://marc.info/?l=openbsd-tech=167185582521873=mbox Hi, I've applied this diff and it's works as expected. wgdesc would be nice features to have when having remote access server with wireguard. old ifconfig wg0 wg0: flags=80c3 mtu 1420 index 15 priority 0 llprio 3 wgport 12345 wgpubkey 123= wgpeer 111= wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.2/32 wgpeer 222= wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.3/32 wgpeer 33= wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.4/32 wgpeer 44= wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.5/32 wgpeer 55= wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.6/32 wgpeer 66= wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.7/32 wgpeer 777= wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.8/32 groups: wg inet 10.123.123.1 netmask 0xff00 broadcast 10.123.123.255 new ifconfig wg0 wg0: flags=80c3 mtu 1420 index 15 priority 0 llprio 3 wgport 12345 wgpubkey 123= wgpeer 111= wgdesc Hrvoje Popovski 1 wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.2/32 wgpeer 222= wgdesc Hrvoje Popovski 2 wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.3/32 wgpeer 333= wgdesc Hrvoje Popovski 3 wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.4/32 wgpeer 444= wgdesc Hrvoje Popovski 4 wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.5/32 wgpeer 555= wgdesc Hrvoje Popovski 5 wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.6/32 wgpeer 666= wgdesc Hrvoje Popovski 6 wgpsk (present) tx: 0, rx: 0 wgaip 10.123.123.7/32 groups: wg inet 10.123.123.1 netmask 0xff00 broadcast 10.123.123.255 cat /etc/hostname.wg0 inet 10.123.123.1 255.255.255.0 wgport 12345 wgkey 456= # old desc - Hrvoje Popovski 1 wgpeer 111= wgdesc "Hrvoje Popovski 1" \ wgpsk 111= \ wgaip 10.123.123.2/32 # old desc - Hrvoje Popovski 2 wgpeer 222= wgdesc "Hrvoje Popovski 2" \ wgpsk 222= \ wgaip 10.123.123.3/32 # old desc - Hrvoje Popovski 3 wgpeer 3= wgdesc "Hrvoje Popovski 3" \ wgpsk 3= \ wgaip 10.123.123.4/32 # old desc - Hrvoje Popovski 4 wgpeer 44= wgdesc "Hrvoje Popovski 4" \ wgpsk 44= \ wgaip 10.123.123.5/32 # old desc .- Hrvoje Popovski 5 wgpeer 555= wgdesc "Hrvoje Popovski 5" \ wgpsk 555= \ wgaip 10.123.123.6/32 # old desc - Hrvoje Popovski 6 wgpeer = wgdesc "Hrvoje Popovski 6" \ wgpsk = \ wgaip 10.123.123.7/32
Re: ifconfig description for wireguard peers
Stuart Henderson wrote: > : Index: share/man/man4/wg.4 > : === > : RCS file: /cvs/src/share/man/man4/wg.4,v > : retrieving revision 1.10 > : diff -u -p -u -r1.10 wg.4 > : --- share/man/man4/wg.4 14 Mar 2021 10:08:38 - 1.10 > : +++ share/man/man4/wg.4 24 Dec 2022 00:49:05 - > : @@ -42,6 +42,19 @@ configuration file for > : .Xr netstart 8 . > : The interface itself can be configured with > : .Xr ifconfig 8 . > : +To display > : +.Cm wgpeer > : +information for each > : +.Nm wg > : +interface option > : +.Fl A > : +to > : +.Xr ifconfig 8 > : +should be used or > : +.Nm wg > : +interface should be specified as an argument to > : +.Xr ifconfig 8 > : +command. > : .Pp > : .Nm wg > : interfaces support the following > > This isn't directly related to the descr diff so should be a separate > commit if done. > > This diff displays like so: > > - > The interfaces can be created at runtime using the ifconfig wgN create > command or by setting up a hostname.if(5) configuration file for > netstart(8). The interface itself can be configured with ifconfig(8). > To display wgpeer information for each wg interface option -A to > ifconfig(8) should be used or wg interface should be specified as an > argument to ifconfig(8) command. > [...] > - > > I find this new text a bit confusing really, and really it's more > general to ifconfig rather than specific to wg, so I'm not sure how > much explanation is really needed in wg(4). How about just adding > a couple of lines to Examples instead? > > Index: wg.4 > === > RCS file: /cvs/src/share/man/man4/wg.4,v > retrieving revision 1.10 > diff -u -p -r1.10 wg.4 > --- wg.4 14 Mar 2021 10:08:38 - 1.10 > +++ wg.4 14 Jan 2023 14:24:58 - > @@ -164,6 +164,11 @@ which resides in the default > Show the listening sockets: > .Pp > .Dl $ netstat -ln > +.Pp > +Show full information for peers on a single interface, or all interfaces: > +.Pp > +.Dl # ifconfig wg1 > +.Dl # ifconfig -A > .Sh DIAGNOSTICS > The > .Nm Or just skip all this text explaining how to use ifconfig, because people need to learn how to use ifconfig from the ifconfig manual page.
Re: ifconfig description for wireguard peers
On 2023/01/12 04:49, Mikolaj Kucharski wrote:
> Hi,
>
> Is there anything else which I can do, to help this diff reviwed and
> increase the chance of getting in?
>
> Thread at https://marc.info/?t=16347829861=1=2
>
> Last version of the diff at
> https://marc.info/?l=openbsd-tech=167185582521873=mbox
Inlining that for a few comments, otherwise it's ok sthen
: Index: sbin/ifconfig/ifconfig.c
: ===
: RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
: retrieving revision 1.460
: diff -u -p -u -r1.460 ifconfig.c
: --- sbin/ifconfig/ifconfig.c 18 Dec 2022 18:56:38 - 1.460
: +++ sbin/ifconfig/ifconfig.c 24 Dec 2022 00:49:05 -
: @@ -355,12 +355,14 @@ voidsetwgpeerep(const char *, const cha
: void setwgpeeraip(const char *, int);
: void setwgpeerpsk(const char *, int);
: void setwgpeerpka(const char *, int);
: +void setwgpeerdesc(const char *, int);
: void setwgport(const char *, int);
: void setwgkey(const char *, int);
: void setwgrtable(const char *, int);
:
: void unsetwgpeer(const char *, int);
: void unsetwgpeerpsk(const char *, int);
: +void unsetwgpeerdesc(const char *, int);
: void unsetwgpeerall(const char *, int);
:
: void wg_status(int);
: @@ -623,11 +625,13 @@ const structcmd {
: { "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
: { "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
: { "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
: + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
: { "wgport", NEXTARG,A_WIREGUARD,setwgport},
: { "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
: { "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
: { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
: { "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
: + { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
: { "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
:
: #else /* SMALL */
: @@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
: }
:
: void
: +setwgpeerdesc(const char *wgdesc, int param)
: +{
: + if (wg_peer == NULL)
: + errx(1, "wgdesc: wgpeer not set");
: + if (strlen(wgdesc))
: + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
: + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
: +}
: +
: +void
: setwgport(const char *port, int param)
: {
: const char *errmsg = NULL;
: @@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
: }
:
: void
: +unsetwgpeerdesc(const char *value, int param)
: +{
: + if (wg_peer == NULL)
: + errx(1, "wgdesc: wgpeer not set");
: + strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
: + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
I was a bit confused by this at first (wondering if it should use
"&= ~WG_PEER_SET_DESCRIPTION"). I understand it now but I think that
a different name would make it clearer. Maybe WG_PEER_UPDATE_DESCR?
: +}
: +
: +void
: unsetwgpeerall(const char *value, int param)
: {
: ensurewginterface();
: @@ -5961,6 +5984,9 @@ wg_status(int ifaliases)
: b64_ntop(wg_peer->p_public, WG_KEY_LEN,
: key, sizeof(key));
: printf("\twgpeer %s\n", key);
: +
: + if (strlen(wg_peer->p_description))
: + printf("\t\twgdesc %s\n",
wg_peer->p_description);
:
: if (wg_peer->p_flags & WG_PEER_HAS_PSK)
: printf("\t\twgpsk (present)\n");
: Index: share/man/man4/wg.4
: ===
: RCS file: /cvs/src/share/man/man4/wg.4,v
: retrieving revision 1.10
: diff -u -p -u -r1.10 wg.4
: --- share/man/man4/wg.4 14 Mar 2021 10:08:38 - 1.10
: +++ share/man/man4/wg.4 24 Dec 2022 00:49:05 -
: @@ -42,6 +42,19 @@ configuration file for
: .Xr netstart 8 .
: The interface itself can be configured with
: .Xr ifconfig 8 .
: +To display
: +.Cm wgpeer
: +information for each
: +.Nm wg
: +interface option
: +.Fl A
: +to
: +.Xr ifconfig 8
: +should be used or
: +.Nm wg
: +interface should be specified as an argument to
: +.Xr ifconfig 8
: +command.
: .Pp
: .Nm wg
: interfaces support the following
This isn't directly related to the descr diff so should be a separate
commit if done.
This diff displays like so:
-
The interfaces can be created at runtime using the ifconfig wgN create
command or by setting up a hostname.if(5) configuration file for
netstart(8). The interface itself can be configured with ifconfig(8).
To display wgpeer information for each wg interface option -A to
ifconfig(8) should be used or wg interface should be specified as an
argument to ifconfig(8) command.
[...]
-
I find
Re: ifconfig description for wireguard peers
Hi,
Is there anything else which I can do, to help this diff reviwed and
increase the chance of getting in?
Thread at https://marc.info/?t=16347829861=1=2
Last version of the diff at
https://marc.info/?l=openbsd-tech=167185582521873=mbox
On Thu, Jan 05, 2023 at 07:41:54PM +0100, Hrvoje Popovski wrote:
> On 2.1.2023. 22:01, Mikolaj Kucharski wrote:
> > This seems to work fine for me.
> >
> > Patch also available at:
> >
> > https://marc.info/?l=openbsd-tech=167185582521873=mbox
> >
>
> I've had some problems with 20+ wgpeers few days ago and at that time it
> would have been good if I had wgdesc in ifconfig wg output ...
>
>
> >
> > On Sat, Dec 24, 2022 at 03:29:35AM +, Mikolaj Kucharski wrote:
> >> On Sat, Nov 19, 2022 at 12:03:59PM +, Mikolaj Kucharski wrote:
> >>> Kind reminder.
> >>>
> >>> Below diff also at:
> >>>
> >>> https://marc.info/?l=openbsd-tech=166806412910623=2
> >>>
> >>> This is diff by Noah Meier with small changes by me.
> >>>
> >>>
> >>> On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
> >>>> On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> >>>>> On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> While wireguard interfaces can have a description set by ifconfig,
> >>>>>> wireguard peers currently cannot. I now have a lot of peers and
> >>>>>> descriptions of them in ifconfig would be helpful.
> >>>>>>
> >>>>>> This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> >>>>>> corresponding '-wgdesc' option). Man page also updated.
> >>>>>>
> >>>>>> NM
> >>>>>
> >>>>> Now that my `ifconfig, wireguard output less verbose, unless -A or `
> >>>>> diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> >>>>> bump of an old thread.
> >>>>>
> >>>>> Below is rebased on -current and tiny modified by me, Noah's diff.
> >>>>>
> >>>>> You need both kernel and ifconfig with below code, otherwise you may see
> >>>>> issues bringing up wg(4) interface. If you may loose access to machine
> >>>>> behind wg(4) VPN, make sure you update on that machine both kernel and
> >>>>> ifconfig(8) at the same time.
> >>>>>
> >>
> >> Rebased again, just a moment ago. Will test runtime again over the weekend,
> >> are there no surprises.
> >>
> >> - ifconfig compiles
> >> - GENERIC.MP/amd64 kernel compiles too
> >>
> >>
> >> Index: sbin/ifconfig/ifconfig.c
> >> ===
> >> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> >> retrieving revision 1.460
> >> diff -u -p -u -r1.460 ifconfig.c
> >> --- sbin/ifconfig/ifconfig.c 18 Dec 2022 18:56:38 - 1.460
> >> +++ sbin/ifconfig/ifconfig.c 24 Dec 2022 00:49:05 -
> >> @@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
> >> void setwgpeeraip(const char *, int);
> >> void setwgpeerpsk(const char *, int);
> >> void setwgpeerpka(const char *, int);
> >> +void setwgpeerdesc(const char *, int);
> >> void setwgport(const char *, int);
> >> void setwgkey(const char *, int);
> >> void setwgrtable(const char *, int);
> >>
> >> void unsetwgpeer(const char *, int);
> >> void unsetwgpeerpsk(const char *, int);
> >> +void unsetwgpeerdesc(const char *, int);
> >> void unsetwgpeerall(const char *, int);
> >>
> >> void wg_status(int);
> >> @@ -623,11 +625,13 @@ const struct cmd {
> >>{ "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
> >>{ "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
> >>{ "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
> >> + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
> >>{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
> >>{ "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
> >>{ "wgrtable", NEXTARG,A_WIREGUARD,
Re: ifconfig description for wireguard peers
On Mon, Nov 29, 2021 at 01:25:20PM +0100, Stefan Sperling wrote: > This looks useful to me. > Did you get any feedback for this patch yet, Noah? Not tested the patch, but this functionality will be huge quality of life improvement for me. So from the user perspective, very, very nice to have. Best regards, Chris Narkiewicz
Re: ifconfig description for wireguard peers
On 2.1.2023. 22:01, Mikolaj Kucharski wrote:
> This seems to work fine for me.
>
> Patch also available at:
>
> https://marc.info/?l=openbsd-tech=167185582521873=mbox
>
I've had some problems with 20+ wgpeers few days ago and at that time it
would have been good if I had wgdesc in ifconfig wg output ...
>
> On Sat, Dec 24, 2022 at 03:29:35AM +, Mikolaj Kucharski wrote:
>> On Sat, Nov 19, 2022 at 12:03:59PM +, Mikolaj Kucharski wrote:
>>> Kind reminder.
>>>
>>> Below diff also at:
>>>
>>> https://marc.info/?l=openbsd-tech=166806412910623=2
>>>
>>> This is diff by Noah Meier with small changes by me.
>>>
>>>
>>> On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
>>>> On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
>>>>> On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
>>>>>> Hi,
>>>>>>
>>>>>> While wireguard interfaces can have a description set by ifconfig,
>>>>>> wireguard peers currently cannot. I now have a lot of peers and
>>>>>> descriptions of them in ifconfig would be helpful.
>>>>>>
>>>>>> This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
>>>>>> corresponding '-wgdesc' option). Man page also updated.
>>>>>>
>>>>>> NM
>>>>>
>>>>> Now that my `ifconfig, wireguard output less verbose, unless -A or `
>>>>> diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
>>>>> bump of an old thread.
>>>>>
>>>>> Below is rebased on -current and tiny modified by me, Noah's diff.
>>>>>
>>>>> You need both kernel and ifconfig with below code, otherwise you may see
>>>>> issues bringing up wg(4) interface. If you may loose access to machine
>>>>> behind wg(4) VPN, make sure you update on that machine both kernel and
>>>>> ifconfig(8) at the same time.
>>>>>
>>
>> Rebased again, just a moment ago. Will test runtime again over the weekend,
>> are there no surprises.
>>
>> - ifconfig compiles
>> - GENERIC.MP/amd64 kernel compiles too
>>
>>
>> Index: sbin/ifconfig/ifconfig.c
>> ===
>> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
>> retrieving revision 1.460
>> diff -u -p -u -r1.460 ifconfig.c
>> --- sbin/ifconfig/ifconfig.c 18 Dec 2022 18:56:38 - 1.460
>> +++ sbin/ifconfig/ifconfig.c 24 Dec 2022 00:49:05 -
>> @@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
>> voidsetwgpeeraip(const char *, int);
>> voidsetwgpeerpsk(const char *, int);
>> voidsetwgpeerpka(const char *, int);
>> +voidsetwgpeerdesc(const char *, int);
>> voidsetwgport(const char *, int);
>> voidsetwgkey(const char *, int);
>> voidsetwgrtable(const char *, int);
>>
>> voidunsetwgpeer(const char *, int);
>> voidunsetwgpeerpsk(const char *, int);
>> +voidunsetwgpeerdesc(const char *, int);
>> voidunsetwgpeerall(const char *, int);
>>
>> voidwg_status(int);
>> @@ -623,11 +625,13 @@ const struct cmd {
>> { "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
>> { "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
>> { "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
>> +{ "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
>> { "wgport", NEXTARG,A_WIREGUARD,setwgport},
>> { "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
>> { "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
>> { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
>> { "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
>> +{ "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
>> { "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
>>
>> #else /* SMALL */
>> @@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
>> }
>>
>> void
>> +setwgpeerdesc(const char *wgdesc, int param)
>> +{
>> +if (wg_peer == NULL)
>> +errx(1, "wgdesc: wgpeer not set");
>> +if (strlen(wgdesc
Re: ifconfig description for wireguard peers
This seems to work fine for me.
Patch also available at:
https://marc.info/?l=openbsd-tech=167185582521873=mbox
On Sat, Dec 24, 2022 at 03:29:35AM +, Mikolaj Kucharski wrote:
> On Sat, Nov 19, 2022 at 12:03:59PM +, Mikolaj Kucharski wrote:
> > Kind reminder.
> >
> > Below diff also at:
> >
> > https://marc.info/?l=openbsd-tech=166806412910623=2
> >
> > This is diff by Noah Meier with small changes by me.
> >
> >
> > On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
> > > On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> > > > On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > > > > Hi,
> > > > >
> > > > > While wireguard interfaces can have a description set by ifconfig,
> > > > > wireguard peers currently cannot. I now have a lot of peers and
> > > > > descriptions of them in ifconfig would be helpful.
> > > > >
> > > > > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> > > > > corresponding '-wgdesc' option). Man page also updated.
> > > > >
> > > > > NM
> > > >
> > > > Now that my `ifconfig, wireguard output less verbose, unless -A or `
> > > > diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> > > > bump of an old thread.
> > > >
> > > > Below is rebased on -current and tiny modified by me, Noah's diff.
> > > >
> > > > You need both kernel and ifconfig with below code, otherwise you may see
> > > > issues bringing up wg(4) interface. If you may loose access to machine
> > > > behind wg(4) VPN, make sure you update on that machine both kernel and
> > > > ifconfig(8) at the same time.
> > > >
>
> Rebased again, just a moment ago. Will test runtime again over the weekend,
> are there no surprises.
>
> - ifconfig compiles
> - GENERIC.MP/amd64 kernel compiles too
>
>
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.460
> diff -u -p -u -r1.460 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 18 Dec 2022 18:56:38 - 1.460
> +++ sbin/ifconfig/ifconfig.c 24 Dec 2022 00:49:05 -
> @@ -355,12 +355,14 @@ voidsetwgpeerep(const char *, const cha
> void setwgpeeraip(const char *, int);
> void setwgpeerpsk(const char *, int);
> void setwgpeerpka(const char *, int);
> +void setwgpeerdesc(const char *, int);
> void setwgport(const char *, int);
> void setwgkey(const char *, int);
> void setwgrtable(const char *, int);
>
> void unsetwgpeer(const char *, int);
> void unsetwgpeerpsk(const char *, int);
> +void unsetwgpeerdesc(const char *, int);
> void unsetwgpeerall(const char *, int);
>
> void wg_status(int);
> @@ -623,11 +625,13 @@ const structcmd {
> { "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
> { "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
> { "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
> + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
> { "wgport", NEXTARG,A_WIREGUARD,setwgport},
> { "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
> { "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
> { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
> { "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
> + { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
> { "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
>
> #else /* SMALL */
> @@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
> }
>
> void
> +setwgpeerdesc(const char *wgdesc, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + if (strlen(wgdesc))
> + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
> + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> +}
> +
> +void
> setwgport(const char *port, int param)
> {
> const char *errmsg = NULL;
> @@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
> }
>
> void
> +unsetwgpeerdesc(const char *value, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + str
Re: ifconfig description for wireguard peers
On Sat, Nov 19, 2022 at 12:03:59PM +, Mikolaj Kucharski wrote:
> Kind reminder.
>
> Below diff also at:
>
> https://marc.info/?l=openbsd-tech=166806412910623=2
>
> This is diff by Noah Meier with small changes by me.
>
>
> On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
> > On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> > > On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > > > Hi,
> > > >
> > > > While wireguard interfaces can have a description set by ifconfig,
> > > > wireguard peers currently cannot. I now have a lot of peers and
> > > > descriptions of them in ifconfig would be helpful.
> > > >
> > > > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> > > > corresponding '-wgdesc' option). Man page also updated.
> > > >
> > > > NM
> > >
> > > Now that my `ifconfig, wireguard output less verbose, unless -A or `
> > > diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> > > bump of an old thread.
> > >
> > > Below is rebased on -current and tiny modified by me, Noah's diff.
> > >
> > > You need both kernel and ifconfig with below code, otherwise you may see
> > > issues bringing up wg(4) interface. If you may loose access to machine
> > > behind wg(4) VPN, make sure you update on that machine both kernel and
> > > ifconfig(8) at the same time.
> > >
Rebased again, just a moment ago. Will test runtime again over the weekend,
are there no surprises.
- ifconfig compiles
- GENERIC.MP/amd64 kernel compiles too
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.460
diff -u -p -u -r1.460 ifconfig.c
--- sbin/ifconfig/ifconfig.c18 Dec 2022 18:56:38 - 1.460
+++ sbin/ifconfig/ifconfig.c24 Dec 2022 00:49:05 -
@@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
void setwgpeeraip(const char *, int);
void setwgpeerpsk(const char *, int);
void setwgpeerpka(const char *, int);
+void setwgpeerdesc(const char *, int);
void setwgport(const char *, int);
void setwgkey(const char *, int);
void setwgrtable(const char *, int);
void unsetwgpeer(const char *, int);
void unsetwgpeerpsk(const char *, int);
+void unsetwgpeerdesc(const char *, int);
void unsetwgpeerall(const char *, int);
void wg_status(int);
@@ -623,11 +625,13 @@ const struct cmd {
{ "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
+ { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
+ { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
#else /* SMALL */
@@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
}
void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ if (strlen(wgdesc))
+ strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
setwgport(const char *port, int param)
{
const char *errmsg = NULL;
@@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
}
void
+unsetwgpeerdesc(const char *value, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
unsetwgpeerall(const char *value, int param)
{
ensurewginterface();
@@ -5961,6 +5984,9 @@ wg_status(int ifaliases)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+ if (strlen(wg_peer->p_description))
+ printf("\t\twgdesc %s\n",
wg_peer->p_description);
if (wg_peer->p_flags & WG_PE
Re: ifconfig description for wireguard peers
Kind reminder.
Below diff also at:
https://marc.info/?l=openbsd-tech=166806412910623=2
This is diff by Noah Meier with small changes by me.
On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
> On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> > On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > > Hi,
> > >
> > > While wireguard interfaces can have a description set by ifconfig,
> > > wireguard peers currently cannot. I now have a lot of peers and
> > > descriptions of them in ifconfig would be helpful.
> > >
> > > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> > > corresponding '-wgdesc' option). Man page also updated.
> > >
> > > NM
> >
> > Now that my `ifconfig, wireguard output less verbose, unless -A or `
> > diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> > bump of an old thread.
> >
> > Below is rebased on -current and tiny modified by me, Noah's diff.
> >
> > You need both kernel and ifconfig with below code, otherwise you may see
> > issues bringing up wg(4) interface. If you may loose access to machine
> > behind wg(4) VPN, make sure you update on that machine both kernel and
> > ifconfig(8) at the same time.
> >
>
> Typo, s/wgpesc/wgdesc/
>
>
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.457
> diff -u -p -u -r1.457 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 26 Oct 2022 17:06:31 - 1.457
> +++ sbin/ifconfig/ifconfig.c 10 Nov 2022 01:03:04 -
> @@ -355,12 +355,14 @@ voidsetwgpeerep(const char *, const cha
> void setwgpeeraip(const char *, int);
> void setwgpeerpsk(const char *, int);
> void setwgpeerpka(const char *, int);
> +void setwgpeerdesc(const char *, int);
> void setwgport(const char *, int);
> void setwgkey(const char *, int);
> void setwgrtable(const char *, int);
>
> void unsetwgpeer(const char *, int);
> void unsetwgpeerpsk(const char *, int);
> +void unsetwgpeerdesc(const char *, int);
> void unsetwgpeerall(const char *, int);
>
> void wg_status(int);
> @@ -620,11 +622,13 @@ const structcmd {
> { "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
> { "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
> { "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
> + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
> { "wgport", NEXTARG,A_WIREGUARD,setwgport},
> { "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
> { "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
> { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
> { "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
> + { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
> { "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
>
> #else /* SMALL */
> @@ -5843,6 +5847,16 @@ setwgpeerpka(const char *pka, int param)
> }
>
> void
> +setwgpeerdesc(const char *wgdesc, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + if (strlen(wgdesc))
> + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
> + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> +}
> +
> +void
> setwgport(const char *port, int param)
> {
> const char *errmsg = NULL;
> @@ -5889,6 +5903,15 @@ unsetwgpeerpsk(const char *value, int pa
> }
>
> void
> +unsetwgpeerdesc(const char *value, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
> + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> +}
> +
> +void
> unsetwgpeerall(const char *value, int param)
> {
> ensurewginterface();
> @@ -5948,6 +5971,9 @@ wg_status(int ifaliases)
> b64_ntop(wg_peer->p_public, WG_KEY_LEN,
> key, sizeof(key));
> printf("\twgpeer %s\n", key);
> +
> + if (strlen(wg_peer->p_description))
> + printf("\t\twgdesc %s\n",
> wg_peer->p_description);
>
> if (wg_peer->p_flags & WG_PEER_HAS_PSK)
> pr
Re: ifconfig description for wireguard peers
On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > Hi,
> >
> > While wireguard interfaces can have a description set by ifconfig,
> > wireguard peers currently cannot. I now have a lot of peers and
> > descriptions of them in ifconfig would be helpful.
> >
> > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> > corresponding '-wgdesc' option). Man page also updated.
> >
> > NM
>
> Now that my `ifconfig, wireguard output less verbose, unless -A or `
> diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> bump of an old thread.
>
> Below is rebased on -current and tiny modified by me, Noah's diff.
>
> You need both kernel and ifconfig with below code, otherwise you may see
> issues bringing up wg(4) interface. If you may loose access to machine
> behind wg(4) VPN, make sure you update on that machine both kernel and
> ifconfig(8) at the same time.
>
Typo, s/wgpesc/wgdesc/
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.457
diff -u -p -u -r1.457 ifconfig.c
--- sbin/ifconfig/ifconfig.c26 Oct 2022 17:06:31 - 1.457
+++ sbin/ifconfig/ifconfig.c10 Nov 2022 01:03:04 -
@@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
void setwgpeeraip(const char *, int);
void setwgpeerpsk(const char *, int);
void setwgpeerpka(const char *, int);
+void setwgpeerdesc(const char *, int);
void setwgport(const char *, int);
void setwgkey(const char *, int);
void setwgrtable(const char *, int);
void unsetwgpeer(const char *, int);
void unsetwgpeerpsk(const char *, int);
+void unsetwgpeerdesc(const char *, int);
void unsetwgpeerall(const char *, int);
void wg_status(int);
@@ -620,11 +622,13 @@ const struct cmd {
{ "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
+ { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
+ { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
#else /* SMALL */
@@ -5843,6 +5847,16 @@ setwgpeerpka(const char *pka, int param)
}
void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ if (strlen(wgdesc))
+ strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
setwgport(const char *port, int param)
{
const char *errmsg = NULL;
@@ -5889,6 +5903,15 @@ unsetwgpeerpsk(const char *value, int pa
}
void
+unsetwgpeerdesc(const char *value, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
unsetwgpeerall(const char *value, int param)
{
ensurewginterface();
@@ -5948,6 +5971,9 @@ wg_status(int ifaliases)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+ if (strlen(wg_peer->p_description))
+ printf("\t\twgdesc %s\n",
wg_peer->p_description);
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
Index: share/man/man4/wg.4
===
RCS file: /cvs/src/share/man/man4/wg.4,v
retrieving revision 1.10
diff -u -p -u -r1.10 wg.4
--- share/man/man4/wg.4 14 Mar 2021 10:08:38 - 1.10
+++ share/man/man4/wg.4 10 Nov 2022 01:03:04 -
@@ -42,6 +42,19 @@ configuration file for
.Xr netstart 8 .
The interface itself can be configured with
.Xr ifconfig 8 .
+To display
+.Cm wgpeer
+information for each
+.Nm wg
+interface option
+.Fl A
+to
+.Xr ifconfig 8
+should be used or
+.Nm wg
+interface should be specified as an argument to
+.Xr ifconfig 8
+command.
Re: ifconfig description for wireguard peers
On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> Hi,
>
> While wireguard interfaces can have a description set by ifconfig, wireguard
> peers currently cannot. I now have a lot of peers and descriptions of them in
> ifconfig would be helpful.
>
> This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> corresponding '-wgdesc' option). Man page also updated.
>
> NM
Now that my `ifconfig, wireguard output less verbose, unless -A or `
diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
bump of an old thread.
Below is rebased on -current and tiny modified by me, Noah's diff.
You need both kernel and ifconfig with below code, otherwise you may see
issues bringing up wg(4) interface. If you may loose access to machine
behind wg(4) VPN, make sure you update on that machine both kernel and
ifconfig(8) at the same time.
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.457
diff -u -p -u -r1.457 ifconfig.c
--- sbin/ifconfig/ifconfig.c26 Oct 2022 17:06:31 - 1.457
+++ sbin/ifconfig/ifconfig.c5 Nov 2022 19:41:22 -
@@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
void setwgpeeraip(const char *, int);
void setwgpeerpsk(const char *, int);
void setwgpeerpka(const char *, int);
+void setwgpeerdesc(const char *, int);
void setwgport(const char *, int);
void setwgkey(const char *, int);
void setwgrtable(const char *, int);
void unsetwgpeer(const char *, int);
void unsetwgpeerpsk(const char *, int);
+void unsetwgpeerdesc(const char *, int);
void unsetwgpeerall(const char *, int);
void wg_status(int);
@@ -620,11 +622,13 @@ const struct cmd {
{ "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
+ { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
+ { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
#else /* SMALL */
@@ -5843,6 +5847,16 @@ setwgpeerpka(const char *pka, int param)
}
void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ if (strlen(wgdesc))
+ strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
setwgport(const char *port, int param)
{
const char *errmsg = NULL;
@@ -5889,6 +5903,15 @@ unsetwgpeerpsk(const char *value, int pa
}
void
+unsetwgpeerdesc(const char *value, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgpesc: wgpeer not set");
+ strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
unsetwgpeerall(const char *value, int param)
{
ensurewginterface();
@@ -5948,6 +5971,9 @@ wg_status(int ifaliases)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+ if (strlen(wg_peer->p_description))
+ printf("\t\twgdesc %s\n",
wg_peer->p_description);
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
Index: share/man/man4/wg.4
===
RCS file: /cvs/src/share/man/man4/wg.4,v
retrieving revision 1.10
diff -u -p -u -r1.10 wg.4
--- share/man/man4/wg.4 14 Mar 2021 10:08:38 - 1.10
+++ share/man/man4/wg.4 5 Nov 2022 19:41:22 -
@@ -42,6 +42,19 @@ configuration file for
.Xr netstart 8 .
The interface itself can be configured with
.Xr ifconfig 8 .
+To display
+.Cm wgpeer
+information for each
+.Nm wg
+interface option
+.Fl A
+to
+.Xr ifconfig 8
+should be used or
+.Nm wg
+interface should be specified as an argument to
+.Xr ifconfig 8
+command.
.Pp
.Nm wg
interfaces support the following
Index: sys/net/if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.26
diff -u -
Re: ifconfig description for wireguard peers
On 2022/07/14 10:57, Claudio Jeker wrote: > On Thu, Jul 14, 2022 at 10:51:42AM +0200, Stefan Sperling wrote: > > On Wed, Jul 13, 2022 at 05:13:49PM +, Mikolaj Kucharski wrote: > > > On Wed, Jul 13, 2022 at 05:43:59PM +0100, Stuart Henderson wrote: > > > > > > > > > > Not sure how to handle long output in different way. If you don't > > > > > specify wgdesc to the ifconfig, the diff doesn't change anything and > > > > > ifconfig(8) output is exactly the same. If you don't find this feature > > > > > useful, by not using it, nothing changes for you. Isn't that fair? > > > > > > > > It might be better if wgpeer details would be skipped from plain > > > > "ifconfig" and only listed if "ifconfig wg0" is used, much like > > > > IPv4 aliases are skipped from "ifconfig" and only listed when > > > > you use either "ifconfig -A" or "ifconfig em0" etc. > > > > > > > > > > Ah, that helps. Thank you! Will look into it. Indeed this makes more > > > sense. I was aware of -A, but was not aware of ifconfig behaviour. > > > > Perhaps also consider moving counters to netstat -s instead of ifconfig. > > I don't think netstat -s is a good place for per peer stats (neither is > ifconfig though). netstat -s shows global per protocol counters. Agreed. These need to either be in ifconfig or some separate thing for wg, and I much prefer ifconfig. (Everything which is listed there now is IMHO required information, per-peer byte counters are useful to help spot 1-way comms issues).
Re: ifconfig description for wireguard peers
On Thu, Jul 14, 2022 at 10:57:51AM +0200, Claudio Jeker wrote: > On Thu, Jul 14, 2022 at 10:51:42AM +0200, Stefan Sperling wrote: > > Perhaps also consider moving counters to netstat -s instead of ifconfig. > > I don't think netstat -s is a good place for per peer stats (neither is > ifconfig though). netstat -s shows global per protocol counters. Ah yes, indeed. netstat only makes sense for global counters. Thanks.
Re: ifconfig description for wireguard peers
On Thu, Jul 14, 2022 at 10:51:42AM +0200, Stefan Sperling wrote: > On Wed, Jul 13, 2022 at 05:13:49PM +, Mikolaj Kucharski wrote: > > On Wed, Jul 13, 2022 at 05:43:59PM +0100, Stuart Henderson wrote: > > > > > > > > Not sure how to handle long output in different way. If you don't > > > > specify wgdesc to the ifconfig, the diff doesn't change anything and > > > > ifconfig(8) output is exactly the same. If you don't find this feature > > > > useful, by not using it, nothing changes for you. Isn't that fair? > > > > > > It might be better if wgpeer details would be skipped from plain > > > "ifconfig" and only listed if "ifconfig wg0" is used, much like > > > IPv4 aliases are skipped from "ifconfig" and only listed when > > > you use either "ifconfig -A" or "ifconfig em0" etc. > > > > > > > Ah, that helps. Thank you! Will look into it. Indeed this makes more > > sense. I was aware of -A, but was not aware of ifconfig behaviour. > > Perhaps also consider moving counters to netstat -s instead of ifconfig. I don't think netstat -s is a good place for per peer stats (neither is ifconfig though). netstat -s shows global per protocol counters. -- :wq Claudio
Re: ifconfig description for wireguard peers
On Wed, Jul 13, 2022 at 05:13:49PM +, Mikolaj Kucharski wrote: > On Wed, Jul 13, 2022 at 05:43:59PM +0100, Stuart Henderson wrote: > > > > > > Not sure how to handle long output in different way. If you don't > > > specify wgdesc to the ifconfig, the diff doesn't change anything and > > > ifconfig(8) output is exactly the same. If you don't find this feature > > > useful, by not using it, nothing changes for you. Isn't that fair? > > > > It might be better if wgpeer details would be skipped from plain > > "ifconfig" and only listed if "ifconfig wg0" is used, much like > > IPv4 aliases are skipped from "ifconfig" and only listed when > > you use either "ifconfig -A" or "ifconfig em0" etc. > > > > Ah, that helps. Thank you! Will look into it. Indeed this makes more > sense. I was aware of -A, but was not aware of ifconfig behaviour. Perhaps also consider moving counters to netstat -s instead of ifconfig.
Re: ifconfig description for wireguard peers
On Wed, Jul 13, 2022 at 05:43:59PM +0100, Stuart Henderson wrote: > > > > Not sure how to handle long output in different way. If you don't > > specify wgdesc to the ifconfig, the diff doesn't change anything and > > ifconfig(8) output is exactly the same. If you don't find this feature > > useful, by not using it, nothing changes for you. Isn't that fair? > > It might be better if wgpeer details would be skipped from plain > "ifconfig" and only listed if "ifconfig wg0" is used, much like > IPv4 aliases are skipped from "ifconfig" and only listed when > you use either "ifconfig -A" or "ifconfig em0" etc. > Ah, that helps. Thank you! Will look into it. Indeed this makes more sense. I was aware of -A, but was not aware of ifconfig behaviour. -- Regards, Mikolaj
Re: ifconfig description for wireguard peers
On 2022/07/13 16:18, Mikolaj Kucharski wrote: > On Wed, Jul 13, 2022 at 10:02:30AM -0600, Theo de Raadt wrote: > > Mikolaj Kucharski wrote: > > > > > I took the libery and refreshed the patch. What I did so far: > > > > > > - compiled GENERIC.MP on amd64 > > > - compiled new ifconfig, same arch > > > - booted up new bsd.mp with the patch > > > - when wgdesc is not set, pre-patch ifconfig seems to work > > > - when with new ifconfig I set wgdesc, old ifconfig wg segfaults > > > > > > Example from running -current: > > > > > > pce-0067# ifconfig.new wg0 > > > wg0: flags=80c3 mtu 1420 > > > index 8 priority 0 llprio 3 > > > wgport 51820 > > > wgpubkey qcb... > > > wgpeer klM... > > > description: ks2 > > > wgpsk (present) > > > wgpka 25 (sec) > > > wgendpoint xxx.xxx.xxx.xxx 51820 > > > tx: 1932, rx: 620 > > > last handshake: 83 seconds ago > > > wgaip fde4:f456:48c2:13c0::/64 > > > groups: wg > > > inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64 > > > > That seems disgustingly verbose to me. > > > > Who is going to read it? > > Me. I find this one additional line useful. > > > Shall we put all the active counters for normal ethernet/wifi into the > > default ifconfig output, so that the default ifconfig output scrolls and > > scrolls and scrolls and noone actually reads it? > > > > I think this has gone off the rails. > > This is the nature of wg(4) interface. I have machine with 25 peers and > each peer adds lines to the ifconfig(8) output. This is on a machine > without patch from this thread, -stable, official kernel: > > # ifconfig wg0 | wc -l > 128 > > Not sure how to handle long output in different way. If you don't > specify wgdesc to the ifconfig, the diff doesn't change anything and > ifconfig(8) output is exactly the same. If you don't find this feature > useful, by not using it, nothing changes for you. Isn't that fair? It might be better if wgpeer details would be skipped from plain "ifconfig" and only listed if "ifconfig wg0" is used, much like IPv4 aliases are skipped from "ifconfig" and only listed when you use either "ifconfig -A" or "ifconfig em0" etc.
Re: ifconfig description for wireguard peers
On Wed, Jul 13, 2022 at 10:02:30AM -0600, Theo de Raadt wrote: > Mikolaj Kucharski wrote: > > > I took the libery and refreshed the patch. What I did so far: > > > > - compiled GENERIC.MP on amd64 > > - compiled new ifconfig, same arch > > - booted up new bsd.mp with the patch > > - when wgdesc is not set, pre-patch ifconfig seems to work > > - when with new ifconfig I set wgdesc, old ifconfig wg segfaults > > > > Example from running -current: > > > > pce-0067# ifconfig.new wg0 > > wg0: flags=80c3 mtu 1420 > > index 8 priority 0 llprio 3 > > wgport 51820 > > wgpubkey qcb... > > wgpeer klM... > > description: ks2 > > wgpsk (present) > > wgpka 25 (sec) > > wgendpoint xxx.xxx.xxx.xxx 51820 > > tx: 1932, rx: 620 > > last handshake: 83 seconds ago > > wgaip fde4:f456:48c2:13c0::/64 > > groups: wg > > inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64 > > That seems disgustingly verbose to me. > > Who is going to read it? Me. I find this one additional line useful. > Shall we put all the active counters for normal ethernet/wifi into the > default ifconfig output, so that the default ifconfig output scrolls and > scrolls and scrolls and noone actually reads it? > > I think this has gone off the rails. This is the nature of wg(4) interface. I have machine with 25 peers and each peer adds lines to the ifconfig(8) output. This is on a machine without patch from this thread, -stable, official kernel: # ifconfig wg0 | wc -l 128 Not sure how to handle long output in different way. If you don't specify wgdesc to the ifconfig, the diff doesn't change anything and ifconfig(8) output is exactly the same. If you don't find this feature useful, by not using it, nothing changes for you. Isn't that fair? -- Regards, Mikolaj
Re: ifconfig description for wireguard peers
Mikolaj Kucharski wrote: > I took the libery and refreshed the patch. What I did so far: > > - compiled GENERIC.MP on amd64 > - compiled new ifconfig, same arch > - booted up new bsd.mp with the patch > - when wgdesc is not set, pre-patch ifconfig seems to work > - when with new ifconfig I set wgdesc, old ifconfig wg segfaults > > Example from running -current: > > pce-0067# ifconfig.new wg0 > wg0: flags=80c3 mtu 1420 > index 8 priority 0 llprio 3 > wgport 51820 > wgpubkey qcb... > wgpeer klM... > description: ks2 > wgpsk (present) > wgpka 25 (sec) > wgendpoint xxx.xxx.xxx.xxx 51820 > tx: 1932, rx: 620 > last handshake: 83 seconds ago > wgaip fde4:f456:48c2:13c0::/64 > groups: wg > inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64 That seems disgustingly verbose to me. Who is going to read it? Shall we put all the active counters for normal ethernet/wifi into the default ifconfig output, so that the default ifconfig output scrolls and scrolls and scrolls and noone actually reads it? I think this has gone off the rails.
Re: ifconfig description for wireguard peers
On Wed, Jul 13, 2022 at 03:37:05PM +, Mikolaj Kucharski wrote:
> Example from running -current:
>
> pce-0067# ifconfig.new wg0
> wg0: flags=80c3 mtu 1420
> index 8 priority 0 llprio 3
> wgport 51820
> wgpubkey qcb...
> wgpeer klM...
> description: ks2
> wgpsk (present)
> wgpka 25 (sec)
> wgendpoint xxx.xxx.xxx.xxx 51820
> tx: 1932, rx: 620
> last handshake: 83 seconds ago
> wgaip fde4:f456:48c2:13c0::/64
> groups: wg
> inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64
>
> Seems to work for me. Looking at above ifconfig(8) output, not sure
> should the output should say `description:` or just follow the pattern
> of other wireguard keywords and just say `wgdesc` (same keyword like
> for ifconfig command and no colon). I think I would prefer that.
>
I mean like this:
--- ifconfig.c Wed Jul 13 15:19:24 2022
+++ ifconfig.c Wed Jul 13 15:39:04 2022
@@ -5972,7 +5972,7 @@
printf("\twgpeer %s\n", key);
if (strlen(wg_peer->p_description))
- printf("\t\tdescription: %s\n", wg_peer->p_description);
+ printf("\t\twgdesc %s\n", wg_peer->p_description);
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
pce-0067# ifconfig.new wg
wg0: flags=80c3 mtu 1420
index 8 priority 0 llprio 3
wgport 51820
wgpubkey qcb...
wgpeer klM...
wgdesc ks2
wgpsk (present)
wgpka 25 (sec)
wgendpoint xxx.xxx.xxx.xxx 51820
tx: 3260, rx: 1116
last handshake: 69 seconds ago
wgaip fde4:f456:48c2:13c0::/64
groups: wg
inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64
--
Regards,
Mikolaj
Re: ifconfig description for wireguard peers
I took the libery and refreshed the patch. What I did so far:
- compiled GENERIC.MP on amd64
- compiled new ifconfig, same arch
- booted up new bsd.mp with the patch
- when wgdesc is not set, pre-patch ifconfig seems to work
- when with new ifconfig I set wgdesc, old ifconfig wg segfaults
Example from running -current:
pce-0067# ifconfig.new wg0
wg0: flags=80c3 mtu 1420
index 8 priority 0 llprio 3
wgport 51820
wgpubkey qcb...
wgpeer klM...
description: ks2
wgpsk (present)
wgpka 25 (sec)
wgendpoint xxx.xxx.xxx.xxx 51820
tx: 1932, rx: 620
last handshake: 83 seconds ago
wgaip fde4:f456:48c2:13c0::/64
groups: wg
inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64
Seems to work for me. Looking at above ifconfig(8) output, not sure
should the output should say `description:` or just follow the pattern
of other wireguard keywords and just say `wgdesc` (same keyword like
for ifconfig command and no colon). I think I would prefer that.
Patch is not mine. It's from Noah Meier and
I just re-applied it to -current. It's at the end of this email.
I hope I didn't screw up anything.
On Wed, Jul 13, 2022 at 11:29:51AM +, Mikolaj Kucharski wrote:
> Hi,
>
> I'm sorry I didn't test this, as I don't have -current OpenBSD on all my
> machines, but I do have one WireGuard gateway running -stable with few
> peers:
>
> # ifconfig wg | grep -cw wgpeer
> 25
>
> I would love to have this merged into main repo, as it would make my
> life a tiny bit easier to see which pubkey is which peer.
>
> Based on what Stefan wrote below, do you have by any chance newer
> version of your diff, Noah?
>
>
>
> On Wed, Dec 01, 2021 at 12:18:52AM +0100, Stefan Sperling wrote:
> > On Tue, Nov 30, 2021 at 02:31:20PM -0500, Noah Meier wrote:
> > > Hi Stefan,
> > >
> > > Richard Procter offered some kind advice on the ordering of options in
> > > the man page
> > > (to be done alphabetically) and commented on an unnecessary cast.
> > >
> > > I also believe that I goofed by failing to initalize the mutex and zero
> > > the description
> > > upon peer creation (in wg_peer_create).
> > >
> > > I’ve attempted to address these issues and have pasted the diff below.
> > >
> > > NM
> >
> > This new patch does not apply cleanly.
> > Leading whitespace was stripped, and thus patch complains as follows:
> >
> > Patching file if_wg.c using Plan A...
> > patch: malformed patch at line 15: };
> >
> > And it would help if you created a patch where all paths are relative to
> > the /usr/src directory. Something like this should do it:
> >
> > cd /usr/src
> > cvs diff -u sbin/ifconfig sys/net > /tmp/wgdesc.patch
> >
> > If your mailer cannot preserve whitespace as-is then please try attaching
> > the patch file instead of inlining it.
> >
> > Thanks!
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.384
diff -u -p -u -r1.384 ifconfig.8
--- sbin/ifconfig/ifconfig.827 Jun 2022 16:27:03 - 1.384
+++ sbin/ifconfig/ifconfig.813 Jul 2022 14:53:22 -
@@ -2251,6 +2251,10 @@ Set the peer's IPv4 or IPv6
range for tunneled traffic.
Repeat the option to set multiple ranges.
By default, no addresses are allowed.
+.It Cm wgdesc Ar value
+Specify a description of the peer.
+.It Cm -wgdesc
+Clear the peer description.
.It Cm wgendpoint Ar peer_address port
Address traffic to the peer's IPv4 or IPv6
.Ar peer_address
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.456
diff -u -p -u -r1.456 ifconfig.c
--- sbin/ifconfig/ifconfig.c8 Jul 2022 07:04:54 - 1.456
+++ sbin/ifconfig/ifconfig.c13 Jul 2022 14:53:22 -
@@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
void setwgpeeraip(const char *, int);
void setwgpeerpsk(const char *, int);
void setwgpeerpka(const char *, int);
+void setwgpeerdesc(const char *, int);
void setwgport(const char *, int);
void setwgkey(const char *, int);
void setwgrtable(const char *, int);
void unsetwgpeer(const char *, int);
void unsetwgpeerpsk(const char *, int);
+void unsetwgpeerdesc(const char *, int);
void unsetwgpeerall(const char *, int);
void wg_status();
@@ -620,11 +622,13 @@ const struct cmd {
{ "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
+ { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
{
Re: ifconfig description for wireguard peers
Hi, I'm sorry I didn't test this, as I don't have -current OpenBSD on all my machines, but I do have one WireGuard gateway running -stable with few peers: # ifconfig wg | grep -cw wgpeer 25 I would love to have this merged into main repo, as it would make my life a tiny bit easier to see which pubkey is which peer. Based on what Stefan wrote below, do you have by any chance newer version of your diff, Noah? On Wed, Dec 01, 2021 at 12:18:52AM +0100, Stefan Sperling wrote: > On Tue, Nov 30, 2021 at 02:31:20PM -0500, Noah Meier wrote: > > Hi Stefan, > > > > Richard Procter offered some kind advice on the ordering of options in the > > man page > > (to be done alphabetically) and commented on an unnecessary cast. > > > > I also believe that I goofed by failing to initalize the mutex and zero the > > description > > upon peer creation (in wg_peer_create). > > > > I’ve attempted to address these issues and have pasted the diff below. > > > > NM > > This new patch does not apply cleanly. > Leading whitespace was stripped, and thus patch complains as follows: > > Patching file if_wg.c using Plan A... > patch: malformed patch at line 15: }; > > And it would help if you created a patch where all paths are relative to > the /usr/src directory. Something like this should do it: > > cd /usr/src > cvs diff -u sbin/ifconfig sys/net > /tmp/wgdesc.patch > > If your mailer cannot preserve whitespace as-is then please try attaching > the patch file instead of inlining it. > > Thanks! > -- Regards, Mikolaj
Re: ifconfig description for wireguard peers
On Tue, Nov 30, 2021 at 02:31:20PM -0500, Noah Meier wrote: > Hi Stefan, > > Richard Procter offered some kind advice on the ordering of options in the > man page > (to be done alphabetically) and commented on an unnecessary cast. > > I also believe that I goofed by failing to initalize the mutex and zero the > description > upon peer creation (in wg_peer_create). > > I’ve attempted to address these issues and have pasted the diff below. > > NM This new patch does not apply cleanly. Leading whitespace was stripped, and thus patch complains as follows: Patching file if_wg.c using Plan A... patch: malformed patch at line 15: }; And it would help if you created a patch where all paths are relative to the /usr/src directory. Something like this should do it: cd /usr/src cvs diff -u sbin/ifconfig sys/net > /tmp/wgdesc.patch If your mailer cannot preserve whitespace as-is then please try attaching the patch file instead of inlining it. Thanks!
Re: ifconfig description for wireguard peers
Hi Stefan,
Richard Procter offered some kind advice on the ordering of options in the man
page
(to be done alphabetically) and commented on an unnecessary cast.
I also believe that I goofed by failing to initalize the mutex and zero the
description
upon peer creation (in wg_peer_create).
I’ve attempted to address these issues and have pasted the diff below.
NM
Index: if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.18
diff -u -p -u -p -r1.18 if_wg.c
--- if_wg.c 5 Aug 2021 13:37:04 - 1.18
+++ if_wg.c 26 Oct 2021 23:01:11 -
@@ -222,6 +222,9 @@ struct wg_peer {
SLIST_ENTRY(wg_peer) p_start_list;
int p_start_onlist;
+
+ struct mutex p_description_mtx;
+ char p_description[IFDESCRSIZE];
};
struct wg_softc {
@@ -276,6 +279,7 @@ int wg_peer_get_sockaddr(struct wg_peer
voidwg_peer_clear_src(struct wg_peer *);
voidwg_peer_get_endpoint(struct wg_peer *, struct wg_endpoint *);
voidwg_peer_counters_add(struct wg_peer *, uint64_t, uint64_t);
+void wg_peer_set_description(struct wg_peer *, char *);
int wg_aip_add(struct wg_softc *, struct wg_peer *, struct wg_aip_io *);
struct wg_peer *
@@ -409,6 +413,9 @@ wg_peer_create(struct wg_softc *sc, uint
peer->p_counters_tx = 0;
peer->p_counters_rx = 0;
+ mtx_init(>p_description_mtx, IPL_NET);
+ memset(peer->p_description, 0, IFDESCRSIZE);
+
mtx_init(>p_endpoint_mtx, IPL_NET);
bzero(>p_endpoint, sizeof(peer->p_endpoint));
@@ -583,6 +590,15 @@ wg_peer_counters_add(struct wg_peer *pee
mtx_leave(>p_counters_mtx);
}
+void
+wg_peer_set_description(struct wg_peer *peer, char *description)
+{
+ mtx_enter(>p_description_mtx);
+ memset(peer->p_description, 0, IFDESCRSIZE);
+ strlcpy(peer->p_description, description, IFDESCRSIZE);
+ mtx_leave(>p_description_mtx);
+}
+
int
wg_aip_add(struct wg_softc *sc, struct wg_peer *peer, struct wg_aip_io *d)
{
@@ -2323,6 +2339,10 @@ wg_ioctl_set(struct wg_softc *sc, struct
}
}
+ if (peer_o.p_flags & WG_PEER_SET_DESCRIPTION) {
+ wg_peer_set_description(peer, peer_o.p_description);
+ }
+
aip_p = _p->p_aips[0];
for (j = 0; j < peer_o.p_aips_count; j++) {
if ((ret = copyin(aip_p, _o, sizeof(aip_o))) != 0)
@@ -2432,6 +2452,8 @@ wg_ioctl_get(struct wg_softc *sc, struct
aip_count++;
}
peer_o.p_aips_count = aip_count;
+
+ strlcpy(peer_o.p_description, peer->p_description, IFDESCRSIZE);
if ((ret = copyout(_o, peer_p, sizeof(peer_o))) != 0)
goto unlock_and_ret_size;
Index: if_wg.h
===
RCS file: /cvs/src/sys/net/if_wg.h,v
retrieving revision 1.4
diff -u -p -u -p -r1.4 if_wg.h
--- if_wg.h 22 Jun 2020 12:20:44 - 1.4
+++ if_wg.h 26 Oct 2021 23:01:11 -
@@ -61,6 +61,7 @@ struct wg_aip_io {
#define WG_PEER_REPLACE_AIPS(1 << 4)
#define WG_PEER_REMOVE (1 << 5)
#define WG_PEER_UPDATE (1 << 6)
+#define WG_PEER_SET_DESCRIPTION(1 << 7)
#define p_sap_endpoint.sa_sa
#define p_sin p_endpoint.sa_sin
@@ -80,6 +81,7 @@ struct wg_peer_io {
uint64_tp_txbytes;
uint64_tp_rxbytes;
struct timespec p_last_handshake; /* nanotime */
+ charp_description[IFDESCRSIZE];
size_t p_aips_count;
struct wg_aip_iop_aips[];
};
Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.375
diff -u -p -u -p -r1.375 ifconfig.8
--- ifconfig.8 18 Aug 2021 18:10:33 - 1.375
+++ ifconfig.8 26 Oct 2021 23:01:34 -
@@ -2309,6 +2309,10 @@ Set the peer's IPv4 or IPv6
range for tunneled traffic.
Repeat the option to set multiple ranges.
By default, no addresses are allowed.
+.It Cm wgdesc Ar value
+Specify a description of the peer.
+.It Cm -wgdesc
+Clear the peer description.
.It Cm wgendpoint Ar peer_address port
Address traffic to the peer's IPv4 or IPv6
.Ar peer_address
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.445
diff -u -p -u -p -r1.445 ifconfig.c
--- ifconfig.c 6 Oct 2021 06:14:08 - 1.445
+++ ifconfig.c 26 Oct 2021 23:01:34 -
@@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
voidsetwgpeeraip(const char *, int);
voidsetwgpeerpsk(const char *, int);
voidsetwgpeerpka(const char *, int);
+void
Re: ifconfig description for wireguard peers
On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> Hi,
>
> While wireguard interfaces can have a description set by ifconfig, wireguard
> peers currently cannot. I now have a lot of peers and descriptions of them in
> ifconfig would be helpful.
>
> This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> corresponding '-wgdesc' option). Man page also updated.
>
> NM
This looks useful to me.
Did you get any feedback for this patch yet, Noah?
> Index: ifconfig.8
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.375
> diff -u -p -u -p -r1.375 ifconfig.8
> --- ifconfig.818 Aug 2021 18:10:33 - 1.375
> +++ ifconfig.821 Oct 2021 00:09:20 -
> @@ -2343,6 +2343,10 @@ It is optional but recommended and can b
> .Dl $ openssl rand -base64 32
> .It Cm -wgpsk
> Remove the pre-shared key for this peer.
> +.It Cm wgdesc Ar value
> +Specify a description of the peer.
> +.It Cm -wgdesc
> +Clear the peer description.
> .El
> .Sh EXAMPLES
> Assign the
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.445
> diff -u -p -u -p -r1.445 ifconfig.c
> --- ifconfig.c6 Oct 2021 06:14:08 - 1.445
> +++ ifconfig.c21 Oct 2021 00:09:20 -
> @@ -355,12 +355,14 @@ voidsetwgpeerep(const char *, const cha
> void setwgpeeraip(const char *, int);
> void setwgpeerpsk(const char *, int);
> void setwgpeerpka(const char *, int);
> +void setwgpeerdesc(const char *, int);
> void setwgport(const char *, int);
> void setwgkey(const char *, int);
> void setwgrtable(const char *, int);
>
> void unsetwgpeer(const char *, int);
> void unsetwgpeerpsk(const char *, int);
> +void unsetwgpeerdesc(const char *, int);
> void unsetwgpeerall(const char *, int);
>
> void wg_status();
> @@ -625,11 +627,13 @@ const structcmd {
> { "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
> { "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
> { "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
> + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
> { "wgport", NEXTARG,A_WIREGUARD,setwgport},
> { "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
> { "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
> { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
> { "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
> + { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
> { "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
>
> #else /* SMALL */
> @@ -5827,6 +5831,16 @@ setwgpeerpka(const char *pka, int param)
> }
>
> void
> +setwgpeerdesc(const char *wgdesc, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + if (strlen(wgdesc))
> + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
> + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> +}
> +
> +void
> setwgport(const char *port, int param)
> {
> const char *errmsg = NULL;
> @@ -5873,6 +5887,15 @@ unsetwgpeerpsk(const char *value, int pa
> }
>
> void
> +unsetwgpeerdesc(const char *value, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgpesc: wgpeer not set");
> + strlcpy(wg_peer->p_description, (const char *)"", IFDESCRSIZE);
> + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> +}
> +
> +void
> unsetwgpeerall(const char *value, int param)
> {
> ensurewginterface();
> @@ -5931,6 +5954,9 @@ wg_status(void)
> b64_ntop(wg_peer->p_public, WG_KEY_LEN,
> key, sizeof(key));
> printf("\twgpeer %s\n", key);
> +
> + if (strlen(wg_peer->p_description))
> + printf("\t\tdescription: %s\n", wg_peer->p_description);
>
> if (wg_peer->p_flags & WG_PEER_HAS_PSK)
> printf("\t\twgpsk (present)\n");
> Index: if_wg.c
> ===
> RCS file: /cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.18
> diff -u -p -u -p -r1.18 if_wg.c
> --- if_wg.c 5 Aug 2021 13:37:04 - 1.18
> +++ if_wg.c 21 Oct 2021 00:10:29 -
> @
ifconfig description for wireguard peers
Hi,
While wireguard interfaces can have a description set by ifconfig, wireguard
peers currently cannot. I now have a lot of peers and descriptions of them in
ifconfig would be helpful.
This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a corresponding
'-wgdesc' option). Man page also updated.
NM
Index: ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.375
diff -u -p -u -p -r1.375 ifconfig.8
--- ifconfig.8 18 Aug 2021 18:10:33 - 1.375
+++ ifconfig.8 21 Oct 2021 00:09:20 -
@@ -2343,6 +2343,10 @@ It is optional but recommended and can b
.Dl $ openssl rand -base64 32
.It Cm -wgpsk
Remove the pre-shared key for this peer.
+.It Cm wgdesc Ar value
+Specify a description of the peer.
+.It Cm -wgdesc
+Clear the peer description.
.El
.Sh EXAMPLES
Assign the
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.445
diff -u -p -u -p -r1.445 ifconfig.c
--- ifconfig.c 6 Oct 2021 06:14:08 - 1.445
+++ ifconfig.c 21 Oct 2021 00:09:20 -
@@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
void setwgpeeraip(const char *, int);
void setwgpeerpsk(const char *, int);
void setwgpeerpka(const char *, int);
+void setwgpeerdesc(const char *, int);
void setwgport(const char *, int);
void setwgkey(const char *, int);
void setwgrtable(const char *, int);
void unsetwgpeer(const char *, int);
void unsetwgpeerpsk(const char *, int);
+void unsetwgpeerdesc(const char *, int);
void unsetwgpeerall(const char *, int);
void wg_status();
@@ -625,11 +627,13 @@ const struct cmd {
{ "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
+ { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
+ { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
#else /* SMALL */
@@ -5827,6 +5831,16 @@ setwgpeerpka(const char *pka, int param)
}
void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ if (strlen(wgdesc))
+ strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
setwgport(const char *port, int param)
{
const char *errmsg = NULL;
@@ -5873,6 +5887,15 @@ unsetwgpeerpsk(const char *value, int pa
}
void
+unsetwgpeerdesc(const char *value, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgpesc: wgpeer not set");
+ strlcpy(wg_peer->p_description, (const char *)"", IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
unsetwgpeerall(const char *value, int param)
{
ensurewginterface();
@@ -5931,6 +5954,9 @@ wg_status(void)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+ if (strlen(wg_peer->p_description))
+ printf("\t\tdescription: %s\n", wg_peer->p_description);
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
Index: if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.18
diff -u -p -u -p -r1.18 if_wg.c
--- if_wg.c 5 Aug 2021 13:37:04 - 1.18
+++ if_wg.c 21 Oct 2021 00:10:29 -
@@ -222,6 +222,9 @@ struct wg_peer {
SLIST_ENTRY(wg_peer) p_start_list;
int p_start_onlist;
+
+ struct mutex p_description_mtx;
+ char p_description[IFDESCRSIZE];
};
struct wg_softc {
@@ -276,6 +279,7 @@ int wg_peer_get_sockaddr(struct wg_peer
void wg_peer_clear_src(struct wg_peer *);
void wg_peer_get_endpoint(struct wg_peer *, struct wg_endpoint *);
void wg_peer_counters_add(struct wg_peer *, uint64_t, uint64_t);
+void wg_peer_set_description(struct wg_peer *, char *);
intwg_aip_add(struct wg_softc *, struct wg_peer *, struct wg_aip_
