From b255ec9040c5acadf982f73eafa7d1fddfe19ae8 Mon Sep 17 00:00:00 2001 From: sthen Date: Mon, 31 Jan 2022 23:51:15 +0000 Subject: [PATCH] In isakmpd's pf_key_v2_enable_sa(), check if proto is NULL and if so, log a warning rather than attempting to dereference it. Check suggested by millert@, ok "your fix shouldn't hurt" mpi@ This isn't fixing the root cause but I don't have a better idea and I'm hitting problems on several systems as I upgrade them, and I think in this case logging rather than dumping core is more helpful. Without this, in recent OpenBSD versions (I have seen it in since at least the snapshot from June 14 2021) I am often seeing isakmpd crashes after SAs come in shortly after isakmpd starts with my usual flags, although they don't seem to occur if I raise logs to heavy debug levels (-DA=90). With this, those connections will fail but isakmpd will stay running and after usually one retry things will be ok. Usually, perhaps always, seen associated with "responder_recv_HASH_SA_NONCE: KEY_EXCH payload without a group desc. attribute" logged previously. Pcap written by isakmpd -L shows a normal-looking proposal though, with proto/ids/group description set, yet printing *isa at the point that message is logged shows zeros in sport/dport/group_desc/etc. (I can give more info and/or test if someone has a better idea!) --- sbin/isakmpd/pf_key_v2.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sbin/isakmpd/pf_key_v2.c b/sbin/isakmpd/pf_key_v2.c index 67fa921bb37..758acf63c1a 100644 --- a/sbin/isakmpd/pf_key_v2.c +++ b/sbin/isakmpd/pf_key_v2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf_key_v2.c,v 1.203 2022/01/16 14:30:11 naddy Exp $ */ +/* $OpenBSD: pf_key_v2.c,v 1.204 2022/01/31 23:51:15 sthen Exp $ */ /* $EOM: pf_key_v2.c,v 1.79 2000/12/12 00:33:19 niklas Exp $ */ /* @@ -1802,6 +1802,11 @@ pf_key_v2_enable_sa(struct sa *sa, struct sa *isakmp_sa) size_t sidlen = 0, didlen = 0; u_int8_t *sid = 0, *did = 0; + if (proto == NULL) { + log_print("pf_key_v2_enable_sa: no proto"); + return EINVAL; + } + sa->transport->vtbl->get_dst(sa->transport, &dst); sa->transport->vtbl->get_src(sa->transport, &src); -- 2.20.1