ports/69065: Some security fixes (Backported the fix from OpenBSD)
Matthijs Mohlmann
matthijs at cacholong.nl
Wed Jul 14 19:40:23 UTC 2004
>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 at 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:
More information about the freebsd-ports-bugs
mailing list