Date: Wed, 14 Jul 2004 21:32:54 +0200 (CEST) From: Matthijs Mohlmann <matthijs@cacholong.nl> To: FreeBSD-gnats-submit@FreeBSD.org Subject: ports/69065: Some security fixes (Backported the fix from OpenBSD) Message-ID: <20040714193254.531D828468@router.cacholong.nl> Resent-Message-ID: <200407141940.i6EJeMje018138@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 69065 >Category: ports >Synopsis: Some security fixes (Backported the fix from OpenBSD) >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-ports-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Jul 14 19:40:22 GMT 2004 >Closed-Date: >Last-Modified: >Originator: Matthijs Mohlmann >Release: FreeBSD 5.2.1-RELEASE-p8 i386 >Organization: >Environment: System: FreeBSD router.cacholong.nl 5.2.1-RELEASE-p8 FreeBSD 5.2.1-RELEASE-p8 #14: Mon Jun 28 17:34:31 CEST 2004 root@router.cacholong.nl:/usr/obj/usr/src/sys/KERNEL i386 >Description: The problem is reported to the security officer. URL: http://www.freebsd.org/ports/portaudit/9a73a5b4-c9b5-11d8-95ca-02e081301d81.html >How-To-Repeat: The above URL gives some example code how to exploit this bug. >Fix: Well here is a fix this problem... There is another problem with this package i think and i've fixed these also (als a security bug i think: The original isakmpd doesn't permit INITIAL CONTACT in aggressive exchange mode. And also it doesn't check if the INTITIAL CONTACT is protected by a SA (Security Association) Another problem is that i have one FreeBSD box. And didn't have much time to test it.. but please can someone look to this pr. --- isakmpd.diff begins here --- diff -ruN isakmpd.orig/files/patch-ike_phase_1.c isakmpd/files/patch-ike_phase_1.c --- isakmpd.orig/files/patch-ike_phase_1.c Thu Jan 1 01:00:00 1970 +++ isakmpd/files/patch-ike_phase_1.c Wed Jul 14 20:18:53 2004 @@ -0,0 +1,12 @@ +--- ike_phase_1.c.orig Fri Aug 8 10:46:59 2003 ++++ ike_phase_1.c Wed Jul 14 20:17:06 2004 +@@ -1094,6 +1094,9 @@ + return -1; + } + ++ /* Mark message as authenticated. */ ++ msg->flags |= MSG_AUTHENTICATED; ++ + return 0; + } + diff -ruN isakmpd.orig/files/patch-ike_quick_mode.c isakmpd/files/patch-ike_quick_mode.c --- isakmpd.orig/files/patch-ike_quick_mode.c Thu Jan 1 01:00:00 1970 +++ isakmpd/files/patch-ike_quick_mode.c Wed Jul 14 20:19:19 2004 @@ -0,0 +1,22 @@ +--- ike_quick_mode.c.orig Tue Jun 10 18:41:29 2003 ++++ ike_quick_mode.c Wed Jul 14 20:17:06 2004 +@@ -1508,6 +1508,9 @@ + free (my_hash); + my_hash = 0; + ++ /* Mark message as authenticated. */ ++ msg->flags |= MSG_AUTHENTICATED; ++ + kep = TAILQ_FIRST (&msg->payload[ISAKMP_PAYLOAD_KEY_EXCH]); + if (kep) + ie->pfs = 1; +@@ -1951,6 +1954,9 @@ + goto cleanup; + } + free (my_hash); ++ ++ /* Mark message as authenticated. */ ++ msg->flags |= MSG_AUTHENTICATED; + + post_quick_mode (msg); + diff -ruN isakmpd.orig/files/patch-ipsec.c isakmpd/files/patch-ipsec.c --- isakmpd.orig/files/patch-ipsec.c Thu Jan 1 01:00:00 1970 +++ isakmpd/files/patch-ipsec.c Wed Jul 14 20:19:43 2004 @@ -0,0 +1,128 @@ +--- ipsec.c.orig Tue Sep 2 20:15:55 2003 ++++ ipsec.c Wed Jul 14 20:17:06 2004 +@@ -1104,7 +1104,17 @@ + if (type == ISAKMP_NOTIFY_INVALID_SPI) + ipsec_invalid_spi (msg, p); + +- p->flags |= PL_MARK; ++// p->flags |= PL_MARK; ++ switch (type) ++ { ++ case IPSEC_NOTIFY_INITIAL_CONTACT: ++ /* Handled by leftover logic. */ ++ break; ++ ++ default: ++ p->flags |= PL_MARK; ++ break; ++ } + } + + /* +@@ -1656,43 +1666,75 @@ + switch (GET_ISAKMP_NOTIFY_MSG_TYPE (payload->p)) + { + case IPSEC_NOTIFY_INITIAL_CONTACT: +- /* +- * Find out who is sending this and then delete every SA that is +- * ready. Exchanges will timeout themselves and then the +- * non-ready SAs will disappear too. +- */ +- msg->transport->vtbl->get_dst (msg->transport, &dst); +- while ((sa = sa_lookup_by_peer (dst, sysdep_sa_len (dst))) != 0) +- { +- /* +- * Don't delete the current SA -- we received the notification +- * over it, so it's obviously still active. We temporarily need ++ /* ++ * Permit INITIAL-CONTACT if ++ * - this is not an AGGRESSIVE mode exchange ++ * - it is protected by an ISAKMP SA ++ * ++ * XXX Instead of the first condition above, we could permit this ++ * XXX only for phase 2. In the last packet of main-mode, this ++ * XXX payload, while encrypted, is not part of the hash digest. ++ * XXX As we currently send our own INITIAL-CONTACTs at this point, ++ * XXX this too would need to be changed. ++ */ ++ if (msg->exchange->type == ISAKMP_EXCH_AGGRESSIVE) ++ { ++ log_print ("ipsec_handle_leftover_payload: got INITIAL-CONTACT " ++ "in AGGRESSIVE mode"); ++ return -1; ++ } ++ ++ if ((msg->exchange->flags & EXCHANGE_FLAG_ENCRYPT) == 0) ++ { ++ log_print ("ipsec_handle_leftover_payload: got INITIAL-CONTACT " ++ "without ISAKMP SA"); ++ return -1; ++ } ++ ++ if ((msg->flags & MSG_AUTHENTICATED) == 0) ++ { ++ log_print("ipsec_handle_leftover_payload: got unauthenticated " ++ "INITIAL-CONTACT"); ++ return -1; ++ } ++ ++ /* ++ * Find out who is sending this and then delete every SA that is ++ * ready. Exchanges will timeout themselves and then the ++ * non-ready SAs will disappear too. ++ */ ++ msg->transport->vtbl->get_dst (msg->transport, &dst); ++ while ((sa = sa_lookup_by_peer (dst, sysdep_sa_len (dst))) != 0) ++ { ++ /* ++ * Don't delete the current SA -- we received the notification ++ * over it, so it's obviously still active. We temporarily need + * to remove the SA from the list to avoid an endless loop, +- * but keep a reference so it won't disappear meanwhile. +- */ +- if (sa == msg->isakmp_sa) +- { +- sa_reference (sa); ++ * but keep a reference so it won't disappear meanwhile. ++ */ ++ if (sa == msg->isakmp_sa) ++ { ++ sa_reference (sa); + sa_remove (sa); + reenter = 1; +- continue; +- } ++ continue; ++ } + +- LOG_DBG ((LOG_SA, 30, +- "ipsec_handle_leftover_payload: " +- "INITIAL-CONTACT made us delete SA %p", +- sa)); +- sa_delete (sa, 0); +- } ++ LOG_DBG ((LOG_SA, 30, ++ "ipsec_handle_leftover_payload: " ++ "INITIAL-CONTACT made us delete SA %p", ++ sa)); ++ sa_delete (sa, 0); ++ } + + if (reenter) +- { +- sa_enter (msg->isakmp_sa); +- sa_release (msg->isakmp_sa); +- } +- payload->flags |= PL_MARK; +- return 0; +- } ++ { ++ sa_enter (msg->isakmp_sa); ++ sa_release (msg->isakmp_sa); ++ } ++ payload->flags |= PL_MARK; ++ return 0; ++ } + } + return -1; + } diff -ruN isakmpd.orig/files/patch-message.c isakmpd/files/patch-message.c --- isakmpd.orig/files/patch-message.c Thu Jan 1 01:00:00 1970 +++ isakmpd/files/patch-message.c Wed Jul 14 20:20:08 2004 @@ -0,0 +1,44 @@ +--- message.c.orig Tue Sep 2 20:14:52 2003 ++++ message.c Wed Jul 14 20:17:06 2004 +@@ -433,6 +433,11 @@ + /* + * Validate the delete payload P in message MSG. As a side-effect, create + * an exchange if we do not have one already. ++ * ++ * Note: DELETEs are only accepted as part of an INFORMATIONAL exchange. ++ * exchange_validate() makes sure a HASH payload is present. Due to the order ++ * of message validation functions in message_validate_payload[] we can be ++ * sure that the HASH payload has been successfully validated at this point. + */ + static int + message_validate_delete (struct message *msg, struct payload *p) +@@ -440,6 +445,13 @@ + u_int8_t proto = GET_ISAKMP_DELETE_PROTO (p->p); + struct doi *doi; + ++ /* Only accpet authenticated DELETEs. */ ++ if ((msg->flags & MSG_AUTHENTICATED) == 0) ++ { ++ log_print("message_validate_delete: got unauthenticated DELETE"); ++ return -1; ++ } ++ + doi = doi_lookup (GET_ISAKMP_DELETE_DOI (p->p)); + if (!doi) + { +@@ -463,6 +475,15 @@ + return -1; + } + } ++ ++ /* Only accept DELETE as part of an INFORMATIONAL exchange. */ ++ if (msg->exchange->type != ISAKMP_EXCH_INFO) { ++ log_print("message_validate_delete: delete in exchange other " ++ "than INFO: %s", constant_name(isakmp_exch_cst, ++ msg->exchange->type)); ++ message_free(msg); ++ return -1; ++ } + + if (proto != ISAKMP_PROTO_ISAKMP && doi->validate_proto (proto)) + { diff -ruN isakmpd.orig/files/patch-message.h isakmpd/files/patch-message.h --- isakmpd.orig/files/patch-message.h Thu Jan 1 01:00:00 1970 +++ isakmpd/files/patch-message.h Wed Jul 14 20:20:20 2004 @@ -0,0 +1,12 @@ +--- message.h.orig Wed Jun 4 09:31:17 2003 ++++ message.h Wed Jul 14 20:17:06 2004 +@@ -160,6 +160,9 @@ + /* This message should be kept on the prioritized sendq. */ + #define MSG_PRIORITIZED 8 + ++/* This message has successfully been authenticated. */ ++#define MSG_AUTHENTICATED 16 ++ + TAILQ_HEAD(msg_head, message); + + extern int message_add_payload (struct message *, u_int8_t, u_int8_t *, --- isakmpd.diff ends here --- >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040714193254.531D828468>