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!)

diff --git a/sbin/isakmpd/pf_key_v2.c b/sbin/isakmpd/pf_key_v2.c
index 67fa921..758acf6 100644 (file)
--- 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);