Re: unauthorized deletion of IPsec (and ISAKMP) SAs in racoon

[itojun () kame net]

> 0 Preface
>
>   Now that most bugs in isakmpd that allowed for unauthorized SA
>   deletion are "fixed", it's time to release some information on racoon.
>
>   By the way: About 5 months ago I tried to contact the KAME developers.

        sorry that we did not take necessary actions that time.  the attached
        patch should remedy the problem (credit: IIJ SEIL team).
        kame as well as netbsd repository are updated, and vendors are informed.

itojun


Index: isakmp_inf.c
===================================================================
RCS file: /cvsroot/kame/kame/kame/kame/racoon/isakmp_inf.c,v
retrieving revision 1.82
diff -u -r1.82 isakmp_inf.c
--- isakmp_inf.c        13 Nov 2003 02:30:20 -0000      1.82
+++ isakmp_inf.c        14 Jan 2004 09:14:31 -0000
@@ -136,10 +136,81 @@
 
        isakmp = (struct isakmp *)msg->v;
        gen = (struct isakmp_gen *)((caddr_t)isakmp + sizeof(struct isakmp));
-       if (isakmp->np == ISAKMP_NPTYPE_HASH)
-               np = gen->np;
-       else
-               np = isakmp->np;
+
+       if (isakmp->np != ISAKMP_NPTYPE_HASH) {
+               plog(LLV_ERROR, LOCATION, NULL,
+                   "ignore information because the message has no hash payload.\n");
+               goto end;
+       }
+
+       if (iph1->status != PHASE1ST_ESTABLISHED) {
+               plog(LLV_ERROR, LOCATION, NULL,
+                   "ignore information because ISAKMP-SA has not been established yet.\n");
+               goto end;
+       }
+
+       np = gen->np;
+
+       {
+               void *p;
+               vchar_t *hash, *payload;
+               struct isakmp_gen *nd;
+
+               /*
+                * XXX: gen->len includes isakmp header length
+                */
+               p = (caddr_t) gen + sizeof(struct isakmp_gen);
+               nd = (struct isakmp_gen *) ((caddr_t) gen + gen->len);
+
+               /* nd length check */
+               if (nd->len > msg->l - (sizeof(struct isakmp) + gen->len)) {
+                       plog(LLV_ERROR, LOCATION, NULL,
+                                "too long payload length (broken message?)\n");
+                       goto end;
+               }
+
+               payload = vmalloc(nd->len);
+               if (payload == NULL) {
+                       plog(LLV_ERROR, LOCATION, NULL,
+                           "cannot allocate memory\n");
+                       goto end;
+               }
+
+               memcpy(payload->v, (caddr_t) nd, nd->len);
+
+               /* compute HASH */
+               hash = oakley_compute_hash1(iph1, isakmp->msgid, payload);
+               if (hash == NULL) {
+                       plog(LLV_ERROR, LOCATION, NULL,
+                           "cannot compute hash\n");
+
+                       vfree(payload);
+                       goto end;
+               }
+               
+               if (gen->len - sizeof(struct isakmp_gen) != hash->l) {
+                       plog(LLV_ERROR, LOCATION, NULL,
+                           "ignore information due to hash length mismatch\n");
+
+                       vfree(hash);
+                       vfree(payload);
+                       goto end;
+               }
+
+               if (memcmp(p, hash->v, hash->l) != 0) {
+                       plog(LLV_ERROR, LOCATION, NULL,
+                           "ignore information due to hash mismatch\n");
+
+                       vfree(hash);
+                       vfree(payload);
+                       goto end;
+               }
+
+               plog(LLV_DEBUG, LOCATION, NULL, "hash validated.\n");
+
+               vfree(hash);
+               vfree(payload);
+       }
                
        /* make sure the packet were encrypted. */
        if (!encrypted) {