clang: compile static analyzer

2022-01-14 Thread Tobias Heider
Hi, clang ships with a pretty useful static analyzer to find all kinds of bugs in C and C++ code: https://clang-analyzer.llvm.org/ I use it regularly to check my own diffs and found plenty of bugs I could have missed otherwise. While we have the code in base we don't actually build it into our

Re: sdmmc: fix malloc error handling in sdmmc_mem_send_scr()

2022-01-10 Thread Tobias Heider
On Mon, Jan 10, 2022 at 04:20:36PM +0100, Stefan Sperling wrote: > On Mon, Jan 10, 2022 at 03:50:45PM +0100, Tobias Heider wrote: > > Makes sense. I also fixed the one in sdmmc_mem_send_cxd_data(). > > Doesn't build here, there a few errors like this: > > /usr/src/sys/dev/sd

Re: sdmmc: fix malloc error handling in sdmmc_mem_send_scr()

2022-01-10 Thread Tobias Heider
On Mon, Jan 10, 2022 at 02:39:58PM +, Visa Hankala wrote: > On Mon, Jan 10, 2022 at 03:21:49PM +0100, Tobias Heider wrote: > > On Mon, Jan 10, 2022 at 01:41:53PM +, Visa Hankala wrote: > > > On Mon, Jan 10, 2022 at 01:12:10PM +0100, Tobias Heider wrote: > > > &

Re: remove ieee80211_find_node_for_beacon()

2022-01-10 Thread Tobias Heider
On Mon, Jan 10, 2022 at 11:00:52AM +0100, Stefan Sperling wrote: > Ping. I have had zero feedback on this so far. Anyone? Makes sense, I remember that part of the code making problems before. ok tobhe. > > On Tue, Jan 04, 2022 at 02:35:52PM +0100, Stefan Sperling wrote: > > The function

Re: sdmmc: fix malloc error handling in sdmmc_mem_send_scr()

2022-01-10 Thread Tobias Heider
On Mon, Jan 10, 2022 at 01:41:53PM +, Visa Hankala wrote: > On Mon, Jan 10, 2022 at 01:12:10PM +0100, Tobias Heider wrote: > > sdmmc_mem_send_scr() tries to malloc() with M_NOWAIT and returns 0 on > > error, which leads to sdmmc_mem_sd_init() passing uninitialized s

sdmmc: fix malloc error handling in sdmmc_mem_send_scr()

2022-01-10 Thread Tobias Heider
sdmmc_mem_send_scr() tries to malloc() with M_NOWAIT and returns 0 on error, which leads to sdmmc_mem_sd_init() passing uninitialized stack memory to sdmmc_mem_decode_scr(). The diff below makes sdmmc_mem_send_scr() return ENOMEM if malloc fails. ok? diff --git a/sys/dev/sdmmc/sdmmc_mem.c

iked: cleanup libcrypto *_free calls

2021-12-13 Thread Tobias Heider
Hey, tb@ noticed that we do a lot of redundant explicit NULL checks before calling libcrypto *_free() functions. A few of the free() calls can also be avoided by using X509_get0_pubkey() instead of X509_get_pubkey(). ok? Index: ca.c

tdb_delete_locked for pfkey

2021-12-03 Thread Tobias Heider
Hi, the diff below adds tdb_delete_locked() for use in pfkeyv2_sa_flush(). This way we won't have to worry about keeping the inline code and tdb_delete() in sync. ok? Index: net/pfkeyv2.c === RCS file: /cvs/src/sys/net/pfkeyv2.c,v

Re: ipsec: refactor TDBF_DELETED

2021-11-25 Thread Tobias Heider
On Fri, Nov 26, 2021 at 01:17:22AM +0300, Vitaliy Makkoveev wrote: > On Thu, Nov 25, 2021 at 10:59:25PM +0100, Alexander Bluhm wrote: > > On Thu, Nov 25, 2021 at 05:13:16PM +0100, Tobias Heider wrote: > > > Now with the missing parts from pfkeyv2.c as noticed by Hrvoje. > >

Re: ipsec: refactor TDBF_DELETED

2021-11-25 Thread Tobias Heider
On Thu, Nov 25, 2021 at 03:50:29PM +0100, Tobias Heider wrote: > As discussed in the previous thread we can simplify the tdb cleanup > code by removing the TDBF_DELETED flag and instead checking if the > tdb was already unlinked. > > ok? > Now with the missing parts from pf

ipsec: refactor TDBF_DELETED

2021-11-25 Thread Tobias Heider
As discussed in the previous thread we can simplify the tdb cleanup code by removing the TDBF_DELETED flag and instead checking if the tdb was already unlinked. ok? Index: ip_ipsp.c === RCS file: /cvs/src/sys/netinet/ip_ipsp.c,v

Re: IPsec tdb ref counting

2021-11-24 Thread Tobias Heider
On Wed, Nov 24, 2021 at 03:52:26PM +0100, Alexander Bluhm wrote: > On Wed, Nov 24, 2021 at 05:12:36PM +0300, Vitaliy Makkoveev wrote: > > Understood. But his means we encoded double unref when we calling > > tdb_unref() just after tdb_delete(tdb). To me it looks better to avoid > > this and rework

Re: IPsec tdb ref counting

2021-11-23 Thread Tobias Heider
On Tue, Nov 23, 2021 at 02:18:26PM +0100, Alexander Bluhm wrote: > On Tue, Nov 23, 2021 at 06:54:59AM +0100, Hrvoje Popovski wrote: > > after 24 hours hitting sasyncd setup one box panic > > Thanks for testing. > > I have reduced my iked lifetime to about 10 seconds and got the > same panic on

OpenIKED 7.0 released

2021-11-03 Thread Tobias Heider
We have released OpenIKED 7.0, which will be arriving in the OpenIKED directory of your local OpenBSD mirror soon. This release includes the following changes to the previous release: * Added client-side support for DNS configuration via OpenBSD resolvd(8) and systemd-resolved(8) *

Re: diff: ipsec.conf(5), clarify "aes" accepts 128:256 bits

2021-11-03 Thread Tobias Heider
On Wed, Nov 03, 2021 at 02:55:11PM +0900, YASUOKA Masahiko wrote: > Hi, > > On Tue, 2 Nov 2021 07:03:43 + > Jason McIntyre wrote: > > On Tue, Nov 02, 2021 at 12:02:07PM +0900, YASUOKA Masahiko wrote: > >> I'd like to clarify "aes" in ipsec.conf accepts 128:256 bits. > >> > >>

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

  1   2   >