Re: IKEv2 tunnel crash when sec(4) pushed with large data

2023-08-14 Thread Jason Tubnor

On Mon, Aug 14, 2023 at 02:07:12AM +, Jason Tubnor wrote:
>> Hi,


> Not sure how this can happen. Have you destroyed and recreated the interface
> in between? Can you easily reproduce this?

No I didn't it just seem to drop. It happened twice yesterday but I have, even 
under continous load, not been able to get it to error again.

> I have added a bit more info to the error message, it now also prints the
> iface id and the errno. It would be useful if you can reproduce it with those.

Thanks and patched across all 3 machines.

Below are metrics/results from today's testing:

Spoke A:

CPU0 states:  1.0% user,  0.0% nice, 27.5% sys,  5.9% spin,  2.9% intr, 62.7% 
idle
CPU1 states:  0.0% user,  0.0% nice, 26.5% sys,  4.1% spin,  0.0% intr, 69.4% 
idle
  PID USERNAME PRI NICE  SIZE   RES STATE WAIT  TIMECPU COMMAND
45123 root   20 1780K 2864K sleep/0   kqread8:39 20.75% iperf3
 |
 |
\|/
Hub:

CPU0 states:  0.0% user,  0.0% nice, 37.4% sys, 16.2% spin,  1.0% intr, 45.5% 
idle
CPU1 states:  0.0% user,  0.0% nice, 40.2% sys, 10.8% spin,  0.0% intr, 49.0% 
idle
 |
 |
\|/
Spoke B:

CPU0 states:  0.0% user,  0.0% nice, 23.0% sys,  1.0% spin,  6.0% intr, 70.0% 
idle
CPU1 states:  0.0% user,  0.0% nice, 30.4% sys,  2.0% spin,  0.0% intr, 67.6% 
idle
  PID USERNAME PRI NICE  SIZE   RES STATE WAIT  TIMECPU COMMAND
32297 _iperf320 1776K 2688K sleep/0   kqread4:24 10.55% iperf3

Performance of sending Spoke A through to Spoke B via Hub (iperf3 -t 3600):

[ ID] Interval   Transfer Bitrate Retr
[  5]   0.00-3600.00 sec   117 GBytes   280 Mbits/sec   85 sender
[  5]   0.00-3600.01 sec   117 GBytes   280 Mbits/sec  receiver

Performance of sending Spoke A to Hub (iperf3 -t 300):

[ ID] Interval   Transfer Bitrate Retr
[  5]   0.00-300.00 sec  37.9 GBytes  1.08 Gbits/sec   17 sender
[  5]   0.00-300.00 sec  37.9 GBytes  1.08 Gbits/sec  receiver

Hub load while under iperf3 test with more throughput above:

CPU0 states:  0.0% user,  0.0% nice, 47.0% sys,  6.0% spin,  1.0% intr, 46.0% 
idle
CPU1 states:  0.0% user,  0.0% nice, 46.6% sys,  4.9% spin,  0.0% intr, 48.5% 
idle
Memory: Real: 23M/273M act/tot Free: 176M Cache: 89M Swap: 22M/256M

  PID USERNAME PRI NICE  SIZE   RES STATE WAIT  TIMECPU COMMAND
 1205 _iperf320 1016K 1736K sleep/1   kqread0:05 11.43% iperf3



Is there a way to improve the forwarding speed at the hub? Using the same IKEv2 
configuration with a VXLAN/VEB/VPORT configuration, I can get ~2.5x the 
performance from spoke to spoke. Spoke to Hub performance is on par.

Cheers


Re: installer: disk crypto: crank KDF rounds to hardware based default

2023-08-14 Thread Lucas
Klemens Nanni  wrote:
> @@ -1117,13 +1117,6 @@ bio_changepass(char *dev)
>  
>   /* Current passphrase. */
>   bio_kdf_derive(, , "Old passphrase: ", 0);
> -
> - /*
> -  * Unless otherwise specified, keep the previous number of rounds as
> -  * long as we're using the same KDF.
> -  */
> - if (kdfhint.generic.type == SR_CRYPTOKDFT_BCRYPT_PBKDF && !rflag)
> - rflag = kdfhint.rounds;
>  
>   /* New passphrase. */
>   bio_kdf_generate();

This will potentially downgrade the amount of rounds on password change
if `-r` is omitted, which is not ideal imo. What about the following to
keep the previous amount of rounds if its bigger than the automatic
estimate?

-Lucas


diff refs/heads/master 758422c5a8c4e618082a6dc3dc0f268ed05e9cd9
commit - d4b9d4747036fa562b886f23a67e486ba94b3dc6
commit + 758422c5a8c4e618082a6dc3dc0f268ed05e9cd9
blob - d6617b14595e278f687a9f114767438f5fe51326
blob + 951df4da4db2e69c058a2bcb0d460543b602cc7a
--- sbin/bioctl/bioctl.8
+++ sbin/bioctl/bioctl.8
@@ -282,11 +282,12 @@ If
 passphrase of an existing encrypted volume.
 A larger number of iterations takes more time, but offers increased resistance
 against passphrase guessing attacks.
-If
+By default, or if
 .Ar rounds
-is specified as "auto", the number of rounds will be automatically determined
-based on system performance.
-Otherwise the minimum is 4 rounds and the default is 16.
+is specified as
+.Cm auto ,
+the number of rounds will automatically be based on system performance.
+The minimum is 16 rounds.
 .It Fl s
 Read the passphrase for the selected crypto volume from
 .Pa /dev/stdin
blob - 2928cfba3d52f5f3a4c6589d4e363e09f6da30d4
blob + ba4a15bab4d8d1ac1211aec9a6c315bfb6f29bb6
--- sbin/bioctl/bioctl.c
+++ sbin/bioctl/bioctl.c
@@ -66,7 +66,7 @@ void  bio_kdf_generate(struct 
sr_crypto_kdfinfo *);
 intbio_parse_devlist(char *, dev_t *);
 void   bio_kdf_derive(struct sr_crypto_kdfinfo *,
struct sr_crypto_pbkdf *, char *, int);
-void   bio_kdf_generate(struct sr_crypto_kdfinfo *);
+void   bio_kdf_generate(struct sr_crypto_kdfinfo *, int);
 intbcrypt_pbkdf_autorounds(void);
 void   derive_key(u_int32_t, int, u_int8_t *, size_t,
u_int8_t *, size_t, char *, int);
@@ -89,7 +89,7 @@ int   rflag = 0;
 inthuman;
 intverbose;
 u_int32_t  cflags = 0;
-intrflag = 0;
+intrflag = -1; /* auto */
 char   *password;
 
 void   *bio_cookie;
@@ -182,7 +182,7 @@ main(int argc, char *argv[])
rflag = -1;
break;
}
-   rflag = strtonum(optarg, 4, 1<<30, );
+   rflag = strtonum(optarg, 16, 1<<30, );
if (errstr != NULL)
errx(1, "number of KDF rounds is %s: %s",
errstr, optarg);
@@ -902,7 +902,7 @@ bio_createraid(u_int16_t level, char *dev_list, char *
bio_kdf_derive(, , "Passphrase: ", 0);
memset(, 0, sizeof(kdfhint));
} else {
-   bio_kdf_generate();
+   bio_kdf_generate(, -1);
}
 
create.bc_opaque = 
@@ -968,17 +968,20 @@ bio_kdf_generate(struct sr_crypto_kdfinfo *kdfinfo)
 }
 
 void
-bio_kdf_generate(struct sr_crypto_kdfinfo *kdfinfo)
+bio_kdf_generate(struct sr_crypto_kdfinfo *kdfinfo, int hint_rounds)
 {
if (!kdfinfo)
errx(1, "invalid KDF info");
 
-   if (rflag == -1)
+   if (rflag == -1) {
rflag = bcrypt_pbkdf_autorounds();
+   if (rflag < hint_rounds)
+   rflag = hint_rounds;
+   }
 
kdfinfo->pbkdf.generic.len = sizeof(kdfinfo->pbkdf);
kdfinfo->pbkdf.generic.type = SR_CRYPTOKDFT_BCRYPT_PBKDF;
-   kdfinfo->pbkdf.rounds = rflag ? rflag : 16;
+   kdfinfo->pbkdf.rounds = rflag;
 
kdfinfo->flags = SR_CRYPTOKDF_KEY | SR_CRYPTOKDF_HINT;
kdfinfo->len = sizeof(*kdfinfo);
@@ -1097,7 +1100,7 @@ bio_changepass(char *dev)
struct sr_crypto_kdfpair kdfpair;
struct sr_crypto_kdfinfo kdfinfo1, kdfinfo2;
struct sr_crypto_pbkdf kdfhint;
-   int rv;
+   int rv, hint_rounds = -1;
 
memset(, 0, sizeof(bd));
memset(, 0, sizeof(kdfhint));
@@ -1119,14 +1122,14 @@ bio_changepass(char *dev)
bio_kdf_derive(, , "Old passphrase: ", 0);
 
/*
-* Unless otherwise specified, keep the previous number of rounds as
-* long as we're using the same KDF.
+* Broadcast the previous number of rounds as long as we're using the
+* same KDF.
 */
-   if 

Re: Have sysupgrade run fw_update -vv

2023-08-14 Thread Mikhail
On Sun, Aug 13, 2023 at 10:02:26PM -0700, Kevin Williams wrote:
> What about giving sysupgrade the same verbosity options as fw_update?
> This would mean -v and -vv I sysupgrade would pass through the same
> options to fw_update.
>
> And for the installer, maybe another question: “Verbose download &
> install progress?” Or no, as default.

I think it's overengineering for such simple task.

Regarding installer - probably simple notice about what's happening will
be enough too - user will either understand that some network activity
in process or will get immediate message that installer can't resolve
whatever etc.

Proposed patch:

diff /usr/src
commit - 8afcf90fb39e4a84606e93137c2b6c20f44312cb
path + /usr/src
blob - 4386ec9873c433a99fa83b9a9091c06bd952
file + distrib/miniroot/install.sub
--- distrib/miniroot/install.sub
+++ distrib/miniroot/install.sub
@@ -3008,7 +3008,9 @@ __EOT
 fi
 __EOT
 
-   [ -x /mnt/usr/sbin/fw_update ] && DESTDIR=/mnt /mnt/usr/sbin/fw_update
+   [ -x /mnt/usr/sbin/fw_update ] &&
+   echo "Downloading firmware..." &&
+   DESTDIR=/mnt /mnt/usr/sbin/fw_update
 
if [[ -f $_kernel_dir.tgz ]]; then
echo -n "Relinking to create unique kernel..."



Re: kqueue(2) and close-on-exec

2023-08-14 Thread Visa Hankala
On Sun, Aug 13, 2023 at 05:58:01PM -0700, Philip Guenther wrote:
> I think that changing the behavior of the existing API in a way that
> gratuitously increases the differences between BSDs is unwise.  IMHO, we
> should follow NetBSD on this and add kqueue1(), that being obviously
> consistent with the previous 'add flags argument' extensions: pipe2(),
> dup3(), and accept4().

I agree that changing the behavior is not good.

Below is an implementation of the kqueue1() syscall and the libc stub.

NetBSD's kqueue1() accepts flags that do not really affect kqueue
behavior, namely O_NONBLOCK and O_NOSIGPIPE (this is specific to
NetBSD). For compatibility, the new syscall code passes O_NONBLOCK to
file flags but otherwise ignores it. I have omitted this flag from the
syscall manual page because it does not make sense with kqueue.
However, I can add a note about the flag if it is deemed necessary.

Index: sys/kern/kern_event.c
===
RCS file: src/sys/kern/kern_event.c,v
retrieving revision 1.197
diff -u -p -r1.197 kern_event.c
--- sys/kern/kern_event.c   13 Aug 2023 08:29:28 -  1.197
+++ sys/kern/kern_event.c   14 Aug 2023 14:52:44 -
@@ -60,6 +60,7 @@
 #define KLIST_ASSERT_LOCKED(kl)((void)(kl))
 #endif
 
+intdokqueue(struct proc *, int, register_t *);
 struct kqueue *kqueue_alloc(struct filedesc *);
 void   kqueue_terminate(struct proc *p, struct kqueue *);
 void   KQREF(struct kqueue *);
@@ -912,12 +913,14 @@ kqueue_alloc(struct filedesc *fdp)
 }
 
 int
-sys_kqueue(struct proc *p, void *v, register_t *retval)
+dokqueue(struct proc *p, int flags, register_t *retval)
 {
struct filedesc *fdp = p->p_fd;
struct kqueue *kq;
struct file *fp;
-   int fd, error;
+   int cloexec, error, fd;
+
+   cloexec = (flags & O_CLOEXEC) ? UF_EXCLOSE : 0;
 
kq = kqueue_alloc(fdp);
 
@@ -925,14 +928,14 @@ sys_kqueue(struct proc *p, void *v, regi
error = falloc(p, , );
if (error)
goto out;
-   fp->f_flag = FREAD | FWRITE;
+   fp->f_flag = FREAD | FWRITE | (flags & FNONBLOCK);
fp->f_type = DTYPE_KQUEUE;
fp->f_ops = 
fp->f_data = kq;
*retval = fd;
LIST_INSERT_HEAD(>fd_kqlist, kq, kq_next);
kq = NULL;
-   fdinsert(fdp, fd, 0, fp);
+   fdinsert(fdp, fd, cloexec, fp);
FRELE(fp, p);
 out:
fdpunlock(fdp);
@@ -942,6 +945,24 @@ out:
 }
 
 int
+sys_kqueue(struct proc *p, void *v, register_t *retval)
+{
+   return (dokqueue(p, 0, retval));
+}
+
+int
+sys_kqueue1(struct proc *p, void *v, register_t *retval)
+{
+   struct sys_kqueue1_args /* {
+   syscallarg(int) flags;
+   } */ *uap = v;
+
+   if (SCARG(uap, flags) & ~(O_CLOEXEC | O_NONBLOCK))
+   return (EINVAL);
+   return (dokqueue(p, SCARG(uap, flags), retval));
+}
+
+int
 sys_kevent(struct proc *p, void *v, register_t *retval)
 {
struct kqueue_scan_state scan;
Index: sys/kern/syscalls.master
===
RCS file: src/sys/kern/syscalls.master,v
retrieving revision 1.249
diff -u -p -r1.249 syscalls.master
--- sys/kern/syscalls.master24 Jul 2023 19:32:23 -  1.249
+++ sys/kern/syscalls.master14 Aug 2023 14:52:44 -
@@ -462,7 +462,7 @@
 267OBSOL   pad_preadv
 268OBSOL   pad_pwritev
 269STD NOLOCK  { int sys_kqueue(void); }
-270OBSOL   t32_kevent
+270STD NOLOCK  { int sys_kqueue1(int flags); }
 271STD { int sys_mlockall(int flags); }
 272STD { int sys_munlockall(void); }
 273UNIMPL  sys_getpeereid
Index: sys/sys/event.h
===
RCS file: src/sys/sys/event.h,v
retrieving revision 1.70
diff -u -p -r1.70 event.h
--- sys/sys/event.h 13 Aug 2023 08:29:28 -  1.70
+++ sys/sys/event.h 14 Aug 2023 14:52:44 -
@@ -369,6 +369,7 @@ struct timespec;
 
 __BEGIN_DECLS
 intkqueue(void);
+intkqueue1(int flags);
 intkevent(int kq, const struct kevent *changelist, int nchanges,
struct kevent *eventlist, int nevents,
const struct timespec *timeout);
Index: lib/libc/Symbols.list
===
RCS file: src/lib/libc/Symbols.list,v
retrieving revision 1.81
diff -u -p -r1.81 Symbols.list
--- lib/libc/Symbols.list   11 Feb 2023 23:07:28 -  1.81
+++ lib/libc/Symbols.list   14 Aug 2023 14:52:42 -
@@ -121,6 +121,7 @@ _thread_sys_issetugid
 _thread_sys_kevent
 _thread_sys_kill
 _thread_sys_kqueue
+_thread_sys_kqueue1
 _thread_sys_ktrace
 _thread_sys_lchown
 _thread_sys_link
@@ -322,6 +323,7 @@ issetugid
 kevent
 kill
 kqueue
+kqueue1
 ktrace
 lchown
 link
Index: lib/libc/shlib_version
===
RCS 

Re: IKEv2 tunnel crash when sec(4) pushed with large data

2023-08-14 Thread Tobias Heider
On Mon, Aug 14, 2023 at 02:07:12AM +, Jason Tubnor wrote:
> Hi,
> 
> Testing sec(4) between 2 end points with iperf3, iked has lost the associated 
> iface for the sec(4) point to point link. Specifically:
> 
> pfkey_sa: unsupported interface

Not sure how this can happen. Have you destroyed and recreated the interface
in between? Can you easily reproduce this?
I have added a bit more info to the error message, it now also prints the
iface id and the errno. It would be useful if you can reproduce it with those.

> 
> Here is the surround log for the event:
> 
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: recv 
> CREATE_CHILD_SA req 3 peer 4.4.4.2:64893 local 4.4.4.1:4500, 305 bytes, 
> policy 'policy1'
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: send 
> CREATE_CHILD_SA res 3 peer 4.4.4.2:64893 local 4.4.4.1:4500, 177 bytes, NAT-T
> Aug 14 11:30:54 terminator iked[93171]: pfkey_sa: unsupported interface
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #1 ENCR=AES_GCM_16-128
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #1 ENCR=AES_GCM_16-256
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #1 ESN=ESN
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #1 ESN=NONE
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 ENCR=AES_CBC-256
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 ENCR=AES_CBC-192
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 ENCR=AES_CBC-128
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 INTEGR=HMAC_SHA2_256_128
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 INTEGR=HMAC_SHA2_384_192
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 INTEGR=HMAC_SHA2_512_256
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 INTEGR=HMAC_SHA1_96
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 ESN=ESN
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_log_proposal: ESP #2 ESN=NONE
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: 
> ikev2_add_error: NO_PROPOSAL_CHOSEN
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: send 
> CREATE_CHILD_SA res 3 peer 4.4.4.2:64893 local 4.4.4.1:4500, 65 bytes, NAT-T
> Aug 14 11:30:54 terminator iked[93171]: spi=0x635987a83a22a13e: deleted 1 
> SPI: 0x37249f77
> Aug 14 11:33:04 terminator iked[93171]: spi=0xffb183f53eae6546: recv 
> IKE_SA_INIT req 0 peer 4.4.4.2:60926 local 4.4.4.1:500, 518 bytes, policy 
> 'policy2'
> Aug 14 11:33:04 terminator iked[93171]: spi=0xffb183f53eae6546: send 
> IKE_SA_INIT res 0 peer 4.4.4.2:60926 local 4.4.4.1:500, 235 bytes
> Aug 14 11:33:04 terminator iked[93171]: spi=0xffb183f53eae6546: recv IKE_AUTH 
> req 1 peer 4.4.4.2:64893 local 4.4.4.1:4500, 475 bytes, policy 'policy2'
> Aug 14 11:33:04 terminator iked[93171]: spi=0xffb183f53eae6546: send IKE_AUTH 
> res 1 peer 4.4.4.2:64893 local 4.4.4.1:4500, 341 bytes, NAT-T
> Aug 14 11:33:04 terminator iked[93171]: pfkey_sa: unsupported interface
> Aug 14 11:35:43 terminator iked[93171]: spi=0xffb183f53eae6546: sa_free: 
> reload
> Aug 14 11:37:45 terminator iked[93171]: spi=0x635987a83a22a13e: retransmit 1 
> INFORMATIONAL req 6 peer 4.4.4.2:64893 local 4.4.4.1:4500
> Aug 14 11:37:49 terminator iked[93171]: spi=0x635987a83a22a13e: retransmit 2 
> INFORMATIONAL req 6 peer 4.4.4.2:64893 local 4.4.4.1:4500
> Aug 14 11:37:57 terminator iked[93171]: spi=0x635987a83a22a13e: retransmit 3 
> INFORMATIONAL req 6 peer 4.4.4.2:64893 local 4.4.4.1:4500
> Aug 14 11:38:13 terminator iked[93171]: spi=0x635987a83a22a13e: retransmit 4 
> INFORMATIONAL req 6 peer 4.4.4.2:64893 local 4.4.4.1:4500
> Aug 14 11:38:45 terminator iked[93171]: spi=0x635987a83a22a13e: retransmit 5 
> INFORMATIONAL req 6 peer 4.4.4.2:64893 local 4.4.4.1:4500
> Aug 14 11:39:04 terminator iked[93171]: spi=0x8e5dc9e7e8a397e0: recv 
> IKE_SA_INIT req 0 peer 4.4.4.2:63301 local 4.4.4.1:500, 518 bytes, policy 
> 'policy6'
> Aug 14 11:39:04 terminator iked[93171]: spi=0x8e5dc9e7e8a397e0: send 
> IKE_SA_INIT res 0 peer 4.4.4.2:63301 local 4.4.4.1:500, 235 bytes
> Aug 14 11:39:04 terminator iked[93171]: spi=0x8e5dc9e7e8a397e0: recv IKE_AUTH 
> req 1 peer 4.4.4.2:64893 local 4.4.4.1:4500, 473 bytes, policy 'policy6'
> Aug 14 11:39:04 terminator iked[93171]: spi=0x8e5dc9e7e8a397e0: send IKE_AUTH 
> res 1 peer 4.4.4.2:64893 local 4.4.4.1:4500, 342 bytes, NAT-T
> Aug 14 11:39:04 terminator iked[93171]: spi=0x8e5dc9e7e8a397e0: 
> ikev2_childsa_enable: loaded SPIs: 0xbd533d62, 0xf7d5e5fd (enc aes-128-gcm 
> esn)
> 

Re: all platforms: separate cpu_initclocks() from cpu_startclock()

2023-08-14 Thread Jonathan Matthew
On Mon, Aug 14, 2023 at 06:24:14PM +1000, Jonathan Matthew wrote:
> On Sun, Aug 13, 2023 at 01:48:21PM -0500, Scott Cheloha wrote:
> > This is the next patch in the clock interrupt reorganization series.
> > 
> > Before we continue breaking up the hardclock(9) we need to detour into
> > the MD code.
> > 
> > This patch divides the "initialization" parts of cpu_initclocks() from
> > the "start the clock interrupt" parts.  Seprating the two parts leaves
> > initclocks() an opportunity to prepare the primary CPU for clock
> > interrupt dispatch in a machine-independent manner before actually
> > pulling the trigger.  It's nearly impossible to do any MI setup during
> > initclocks() because cpu_initclocks() does everything in one go: both
> > initialization and kickoff are done when cpu_initclocks() returns.
> > 
> > Many platforms have a "cpu_startclock()" function, so this patch takes
> > that de facto standard and makes it a rule: cpu_startclock() is now
> > required.  It is prototyped in sys/systm.h and every platform must
> > implement it.
> > 
> > The revised initclocks() sequence is then:
> > 
> > 1. Call cpu_initclocks().  At minimum, cpu_initclocks() ensures
> >hz, stathz, and profhz are initialized.  All the machine
> >independent setup in step (2) (currently) depends upon
> >these machine-dependent values.
> > 
> > 2. Compute intervals using hz, stathz, and profhz.
> > 
> >In a later step I will move the full contents of clockintr_init()
> >up into initclocks() and get rid of clockintr_init() entirely.
> > 
> > 3. Call cpu_startclock().  At minimum, cpu_startclock() starts the
> >clock interrupt dispatch cycle on the primary CPU.
> > 
> > I have compiled/booted this patch on amd64 (lapic path), arm64, i386
> > (lapic path), macppc, octeon, and sparc64 (sun4v).
> > 
> > I am looking for compile/boot tests on alpha, armv7, hppa, landisk,
> > luna88k, powerpc64, and riscv64.  I think armv7 is the tricky one
> > here.  Everything else is relatively straightforward, though I may
> > have missed a few stray variables here or there.
> > 
> > Test results?  Ok?
> 
> Compiles on armv7 and boots on an Allwinner A20 machine using agtimer(4).
> I don't think I have any armv7 systems using other timer devices.
> 

Also compiles and boots on riscv64 (visionfive 2).



Re: all platforms: separate cpu_initclocks() from cpu_startclock()

2023-08-14 Thread Jonathan Matthew
On Sun, Aug 13, 2023 at 01:48:21PM -0500, Scott Cheloha wrote:
> This is the next patch in the clock interrupt reorganization series.
> 
> Before we continue breaking up the hardclock(9) we need to detour into
> the MD code.
> 
> This patch divides the "initialization" parts of cpu_initclocks() from
> the "start the clock interrupt" parts.  Seprating the two parts leaves
> initclocks() an opportunity to prepare the primary CPU for clock
> interrupt dispatch in a machine-independent manner before actually
> pulling the trigger.  It's nearly impossible to do any MI setup during
> initclocks() because cpu_initclocks() does everything in one go: both
> initialization and kickoff are done when cpu_initclocks() returns.
> 
> Many platforms have a "cpu_startclock()" function, so this patch takes
> that de facto standard and makes it a rule: cpu_startclock() is now
> required.  It is prototyped in sys/systm.h and every platform must
> implement it.
> 
> The revised initclocks() sequence is then:
> 
> 1. Call cpu_initclocks().  At minimum, cpu_initclocks() ensures
>hz, stathz, and profhz are initialized.  All the machine
>independent setup in step (2) (currently) depends upon
>these machine-dependent values.
> 
> 2. Compute intervals using hz, stathz, and profhz.
> 
>In a later step I will move the full contents of clockintr_init()
>up into initclocks() and get rid of clockintr_init() entirely.
> 
> 3. Call cpu_startclock().  At minimum, cpu_startclock() starts the
>clock interrupt dispatch cycle on the primary CPU.
> 
> I have compiled/booted this patch on amd64 (lapic path), arm64, i386
> (lapic path), macppc, octeon, and sparc64 (sun4v).
> 
> I am looking for compile/boot tests on alpha, armv7, hppa, landisk,
> luna88k, powerpc64, and riscv64.  I think armv7 is the tricky one
> here.  Everything else is relatively straightforward, though I may
> have missed a few stray variables here or there.
> 
> Test results?  Ok?

Compiles on armv7 and boots on an Allwinner A20 machine using agtimer(4).
I don't think I have any armv7 systems using other timer devices.



Re: installer: disk crypto: crank KDF rounds to hardware based default

2023-08-14 Thread Klemens Nanni
On Fri, Aug 11, 2023 at 05:38:41PM +0200, Mark Kettenis wrote:
> > From: "Theo de Raadt" 
> > I think this case is different, because the ramdisk has no process
> > contention.
> > 
> > The code still sticks to minimum 16:
> > 
> > if (r < 16)
> > r = 16;
> > 
> > On faster machines, it will increase the rounds.  For that machine, for
> > that disk configuration.  This is processed early on to bring the disk up,
> > when there is little or no contention.  So it will not have a regressive
> > performance impact.
> 
> So the man page just lies?

Incomplete and unclear, yes.  Code has four default decision points:

* "-r auto" falls back to 16 if the performance based value is below that
  (snippet above, undocumented)

* "-r N" must be 4 or higher (strtonum(), manual refers to that)

* no "-r rounds" given means
  - default of 16 rounds for new volumes (see 3rd hunk, undocumented)
  - previous value when changing passphrase (4th hunk)


New diff a) defaults to "auto" and b) enforces a minium of 16 rounds,
making code easier to follow, imho.

Default rflag = -1 means rflag == 0 is now impossible, so the passphrase
change code can no longer distinguish between explicit "-r auto" and new
default, so it no longer sticks with the old number of rounds, but instead
goes with auto or explicit "-r N".

Does that make more sense?

Index: bioctl.8
===
RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
retrieving revision 1.111
diff -u -p -r1.111 bioctl.8
--- bioctl.86 Jul 2023 21:08:50 -   1.111
+++ bioctl.814 Aug 2023 06:12:46 -
@@ -282,11 +282,12 @@ passphrase into a key, in order to creat
 passphrase of an existing encrypted volume.
 A larger number of iterations takes more time, but offers increased resistance
 against passphrase guessing attacks.
-If
+By default, or if
 .Ar rounds
-is specified as "auto", the number of rounds will be automatically determined
-based on system performance.
-Otherwise the minimum is 4 rounds and the default is 16.
+is specified as
+.Cm auto ,
+the number of rounds will automatically be based on system performance.
+The minimum is 16 rounds.
 .It Fl s
 Read the passphrase for the selected crypto volume from
 .Pa /dev/stdin
Index: bioctl.c
===
RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
retrieving revision 1.151
diff -u -p -r1.151 bioctl.c
--- bioctl.c18 Oct 2022 07:04:20 -  1.151
+++ bioctl.c14 Aug 2023 06:52:04 -
@@ -89,7 +89,7 @@ int   devh = -1;
 inthuman;
 intverbose;
 u_int32_t  cflags = 0;
-intrflag = 0;
+intrflag = -1; /* auto */
 char   *password;
 
 void   *bio_cookie;
@@ -182,7 +182,7 @@ main(int argc, char *argv[])
rflag = -1;
break;
}
-   rflag = strtonum(optarg, 4, 1<<30, );
+   rflag = strtonum(optarg, 16, 1<<30, );
if (errstr != NULL)
errx(1, "number of KDF rounds is %s: %s",
errstr, optarg);
@@ -978,7 +978,7 @@ bio_kdf_generate(struct sr_crypto_kdfinf
 
kdfinfo->pbkdf.generic.len = sizeof(kdfinfo->pbkdf);
kdfinfo->pbkdf.generic.type = SR_CRYPTOKDFT_BCRYPT_PBKDF;
-   kdfinfo->pbkdf.rounds = rflag ? rflag : 16;
+   kdfinfo->pbkdf.rounds = rflag;
 
kdfinfo->flags = SR_CRYPTOKDF_KEY | SR_CRYPTOKDF_HINT;
kdfinfo->len = sizeof(*kdfinfo);
@@ -1117,13 +1117,6 @@ bio_changepass(char *dev)
 
/* Current passphrase. */
bio_kdf_derive(, , "Old passphrase: ", 0);
-
-   /*
-* Unless otherwise specified, keep the previous number of rounds as
-* long as we're using the same KDF.
-*/
-   if (kdfhint.generic.type == SR_CRYPTOKDFT_BCRYPT_PBKDF && !rflag)
-   rflag = kdfhint.rounds;
 
/* New passphrase. */
bio_kdf_generate();