Re: move the pf_state_tree type around

[Alexandr Nedvedicky]

Hello,

On Tue, Jan 03, 2023 at 03:31:42PM +1000, David Gwynne wrote:
> the pf_state_tree type represents the rb tree that pf_state_key structs
> go into. currently pf_state_key is declared in pfvar_priv.h (because
> it's a kernel private data structure) but pf_state_tree was left in
> pfvar.h. this moves it to pfvar_priv.h, because it is also kernel
> private.
> 
> while here, this moves it from the RB tree macros to RBT which shrinks
> pf.o by about 13k on amd64.

diff reads OK to me. Do I assume right 'tree_id' tree will
be also moved from RB_*() to RBT_() in another commit?

thanks and OK sashan

move the pf_state_tree type around

[David Gwynne]

the pf_state_tree type represents the rb tree that pf_state_key structs
go into. currently pf_state_key is declared in pfvar_priv.h (because
it's a kernel private data structure) but pf_state_tree was left in
pfvar.h. this moves it to pfvar_priv.h, because it is also kernel
private.

while here, this moves it from the RB tree macros to RBT which shrinks
pf.o by about 13k on amd64.

ok?

Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1165
diff -u -p -r1.1165 pf.c
--- pf.c2 Jan 2023 05:32:40 -   1.1165
+++ pf.c3 Jan 2023 04:16:25 -
@@ -303,8 +303,8 @@ struct pf_pool_limit pf_pool_limits[PF_L
} while (0)
 
 static __inline int pf_src_compare(struct pf_src_node *, struct pf_src_node *);
-static __inline int pf_state_compare_key(struct pf_state_key *,
-   struct pf_state_key *);
+static inline int pf_state_compare_key(const struct pf_state_key *,
+   const struct pf_state_key *);
 static __inline int pf_state_compare_id(struct pf_state *,
struct pf_state *);
 #ifdef INET6
@@ -319,12 +319,13 @@ struct pf_state_tree_id tree_id;
 struct pf_state_list pf_state_list = PF_STATE_LIST_INITIALIZER(pf_state_list);
 
 RB_GENERATE(pf_src_tree, pf_src_node, entry, pf_src_compare);
-RB_GENERATE(pf_state_tree, pf_state_key, sk_entry, pf_state_compare_key);
+RBT_GENERATE(pf_state_tree, pf_state_key, sk_entry, pf_state_compare_key);
 RB_GENERATE(pf_state_tree_id, pf_state,
 entry_id, pf_state_compare_id);
 
-__inline int
-pf_addr_compare(struct pf_addr *a, struct pf_addr *b, sa_family_t af)
+int
+pf_addr_compare(const struct pf_addr *a, const struct pf_addr *b,
+sa_family_t af)
 {
switch (af) {
case AF_INET:
@@ -689,8 +690,9 @@ pf_state_rm_src_node(struct pf_state *s,
 
 /* state table stuff */
 
-static __inline int
-pf_state_compare_key(struct pf_state_key *a, struct pf_state_key *b)
+static inline int
+pf_state_compare_key(const struct pf_state_key *a,
+const struct pf_state_key *b)
 {
int diff;
 
@@ -743,7 +745,7 @@ pf_state_key_attach(struct pf_state_key 
 
KASSERT(s->key[idx] == NULL);
sk->sk_removed = 0;
-   cur = RB_INSERT(pf_state_tree, _statetbl, sk);
+   cur = RBT_INSERT(pf_state_tree, _statetbl, sk);
if (cur != NULL) {
sk->sk_removed = 1;
/* key exists. check for same kif, if none, add to key */
@@ -798,7 +800,7 @@ pf_state_key_attach(struct pf_state_key 
if ((si = pool_get(_state_item_pl, PR_NOWAIT)) == NULL) {
if (TAILQ_EMPTY(>sk_states)) {
KASSERT(cur == NULL);
-   RB_REMOVE(pf_state_tree, _statetbl, sk);
+   RBT_REMOVE(pf_state_tree, _statetbl, sk);
sk->sk_removed = 1;
pf_state_key_unref(sk);
}
@@ -856,7 +858,7 @@ pf_state_key_detach(struct pf_state *s, 
pool_put(_state_item_pl, si);
 
if (TAILQ_EMPTY(>sk_states)) {
-   RB_REMOVE(pf_state_tree, _statetbl, sk);
+   RBT_REMOVE(pf_state_tree, _statetbl, sk);
sk->sk_removed = 1;
pf_state_key_unlink_reverse(sk);
pf_state_key_unlink_inpcb(sk);
@@ -1165,7 +1167,7 @@ pf_find_state(struct pf_pdesc *pd, struc
}
 
if (sk == NULL) {
-   if ((sk = RB_FIND(pf_state_tree, _statetbl,
+   if ((sk = RBT_FIND(pf_state_tree, _statetbl,
(struct pf_state_key *)key)) == NULL)
return (PF_DROP);
if (pd->dir == PF_OUT && pkt_sk &&
@@ -1220,7 +1222,7 @@ pf_find_state_all(struct pf_state_key_cm
 
pf_status.fcounters[FCNT_STATE_SEARCH]++;
 
-   sk = RB_FIND(pf_state_tree, _statetbl, (struct pf_state_key *)key);
+   sk = RBT_FIND(pf_state_tree, _statetbl, (struct pf_state_key *)key);
 
if (sk != NULL) {
TAILQ_FOREACH(si, >sk_states, si_entry) {
Index: pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.393
diff -u -p -r1.393 pf_ioctl.c
--- pf_ioctl.c  21 Dec 2022 02:23:10 -  1.393
+++ pf_ioctl.c  3 Jan 2023 04:16:26 -
@@ -1758,7 +1758,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
key.port[sidx] = psk->psk_src.port[0];
key.port[didx] = psk->psk_dst.port[0];
 
-   sk = RB_FIND(pf_state_tree, _statetbl, );
+   sk = RBT_FIND(pf_state_tree, _statetbl,
+   );
if (sk == NULL)
continue;
 
Index: pfvar.h
===
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.525
diff -u -p -r1.525 pfvar.h
--- 

Re: Making wscons a bit more like xterm

[Crystal Kolipe]

Here is an updated version of this patch.

Unlike the first version which just added some new control sequences in order
to fix the display, this also touches some input code.

New in this version:

* F1 - F4 now send different sequences.
* F13 - F24 now send different sequences, but see notes below about F13 - F16.
* With numlock OFF, keypad 5 is now 'begin' rather than unused.
* Home, End, keypad home, and keypad end now send different sequences.
* A new keysym has been added - KS_Backtab.
* Shift-TAB is now defined as KS_Backtab and sends ESC [ Z.
* Shift-F1 - Shift-F12 now send F13 - F24.

Details:

* F1 - F24 now send different sequences

F1 - F4 and F17 - F24 now send the sequences expected by the xterm terminfo
entry.  Those for F1 - F4 are the same as the PF1 - PF4 sequences.  The
sequences previously used for F1 - F4 did not correspond to a particular
hardware terminal.

F13 - F16 are interesting.  By default, a real xterm running on OpenBSD does
not get the sequences it wants for these keys:

F13, for example, sends 'ESC O 2 P'.  But the terminfo wants 'ESC [ 1 ; 2 P'.

The codes that xterm actually sends for these keys by default on OpenBSD are
the codes defined in xterm-xf86-v40 and xterm+pcf0, but by using the plain
xterm entry, we're expecting the codes for xterm+pcf2.

So F13 - F16 have long been broken in xterm, and nobody has complained.

F13 and F14 currently work in vt220, but F15 and F16 don't.  A real hardware
vt220 doesn't have keys marked F15 and F16, instead they are marked help and
do.  This is reflected in the vt220 terminfo entry.

F13 - F16 do currently work in pccon, (without my patch).

My patch changes wscons to send the codes that will be recognised by the
xterm terminfo entry, making this aspect of TERM=xterm work on the console.

So the only users who might be disappointed will be those who currently rely
on the buggy behaviour of a real xterm running with the OpenBSD default
settings, and who hope or expect to now see this same buggy behaviour on the
console.

* With numlock OFF, keypad 5 is now 'begin' rather than being unused.

Previously, KS_Begin was a NOP.  However, the xterm terminfo entry includes
a sequence for keypad begin, so I've made wscons send that sequence.

This matches the behaviour of a real xterm, so anybody actually using this key
in a program should be happy that it now works on the console.

* Home, End, keypad home, and keypad end now send different sequences.

These codes have been changed to match the xterm terminfo entry.

The vt220 terminfo entry does not recognise the old codes for home and end, so
only users using pccon or a similar terminal type would have been able to use
it in programs that parse terminfo.  Now it works fine on the console with
TERM=xterm.

* A new keysym has been added - KS_Backtab, mapped to Shift-TAB

X sees shift-tab as ISO_Left_Tab, and xterm maps this to ESC [ Z, which the
terminfo interprets as Backtab.

With my patch, a new keysym KS_Backtab is defined, and the console also
generates the sequence ESC [ Z.  This is then also mapped to Backtab.

* Shift-F1 - Shift-F12 now send F13 - F24.

This matches what X does, and also makes testing F13 - F24 possible for the
majority of users who don't have a keyboard with these physical keys.


* Issues remaining *


The main area left in which the console does not match a real xterm in terms
of keyboard input is in _application mode_.

Note to those testing this: when switching between the console and X, the
numlock LED on the keyboard may become out of sync with the actual status of
numlock on the system.  So toggle it before doing further tests.

In xterm, with keypad application mode ON, (DECKPAM), and numlock OFF:
 * The keypad keys work as their cursor movement counterparts on the main
   keyboard, I.E. they send the sequences for home, end, page up, etc.
 * The keypad - key sends ESC O m, like a real vt220 
 * The keypad / * and + keys send ESC O o, ESC O j, and ESC O k, but these do
   not correspond to a real vt220, because the real vt220 didn't have these
   keys.

In xterm, with keypad application mode ON, (DECKPAM), and numlock ON:
 * The keypad works the same as in non-application mode, I.E. numbers produce
   numbers, and / * - and + produce / * - and +.

This means that with a real xterm in our current default configuration, you
_cannot_ get distinct sequences for the keypad keys.

On the console, with keypad application mode ON, (DECKPAM), and numlock OFF:
 * The keypad keys behave the same as described above for xterm in this
   situation.

On the console, with keypad application mode ON, (DECKPAM), and numlock ON:
 * The keypad generates sequences consistent with a real vt220 for keys that
   exist on a real vt220, and ones in a similar vein for those that don't.

So on the console, you currently can get distinct sequences.

This is a bit awkward, because even though the distinct sequences generated in
application mode 

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

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");
> + 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_PEER_HAS_PSK)
>   printf("\t\twgpsk (present)\n");
> Index: share/man/man4/wg.4
> 

Re: ESRT support for the amd64 bootloader

[Peter Stuge]

Mark Kettenis wrote:
> patrick@, mlarkin@, yasuoka@ and other devs: ok?

Do what you have to do, but I plead to avoid (U)EFI lock-in as far as
technically possible, since most (U)EFI implementations are not open
source, and that does exist.

I understand too well that (U)EFI is the modern popular firmware
interface and thus the only choice sometimes, just please do not
indulge here.


Thanks and kind regards

//Peter

Suggested change to ports.7 man page

[Jon Fineman]

I was trying to make clean a port I was updating and it wasn't clear 
to me that I should be adding the FLAVOR variable to the make line to 
clean the correct build.


My suggested change below might help other people.

Jon



Index: ports.7
===
RCS file: /cvs/src/share/man/man7/ports.7,v
retrieving revision 1.130
diff -u -p -r1.130 ports.7
--- ports.7 20 Jul 2022 16:37:50 -  1.130
+++ ports.7 2 Jan 2023 11:38:27 -
@@ -324,6 +324,9 @@ This does not recurse to dependencies un
 .Ev CLEANDEPENDS
 is defined to
 .Dv Yes .
+To clean a build of
+.Cm FLAVORS
+you need to specify all of the flavors you built with.
 .It Cm distclean
 Remove the port's distfile(s).
 This does not recurse to dependencies.

Re: [RFC resend v1 2/2] Use arc4random_range() instead of arc4random_uniform() when appropriate

[Alejandro Colomar]

On 1/1/23 01:08, Alejandro Colomar wrote:



On 12/31/22 20:08, Alejandro Colomar wrote:

This makes the code much more readable and self-documented.  While doing
this, I noticed a few bugs, and other cases which may be bugs or not.
Switching to this specialized API makes it easier to spot such bugs, but
since I'm not familiar with the code, I kept some bugs unfixed.  The
most obvious ones (although I may be wrong) I fixed them.  And in some
cases where it was very unclear, I didn't touch the old *_uniform() code.

Below are the cases where I changed the behavior (I considered it a bug):

*  usr.bin/ssh/auth.c:

    -  *cp = hashchars[arc4random_uniform(sizeof(hashchars) - 1)];
    +  *cp = hashchars[arc4random_range(0, sizeof(hashchars) - 1)];


Reconsidering, this one is probably better just as 
arc4random_uniform(sizeof(hashchars)).


I was also wrong here.
I was confused by the implicit strlen() calculation with sizeof()-1, whose -1 
was cancelled by the +1.



--



OpenPGP_signature
Description: OpenPGP digital signature

Re: athn auto media type (Was: Re: Explicit NULL assignment after free)

[Stefan Sperling]

On Sun, Jan 01, 2023 at 10:37:44PM +, Ali Farzanrad wrote:
> OK, I started in a working state where I was connected to my Android
> phone:

OK, let's untangle this... see below...

> # cat /etc/hostname.athn0
> nwid [..name..] wpakey [..pass..]
> inet autoconf

Before you get to "status: active" the interface was scanning for APs.
While scanning, it did already behave just as you describe below...

> # ifconfig athn0
> athn0: flags=808843 mtu 1500
>   lladdr [..lladdr..]
>   index 1 priority 4 llprio 3
>   groups: wlan egress
>   media: IEEE802.11 autoselect (HT-MCS7 mode 11n)
>   status: active
>   ieee80211: nwid [..name..] chan 9 bssid [..bssid..] -45dBm wpakey 
> wpaprotos wpa2 wpaakms psk wpaciphers ccmp wpagroupcipher ccmp
>   inet 192.168.131.83 netmask 0xff00 broadcast 192.168.131.255
> 
> then I simply re-run netstart:
> 
> # sh /etc/netstart athn0
> # ifconfig athn0
> athn0: flags=808843 mtu 1500
>   lladdr [..lladdr..]
>   index 1 priority 4 llprio 3
>   groups: wlan egress
>   media: IEEE802.11 autoselect (DS1 mode 11b)
>   status: no network

... here.

The interface is not connected to any AP (status: no network) so it will
keep scanning. While scanning, the interface will switch between modes,
because the 11b/g and 11a bands require separate sets of channels to
be scanned. To implement switching between channel sets the net80211 stack
will keep switching modes internally. This shows up on the ifconfig media
line as a side-effect. But this unrelated to Tx rates used for data frames
because no data frames will even be sent before the interface is associated.

So now, you are in 11b mode and scanning...

>   ieee80211: nwid [..name..] wpakey wpaprotos wpa2 wpaakms psk wpaciphers 
> ccmp wpagroupcipher ccmp
>   inet 192.168.131.83 netmask 0xff00 broadcast 192.168.131.255
> 
> [..after some time..]
> 
> # ifconfig athn0
> athn0: flags=808843 mtu 1500
>   lladdr [..lladdr..]
>   index 1 priority 4 llprio 3
>   groups: wlan egress
>   media: IEEE802.11 autoselect (DS1 mode 11g)
>   status: no network

... and here, you are in 11g mode, but still not associated, sill scanning.

The real problem is that your AP seems to be disappearing.
Your phone might be switching between WLAN channels, or it might be
turning the AP off for some reason. In either case the AP will have
disappeared from the client's point of view.

You could try:
ifconfig athn0 debug
tail -f /var/log/messages

Hopefully the additional debug info now written to /var/log/messages will
help you figure out why the AP is not being seen anymore.

>   ieee80211: nwid [..name..] chan 9 bssid [..bssid..] -100dBm wpakey 
> wpaprotos wpa2 wpaakms psk wpaciphers ccmp wpagroupcipher ccmp
>   inet 192.168.131.83 netmask 0xff00 broadcast 192.168.131.255
> 
> [..after some time..]
> 
> # ifconfig athn0
> athn0: flags=808843 mtu 1500
>   lladdr [..lladdr..]
>   index 1 priority 4 llprio 3
>   groups: wlan egress
>   media: IEEE802.11 autoselect (DS1)
>   status: no network
>   ieee80211: nwid [..name..] wpakey wpaprotos wpa2 wpaakms psk wpaciphers 
> ccmp wpagroupcipher ccmp
>   inet 192.168.131.83 netmask 0xff00 broadcast 192.168.131.255
> 
> and it repeats showing different modes but always in DS1 media type.
> 2 days ago I waited for hours, but it didn't find correct media type.
> This time even after reboot it didn't find correct media type, so I
> forced to turn my phone's Internet sharing off and on.
> 
> I wanted to find the problem by myself but unfortunately I don't know
> how :(
>