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.
>
> : 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 -0000 1.460
> : +++ sbin/ifconfig/ifconfig.c 24 Dec 2022 00:49:05 -0000
> : @@ -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
> : ===================================================================
> : RCS file: /cvs/src/sys/net/if_wg.c,v
> : retrieving revision 1.26
> : diff -u -p -u -r1.26 if_wg.c
> : --- sys/net/if_wg.c 21 Jul 2022 11:26:50 -0000 1.26
> : +++ sys/net/if_wg.c 24 Dec 2022 00:49:06 -0000
> : @@ -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,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 *);
> :
> : int wg_aip_add(struct wg_softc *, struct wg_peer *, struct
> wg_aip_io *);
> : struct wg_peer *
> : @@ -407,6 +411,9 @@ wg_peer_create(struct wg_softc *sc, uint
> : peer->p_counters_tx = 0;
> : peer->p_counters_rx = 0;
> :
> : + mtx_init(&peer->p_description_mtx, IPL_NET);
> : + memset(peer->p_description, 0, IFDESCRSIZE);
> : +
I used strlcpy with the empty string to be consistent with how it is cleared
in other hunks and clarify that this is always a string.
> : mtx_init(&peer->p_endpoint_mtx, IPL_NET);
> : bzero(&peer->p_endpoint, sizeof(peer->p_endpoint));
> :
> : @@ -581,6 +588,15 @@ wg_peer_counters_add(struct wg_peer *pee
> : mtx_leave(&peer->p_counters_mtx);
> : }
> :
> : +void
> : +wg_peer_set_description(struct wg_peer *peer, char *description)
> : +{
> : + mtx_enter(&peer->p_description_mtx);
> : + memset(peer->p_description, 0, IFDESCRSIZE);
> : + strlcpy(peer->p_description, description, IFDESCRSIZE);
memset is not needed here.
> : + mtx_leave(&peer->p_description_mtx);
> : +}
> : +
> : int
> : wg_aip_add(struct wg_softc *sc, struct wg_peer *peer, struct wg_aip_io *d)
> : {
> : @@ -2320,6 +2336,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);
> : + }
> : +
Dropped brackets to be consistent with code around this block.
> : 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 +2449,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 -u -r1.4 if_wg.h
> : --- sys/net/if_wg.h 22 Jun 2020 12:20:44 -0000 1.4
> : +++ sys/net/if_wg.h 24 Dec 2022 00:49:06 -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[];
> : };
> :
> : --
> : Regards,
> : Mikolaj
> :
>
Index: sys/net/if_wg.c
===================================================================
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.26
diff -u -p -r1.26 if_wg.c
--- sys/net/if_wg.c 21 Jul 2022 11:26:50 -0000 1.26
+++ sys/net/if_wg.c 23 May 2023 18:37:52 -0000
@@ -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,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 *, const char *);
int wg_aip_add(struct wg_softc *, struct wg_peer *, struct wg_aip_io *);
struct wg_peer *
@@ -407,6 +411,9 @@ wg_peer_create(struct wg_softc *sc, uint
peer->p_counters_tx = 0;
peer->p_counters_rx = 0;
+ mtx_init(&peer->p_description_mtx, IPL_NET);
+ strlcpy(peer->p_description, "", IFDESCRSIZE);
+
mtx_init(&peer->p_endpoint_mtx, IPL_NET);
bzero(&peer->p_endpoint, sizeof(peer->p_endpoint));
@@ -581,6 +588,14 @@ wg_peer_counters_add(struct wg_peer *pee
mtx_leave(&peer->p_counters_mtx);
}
+void
+wg_peer_set_description(struct wg_peer *peer, const char *description)
+{
+ mtx_enter(&peer->p_description_mtx);
+ strlcpy(peer->p_description, description, IFDESCRSIZE);
+ mtx_leave(&peer->p_description_mtx);
+}
+
int
wg_aip_add(struct wg_softc *sc, struct wg_peer *peer, struct wg_aip_io *d)
{
@@ -2320,6 +2335,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 = &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 +2447,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 23 May 2023 18:41:24 -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 23 May 2023 18:49:49 -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 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 23 May 2023 18:40:47 -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");
