ipsec(4): add sockopt to manage additional udpencap ports

2021-10-25 Thread Tobias Heider
UDP-encapsulaton has two common use-cases: to make ESP work with NATs and to circumvent firewalls. Currently we only support a single UDP-encap port at a time, that is globally configured via sysctl. With an iked server, having a single client using a non-standard port to circumvent a firewall

Re: ipsec(4): cleanup ipcomp

2021-10-24 Thread Tobias Heider
On Sun, Oct 24, 2021 at 07:16:27PM +0200, Tobias Heider wrote: > On Sun, Oct 24, 2021 at 07:10:22PM +0200, Tobias Heider wrote: > > The diff below removes ipcomp_input_cb(), ipcomp_output_cb() and some other > > things related to the old crypto API. > > > > ok? >

Re: ipsec(4): cleanup ipcomp

2021-10-24 Thread Tobias Heider
On Sun, Oct 24, 2021 at 07:10:22PM +0200, Tobias Heider wrote: > The diff below removes ipcomp_input_cb(), ipcomp_output_cb() and some other > things related to the old crypto API. > > ok? small update to move ip6_hdr out of '#if NBPFILTER > 0' Ind

ipsec(4): cleanup ipcomp

2021-10-24 Thread Tobias Heider
The diff below removes ipcomp_input_cb(), ipcomp_output_cb() and some other things related to the old crypto API. ok? Index: ip_ipcomp.c === RCS file: /cvs/src/sys/netinet/ip_ipcomp.c,v retrieving revision 1.84 diff -u -p -r1.84

Re: ipsec(4): speedup ESP

2021-10-24 Thread Tobias Heider
On Sun, Oct 24, 2021 at 05:05:06PM +0200, Tobias Heider wrote: > On Sat, Oct 23, 2021 at 10:17:54PM +0200, Tobias Heider wrote: > > The diff below removes a few leftover tdb_crypto allocations in esp_input() > > and esp_output(). The allocations were needed to pass arguments to t

Re: ipsec(4): speedup ESP

2021-10-24 Thread Tobias Heider
On Sat, Oct 23, 2021 at 10:17:54PM +0200, Tobias Heider wrote: > The diff below removes a few leftover tdb_crypto allocations in esp_input() > and esp_output(). The allocations were needed to pass arguments to the > callback function with the non-blocking crypto API and are redu

ipsec(4): speedup ESP

2021-10-23 Thread Tobias Heider
The diff below removes a few leftover tdb_crypto allocations in esp_input() and esp_output(). The allocations were needed to pass arguments to the callback function with the non-blocking crypto API and are redundant now that crypto is blocking. This should result in a notable speedup for ESP.

crypto: retire async crypto API

2021-10-23 Thread Tobias Heider
Now that we have removed all the legacy crypto offloading drivers we can simplify the crypto framework API by ripping out the async callbacks. The diff below removes crypto_dispatch() and crypto_done() and replaces them with crypto_invoke() which is blocking and only returns after the operation

crypto: remove crypto queue

2021-10-21 Thread Tobias Heider
Currently, all crypto users set CRYPTO_F_NOQUEUE to run crypto operations without queue and there are no plans to switch back to using the queue. The diff below removes the flag together with the queueing code. ok? Index: dev/softraid_crypto.c

Re: iked(8): make proto option accept lists

2021-09-04 Thread Tobias Heider
Here's an updated diff including the man page bits. Looking at pf.conf(5) and ipsec.conf(5), there does not really seem to be a standard way to document which parameters accept lists. Index: iked.conf.5 === RCS file:

Re: iked(8): make proto option accept lists

2021-09-03 Thread Tobias Heider
On Fri, Sep 03, 2021 at 10:12:57AM +0200, Sebastian Benoit wrote: > Tobias Heider(tobias.hei...@stusta.de) on 2021.09.02 15:39:46 +0200: > > The diff below makes iked accept a list of protocols for the "proto" config > > option in iked.conf(5). > > This would a

iked(8): make proto option accept lists

2021-09-02 Thread Tobias Heider
The diff below makes iked accept a list of protocols for the "proto" config option in iked.conf(5). This would allow us to have a single policy with "proto { ipencap, ipv6 }" to secure a gif(4) tunnel, instead of requiring one policy for each protocol. ok? Index: iked.h

Re: iked(8): client-side DNS support via resolvd(8)

2021-09-01 Thread Tobias Heider
Here's an updated diff with the following changes: - Send the ifidx of the configured 'iface' instead of ifidx 0 to prevent name collisions - Cache the first received DNS server locally for cleanup/resending. - Handle RTP_PROPOSAL_SOLICIT by resending the cached server. - Remove the cached

iked(8): client-side DNS support via resolvd(8)

2021-08-31 Thread Tobias Heider
IKEv2 allows road warrior servers to announce internal name servers in a configuration payload. iked responders can be configured to send such payloads with the 'config name-server' option. This diff adds support for receiving DNS server configuration payloads as a road warrior client and

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

Re: ipsec: remove unused `PolicyHead' from 'sockaddr_encap' structure

2021-07-12 Thread Tobias Heider
On Sun, Jul 11, 2021 at 05:33:18AM +0300, Vitaliy Makkoveev wrote: > This member is never set or used. Also I kept 'SENT_IP6' definition for > prevent the potential break of third party software. Is it ok to > redefine it to '0x0002'? At least openswan wants this [1]. > > 1. >

pfkey: export tdb mtu

2021-07-04 Thread Tobias Heider
Hi, here's a diff to export tdb MTUs via pfkey and view them with ipsecctl. This turned out to be quite useful to debug path MTU discovery issues with IPsec UDP encapsulation. ok? Index: sys/net/pfkeyv2.c === RCS file:

Re: Patch: Send AUTHENTICATION_FAILED in case of unexpected auth method or auth data not being accessible

2021-06-29 Thread Tobias Heider
On Tue, Jun 29, 2021 at 05:26:09PM +0200, Patrick Wildt wrote: > Am Tue, Jun 29, 2021 at 10:39:06AM + schrieb Claudia Priesterjahn: > > We added two AUTHENTICATION_FAILED notifications for the cases that > > the peer used an unexepected authentication method and for the case > > that the

dt(4): skip probe frames on arm64

2021-06-09 Thread Tobias Heider
Hi, the diff below adds DT_FA_PROFILE and DT_FA_STATIC defines for arm64 to skip the probe context frames. Here is how a typical arm64 stack trace looks with and without diff: dt_pcb_ring_get+0x130 dt_prov_profile_enter+0x90 hardclock+0x1b0 agtimer_intr+0xa4 ampintc_irq_handler+0x1c0

OpenIKED 6.9.0 Released

2021-05-18 Thread Tobias Heider
OpenIKED 6.9.0 has just been released. It will be arriving in the OpenIKED directory of your local OpenBSD mirror soon. OpenIKED is a free, permissively licensed Internet Key Exchange (IKEv2) implementation, developed as part of the OpenBSD project. It is intended to be a lean, secure and

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

2021-05-14 Thread Tobias Heider
On Thu, May 13, 2021 at 02:39:37PM +0900, Katsuhiro Ueno wrote: > Hi, > > I would be happy if iked(8) supports intermediate CAs and sends the > entire certificate chain to the clients. The diff attached adds > supports for intermediate CAs and multiple CERT payloads to iked(8). > > What I would

Re: LibreSSL: handle EXFLAG_INVALID

2021-03-13 Thread Tobias Heider
On Wed, Mar 03, 2021 at 05:36:12PM +0100, Theo Buehler wrote: > On Thu, Feb 25, 2021 at 09:34:30PM +0100, Tobias Heider wrote: > > Hi, > > > > while testing different x509 validator corner cases i found that a bunch of > > errors are currently not handled in libcry

LibreSSL: handle EXFLAG_INVALID

2021-02-25 Thread Tobias Heider
Hi, while testing different x509 validator corner cases i found that a bunch of errors are currently not handled in libcrypto. In particular duplicate or undecodable extensions are ignored. The diff below sets EXFLAG_INVALID whenever X509_get_ext_d2i() returns an error (other than "not found")

iked(8): roadwarrior client support

2021-02-08 Thread Tobias Heider
d_type = get_id_type(srcid); pol.pol_localid.id_length = strlen(srcid); diff --git a/sbin/iked/policy.c b/sbin/iked/policy.c index df7f2676dd1..a21099afe39 100644 --- a/sbin/iked/policy.c +++ b/sbin/iked/policy.c @@ -1,6 +1,7 @@ /* $OpenBSD: policy.c,v 1.75 2021/02/01 16:37:

ieee80211: Flush reorder buffer after gap timeout

2020-12-08 Thread Tobias Heider
Hi, here is another diff that should fix associating with some APs that currently don't work. If block ack is active and the first frame got lost, subsequent packets are held back until a timeout expires. When this timeout expires, the gap at the start of the reorder buffer is skipped and

Re: net80211: Use a BA agreement for rx immediately

2020-12-07 Thread Tobias Heider
On Mon, Dec 07, 2020 at 02:33:10PM +0100, Stefan Sperling wrote: > On Mon, Dec 07, 2020 at 01:31:09PM +0100, Tobias Heider wrote: > > Some APs request a BA agreement and continue to send QOS packets > > for the same tid (with normal ack policy). Currently, these packets > >

iwx(4): decoding of multiple MPDUs in one receive packet

2020-12-07 Thread Tobias Heider
Hi, this is an iwx(4) port of the iwm(4) fix by Christian Erhardt which I sent in a previous mail: https://marc.info/?l=openbsd-tech=160733342209497=2 I don't have a iwx(4) card to test this, but the diff to iwm(4) is trivial. ok? Index: if_iwx.c

net80211: Better gapwait accounting

2020-12-07 Thread Tobias Heider
Hi, our net80211 gapwait accounting implementation seems to have several problems: - If we lose packets with serial numbers 0 und 2 but receive the packet with serial number 1, the first gap wait timeout will skip serial number 0, flush out serial number 1 and then wait for serial number 2.

net80211: Use a BA agreement for rx immediately

2020-12-07 Thread Tobias Heider
Some APs request a BA agreement and continue to send QOS packets for the same tid (with normal ack policy). Currently, these packets make it to the higher layers without going through BA reordering or the BA buffer. This results in reduced performance later on as the sequence numbers are expected

iwm(4): decoding of multiple MPDUs in one receive packet

2020-12-07 Thread Tobias Heider
Hi, In iwm_rx_pkt() the calculation of "remain" seems to be wrong if there are three or more MPDUs in one packet. "remain" is initialized with the output buffer size. Each time an MPDU is found in the packet remain is reduced by the offset of the MPDU in the receive buffer, which is only correct

Re: [PATCH] Macbook1,1 - minimal diff to swap 2 key for ISO internal kbd

2020-10-28 Thread Tobias Heider
On Wed, Oct 28, 2020 at 08:54:13PM +0100, Mathias Schmocker wrote: > Hello, > Here a minimal diff to solve the swapped keys of the internal ISO > keyboard/trackpad found on my older Macbook1,1 13inch black laptop Looks ok. One nit: I would propose changing the name of the device. It seems the

macppc: fix initial wsconsctl display.brighness

2020-10-28 Thread Tobias Heider
Hi, playing around with the display brightness i found that the initial state seems to be broken. We initiate the value at MAX_BRIGHTNESS while in reality it is much lower than that after boot. Increasing the brightness won't work after booting because wscons thinks we are at 100%, while

Re: ukbd(4): support apple brightness keys

2020-10-27 Thread Tobias Heider
> What about KS_Cmd_BrightnessUp and KS_Cmd_BrightnessDown? Right, here's a new diff using those wskbd commands. I couldn't find any standardized UHID key codes for brightness keys so I chose 232 and 233 which are currently unused and in the RESERVED range. I also included the regenerated

Re: ukbd(4): support apple brightness keys

2020-10-27 Thread Tobias Heider
On Tue, Oct 27, 2020 at 02:22:24PM +0100, Klemens Nanni wrote: > On Tue, Oct 27, 2020 at 12:16:16AM +0100, Tobias Heider wrote: > > the diff below makes the brightness keys work on apple powerbooks > 5,6 > > where the keyboard attaches via ukbd(4). > > I'm wondering if

ukbd(4): support apple brightness keys

2020-10-26 Thread Tobias Heider
Hi, the diff below makes the brightness keys work on apple powerbooks > 5,6 where the keyboard attaches via ukbd(4). I'm wondering if it would be better to go through wskbd as is done with audio, but we don't have keycodes allocated for brightness up/down. Thoughts? ok? Index: ukbd.c

macppc: fix wsconsctl for non-vgafb

2020-10-26 Thread Tobias Heider
Hi, i am trying to get backlight and brightness control running on my Powerbook G4. Both can be controlled via ofw. The vgafb(4) video driver currently directly calls of_setbrightness() directly instead of relying on wscons, DRM users are out of luck. The diff below wires the ofw backlight

utpms(4): less jumpy mouse movement

2020-10-19 Thread Tobias Heider
Hi, i noticed that the mouse movement on my powermac can be pretty jittery at times. One of the reasons I have identified is our use of a position change threshold. The driver ignores all finger position changes below a certain threshold. If the finger position change is > threshold it is used

iked(8): check ibuf_seek() return value

2020-09-29 Thread Tobias Heider
Hi, the diff below adds a missing return value check for ibuf_seek() in IKEv2 fragment reassembly. ok? diff --git a/sbin/iked/ikev2_pld.c b/sbin/iked/ikev2_pld.c index 7102cfff6fd..2475be07299 100644 --- a/sbin/iked/ikev2_pld.c +++ b/sbin/iked/ikev2_pld.c @@ -1793,6 +1793,7 @@

httpd(8): don't leak iov

2020-09-20 Thread Tobias Heider
iov is allocated with calloc. I think we should free it after the imsg is sent. ok? Index: config.c === RCS file: /cvs/src/usr.sbin/httpd/config.c,v retrieving revision 1.57 diff -u -p -r1.57 config.c --- config.c8 May 2019

iked(8): add stronger RNG/INTEGR transforms by default

2020-09-18 Thread Tobias Heider
Hi, I would like to activate the stronger SHA2-364 and SHA2-512 by default for INTEGR and PRF transforms to get a bit better out of the box compatibility. iked to iked connections default to AES-GCM-128 and don't use an explicit INTEGR transform, so performance should not suffer. Any objections

utpms(4): fix geyser 2 y_sensors

2020-09-10 Thread Tobias Heider
Hi, the newer Geyser 2 touchpad has only 9 sensors in the Y-direction instead of 16 like the other Apple touch pads. The driver sets sc_y_sensors correctly and then immediately overwrites it with the wrong default. I think we should first set the defaults and then treat the special cases. ok?

Re: iked.conf.5: provide gre example

2020-07-15 Thread Tobias Heider
On Wed, Jul 15, 2020 at 05:34:31PM +0200, Klemens Nanni wrote: > Here's an addition to EXAMPLES for one of my frequent use cases that > finally "just works". > > First transport mode for child SAs was implemented, then a few > interoperability issues have been identified with peers other than

Re: 11n Tx aggregation for iwm(4)

2020-06-27 Thread Tobias Heider
Works for me on a 7260. [ ID] Interval Transfer Bandwidth [ 3] 0.0-10.1 sec 108 MBytes 90.1 Mbits/sec

Re: Blacklist Ericsson F5521GW from umass

2020-06-22 Thread Tobias Heider
On Mon, Jun 22, 2020 at 02:01:43PM +0200, Tobias Heider wrote: > Hi, > > I noticed that the ramdisk takes ages to boot on my T420. > It seems that without umodem in the kernel, umass tries to attach to my > Erricson F5521GW WAN modem and fails after a annoyingly long timeout. >

Blacklist Ericsson F5521GW from umass

2020-06-22 Thread Tobias Heider
Hi, I noticed that the ramdisk takes ages to boot on my T420. It seems that without umodem in the kernel, umass tries to attach to my Erricson F5521GW WAN modem and fails after a annoyingly long timeout. The diff below should prevent umass from matching this device. ok? diff --git

Re: Error reporting in ikev2_ike_sa_alive (was: Improve error reporting in pfkey_sa_last_used)

2020-05-31 Thread Tobias Heider
On Tue, May 26, 2020 at 12:08:08PM -0400, matthew j weaver wrote: > During childsa last use checks, iked debug logs results, per SA, after a > successful pfkey_sa_last_used call. > > This patch makes logging behavior more closely match that, on error. > > I chose log_warn instead of log_debug

iked(8): AES-GCM in default proposals

2020-05-28 Thread Tobias Heider
Hi, now that we finally have AES-GCM in IKE and ESP I would like to add them as defaults. This is a bit more complicated than one might think because AEADs and non-AEADs can not be sent in the same proposal. Instead we must send one proposal for each and adjust the config parser a bit. Test

Re: iked(8): AES_GCM ciphers for IKE

2020-05-26 Thread Tobias Heider
On Thu, May 14, 2020 at 10:07:30PM +0200, Tobias Heider wrote: > Hi, > > currently iked(8) supports AES-GCM only for ESP. > The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants for IKE. > (for more information see [1] and [2]). > Both variants support the 128, 19

Re: WireGuard patchset for OpenBSD, rev. 2

2020-05-26 Thread Tobias Heider
On Tue, May 26, 2020 at 07:39:01PM +1000, Matt Dunwoodie wrote: > Hi tech, > > After some feedback and comments, we've addressed the concerns, and > fixed a few things from our side too. Overall the structure is familiar > with no major changes, so any prior readings mostly carry over. > > This

Re: WireGuard patchset for OpenBSD

2020-05-25 Thread Tobias Heider
Hi Matt, i finally found some time to look at your diff and it looks pretty good to me so far. I have a few question about the SIOCGWG ioctl. > +void > +wg_status(void) > +{ > + size_t i, j, size = 0; > + struct timespec now; > + char

Re: iked(8): AES_GCM ciphers for IKE

2020-05-18 Thread Tobias Heider
On Sat, May 16, 2020 at 02:24:45PM +0200, Christian Weisgerber wrote: > Tobias Heider: > > > currently iked(8) supports AES-GCM only for ESP. > > The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants for > > IKE. > > (for more information see [

Re: iked(8): AES_GCM ciphers for IKE

2020-05-14 Thread Tobias Heider
On Thu, May 14, 2020 at 10:47:52PM +0200, Tobias Heider wrote: > On Thu, May 14, 2020 at 10:07:30PM +0200, Tobias Heider wrote: > > Hi, > > > > currently iked(8) supports AES-GCM only for ESP. > > The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants

Re: iked(8): AES_GCM ciphers for IKE

2020-05-14 Thread Tobias Heider
Looks like you are missing the previous commit: https://marc.info/?l=openbsd-cvs=158946893417378=2

Re: iked(8): AES_GCM ciphers for IKE

2020-05-14 Thread Tobias Heider
On Thu, May 14, 2020 at 10:07:30PM +0200, Tobias Heider wrote: > Hi, > > currently iked(8) supports AES-GCM only for ESP. > The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants for IKE. > (for more information see [1] and [2]). > Both variants support the 128, 19

iked(8): AES_GCM ciphers for IKE

2020-05-14 Thread Tobias Heider
Hi, currently iked(8) supports AES-GCM only for ESP. The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants for IKE. (for more information see [1] and [2]). Both variants support the 128, 196, and 256 bit key lengths. The new new ciphers can be configured with: - aes-128-gcm,

Re: WireGuard patchset for OpenBSD

2020-05-12 Thread Tobias Heider
Hi, thanks for the diff! > SipHash and ChaCha20Poly1305 are already available in the kernel. The > only modification here is add the short and simple chapoly AEAD > construction alongside the existing AE one. At first glance, I think you could use the crypto framework implementation for the

Re: incorrect time in iked

2020-05-02 Thread Tobias Heider
On Sat, May 02, 2020 at 10:15:53AM +0200, René Ammerlaan wrote: > Hi, > > I've found incorrect use of time in iked (-current). The event API doen’t use > the monotonic clock, so this breaks the timer: > ikev2_ike_sa_alive: outgoing CHILD SA spi 0x07409b52 last used > 7466(gettime) -

Re: iked(8): Removing SHA1 from default transforms

2020-05-01 Thread Tobias Heider
04 [2] https://cseweb.ucsd.edu/~mihir/papers/hmac-new.html > > On 02/05/2020 00:03, Tobias Heider wrote: > > On Fri, May 01, 2020 at 11:35:23PM +0200, Stephan Mending wrote: > > > Hi *, > > > > > > this diff removes SHA1 as default transform for integrity a

Re: iked(8): Removing SHA1 from default transforms

2020-05-01 Thread Tobias Heider
On Fri, May 01, 2020 at 11:35:23PM +0200, Stephan Mending wrote: > Hi *, > > this diff removes SHA1 as default transform for integrity algorithms. > > It's been broken long enough. Let's at least get rid of it in iked's > defaults. > > SHA1 is officially broken since 2011 and there have been

Re: iked(8): Add ECDH groups and AEADs to defaults

2020-04-30 Thread Tobias Heider
On Thu, Apr 30, 2020 at 09:33:28PM +0100, Stuart Henderson wrote: > On 2020/04/30 20:11, Tobias Heider wrote: > > Hi, > > > > I would like to modernize our crypto defaults a bit and add some of the > > supported ECDH Diffie-Hellman groups to the default IKE crypto

iked(8): Add ECDH groups and AEADs to defaults

2020-04-30 Thread Tobias Heider
Hi, I would like to modernize our crypto defaults a bit and add some of the supported ECDH Diffie-Hellman groups to the default IKE crypto proposal. There should be no downside to this, if they are not supported by the other side one of the old MODP groups will be used. The same for AEADs in the

Re: iked(8): remove insecure EC2N curves

2020-04-28 Thread Tobias Heider
On Tue, Apr 28, 2020 at 11:22:02AM +0100, Stuart Henderson wrote: > On 2020/04/28 01:09, Tobias Heider wrote: > > Hi, > > > > the EC2N family of curves have been marked as insecure for at least 10 > > years. > > In fact, IANA has stopped listing them altogether

iked(8): remove insecure EC2N curves

2020-04-27 Thread Tobias Heider
Hi, the EC2N family of curves have been marked as insecure for at least 10 years. In fact, IANA has stopped listing them altogether [1]. Their former IDs are now 'reserved'. I think it's time for us to drop them as well. ok? [1]

Re: AEAD Suites in IKEX (iked) and Phase 1 (isakmpd)

2020-04-20 Thread Tobias Heider
On Mon, Apr 20, 2020 at 12:52:24PM +0200, Stephan Mending wrote: > Hi, > I was wondering if there was a reason why there are no AEAD Suites > implemented for initial IKEX in iked or phase 1 in isamkmpd ? Even though > iked's childSAs > support it and Phase 2 in isakmpd does as well ? Is it just

Re: ipsec(4)/iked(8): separate rdomains for encrypted/unecrypted traffic

2020-04-18 Thread Tobias Heider
On Mon, Apr 13, 2020 at 11:56:36AM +0200, Tobias Heider wrote: > Hi, > > the diff below adds a new feature that allows the use of separate rdomains > for the encrypted and unencrypted side of ipsec(4) flows. > > The idea is that an edge router that controls access to a pri

sdmmc(4): use DMA for all commands if supported

2020-04-18 Thread Tobias Heider
Hi, the attached diff allows sdmmc(4) to use DMA for all commands instead of just mem_read and mem_write. There were problems in the past with some controllers not liking small DMA transfers, so it would be nice to get this tested thorougly on different hardware. So far I have successfully

ipsec(4)/iked(8): separate rdomains for encrypted/unecrypted traffic

2020-04-13 Thread Tobias Heider
Hi, the diff below adds a new feature that allows the use of separate rdomains for the encrypted and unencrypted side of ipsec(4) flows. The idea is that an edge router that controls access to a private network via ipsec can have its uplink in one rdomain and the private network in another. The

Re: iked(8): simplify data in sc_sock4 and sc_sock6

2020-04-07 Thread Tobias Heider
Hi, thank you, most of this diff looks good to me. I left some comments inline. On Sun, Apr 05, 2020 at 01:58:04AM +0900, Wataru Ashihara wrote: > The data wich sc_sock4 has is a little bit complicated: > > >

Re: iked(8): boolify

2020-04-03 Thread Tobias Heider
On Fri, Apr 03, 2020 at 12:52:24AM +0900, Wataru Ashihara wrote: > It would save our time of thinking and reading the source (i.e. > eliminate the process of "what if the variable 'mobike' was 2 or more? > ...aha it's just a bool"). > > This is still work in progress. I would continue if you

Re: vmm(4): unterminated vm_name after strncpy

2020-03-28 Thread Tobias Heider
On Sat, Mar 28, 2020 at 06:47:47PM -0600, Theo de Raadt wrote: > Or strncpy with length - 1 would be also good, since it won't copy >foo\0bar\0 > fully, but only >foo\0 > into the buffer and store it as >foo\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0 > and gaurantee the \0 on the

Re: vmm(4): unterminated vm_name after strncpy

2020-03-28 Thread Tobias Heider
On Sat, Mar 28, 2020 at 05:33:05PM -0600, Theo de Raadt wrote: > Pretty obvious why. > > The kernel doesn't check it's a string, before calling strlcpy > which (correctly) runs off the array hunting for the terminal NUL, > and into the next object, and I guess it finds a NUL in the next > VA page

Re: iked ikev2_ikesa_enable is not copying sa_eapid

2020-03-27 Thread Tobias Heider
On Fri, Mar 27, 2020 at 10:39:52AM -0300, Bernardo Vieira wrote: > Hi, > inside the function ikev2_ikesa_enable the atribute sa_eapid should > be copied to the new sa. > Regards, > Bernardo Looks correct, thank you. Committed! > > Index: ikev2.c >

softraid_raid5: possible NULL dereference

2020-03-25 Thread Tobias Heider
sr_block_get() returns dma_alloc(length, PR_NOWAIT | PR_ZERO) which may be NULL if the memory pool is depleted. The result is used as 'dst' argument to memcpy() in the following call to sr_raid5_regenerate(), resulting in a possible NULL dereference. ok? Index: softraid_raid5.c

Re: iked users database misses entries after ikectl reload

2020-03-24 Thread Tobias Heider
On Mon, Mar 23, 2020 at 05:53:00PM -0300, Bernardo Cunha Vieira wrote: > Hi, > This fixes the users' database corruption after an iked reload. > The old code overwrites the pointers in the RB tree, losing users > if a list of users is provided in config file. > Regards, > Bernardo Good find,

umidi: missing NULL checks

2020-03-23 Thread Tobias Heider
In alloc_all_jacks() the variables 'sc_in_jacks' and 'sc_out_checks' are set to NULL if 'sc_in_num_jacks' and 'sc_out_num_jacks' are 0. Further down both are dereferenced unconditionally. I added explicit NULL checks where I think they belong. I think 'sc_in_ep' and 'sc_out_ep' can also be NULL,

rtsock: redundant NULL pointer check

2020-03-23 Thread Tobias Heider
It seems that there is no way 'rtm' could actually be NULL here, which means we can get rid of the check. ok? Index: net/rtsock.c === RCS file: /mount/openbsd/cvs/src/sys/net/rtsock.c,v retrieving revision 1.297 diff -u -p -r1.297

ikectl(8): Reset SAs by policy ID

2020-03-17 Thread Tobias Heider
Hi, this diff adds a new command to ikectl(8) and iked(8) that allows to reset SAs based on the peers ID, which is equivalent to resetting a single policy. The expected ID format is the same as printed by 'ipsecctl -sf' in the 'dstid' field. Example: $ ikectl reset id FQDN/peer1 ok? diff

smtpd: handle buf == NULL from m_get_string

2020-03-16 Thread Tobias Heider
m_get_string(m, ) may set 'buf == NULL', which would lead to strlen(NULL) in m_get_envelope. I chose fatalx because that's what seems to be the common way to handle errors in mproc but I don't know the code base to well. Index: mproc.c

Re: smtpd: mail.lmtp uninitialzed stack access

2020-03-16 Thread Tobias Heider
On Mon, Mar 16, 2020 at 04:54:19PM -0600, Todd C. Miller wrote: > On Mon, 16 Mar 2020 23:46:35 +0100, Tobias Heider wrote: > > > In main() mail.lmtp checks 'if (argc == 0 && session.rcptto == NULL)' > > after getopt(). If neither an 'r' nor an 'u' option was specified,

smtpd: mail.lmtp uninitialzed stack access

2020-03-16 Thread Tobias Heider
In main() mail.lmtp checks 'if (argc == 0 && session.rcptto == NULL)' after getopt(). If neither an 'r' nor an 'u' option was specified, 'session.rcptto' seems to be uninitialized. The obvious solution would be to NULL initialize 'struct session'. ok? Index: mail.lmtp.c

in6_ifattach: strncpy to strlcpy

2020-03-16 Thread Tobias Heider
Using strncpy with sizeof(string) may result in a non-nul-terminated string at 'dst'. This is not too problematic here because if_xname is the same size as 'ifra_name' and should always be NUL terminated. I would still like to replace strncpy with strlcpy which implicitly includes the null byte in

vmm(4): unterminated vm_name after strncpy

2020-03-12 Thread Tobias Heider
vmm uses 'strncpy(vm->vm_name, vcp->vcp_name, VMM_MAX_NAME_LEN)' to copy to buffers of size VMM_MAX_NAME_LEN, which can leave the resulting string unterminated. >From strncpy(3): strncpy() only NUL terminates the destination string when the length of the source string is less than the length

Re: net/if.c: nullptr deref in if_hooks_run

2020-03-09 Thread Tobias Heider
On Mon, Mar 09, 2020 at 11:56:09PM +0100, Klemens Nanni wrote: > On Mon, Mar 09, 2020 at 10:33:17PM +0100, Tobias Heider wrote: > > there seems to be a nullptr dereference in if_hooks_run. > Did your kernel crash here or did you find reading alone? Coverity Scan found it > > Wh

Re: softraid: too many arguments for sr_error

2020-03-09 Thread Tobias Heider
On Tue, Mar 10, 2020 at 12:01:45AM +0100, Klemens Nanni wrote: > On Mon, Mar 09, 2020 at 11:41:14PM +0100, Tobias Heider wrote: > > sr_error takes a sr_softc and a printf like format string + varargs. > > There's no need to pass DEVNAME(sc) here. > Either that or emb

ifq: ifq_dec_sleep may return garbage

2020-03-09 Thread Tobias Heider
If 'm = ifq->ifq_ops->ifqop_deq_begin(ifq, )' is not NULL the loop is exited and an uninitialized 'int error' is returned. Several lines below error is checked for '!= 0', so i assume it was meant to be initialized to '0'. ok? Index: ifq.c

softraid: too many arguments for sr_error

2020-03-09 Thread Tobias Heider
sr_error takes a sr_softc and a printf like format string + varargs. There's no need to pass DEVNAME(sc) here. ok? Index: softraid.c === RCS file: /mount/openbsd/cvs/src/sys/dev/softraid.c,v retrieving revision 1.398 diff -u -p

net/if.c: nullptr deref in if_hooks_run

2020-03-09 Thread Tobias Heider
Hi, there seems to be a nullptr dereference in if_hooks_run. When the inner while loop is exited because 't == NULL' the next line is an access to 't->t_func'. Because 't==NULL' means the TAILQ is fully traversed I think we should break and exit instead. ok? Index: if.c

Re: iked(8): Use TAILQ_FOREACH_SAFE(3)

2020-03-09 Thread Tobias Heider
Hi Wataru, On Mon, Mar 09, 2020 at 08:09:24PM +0900, Wataru Ashihara wrote: > to improve readability. > > This is the first time of my commit to OpenBSD, so if I went something > wrong, let me know that. Thanks for sharing, committed! > > Index: sbin/iked/config.c >

rnd: initialize 'timespec ts'

2020-03-06 Thread Tobias Heider
Hi, if timeout_initialized() returns 0, enqueue_randomness() may use 'ts' uninitialized. This is not really a problem because the value is blended with other collected entropy. To make things clearer I would still prefer to always initialize 'ts'. ok? Index: rnd.c

Re: net80211: properly wrap sequence numbers on increment

2020-03-06 Thread Tobias Heider
On Fri, Mar 06, 2020 at 10:47:44AM +0100, Stefan Sperling wrote: > 802.11 frame sequence numbers are in the range 0x0 - 0xfff. > > Don't let internal representations of sequence numbers grow beyond 0xfff. > > ok? > > diff 582540bcd55abf4efa3abe8c23ebc7f3c247245d >

iked(8): Recheck SA proposals when updating policy

2020-02-26 Thread Tobias Heider
Due to the design of the IKEv2 protocol, the receiver does not know which policy the initiator tries to negotiate an SA for until the second exchange (IKE_AUTH). The IKE_AUTH request contains the ID payload which the responder uses to match a policy (and lookup authentication keys). Until then,

Re: iwm(4): sync RX interrupt handling from iwlwifi

2020-02-26 Thread Tobias Heider
On Tue, Feb 25, 2020 at 03:07:47PM +0100, Stefan Sperling wrote: > On Tue, Feb 25, 2020 at 01:06:31PM +0100, Tobias Heider wrote: > > Hi, > > > > I tried to figure out the reason for the lost Tx receive interrupt > > by comparing iwm with iwlwifi and I think our hand

iwm(4): sync RX interrupt handling from iwlwifi

2020-02-25 Thread Tobias Heider
Hi, I tried to figure out the reason for the lost Tx receive interrupt by comparing iwm with iwlwifi and I think our handling of the periodic RX interrupt is a bit off. In linux on receive of any of the possible RX interrupts the periodic interrupt is disabled and then reenabled. As far as I

ieee80211: handle SIOCSIFLLADDR in ifioctl()

2020-02-24 Thread Tobias Heider
Hi, I noticed that my iwm(4) device does not send RTM_IFINFO messages when the lladdr is changed with ifconfig. For other interface types ifioctl() handles the lladdr change in if_setlladdr() and ifnewlladdr(). I attached a diff that seems to fix this. Is there a specific reason IFT_IEEE80211 is

Re: iked.conf.5: Provide GRE tunnel in transport mode example

2020-02-22 Thread Tobias Heider
> > > > We should rather fix the defaults to do what we expect them to do. > > In your example case that would be using fqdn/D.example.com > Agreed; do you take a stab at it? I'm happy to test. > Try this Index: parse.y === RCS

Re: iked.conf.5: Provide GRE tunnel in transport mode example

2020-02-22 Thread Tobias Heider
On Sat, Feb 22, 2020 at 01:47:35PM +0100, Klemens Nanni wrote: > On Sat, Feb 22, 2020 at 01:18:13PM +0100, Tobias Heider wrote: > > It seems I was mistaken because I usually use IPs in local > > and peer. What I said is true for IPs. When using > > FQDNs for local/peer how

Re: iked.conf.5: Provide GRE tunnel in transport mode example

2020-02-22 Thread Tobias Heider
On Sat, Feb 22, 2020 at 12:50:27PM +0100, Klemens Nanni wrote: > On Sat, Feb 22, 2020 at 12:24:36PM +0100, Klemens Nanni wrote: > > On Sat, Feb 22, 2020 at 10:19:27AM +0100, Tobias Heider wrote: > > > This is not what dstid does. When setting 'dstid D.example.com' the

Re: iked.conf.5: Provide GRE tunnel in transport mode example

2020-02-22 Thread Tobias Heider
On Sat, Feb 22, 2020 at 12:41:12PM +0100, Landry Breuil wrote: > On Sat, Feb 22, 2020 at 12:24:36PM +0100, Klemens Nanni wrote: > > On Sat, Feb 22, 2020 at 10:19:27AM +0100, Tobias Heider wrote: > > > This is not what dstid does. When setting 'dstid D.example.com' the

Re: iked.conf.5: Provide GRE tunnel in transport mode example

2020-02-22 Thread Tobias Heider
On Sat, Feb 22, 2020 at 12:26:01AM +0100, Klemens Nanni wrote: > On Fri, Feb 21, 2020 at 10:28:50PM +, Jason McIntyre wrote: > > it should be "a gre tunnel", not "an" > Sure, leftover from previous wording/reshuffling. > > > > +.Xr gre 4 > > > +tunnel from the local machine A to peer D using

Re: iked(8): add transport mode for childsas

2020-02-18 Thread Tobias Heider
Hi, here is an update of the last diff rebased onto current with minor fixes. There were some problems when multiple transport and non-transport policies were configured, which should now be fixed. I also have a test case for the new regression test which runs successfully. ok? diff --git

<    1   2   3   >