Re: Picky, but much more efficient arc4random_uniform!

2022-05-20 Thread Luke Small
Crystal: You can prove that for random, repetitive, correct, database
record name generation using small upperbounds, the demonstrated 1/3-1/2
runtime isn’t worth it for an upperbound like 26 - 92 in a business context
that fights for every last millisecond?

Bring it.

Prove the correctness of whatever algorithm you’re using while you’re at it.

…unless a lot of those processes are threaded of course. :/

On Fri, May 20, 2022 at 7:27 PM Crystal Kolipe 
wrote:

>
> I've actually written a program to demonstrate how pointless your chacha
> bit
> saving routine is :).  I just haven't posted it yet because I'm too busy
> with
> other, (more useful), things to bother finishing it off.
>
> Your thread on -tech has already wasted more bits than your random number
> routine would save in a very long time, ha ha ha.
>
-- 
-Luke


Re: turn rc_exec variable into an rcexec function

2022-05-20 Thread Antoine Jacoutot
On Tue, May 17, 2022 at 09:12:08AM +0200, Antoine Jacoutot wrote:
> Hi folks.
> 
> I would like to move away from the rc_exec variable to an rcexec() function 
> for
> rc.d scripts.
> That will allow daemon_logger to work out of the box even for manually
> crafted rc_start() functions.
> I will also simplify the addition of new features like daemon_startdir.
> 
> I have tested this with a few daemons only.
> So I am calling for testing this diff on your own setup to detect any possible
> regression (especially for daemons with a complex starting sequence).
> To test, just apply the diff to rc.subr and replace ${rcexec} with rc_exec in
> your rc.d scripts.
> 
> As I don't want to spam the list, please report OK tests directly to me and
> KO ones to the list.
> 
> Thanks a lot!

FYI I intend to commit this over the week-end.
Best way to get test reports I guess.


> Index: rc.subr
> ===
> RCS file: /cvs/src/etc/rc.d/rc.subr,v
> retrieving revision 1.152
> diff -u -p -r1.152 rc.subr
> --- rc.subr   10 Feb 2022 16:57:33 -  1.152
> +++ rc.subr   17 May 2022 07:05:57 -
> @@ -159,11 +159,19 @@ _rc_wait_for_start() {
>   return
>  }
>  
> -rc_start() {
> - ${rcexec} "${daemon_logger:+set -o pipefail; }${daemon} 
> ${daemon_flags}${daemon_logger:+ 2>&1 |
> +rc_exec() {
> + local _rcexec="su -fl -c ${daemon_class} -s /bin/sh ${daemon_user} -c"
> + [ "${daemon_rtable}" -eq "$(id -R)" ] ||
> + _rcexec="route -T ${daemon_rtable} exec ${_rcexec}"
> +
> + ${_rcexec} "${daemon_logger:+set -o pipefail; }$@${daemon_logger:+ 2>&1 
> |
>   logger -ip ${daemon_logger} -t ${_name}}"
>  }
>  
> +rc_start() {
> + rc_exec "${daemon} ${daemon_flags}"
> +}
> +
>  rc_check() {
>   pgrep -T "${daemon_rtable}" -q -xf "${pexp}"
>  }
> @@ -343,6 +351,3 @@ unset _rcflags _rclogger _rcrtable _rcti
>  # the shell will strip the quotes from daemon_flags when starting a daemon;
>  # make sure pexp matches the process (i.e. doesn't include the quotes)
>  pexp="$(eval echo ${daemon}${daemon_flags:+ ${daemon_flags}})"
> -rcexec="su -fl -c ${daemon_class} -s /bin/sh ${daemon_user} -c"
> -[ "${daemon_rtable}" -eq "$(id -R)" ] ||
> - rcexec="route -T ${daemon_rtable} exec ${rcexec}"
> 
> 
> -- 
> Antoine
> 

-- 
Antoine



Re: move vmm(4) spinout paranoia behind MP_LOCKDEBUG

2022-05-20 Thread Mike Larkin
On Sat, Apr 16, 2022 at 12:09:46PM -0400, Dave Voutila wrote:
> This tucks all the spinout paranoia behind `#ifdef MP_LOCKDEBUG` and
> uses the same approach used in amd64's pmap's TLB shootdown code.
>
> Part of me wants to remove this altogether, but I'm not sure it's
> outlived its usefulness quite yet.
>
> Three areas that busy wait on ipi's are modified:
>
> 1. vmm_start - performs ipi to enable vmm on all cpus
> 2. vmm_stop - performs ipi to disable vmm on all cpus
> 3. vmx_remote_vmclear - performs ipi to vmclear a cpu (only pertinent to
>Intel hosts)
>
> (3) is the most likely to spin out and prior to bumping the spinout to
> the current value (based on __mp_lock_spinout) we had reports from users
> of hitting it on slower/older MP hardware.
>
> For vmm_{start, stop}, I moved the current cpu start/stop routine to
> before performing the ipi broadcast because if we're going to fail to
> (dis)enable vmm we should fail fast. If we fail, there's no need to
> broadcast the ipi. This simplifies the code paths and removes a local
> variable.
>
> All three migrate to infinite busy waits and only have a spinout if
> built with MP_LOCKDEBUG. On a spinout, we enter ddb.
>
> Compiled on amd64 GENERIC, GENERIC.MP, and GENERIC.MP with
> MP_LOCKDEBUG. (This time I won't break GENERIC :)
>
> OK?
>
> -dv

Sorry for the delay. ok mlarkin. I've had this on a few machines for
the better part of a month and haven't seen any problems.

-ml

>
> Index: sys/arch/amd64/amd64/vmm.c
> ===
> RCS file: /opt/cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.305
> diff -u -p -r1.305 vmm.c
> --- sys/arch/amd64/amd64/vmm.c28 Mar 2022 06:28:47 -  1.305
> +++ sys/arch/amd64/amd64/vmm.c16 Apr 2022 18:49:01 -
> @@ -43,6 +43,11 @@
>  #include 
>  #include 
>
> +#ifdef MP_LOCKDEBUG
> +#include 
> +extern int __mp_lock_spinout;
> +#endif /* MP_LOCKDEBUG */
> +
>  /* #define VMM_DEBUG */
>
>  void *l1tf_flush_region;
> @@ -1328,17 +1333,26 @@ int
>  vmm_start(void)
>  {
>   struct cpu_info *self = curcpu();
> - int ret = 0;
>  #ifdef MULTIPROCESSOR
>   struct cpu_info *ci;
>   CPU_INFO_ITERATOR cii;
> - int i;
> -#endif
> +#ifdef MP_LOCKDEBUG
> + int nticks;
> +#endif /* MP_LOCKDEBUG */
> +#endif /* MULTIPROCESSOR */
>
>   /* VMM is already running */
>   if (self->ci_flags & CPUF_VMM)
>   return (0);
>
> + /* Start VMM on this CPU */
> + start_vmm_on_cpu(self);
> + if (!(self->ci_flags & CPUF_VMM)) {
> + printf("%s: failed to enter VMM mode\n",
> + self->ci_dev->dv_xname);
> + return (EIO);
> + }
> +
>  #ifdef MULTIPROCESSOR
>   /* Broadcast start VMM IPI */
>   x86_broadcast_ipi(X86_IPI_START_VMM);
> @@ -1346,25 +1360,23 @@ vmm_start(void)
>   CPU_INFO_FOREACH(cii, ci) {
>   if (ci == self)
>   continue;
> - for (i = 10; (!(ci->ci_flags & CPUF_VMM)) && i>0;i--)
> - delay(10);
> - if (!(ci->ci_flags & CPUF_VMM)) {
> - printf("%s: failed to enter VMM mode\n",
> - ci->ci_dev->dv_xname);
> - ret = EIO;
> +#ifdef MP_LOCKDEBUG
> + nticks = __mp_lock_spinout;
> +#endif /* MP_LOCKDEBUG */
> + while (!(ci->ci_flags & CPUF_VMM)) {
> + CPU_BUSY_CYCLE();
> +#ifdef MP_LOCKDEBUG
> + if (--nticks <= 0) {
> + db_printf("%s: spun out", __func__);
> + db_enter();
> + nticks = __mp_lock_spinout;
> + }
> +#endif /* MP_LOCKDEBUG */
>   }
>   }
>  #endif /* MULTIPROCESSOR */
>
> - /* Start VMM on this CPU */
> - start_vmm_on_cpu(self);
> - if (!(self->ci_flags & CPUF_VMM)) {
> - printf("%s: failed to enter VMM mode\n",
> - self->ci_dev->dv_xname);
> - ret = EIO;
> - }
> -
> - return (ret);
> + return (0);
>  }
>
>  /*
> @@ -1376,17 +1388,26 @@ int
>  vmm_stop(void)
>  {
>   struct cpu_info *self = curcpu();
> - int ret = 0;
>  #ifdef MULTIPROCESSOR
>   struct cpu_info *ci;
>   CPU_INFO_ITERATOR cii;
> - int i;
> -#endif
> +#ifdef MP_LOCKDEBUG
> + int nticks;
> +#endif /* MP_LOCKDEBUG */
> +#endif /* MULTIPROCESSOR */
>
>   /* VMM is not running */
>   if (!(self->ci_flags & CPUF_VMM))
>   return (0);
>
> + /* Stop VMM on this CPU */
> + stop_vmm_on_cpu(self);
> + if (self->ci_flags & CPUF_VMM) {
> + printf("%s: failed to exit VMM mode\n",
> + self->ci_dev->dv_xname);
> + return (EIO);
> + }
> +
>  #ifdef MULTIPROCESSOR
>   /* Stop VMM on other CPUs */
>   x86_broadcast_ipi(X86_IPI_STOP_VMM);
> @@ -1394,25 +1415,23 @@ vmm_stop(void)
>

Re: vmm: load vmcs before reading vcpu registers

2022-05-20 Thread Mike Larkin
On Wed, May 18, 2022 at 10:27:11AM -0400, Dave Voutila wrote:
>
> ping...would like to get this in if possible so I can move onto fixing
> some things in vmm.
>

sorry. ok mlarkin

> Dave Voutila  writes:
>
> > tech@,
> >
> > Continuing my vmm/vmd bug hunt, the following diff adapts
> > vcpu_readregs_vmx to optionally load the vmcs on the current cpu. This
> > has gone unnoticed as the ioctl isn't used in typical vmd usage and the
> > usage of vcpu_readregs_vmx in the run ioctl is after the vmcs is already
> > loaded on the current cpu.
> >
> > This fixes `vmctl send` on Intel hosts. (A fix for `vmctl receive` comes
> > next.)
> >
> > Currently, `vmctl send` tries to serialize the vcpu registers as part of
> > serializing the vm state. On an MP machine, it's highly probable that
> > the vmread instructions will fail as they'll be executed on a cpu that
> > doesn't have the vmcs loaded.
> >
> > While here, I noticed the vcpu_writeregs_vmx function doesn't set the
> > vcpu's vmcs state variable to VMCS_CLEARED after running vmclear. This
> > can cause failure to vm-enter as vmm uses that state to determine which
> > of the two Intel instructions to call (vmlaunch or vmresume).
> >
> > ok?
> >
> > -dv
> >
> > Index: sys/arch/amd64/amd64/vmm.c
> > ===
> > RCS file: /opt/cvs/src/sys/arch/amd64/amd64/vmm.c,v
> > retrieving revision 1.308
> > diff -u -p -r1.308 vmm.c
> > --- sys/arch/amd64/amd64/vmm.c  4 May 2022 02:24:26 -   1.308
> > +++ sys/arch/amd64/amd64/vmm.c  8 May 2022 18:37:42 -
> > @@ -140,7 +140,7 @@ int vm_rwregs(struct vm_rwregs_params *,
> >  int vm_mprotect_ept(struct vm_mprotect_ept_params *);
> >  int vm_rwvmparams(struct vm_rwvmparams_params *, int);
> >  int vm_find(uint32_t, struct vm **);
> > -int vcpu_readregs_vmx(struct vcpu *, uint64_t, struct vcpu_reg_state *);
> > +int vcpu_readregs_vmx(struct vcpu *, uint64_t, int, struct vcpu_reg_state 
> > *);
> >  int vcpu_readregs_svm(struct vcpu *, uint64_t, struct vcpu_reg_state *);
> >  int vcpu_writeregs_vmx(struct vcpu *, uint64_t, int, struct vcpu_reg_state 
> > *);
> >  int vcpu_writeregs_svm(struct vcpu *, uint64_t, struct vcpu_reg_state *);
> > @@ -978,7 +978,7 @@ vm_rwregs(struct vm_rwregs_params *vrwp,
> > if (vmm_softc->mode == VMM_MODE_VMX ||
> > vmm_softc->mode == VMM_MODE_EPT)
> > ret = (dir == 0) ?
> > -   vcpu_readregs_vmx(vcpu, vrwp->vrwp_mask, vrs) :
> > +   vcpu_readregs_vmx(vcpu, vrwp->vrwp_mask, 1, vrs) :
> > vcpu_writeregs_vmx(vcpu, vrwp->vrwp_mask, 1, vrs);
> > else if (vmm_softc->mode == VMM_MODE_SVM ||
> > vmm_softc->mode == VMM_MODE_RVI)
> > @@ -1986,6 +1986,7 @@ vcpu_reload_vmcs_vmx(struct vcpu *vcpu)
> >   * Parameters:
> >   *  vcpu: the vcpu to read register values from
> >   *  regmask: the types of registers to read
> > + *  loadvmcs: bit to indicate whether the VMCS has to be loaded first
> >   *  vrs: output parameter where register values are stored
> >   *
> >   * Return values:
> > @@ -1993,7 +1994,7 @@ vcpu_reload_vmcs_vmx(struct vcpu *vcpu)
> >   *  EINVAL: an error reading registers occurred
> >   */
> >  int
> > -vcpu_readregs_vmx(struct vcpu *vcpu, uint64_t regmask,
> > +vcpu_readregs_vmx(struct vcpu *vcpu, uint64_t regmask, int loadvmcs,
> >  struct vcpu_reg_state *vrs)
> >  {
> > int i, ret = 0;
> > @@ -2005,6 +2006,11 @@ vcpu_readregs_vmx(struct vcpu *vcpu, uin
> > struct vcpu_segment_info *sregs = vrs->vrs_sregs;
> > struct vmx_msr_store *msr_store;
> >
> > +   if (loadvmcs) {
> > +   if (vcpu_reload_vmcs_vmx(vcpu))
> > +   return (EINVAL);
> > +   }
> > +
> >  #ifdef VMM_DEBUG
> > /* VMCS should be loaded... */
> > paddr_t pa = 0ULL;
> > @@ -2393,6 +2399,7 @@ out:
> > if (loadvmcs) {
> > if (vmclear(>vc_control_pa))
> > ret = EINVAL;
> > +   atomic_swap_uint(>vc_vmx_vmcs_state, VMCS_CLEARED);
> > }
> > return (ret);
> >  }
> > @@ -4631,7 +4638,7 @@ vmm_translate_gva(struct vcpu *vcpu, uin
> >
> > if (vmm_softc->mode == VMM_MODE_EPT ||
> > vmm_softc->mode == VMM_MODE_VMX) {
> > -   if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, ))
> > +   if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, 1, ))
> > return (EINVAL);
> > } else if (vmm_softc->mode == VMM_MODE_RVI ||
> > vmm_softc->mode == VMM_MODE_SVM) {
> > @@ -5111,7 +5118,7 @@ vcpu_run_vmx(struct vcpu *vcpu, struct v
> > vcpu->vc_last_pcpu = curcpu();
> >
> > /* Copy the VCPU register state to the exit structure */
> > -   if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, >vc_exit.vrs))
> > +   if (vcpu_readregs_vmx(vcpu, VM_RWREGS_ALL, 0, >vc_exit.vrs))
> > ret = EINVAL;
> > vcpu->vc_exit.cpl = vmm_get_guest_cpu_cpl(vcpu);
>
>
> --
> -Dave Voutila
>



Re: Picky, but much more efficient arc4random_uniform!

2022-05-20 Thread Luke Small
I appreciate your response, Damien.

I did do the bare minimum of correctness testing and it was the post right
after Guenther was congratulated on proving incorrectness:

https://marc.info/?l=openbsd-tech=165259528425835=2

The post includes software to reproduce the results.



I wrote a highly dynamically allocated program to test at intervals of
intervals to show at various stages to show the degree that the output
remains random

This is an example of some output:

testing arc4random_uniform(5) and arc4random_uniform_small_unlocked(5):

 256X 2048X16384X   131072X
 1048576X

 
   0 original246  2053 16400131115
1048306
 mine263  2042 16312131258
1046989

   1 original234  2013 16337130894
1047810
 mine249  2022 16304131161
1049511

   2 original236  2061 16367130802
1047430
 mine257  2117 16597130718
1046375

   3 original284  2089 16444131092
1050332
 mine266  2058 16379131190
1049877

   4 original280  2024 16372131457
1049002
 mine245  2001 16328131033
1050128



max-min values:

 original 5076   107   655
 2902
 mine 21   116   293   540
 3753


The program is set up to test values 2 through 50. This will create 224KiB
of output.
I suspected that you'd prefer that I didn't attach it.

Some progress output has been relegated to stderr so that you can easily
pipe it to a file.

I added some getrlimit an setrlimit functions to maximize memory limits if
necessary
In the case that I created for you, this will not be needed.

It uses 1276K RAM in the current configuration.


> Just to bring this back to where we came in: even putting thread-safety
> aside (which you absolutely can't): it doesn't matter how much faster
> it is, your replacement function isn't useful until you do the work to
> demonstrate correctness.
>
> You have done literally zero so far, not even the bare minimum of
> testing the output. As a result your first version was demonstrated to
> be completely broken by literally the most basic of possible tests, a
> mere 10 lines of code.
>
> That you left this to others to do tells me that you fundamentally don't
> understand the environment in which you're trying to operate, and that
> you don't respect the time of the people you're trying to convince.
>
> Please stop wasting our time.
>
> -d

-- 
-Luke
#include 
#include 
#include 
#include 
#include 
#include 
#include 

extern char *malloc_options;


/*
cc arc4random_uniform_small.c -O2 -march=native -mtune=native -flto=full \
-static -p -fno-inline && ./a.out && gprof a.out gmon.out

cc arc4random_uniform_small.c -O2 && ./a.out
 */


/* 
 * uint32_t
 * arc4random_uniform(uint32_t upper_bound);
 * 
 * for which this function is named, provides a cryptographically secure:
 * uint32_t arc4random() % upper_bound;
 * 
 * this function minimizes the waste of randomly generated bits,
 * for small upper bounds.
 * 
 * 'bits' is the requested number of random bits and it needs to be
 * within the following parameters:
 * 
 *   1 << bits >= upper_bound > (1 << bits) >> 1
 * 
 * I make a possibly incorrect presumption that size_t is
 * at the very least a uint32_t, but probably a uint64_t for speed
 */
static uint32_t
arc4random_uniform_small_unlocked(uint32_t upper_bound)
{	
	static uint64_t rand_holder = 0;
	static uint64_t rand_bits = 0;
	
	static uint64_t upper_bound0 = 2;
	static uint64_t bits0 = 1;

	uint64_t bits = 16, i = 1, n = 32, ret, ub = upper_bound;

	if (ub != upper_bound0) {
		
		if (ub < 2) {
			
			/*
			 * reset static cache for if a process needs to fork()
			 * to make it manually fork-safe
			 */
			if (ub == 0) { 
rand_holder = 0;
rand_bits = 0;
			}
			return 0;
		}
		
		/*
		 * binary search for ideal size random bitstream selection
		 */
		for (;;) {
			
			if ( ub > ((uint64_t)1 << bits) ) {

if (n - i == 1) {
	bits = n;
	break;
}

i = bits;
bits = (i + n) / 2;
continue;
			}
			if ( ub <= ((uint64_t)1 << bits) >> 1 ) {

if (n - i == 1) {
	bits = n;
	break;
}

n = bits;
bits = (i + n) / 2;
continue;
			}
			
			break;
		}
		upper_bound0 = ub;
		bits0 = bits;
	} else
		bits = bits0;
		
	const uint64_t bitmask = ((uint64_t)1 << bits) - 1;
		
	do {
		if (rand_bits < bits) {
			rand_holder |= ((uint64_t)arc4random()) << rand_bits;
			
			/* 
			 * rand_bits will be a number between 0 and 31 here
			 * so the 0x20 bit will be empty
			 * rand_bits += 32;
			 */ 
			rand_bits |= 32;
		}
		
		

Re: hidmt: default to clickpad unless report says otherwise

2022-05-20 Thread joshua stein
On Fri, 20 May 2022 at 16:46:01 +0200, Ulf Brosziewski wrote:
> Shouldn't the check of the button page remain in place, for identifying
> plain old touchpads with external buttons?

The device in question reports it has multiple buttons when it's 
just a clickpad, so keeping that check would defeat the purpose of 
this change.  I suppose this diff is not worth it.

I guess a hardware-specific quirk can be added for the Framework, or 
just a wsconsctl knob to turn on the soft button stuff for devices 
that are detected as touchpads instead of clickpads.



Re: hidmt: default to clickpad unless report says otherwise

2022-05-20 Thread Ulf Brosziewski
Shouldn't the check of the button page remain in place, for identifying
plain old touchpads with external buttons?  Looking for the "Pad Type"
feature (HUD_BUTTON_TYPE in hid.h) doesn't make much sense for these
devices because the feature report is optional:

"If the device has a non-button reporting digitizer surface and relies
instead on external buttons only for mouse clicks, then this usage can
be optionally reported."
( 
https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection
 )

It seems that we have, once more, a specification mess here.  My quote is
from the current web page, which describes the Pad Type as a three-valued
feature, with value 2 indicating a "Non-Clickable" touchpad.  HUT1_3_0.pdf
("HID Usage Tables") just states that
"When set, the touchpad is non-depressible (pressure-pad); when clear,
the touchpad is depressible (click-pad)." (p. 177)

A pad type and the HUP_BUTTON 1 feature (the "clickpad button") aren't
reported by the Synaptics touchpad on my Clevo N151CU, it correctly reports
only HUP_BUTTON 2 (the external left button) and 3 (the external right
button).

On 5/19/22 15:01, joshua stein wrote:
> Most Windows Precision Touchpad-style touchpads will be clickpads, 
> with no-button "pressure pad" style devices being the outlier.  Make 
> clickpad the default unless the report says otherwise.
> 
> This fixes the Framework laptop which has a PixArt touchpad with a 
> weird HID descriptor report which puts the button type on its own 
> report (so this heuristic doesn't find it) but also claims to have 3 
> buttons, failing the second heuristic.  If it's not setup as a 
> clickpad, wstpad doesn't do left/center/right button emulation 
> properly.
> 
> I'm curious if this breaks anyone's laptop, especially those ones 
> with a touchpad that also have a separate row of buttons like some 
> Dells.
> 
> 
> diff --git sys/dev/hid/hidmt.c sys/dev/hid/hidmt.c
> index 5ca26899e50..0baf724a9da 100644
> --- sys/dev/hid/hidmt.c
> +++ sys/dev/hid/hidmt.c
> @@ -149,16 +149,13 @@ hidmt_setup(struct device *self, struct hidmt *mt, void 
> *desc, int dlen)
>   mt->sc_num_contacts = d;
>   }
>  
> - /* find whether this is a clickpad or not */
> + /* a "pressure pad" has no buttons, so wstpad needs to know */
> + mt->sc_clickpad = 1;
>   if (hid_locate(desc, dlen, HID_USAGE2(HUP_DIGITIZERS, HUD_BUTTON_TYPE),
>   mt->sc_rep_cap, hid_feature, , NULL)) {
>   d = hid_get_udata(rep, capsize, );
> - mt->sc_clickpad = (d == 0);
> - } else {
> - /* if there's not a 2nd button, this is probably a clickpad */
> - if (!hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 2),
> - mt->sc_rep_input, hid_input, , NULL))
> - mt->sc_clickpad = 1;
> + if (d == 0x1)
> + mt->sc_clickpad = 0;
>   }
>  
>   /*
> 



iked problems with Apple clients in 7.1

2022-05-20 Thread Stuart Henderson
I ran into problems with Apple clients failing to connect to
iked after updating a machine to 7.1, introduced by
https://github.com/openbsd/src/commit/e3f5cf2ee26929d75dc2df9e86d97c36b2a94268

spi=0xac3d46687441f957: recv IKE_SA_INIT req 0 peer rrr.rrr.rrr.rr:49436 local 
lll.ll.lll.lll:500, 308 bytes, policy 'default'
spi=0xac3d46687441f957: send IKE_SA_INIT res 0 peer rrr.rrr.rrr.rr:49436 local 
lll.ll.lll.lll:500, 341 bytes
spi=0xac3d46687441f957: recv IKE_AUTH req 1 peer rrr.rrr.rrr.rr:64892 local 
lll.ll.lll.lll:4500, 368 bytes, policy 'default'
policy_test: localid mismatch
spi=0xac3d46687441f957: ikev2_ike_auth_recv: no compatible policy found
spi=0xac3d46687441f957: ikev2_send_auth_failed: authentication failed for
spi=0xac3d46687441f957: send IKE_AUTH res 1 peer rrr.rrr.rrr.rr:64892 local 
lll.ll.lll.lll:4500, 80 bytes, NAT-T
spi=0xac3d46687441f957: sa_free: authentication failed

I don't have full details of config done on the other side nor any
fruit-based phones to test from myself, did anyone already run into this
and figure out a way around it?

I'm currently running code backed out to "cvs up -D'2021/11/26 15:00'"
as a workaround.  My config looks like

-
set fragmentation

ikev2 "default" passive esp from 0.0.0.0/0 to dynamic \
 \
  local lll.ll.lll.lll \
  peer any \
 \
  ikesa enc aes-128-gcm group curve25519 group ecp521 group ecp256 group 
modp2048 \
  ikesa enc aes-128 enc aes-256 auth hmac-sha2-256 auth hmac-sha1 group 
curve25519 group ecp521 group ecp256 group modp2048 group modp1024 \
 \
  childsa enc aes-128-gcm group curve25519 group ecp521 group ecp256 group 
modp2048 \
  childsa enc aes-128 enc aes-256 auth hmac-sha2-256 auth hmac-sha1 group 
curve25519 group ecp521 group ecp256 group modp2048 group modp1024 \
 \
  childsa enc aes-128 enc aes-256 auth hmac-sha2-256 auth hmac-sha1 \
 \
  srcid "" \
  lifetime 3h bytes 5G \
  eap "mschap-v2" \
  config address ttt.ttt.tt.ttt/26 \
  config name-server lll.ll.lll.aa \
  config name-server lll.ll.lll.bb \
  tag "$name-$id"

ikev2 "keysim" active tunnel esp from 0.0.0.0/0 to 100.70.76.0/22 \
local lll.ll.lll.lll peer kk.kkk.kkk.kkk \
ikesa auth hmac-sha2-256 enc aes-256 group modp3072 \
childsa auth hmac-sha2-256 enc aes-256 group modp3072 \
srcid lll.ll.lll.lll dstid kk.kkk.kkk.kkk \
lifetime 3h bytes 20G \
psk  \
tag "keysim"

include "/etc/iked.users"
-



Fwd: Picky, but much more efficient arc4random_uniform!

2022-05-20 Thread Luke Small
I appreciate your response, Damien.

I did do the bare minimum of correctness testing and it was the post right
after Guenther was congratulated on proving incorrectness:

https://marc.info/?l=openbsd-tech=165259528425835=2

The post includes software to reproduce the results.



I wrote a highly dynamically allocated program to test at intervals of
intervals to show at various stages to show the degree that the output
remains random

This is an example of some output:

testing arc4random_uniform(5) and arc4random_uniform_small_unlocked(5):

 256X 2048X16384X   131072X
 1048576X

 
   0 original246  2053 16400131115
1048306
 mine263  2042 16312131258
1046989

   1 original234  2013 16337130894
1047810
 mine249  2022 16304131161
1049511

   2 original236  2061 16367130802
1047430
 mine257  2117 16597130718
1046375

   3 original284  2089 16444131092
1050332
 mine266  2058 16379131190
1049877

   4 original280  2024 16372131457
1049002
 mine245  2001 16328131033
1050128



max-min values:

 original 5076   107   655
 2902
 mine 21   116   293   540
 3753


The program is set up to test values 2 through 50. This will create 224KiB
of output.
I suspected that you'd prefer that I didn't attach it.

Some progress output has been relegated to stderr so that you can easily
pipe it to a file.

I added some getrlimit an setrlimit functions to maximize memory limits if
necessary
In the case that I created for you, this will not be needed.

It uses 1276K RAM in the current configuration.


> Just to bring this back to where we came in: even putting thread-safety
> aside (which you absolutely can't): it doesn't matter how much faster
> it is, your replacement function isn't useful until you do the work to
> demonstrate correctness.
>
> You have done literally zero so far, not even the bare minimum of
> testing the output. As a result your first version was demonstrated to
> be completely broken by literally the most basic of possible tests, a
> mere 10 lines of code.
>
> That you left this to others to do tells me that you fundamentally don't
> understand the environment in which you're trying to operate, and that
> you don't respect the time of the people you're trying to convince.
>
> Please stop wasting our time.
>
> -d

-- 
-Luke


arc4random_uniform_small.c
Description: Binary data


Re: iked(8): support for intermediate CAs and multiple CERT payloads

2022-05-20 Thread Stuart Henderson
On 2022/05/20 00:39, Loïc Revest wrote:
> ()Hello Stuart,
> 
> Thanks for giving it also a try - I was the one bothering Tobias
> earlier today with
> this use case of a Windows 10 (21H2) client trying to connect to an iked 
> server
> whose CA certificate wasn't self-signed, but signed by a root CA.

I think if it were actually signed _directly_ by a root CA then it would
have worked anyway, the issue is where it's signed by an intermediate (which
is the case for most, possibly all, CAs now in browser/OS root stores)

> > For Windows this works provided that ISRG Root X1 is already in the
> > computer trust store. It seems this is present on some but not other
> > Windows installations - if you get "IKE authentication credentials
> > are unacceptable" this is the likely failure.
> 
> Windows initiates the connection as long as, as you note, the root CA is in
> the computer trust store - along with the iked CA, otherwise I get the "IKE
> authentication credentials are unacceptable" error you were referring to.
> 
> On the other hand I initially (OpenBSD 7.1 release) got on iked side
> (iked -dv) the following "errors":
> 
> ikev2_pld_cert: multiple cert payloads not supported
> ikev2_resp_recv: failed to parse message
> 
> Leading to a timeout (sa_free: SA_INIT timeout) notified on both ends.
> 
> I did try with a "consolidated" /etc/iked/ca/ca.crt (iked cert + the one
> from the signing CA) as well as with a "lone" one (iked cert

This definitely doesn't work without the diff. If it is connecting
anyway then it is something that gets quietly fixed up on the client
side. (In the case of https webservers, it is common for gui browsers
to do that, whereas command-line clients typically don't - similar
situation with IKEv2 clients where some handle it and others don't).

> >
> >
> > I have connected successfully with Windows 20H2.
> 
> I got the same "error" after having applied Tobias' updated diff.
> 
> It's therefore likely that I did something different (wrong?) from you on
> Windows and/or iked side...

With this diff as-is, on the iked side, certs/hostname.example.org.crt
needs to contain only the server cert, and ca/ca.crt should contain either
the intermediate, or intermediate + root.

On the Windows client side, if you still see the error with the above
(I don't recall whether you need to restart iked or not to get it to take
effect) then you _may_ need to open https://valid-isrgrootsx1.letsencrypt.org/
in MSIE or perhaps some other browser to get it to update the root store.