On Wed, May 31, 2023 at 10:27:13AM +0200, Claudio Jeker wrote:
> On Tue, May 30, 2023 at 11:56:01PM +0000, Klemens Nanni wrote:
> > On Tue, May 23, 2023 at 07:13:28PM +0000, Klemens Nanni wrote:
> > > On Sat, Jan 14, 2023 at 02:28:27PM +0000, 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=163478298600001&r=1&w=2
> > > > >
> > > > > Last version of the diff at
> > > > > https://marc.info/?l=openbsd-tech&m=167185582521873&q=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 -0000 1.27
+++ sys/net/if_wg.c 31 May 2023 12:41:50 -0000
@@ -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(&peer->p_endpoint_mtx, IPL_NET);
bzero(&peer->p_endpoint, sizeof(peer->p_endpoint));
@@ -581,6 +585,11 @@ wg_peer_counters_add(struct wg_peer *pee
mtx_leave(&peer->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 = &peer_p->p_aips[0];
for (j = 0; j < peer_o.p_aips_count; j++) {
if ((ret = copyin(aip_p, &aip_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(&peer_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 -0000 1.4
+++ sys/net/if_wg.h 29 May 2023 22:20:02 -0000
@@ -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_t p_txbytes;
uint64_t p_rxbytes;
struct timespec p_last_handshake; /* nanotime */
+ char p_description[IFDESCRSIZE];
size_t p_aips_count;
struct wg_aip_io p_aips[];
};
Index: sbin/ifconfig/ifconfig.8
===================================================================
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.395
diff -u -p -r1.395 ifconfig.8
--- sbin/ifconfig/ifconfig.8 16 May 2023 14:32:54 -0000 1.395
+++ sbin/ifconfig/ifconfig.8 30 May 2023 23:38:33 -0000
@@ -2316,6 +2316,7 @@ Packets on a VLAN interface without a ta
.Op Fl wgpeerall
.Oo
.Oo Fl Oc Ns Cm wgpeer Ar publickey
+.Op Oo Fl Oc Ns Cm wgdescr Ns Oo Cm iption Oc Ar value
.Op Cm wgaip Ar allowed-ip_address/prefix
.Op Cm wgendpoint Ar peer_address port
.Op Cm wgpka Ar interval
@@ -2383,6 +2384,13 @@ Peer configuration options, which apply
immediately preceding them,
are as follows:
.Bl -tag -width Ds
+.Tg wgdescription
+.It Cm wgdescr Ns Oo Cm iption Oc Ar value
+Set the peer's description.
+This can be used to label peers in situations where they may
+otherwise be difficult to distinguish.
+.It Cm -wgdescr Ns Op Cm iption
+Clear the peer description.
.It Cm wgaip Ar allowed-ip_address/prefix
Set the peer's IPv4 or IPv6
.Ar allowed-ip_address
Index: sbin/ifconfig/ifconfig.c
===================================================================
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.464
diff -u -p -r1.464 ifconfig.c
--- sbin/ifconfig/ifconfig.c 16 May 2023 14:32:54 -0000 1.464
+++ sbin/ifconfig/ifconfig.c 29 May 2023 22:20:02 -0000
@@ -351,6 +351,7 @@ void transceiverdump(const char *, int);
/* WG */
void setwgpeer(const char *, int);
+void setwgpeerdesc(const char *, int);
void setwgpeerep(const char *, const char *);
void setwgpeeraip(const char *, int);
void setwgpeerpsk(const char *, int);
@@ -360,6 +361,7 @@ void setwgkey(const char *, int);
void setwgrtable(const char *, int);
void unsetwgpeer(const char *, int);
+void unsetwgpeerdesc(const char *, int);
void unsetwgpeerpsk(const char *, int);
void unsetwgpeerall(const char *, int);
@@ -619,6 +621,8 @@ const struct cmd {
{ "sffdump", 0, 0, transceiverdump },
{ "wgpeer", NEXTARG, A_WIREGUARD, setwgpeer},
+ { "wgdescription", NEXTARG, A_WIREGUARD, setwgpeerdesc},
+ { "wgdescr", NEXTARG, A_WIREGUARD, setwgpeerdesc},
{ "wgendpoint", NEXTARG2, A_WIREGUARD, NULL, setwgpeerep},
{ "wgaip", NEXTARG, A_WIREGUARD, setwgpeeraip},
{ "wgpsk", NEXTARG, A_WIREGUARD, setwgpeerpsk},
@@ -627,7 +631,8 @@ const struct cmd {
{ "wgkey", NEXTARG, A_WIREGUARD, setwgkey},
{ "wgrtable", NEXTARG, A_WIREGUARD, setwgrtable},
{ "-wgpeer", NEXTARG, A_WIREGUARD, unsetwgpeer},
- { "-wgpsk", 0, A_WIREGUARD, unsetwgpeerpsk},
+ { "-wgdescription", 0, A_WIREGUARD, unsetwgpeerdesc},
+ { "-wgdescr", 0, A_WIREGUARD, unsetwgpeerdesc},
{ "-wgpeerall", 0, A_WIREGUARD, unsetwgpeerall},
#else /* SMALL */
@@ -5736,6 +5741,15 @@ setwgpeer(const char *peerkey_b64, int p
}
void
+setwgpeerdesc(const char *descr, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdescr: wgpeer not set");
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+ strlcpy(wg_peer->p_description, descr, IFDESCRSIZE);
+}
+
+void
setwgpeeraip(const char *aip, int param)
{
int res;
@@ -5839,6 +5853,15 @@ unsetwgpeer(const char *peerkey_b64, int
}
void
+unsetwgpeerdesc(const char *descr, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdescr: wgpeer not set");
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+ strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+}
+
+void
unsetwgpeerpsk(const char *value, int param)
{
if (wg_peer == NULL)
@@ -5907,6 +5930,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\twgdescr: %s\n",
wg_peer->p_description);
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
