Re: update xf86-video-amdgpu to latest git

2021-08-03 Thread rgc
@tech

this combo has been working great for me the past few days.
i have not encountered any sort of crash since doing a sysupgrade.

$ sysctl kern.version
kern.version=OpenBSD 6.9-current (GENERIC.MP) #158: Sat Jul 31 11:00:00 MDT 2021
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP

$ fw_update -i
Installed: vmm-firmware-1.14.0 amdgpu-firmware-20210716

rgc

On Wed, Jul 21, 2021 at 05:36:08AM +0900, rgc wrote:
> @tech
> 
> i've been using this patch for a week
> i've experience 4 hangs ... X hangs ... network is alive so i can do a safe 
> reboot
> via ssh. last hang occured just 5 mins ago while watching some YouTube videos.
> 
> 2 behaviours:
> - video/screen freezes indifinitely, which requires me to login via ssh and 
> reboot
> - video/screen freezes and X restarts, allowing me to just re-login
> 
> 
> this laptop acts mostly like a server. it gets rebooted only when it hangs, 
> or if
> there is a new snapshot.
> 
> rgc
> 
> On Sat, Jul 10, 2021 at 08:04:54AM +0900, rgc wrote:
> > On Thu, Jul 08, 2021 at 05:29:01PM +1000, Jonathan Gray wrote:
> > > The latest xf86-video-amdgpu release was in 2019.
> > > 
> > > xf86-video-amdgpu-19.1.0..origin/master
> > 
> > 8>< snipped
> > 
> > just finished rebuilding xenocara
> > build fine
> > booted fine
> > none of my previous issues (see links below) happening yet.
> > 
> > -current:
> > 
> > kern.version=OpenBSD 6.9-current (GENERIC.MP) #120: Thu Jul  8 23:45:06 MDT 
> > 2021
> > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > 
> > amdgpu0: PICASSO 10 CU rev 0x01
> > 
> > 
> > this were my system issues/reports:
> > https://marc.info/?l=openbsd-misc=161131537616993=2
> > https://marc.info/?l=openbsd-misc=161736536311231=2
> > 
> > since my last update i have updated the BIOS FW (3.06)
> > 
> > rgc
> > 
> 



Re: Fix unsafe snmpd defaults

2021-08-03 Thread Martijn van Duren
On Tue, 2021-08-03 at 21:58 +0100, Stuart Henderson wrote:
> On 2021/08/03 22:07, Martijn van Duren wrote:
> > On Tue, 2021-08-03 at 18:24 +0100, Stuart Henderson wrote:
> > > On 2021/06/15 17:39, Stuart Henderson wrote:
> > > > > Then again, I don't get the feeling many people use snmpd at this time
> > > > > and maybe it's a good moment to bite the bullet and go for safest
> > > > > defaults possible at this time. But if that's the case I would like to
> > > > > follow up with a diff to changes the default auth to hmac-sha512,
> > > > > because snmp drops trailing bytes of the result and enc to aes instead
> > > > > of des.
> > > > 
> > > > This is the change that feels most likely to affect existing SNMPv3 
> > > > users.
> > > > Support in management software beyond aes/sha1 is a bit lacking and 
> > > > prone
> > > > to incompatibility (I had issues with net-snmp and snmpd using 
> > > > hmac-sha256
> > > > though it seems it will work with hmac-sha512..)
> > > 
> > > BTW, having updated a few machines now, I am finding the change to
> > > sha2-256 by default to be a complete pain, especially considering that
> > > /etc/examples/snmpd.conf uses "enc aes" but has no setting for auth
> > > so relies on defaults for that..
> > > 
> > I can't do a lot with "a complete pain".
> > 
> > Does something like the diff below make things more intuitive? If not,
> > could you be a little more concrete?
> > 
> > martijn@
> > 
> > Index: snmpd.conf
> > ===
> > RCS file: /cvs/src/etc/examples/snmpd.conf,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 snmpd.conf
> > --- snmpd.conf  11 Jul 2014 21:20:10 -  1.1
> > +++ snmpd.conf  3 Aug 2021 20:05:53 -
> > @@ -18,7 +18,9 @@ system services 74
> >  oid 1.3.6.1.4.1.30155.42.3.1 name testStringValue read-only string "Test"
> >  oid 1.3.6.1.4.1.30155.42.3.4 name testIntValue read-write integer 1
> >  
> > -# Enable SNMPv3 USM with authentication, encryption and two defined users
> > -#seclevel enc
> > -#user "user1" authkey "password123" enc aes enckey "321drowssap"
> > -#user "user2" authkey "password456" enckey "654drowssap"
> > +# Create two SNMPv3 USM users:
> > +# User with default crypto values
> > +#user "defaultuser" authkey "password123" enckey "321drowssap"
> > +# User with backwards compatible crypto:
> > +# Only enable and use when client absolutely can't deal with modern 
> > defaults.
> > +#user "compatuser" authkey "password456" auth hmac-md5 enckey 
> > "654drowssap" enc des
> > 
> > 
> 
> Given the lack of support for SHA2-256 in much management software until
> recently AES+SHA is a pretty common configuration. And given the old 
> snmpd.conf
> example I think that is often done by copying/editing so just "enc aes" is 
> there
> with no auth setting. Wondering if that part might not have been such a good
> change and what anyone else thinks..
> 
I think that these management software applications should join 2016 and start
implementing it and until then its just two or four minor keywords per user.
But I'm not a heavy user of 3rd party mangement software.

Also note that the first time I suggested changing the defaults[0] I offered
to help with getting perl's snmp into shape. That offer still stands with the
same caveats. Similar for other open source software that I'm not aware of.

[0] https://marc.info/?l=openbsd-tech=157226549212943=2



Re: Fix unsafe snmpd defaults

2021-08-03 Thread Stuart Henderson
On 2021/08/03 22:07, Martijn van Duren wrote:
> On Tue, 2021-08-03 at 18:24 +0100, Stuart Henderson wrote:
> > On 2021/06/15 17:39, Stuart Henderson wrote:
> > > > Then again, I don't get the feeling many people use snmpd at this time
> > > > and maybe it's a good moment to bite the bullet and go for safest
> > > > defaults possible at this time. But if that's the case I would like to
> > > > follow up with a diff to changes the default auth to hmac-sha512,
> > > > because snmp drops trailing bytes of the result and enc to aes instead
> > > > of des.
> > > 
> > > This is the change that feels most likely to affect existing SNMPv3 users.
> > > Support in management software beyond aes/sha1 is a bit lacking and prone
> > > to incompatibility (I had issues with net-snmp and snmpd using hmac-sha256
> > > though it seems it will work with hmac-sha512..)
> > 
> > BTW, having updated a few machines now, I am finding the change to
> > sha2-256 by default to be a complete pain, especially considering that
> > /etc/examples/snmpd.conf uses "enc aes" but has no setting for auth
> > so relies on defaults for that..
> > 
> I can't do a lot with "a complete pain".
> 
> Does something like the diff below make things more intuitive? If not,
> could you be a little more concrete?
> 
> martijn@
> 
> Index: snmpd.conf
> ===
> RCS file: /cvs/src/etc/examples/snmpd.conf,v
> retrieving revision 1.1
> diff -u -p -r1.1 snmpd.conf
> --- snmpd.conf11 Jul 2014 21:20:10 -  1.1
> +++ snmpd.conf3 Aug 2021 20:05:53 -
> @@ -18,7 +18,9 @@ system services 74
>  oid 1.3.6.1.4.1.30155.42.3.1 name testStringValue read-only string "Test"
>  oid 1.3.6.1.4.1.30155.42.3.4 name testIntValue read-write integer 1
>  
> -# Enable SNMPv3 USM with authentication, encryption and two defined users
> -#seclevel enc
> -#user "user1" authkey "password123" enc aes enckey "321drowssap"
> -#user "user2" authkey "password456" enckey "654drowssap"
> +# Create two SNMPv3 USM users:
> +# User with default crypto values
> +#user "defaultuser" authkey "password123" enckey "321drowssap"
> +# User with backwards compatible crypto:
> +# Only enable and use when client absolutely can't deal with modern defaults.
> +#user "compatuser" authkey "password456" auth hmac-md5 enckey "654drowssap" 
> enc des
> 
> 

Given the lack of support for SHA2-256 in much management software until
recently AES+SHA is a pretty common configuration. And given the old snmpd.conf
example I think that is often done by copying/editing so just "enc aes" is there
with no auth setting. Wondering if that part might not have been such a good
change and what anyone else thinks..



Re: Fix unsafe snmpd defaults

2021-08-03 Thread Martijn van Duren
On Tue, 2021-08-03 at 18:24 +0100, Stuart Henderson wrote:
> On 2021/06/15 17:39, Stuart Henderson wrote:
> > > Then again, I don't get the feeling many people use snmpd at this time
> > > and maybe it's a good moment to bite the bullet and go for safest
> > > defaults possible at this time. But if that's the case I would like to
> > > follow up with a diff to changes the default auth to hmac-sha512,
> > > because snmp drops trailing bytes of the result and enc to aes instead
> > > of des.
> > 
> > This is the change that feels most likely to affect existing SNMPv3 users.
> > Support in management software beyond aes/sha1 is a bit lacking and prone
> > to incompatibility (I had issues with net-snmp and snmpd using hmac-sha256
> > though it seems it will work with hmac-sha512..)
> 
> BTW, having updated a few machines now, I am finding the change to
> sha2-256 by default to be a complete pain, especially considering that
> /etc/examples/snmpd.conf uses "enc aes" but has no setting for auth
> so relies on defaults for that..
> 
I can't do a lot with "a complete pain".

Does something like the diff below make things more intuitive? If not,
could you be a little more concrete?

martijn@

Index: snmpd.conf
===
RCS file: /cvs/src/etc/examples/snmpd.conf,v
retrieving revision 1.1
diff -u -p -r1.1 snmpd.conf
--- snmpd.conf  11 Jul 2014 21:20:10 -  1.1
+++ snmpd.conf  3 Aug 2021 20:05:53 -
@@ -18,7 +18,9 @@ system services 74
 oid 1.3.6.1.4.1.30155.42.3.1 name testStringValue read-only string "Test"
 oid 1.3.6.1.4.1.30155.42.3.4 name testIntValue read-write integer 1
 
-# Enable SNMPv3 USM with authentication, encryption and two defined users
-#seclevel enc
-#user "user1" authkey "password123" enc aes enckey "321drowssap"
-#user "user2" authkey "password456" enckey "654drowssap"
+# Create two SNMPv3 USM users:
+# User with default crypto values
+#user "defaultuser" authkey "password123" enckey "321drowssap"
+# User with backwards compatible crypto:
+# Only enable and use when client absolutely can't deal with modern defaults.
+#user "compatuser" authkey "password456" auth hmac-md5 enckey "654drowssap" 
enc des




Re: Fix unsafe snmpd defaults

2021-08-03 Thread Stuart Henderson
On 2021/06/15 17:39, Stuart Henderson wrote:
> > Then again, I don't get the feeling many people use snmpd at this time
> > and maybe it's a good moment to bite the bullet and go for safest
> > defaults possible at this time. But if that's the case I would like to
> > follow up with a diff to changes the default auth to hmac-sha512,
> > because snmp drops trailing bytes of the result and enc to aes instead
> > of des.
> 
> This is the change that feels most likely to affect existing SNMPv3 users.
> Support in management software beyond aes/sha1 is a bit lacking and prone
> to incompatibility (I had issues with net-snmp and snmpd using hmac-sha256
> though it seems it will work with hmac-sha512..)

BTW, having updated a few machines now, I am finding the change to
sha2-256 by default to be a complete pain, especially considering that
/etc/examples/snmpd.conf uses "enc aes" but has no setting for auth
so relies on defaults for that..



Re: iked(8): Increase the default Child SA data lifetime limit

2021-08-03 Thread Stuart Henderson
On 2021/08/03 17:02, Vitaliy Makkoveev wrote:
> > - a 50% lower limit feels too low to me
> > 
> 
> Why? The 95% limit is too close to lifetime expiration and as it was
> exposed we don't have enough time to perform rekeying. I also had this
> problem while tested iked(8) over WIFI connection and this is one of
> real-world usage cases.

Rekeying with 9-18 minutes spare out of a 180 minute lifetime seems pretty
good to me.

Rekeying with 72-90 minutes left of a 180 minute lifetime seems way too
early.

For bytes, if 10% of the byte lifetime isn't long enough to complete a
rekey, that really suggests to me the lifetime is set too low, not that
the jitter amount is too low.



Re: iked(8): Increase the default Child SA data lifetime limit

2021-08-03 Thread Vitaliy Makkoveev
On Mon, Aug 02, 2021 at 09:09:03PM -0600, Theo de Raadt wrote:
> 
> I suspect the first step is to make the rekey decision be based upon the
> strength of the ciphers.
> 

Do you mean the special default limits for each cipher?



Re: iked(8): Increase the default Child SA data lifetime limit

2021-08-03 Thread Vitaliy Makkoveev
On Tue, Aug 03, 2021 at 12:17:38PM +0100, Stuart Henderson wrote:
> On 2021/08/03 01:12, Vitaliy Makkoveev wrote:
> > iked(8) uses 3 hours and 512 megabytes of processed data as default
> > lifetime hard limits for Child SA. Also it sets 85-95% of these values as
> > soft limit. iked(8) should perform rekeying before we reach hard limit
> > otherwise this SA will be killed and the tunnel stopped. With default
> > values the window is only 25-52 megabytes and we easily consume them
> > before rekeying and the tunnel stops.
> > 
> > Hrvoje Popovski complained about such stops when he has tested ipsec(4)
> > related diffs. I also tried iked(8) with my macos and found that simple
> > "ping -f ..." makes rekeying impossible.
> > 
> > The hard limit could be modified in iked.conf(5) by setting "lifetime
> > xxx bytes yyy", but the 5% difference between hard and soft limits forces
> > to set bytes limit big enough, about 4G and more, which could be bad for
> > security reason.
> > 
> > I propose to increase the default hard limit at least up to 1G. Also I
> > propose to decrease the soft limit down to 50-60% of hard limit. This
> > keeps the rekeying frequency but increases the update window to 410-512
> > megabytes. Also this allow to keep bytes in "lifetime" setting small
> > enough.
> 
> I have a couple of comments;
> 
> - this isn't a problem I've run into with real-world usage or when
> running tcpbench over (moderately fast) internet connections - I'm not
> saying it doesn't happen, but it seems relatively uncommon, with
> connections at LAN speeds of course it's much more likely
> 
> - a 50% lower limit feels too low to me
> 

Why? The 95% limit is too close to lifetime expiration and as it was
exposed we don't have enough time to perform rekeying. I also had this
problem while tested iked(8) over WIFI connection and this is one of
real-world usage cases.

> - your jitter change affects lifetime both in seconds and in bytes,
> I think changing the jitter for the seconds lifetime is a mistake
> 
> - the jitter change could result in some really short rekey intervals
> if somebody has manually specified lifetimes which are the same as or less
> than the current default
> 

The original code permits you to set "lifetime 1 bytes 1" in
iked.conf(5) so you could have SA with hard lifetime 1 second and 1 byte
and soft lifetime with 0 seconds (disabled) and 0 bytes (disabled)
limit. You could successfully connect but rekeying will never happened.

Is this iked(8) problem or the wrong iked(8) setup problem? Who should
solve it (for example, print error message and don't startup)?

> - looking at other IKEv2 implementations: if bytes lifetime is supported
> at all (several implementations don't have it, only time-based lifetime),
> the default settings rarely seem to use it
> 
> - 512MB is not really a lot of data
>

As I said, 410-512Mb limit was chosen because the "lifetime 3h bytes 4G"
pretty works with original iked(8) 95% jitter. At least for me and
Hrvoje. 95% jitter with 4G limit provides us 205Mb for rekeying.



Re: iked(8): Increase the default Child SA data lifetime limit

2021-08-03 Thread Vitaliy Makkoveev
On Tue, Aug 03, 2021 at 01:40:51PM +0200, Tobias Heider wrote:
> On Tue, Aug 03, 2021 at 12:17:38PM +0100, Stuart Henderson wrote:
> > On 2021/08/03 01:12, Vitaliy Makkoveev wrote:
> > > iked(8) uses 3 hours and 512 megabytes of processed data as default
> > > lifetime hard limits for Child SA. Also it sets 85-95% of these values as
> > > soft limit. iked(8) should perform rekeying before we reach hard limit
> > > otherwise this SA will be killed and the tunnel stopped. With default
> > > values the window is only 25-52 megabytes and we easily consume them
> > > before rekeying and the tunnel stops.
> > > 
> > > Hrvoje Popovski complained about such stops when he has tested ipsec(4)
> > > related diffs. I also tried iked(8) with my macos and found that simple
> > > "ping -f ..." makes rekeying impossible.
> > > 
> > > The hard limit could be modified in iked.conf(5) by setting "lifetime
> > > xxx bytes yyy", but the 5% difference between hard and soft limits forces
> > > to set bytes limit big enough, about 4G and more, which could be bad for
> > > security reason.
> > > 
> > > I propose to increase the default hard limit at least up to 1G. Also I
> > > propose to decrease the soft limit down to 50-60% of hard limit. This
> > > keeps the rekeying frequency but increases the update window to 410-512
> > > megabytes. Also this allow to keep bytes in "lifetime" setting small
> > > enough.
> > 
> > I have a couple of comments;
> > 
> > - this isn't a problem I've run into with real-world usage or when
> > running tcpbench over (moderately fast) internet connections - I'm not
> > saying it doesn't happen, but it seems relatively uncommon, with
> > connections at LAN speeds of course it's much more likely
> > 
> > - a 50% lower limit feels too low to me
> > 
> > - your jitter change affects lifetime both in seconds and in bytes,
> > I think changing the jitter for the seconds lifetime is a mistake
> > 
> > - the jitter change could result in some really short rekey intervals
> > if somebody has manually specified lifetimes which are the same as or less
> > than the current default
> > 
> > - looking at other IKEv2 implementations: if bytes lifetime is supported
> > at all (several implementations don't have it, only time-based lifetime),
> > the default settings rarely seem to use it
> > 
> > - 512MB is not really a lot of data
> > 
> > My first though now I know about this problem is just to increase the
> > default byte limit and leave the jitter range as-is. I don't know enough
> > about the security requirements of IKEv2 to know what demands it places
> > on rekeying, but given the above (especially that other implementations
> > mostly don't use byte limits at all), the figure I'd pull out of the air
> > would be more like 4GB.
> > 
> 
> I agree with Stuart here. In my experience raising the limit to 4 GB is enough
> to solve the problem and the current jitter works well enough.
> 
> In a next step we can think about relaxing the limits even further for "safe"
> algorithms like Theo proposed, but this would need a bit more work.
> 
> FWIW here's a diff I sent to bluhm a few weeks ago.  We didn't commit it yet
> because the low limit helped us find and reproduce a PMTU bug (that should
> be gone now).
> 

As I said, I tested the iked(8) tunnels with "lifetime 3h bytes 4G" in
my iked.conf(5) and the problem disappeared. With this case 95% jitter
provides ~205 megabytes window for rekeying and this is pretty enough.
Hrvoje also tested his setup with "lifetime 3h bytes 4G" and didn't hit
the problem. So I decided the 410 megabytes window is pretty enough as
default.

Raising the bytes default limit to 4G is ok by me, but you forget to
modify iked.conf(5) man page:

lifetime time [bytes bytes]
The optional lifetime parameter defines the Child SA
expiration timeout by the time SA was in use and by
the number of bytes that were processed using the SA.
Default values are 3 hours and 512...

> Index: types.h
> ===
> RCS file: /cvs/src/sbin/iked/types.h,v
> retrieving revision 1.43
> diff -u -p -r1.43 types.h
> --- types.h   13 May 2021 15:20:48 -  1.43
> +++ types.h   3 Aug 2021 11:35:26 -
> @@ -67,7 +67,7 @@
>  #define IKED_CYCLE_BUFFERS   8   /* # of static buffers for mapping */
>  #define IKED_PASSWORD_SIZE   256 /* limited by most EAP types */
>  
> -#define IKED_LIFETIME_BYTES  536870912 /* 512 Mb */
> +#define IKED_LIFETIME_BYTES  4294967296 /* 4 GB */
>  #define IKED_LIFETIME_SECONDS10800 /* 3 hours */
>  
>  #define IKED_E   0x1000  /* Decrypted flag */



Re: iked(8): Increase the default Child SA data lifetime limit

2021-08-03 Thread Patrick Wildt
Am Tue, Aug 03, 2021 at 01:40:51PM +0200 schrieb Tobias Heider:
> On Tue, Aug 03, 2021 at 12:17:38PM +0100, Stuart Henderson wrote:
> > On 2021/08/03 01:12, Vitaliy Makkoveev wrote:
> > > iked(8) uses 3 hours and 512 megabytes of processed data as default
> > > lifetime hard limits for Child SA. Also it sets 85-95% of these values as
> > > soft limit. iked(8) should perform rekeying before we reach hard limit
> > > otherwise this SA will be killed and the tunnel stopped. With default
> > > values the window is only 25-52 megabytes and we easily consume them
> > > before rekeying and the tunnel stops.
> > > 
> > > Hrvoje Popovski complained about such stops when he has tested ipsec(4)
> > > related diffs. I also tried iked(8) with my macos and found that simple
> > > "ping -f ..." makes rekeying impossible.
> > > 
> > > The hard limit could be modified in iked.conf(5) by setting "lifetime
> > > xxx bytes yyy", but the 5% difference between hard and soft limits forces
> > > to set bytes limit big enough, about 4G and more, which could be bad for
> > > security reason.
> > > 
> > > I propose to increase the default hard limit at least up to 1G. Also I
> > > propose to decrease the soft limit down to 50-60% of hard limit. This
> > > keeps the rekeying frequency but increases the update window to 410-512
> > > megabytes. Also this allow to keep bytes in "lifetime" setting small
> > > enough.
> > 
> > I have a couple of comments;
> > 
> > - this isn't a problem I've run into with real-world usage or when
> > running tcpbench over (moderately fast) internet connections - I'm not
> > saying it doesn't happen, but it seems relatively uncommon, with
> > connections at LAN speeds of course it's much more likely
> > 
> > - a 50% lower limit feels too low to me
> > 
> > - your jitter change affects lifetime both in seconds and in bytes,
> > I think changing the jitter for the seconds lifetime is a mistake
> > 
> > - the jitter change could result in some really short rekey intervals
> > if somebody has manually specified lifetimes which are the same as or less
> > than the current default
> > 
> > - looking at other IKEv2 implementations: if bytes lifetime is supported
> > at all (several implementations don't have it, only time-based lifetime),
> > the default settings rarely seem to use it
> > 
> > - 512MB is not really a lot of data
> > 
> > My first though now I know about this problem is just to increase the
> > default byte limit and leave the jitter range as-is. I don't know enough
> > about the security requirements of IKEv2 to know what demands it places
> > on rekeying, but given the above (especially that other implementations
> > mostly don't use byte limits at all), the figure I'd pull out of the air
> > would be more like 4GB.
> > 
> 
> I agree with Stuart here. In my experience raising the limit to 4 GB is enough
> to solve the problem and the current jitter works well enough.
> 
> In a next step we can think about relaxing the limits even further for "safe"
> algorithms like Theo proposed, but this would need a bit more work.
> 
> FWIW here's a diff I sent to bluhm a few weeks ago.  We didn't commit it yet
> because the low limit helped us find and reproduce a PMTU bug (that should
> be gone now).

ok patrick@

> Index: types.h
> ===
> RCS file: /cvs/src/sbin/iked/types.h,v
> retrieving revision 1.43
> diff -u -p -r1.43 types.h
> --- types.h   13 May 2021 15:20:48 -  1.43
> +++ types.h   3 Aug 2021 11:35:26 -
> @@ -67,7 +67,7 @@
>  #define IKED_CYCLE_BUFFERS   8   /* # of static buffers for mapping */
>  #define IKED_PASSWORD_SIZE   256 /* limited by most EAP types */
>  
> -#define IKED_LIFETIME_BYTES  536870912 /* 512 Mb */
> +#define IKED_LIFETIME_BYTES  4294967296 /* 4 GB */
>  #define IKED_LIFETIME_SECONDS10800 /* 3 hours */
>  
>  #define IKED_E   0x1000  /* Decrypted flag */
> 



Re: iked(8): Increase the default Child SA data lifetime limit

2021-08-03 Thread Tobias Heider
On Tue, Aug 03, 2021 at 12:17:38PM +0100, Stuart Henderson wrote:
> On 2021/08/03 01:12, Vitaliy Makkoveev wrote:
> > iked(8) uses 3 hours and 512 megabytes of processed data as default
> > lifetime hard limits for Child SA. Also it sets 85-95% of these values as
> > soft limit. iked(8) should perform rekeying before we reach hard limit
> > otherwise this SA will be killed and the tunnel stopped. With default
> > values the window is only 25-52 megabytes and we easily consume them
> > before rekeying and the tunnel stops.
> > 
> > Hrvoje Popovski complained about such stops when he has tested ipsec(4)
> > related diffs. I also tried iked(8) with my macos and found that simple
> > "ping -f ..." makes rekeying impossible.
> > 
> > The hard limit could be modified in iked.conf(5) by setting "lifetime
> > xxx bytes yyy", but the 5% difference between hard and soft limits forces
> > to set bytes limit big enough, about 4G and more, which could be bad for
> > security reason.
> > 
> > I propose to increase the default hard limit at least up to 1G. Also I
> > propose to decrease the soft limit down to 50-60% of hard limit. This
> > keeps the rekeying frequency but increases the update window to 410-512
> > megabytes. Also this allow to keep bytes in "lifetime" setting small
> > enough.
> 
> I have a couple of comments;
> 
> - this isn't a problem I've run into with real-world usage or when
> running tcpbench over (moderately fast) internet connections - I'm not
> saying it doesn't happen, but it seems relatively uncommon, with
> connections at LAN speeds of course it's much more likely
> 
> - a 50% lower limit feels too low to me
> 
> - your jitter change affects lifetime both in seconds and in bytes,
> I think changing the jitter for the seconds lifetime is a mistake
> 
> - the jitter change could result in some really short rekey intervals
> if somebody has manually specified lifetimes which are the same as or less
> than the current default
> 
> - looking at other IKEv2 implementations: if bytes lifetime is supported
> at all (several implementations don't have it, only time-based lifetime),
> the default settings rarely seem to use it
> 
> - 512MB is not really a lot of data
> 
> My first though now I know about this problem is just to increase the
> default byte limit and leave the jitter range as-is. I don't know enough
> about the security requirements of IKEv2 to know what demands it places
> on rekeying, but given the above (especially that other implementations
> mostly don't use byte limits at all), the figure I'd pull out of the air
> would be more like 4GB.
> 

I agree with Stuart here. In my experience raising the limit to 4 GB is enough
to solve the problem and the current jitter works well enough.

In a next step we can think about relaxing the limits even further for "safe"
algorithms like Theo proposed, but this would need a bit more work.

FWIW here's a diff I sent to bluhm a few weeks ago.  We didn't commit it yet
because the low limit helped us find and reproduce a PMTU bug (that should
be gone now).

Index: types.h
===
RCS file: /cvs/src/sbin/iked/types.h,v
retrieving revision 1.43
diff -u -p -r1.43 types.h
--- types.h 13 May 2021 15:20:48 -  1.43
+++ types.h 3 Aug 2021 11:35:26 -
@@ -67,7 +67,7 @@
 #define IKED_CYCLE_BUFFERS 8   /* # of static buffers for mapping */
 #define IKED_PASSWORD_SIZE 256 /* limited by most EAP types */
 
-#define IKED_LIFETIME_BYTES536870912 /* 512 Mb */
+#define IKED_LIFETIME_BYTES4294967296 /* 4 GB */
 #define IKED_LIFETIME_SECONDS  10800 /* 3 hours */
 
 #define IKED_E 0x1000  /* Decrypted flag */



Re: iked(8): Increase the default Child SA data lifetime limit

2021-08-03 Thread Stuart Henderson
On 2021/08/03 01:12, Vitaliy Makkoveev wrote:
> iked(8) uses 3 hours and 512 megabytes of processed data as default
> lifetime hard limits for Child SA. Also it sets 85-95% of these values as
> soft limit. iked(8) should perform rekeying before we reach hard limit
> otherwise this SA will be killed and the tunnel stopped. With default
> values the window is only 25-52 megabytes and we easily consume them
> before rekeying and the tunnel stops.
> 
> Hrvoje Popovski complained about such stops when he has tested ipsec(4)
> related diffs. I also tried iked(8) with my macos and found that simple
> "ping -f ..." makes rekeying impossible.
> 
> The hard limit could be modified in iked.conf(5) by setting "lifetime
> xxx bytes yyy", but the 5% difference between hard and soft limits forces
> to set bytes limit big enough, about 4G and more, which could be bad for
> security reason.
> 
> I propose to increase the default hard limit at least up to 1G. Also I
> propose to decrease the soft limit down to 50-60% of hard limit. This
> keeps the rekeying frequency but increases the update window to 410-512
> megabytes. Also this allow to keep bytes in "lifetime" setting small
> enough.

I have a couple of comments;

- this isn't a problem I've run into with real-world usage or when
running tcpbench over (moderately fast) internet connections - I'm not
saying it doesn't happen, but it seems relatively uncommon, with
connections at LAN speeds of course it's much more likely

- a 50% lower limit feels too low to me

- your jitter change affects lifetime both in seconds and in bytes,
I think changing the jitter for the seconds lifetime is a mistake

- the jitter change could result in some really short rekey intervals
if somebody has manually specified lifetimes which are the same as or less
than the current default

- looking at other IKEv2 implementations: if bytes lifetime is supported
at all (several implementations don't have it, only time-based lifetime),
the default settings rarely seem to use it

- 512MB is not really a lot of data

My first though now I know about this problem is just to increase the
default byte limit and leave the jitter range as-is. I don't know enough
about the security requirements of IKEv2 to know what demands it places
on rekeying, but given the above (especially that other implementations
mostly don't use byte limits at all), the figure I'd pull out of the air
would be more like 4GB.

> Index: sbin/iked/iked.conf.5
> ===
> RCS file: /cvs/src/sbin/iked/iked.conf.5,v
> retrieving revision 1.85
> diff -u -p -r1.85 iked.conf.5
> --- sbin/iked/iked.conf.5 11 Apr 2021 23:27:06 -  1.85
> +++ sbin/iked/iked.conf.5 2 Aug 2021 21:41:55 -
> @@ -586,8 +586,8 @@ parameter defines the Child SA expiratio
>  SA was in use and by the number of
>  .Ar bytes
>  that were processed using the SA.
> -Default values are 3 hours and 512 megabytes which means that SA will be
> -rekeyed before reaching the time limit or 512 megabytes of data
> +Default values are 3 hours and 1024 megabytes which means that SA will be
> +rekeyed before reaching the time limit or 1024 megabytes of data
>  will pass through.
>  Zero values disable rekeying.

hm, this doesn't mention jitter. I think it should. Perhaps something
like this.

Default values are X hours and Y megabytes.
Rekeying is initiated at between A% and B% of the limit;
if unsuccessful the SA will not be allowed to continue beyond the
hard limit.

> -#define IKED_LIFETIME_BYTES  536870912 /* 512 Mb */
> +#define IKED_LIFETIME_BYTES  1073741824 /* 512 Mb */

Comment is now wrong. I think I would write this as N * 1024 * 1024
and remove the comment, but if not then the comment needs to match.

>  #define IKED_LIFETIME_SECONDS10800 /* 3 hours */
>  
>  #define IKED_E   0x1000  /* Decrypted flag */
> 



Re: Rationale behind exec clearing out unveil paths

2021-08-03 Thread Sebastian Benoit
dz...@disroot.org(dz...@disroot.org) on 2021.06.15 14:12:22 +:
> > Seems to be working as intended. You are letting someone run all binaries.
> And I am not letting someone write to the filesystem. Yet, they can
> bypass that easily. `unveil("/", "rx")` gives a false illusion of
> security, which can even trip up OpenBSD maintainers (more below).
> 
> > Or is it your expectation is that all binaries should crash when they
> > cannot start ld.so or load libc?
> "/" is mounted for reads, why would a program crash while loading
> libc? You don't need write access to execute a program.
> 
> > I'd say the problem is whoever wrote this code unrealistic 2-liner code
> > example, oh wait, that is you.
>   (and)
> > The expected uses of unveil and pledge aren't some weird fiction
> > of "oh look I can use them wrong".
> https://github.com/openbsd/src/commit/15e2c6823410e554b348cd3fb137566da656e866
> 
> 
> Also to be clear - I'm not throwing blame to the author of the commit
> here, it's not their fault. This behaviour isn't documented, so unless
> you have seen the exec() source, you wouldn't know about it.

If anything, that example shows that relayd needs to be redesigned to make
good use of pledge() and unveil(). Because of the capability to reload the
configuration (making it possible to change the patch to external check
programms), with the current structure of relayd it is impossible to do this
better.

The way relayd runs external check scripts need to be changed such
that a tighter unveil() becomes possible.

Maybe you can come up with a patch?