[Design] some changes to delete patch, as commited by Sam

From: D. Hugh Redelmeier <hugh(at)mimosa.com>
Date: Thu Feb 13 2003 - 17:58:52 EST

-----BEGIN PGP SIGNED MESSAGE-----

1. The test to detect and exclude unencrypted delete messages was: (st == NULL && (md->hdr.isa_flags & ISAKMP_FLAG_ENCRYPTION) == 0)

    This will not reject unencrypted delete payloads if they are     associated with a state object (i.e. if st != NULL).

    This could happen with a separate delete notification that was not     encrypted or with a delete payload within a negotiation.

    The first case is forbidden by other code within Pluto, but     that is a fragile basis for confidence.

    The second case is possible, I think.

    I think that the right test is:

            (!md->encrypted)

2. When deleting an ISAKMP SA, there is no check that the

    authenticated IDs of the ISAKMP SA that transmitted the delete     match those of the ISAKMP SA being deleted. What makes this much     less serious is that the IP address of the peer for both SAs must     match.

3. (not a bug) Eliminate a redundant test for rc != NULL. If it

    isn't redundant, I'd like to know.

There are now declarations in ipsec_doi.h for two functions that are never defined or used (send_notification_from_state and send_notification_from_md). It turns out that this is legal C. Still, it would be better to delete these declarations.

Hugh Redelmeier
hugh@mimosa.com voice: +1 416 482-8253

X--- ipsec_doi.c.OLD	2003-02-13 02:12:06.000000000 -0500
X+++ ipsec_doi.c	2003-02-13 17:49:01.000000000 -0500
X@@ -661,7 +661,7 @@
X     size_t sizespi;
X     int i;

X
X- if (st == NULL && (md->hdr.isa_flags & ISAKMP_FLAG_ENCRYPTION) == 0) X+ if (!md->encrypted)

X     {
X 	loglog(RC_LOG_SERIOUS, "ignoring Delete SA payload: not encrypted");
X 	return;
X@@ -734,6 +734,12 @@
X 		loglog(RC_LOG_SERIOUS, "ignoring Delete SA payload: "
X 		    "ISAKMP SA not found (maybe expired)");
X 	    }
X+	    else if (!same_peer_ids(st->st_connection, dst->st_connection, NULL))
X+	    {
X+		/* we've not authenticated the relevant identities */
X+		loglog(RC_LOG_SERIOUS, "ignoring Delete SA payload: "
X+		    "ISAKMP SA used to convey Delete has different IDs from ISAKMP SA it deletes");
X+	    }
X 	    else
X 	    {
X 		loglog(RC_LOG_SERIOUS, "received Delete SA payload: "
X@@ -763,8 +769,7 @@
X 	    {
X 		struct connection *rc = dst->st_connection;
X 
X-		if (rc != NULL
X-		&& rc->newest_ipsec_sa == dst->st_serialno
X+		if (rc->newest_ipsec_sa == dst->st_serialno
X 		&& (rc->policy & POLICY_UP))
X 		    {
X 		    /* Last IPSec SA for a permanent connection that we
================ end ================

-----BEGIN PGP SIGNATURE-----

Version: 2.6.3ia
Charset: noconv

iQCVAwUBPkwjMsFAuQPManGZAQHGtgP+Nxt/7Rqmhth3xBnRTa0Z+v0njJwpAYEO Yo9mYgYCXdIXL+8XM2DnCt4jCFKNNALAq/X4ZSfNsEm/hmGx8FaQh5dcHVamT53M ROUl7qse4eyAfcSlGnt7QHh21IZALc+9npH9yLzkDLYnQ45r9yxnPMf3XkhfPR5L bVw0XWBTtKU=
=fMRq
-----END PGP SIGNATURE-----

---

Design mailing list
Design@lists.freeswan.org
http://lists.freeswan.org/mailman/listinfo/design Received on Thu Feb 13 18:39:43 2003