[Swan] Crash with Libreswan 3.11

[Roel van Meer]

Roel van Meer writes:

> I've tried several versions of Libreswan (3.8, 3.9, 3.11) and two versions  
> of mozilla-nss (3.13.5, 3.16.5), but the crash is seen in all combinations.

I've now also tried with nss-3.17.2, but that has the same result.

Together with a colleague I added some null pointer checks to nss (see the  
attached patch). When we do that, the crash disappears as such, but now we  
hit a failed assert:

pluto[22944]: "conn" #26: ASSERTION FAILED at /tmp/tmp.pCVGT6/libreswan-3.11/programs/pluto/crypt_dh.c:706: tkey19 != NULL
pluto[22944]: "conn" #26: ABORT at /tmp/tmp.pCVGT6/libreswan-3.11/programs/pluto/crypt_dh.c:706
pluto[22944]: "conn" #26: ABORT at /tmp/tmp.pCVGT6/libreswan-3.11/programs/pluto/crypt_dh.c:706
ipsec__plutorun: !pluto failure!:  exited with error status 134 (signal 6)

and on the next run:

pluto[23830]: "conn" #45: ASSERTION FAILED at /tmp/tmp.pCVGT6/libreswan-3.11/programs/pluto/crypt_dh.c:877: tkey37 != NULL
pluto[23830]: "conn" #45: ABORT at /tmp/tmp.pCVGT6/libreswan-3.11/programs/pluto/crypt_dh.c:877
pluto[23830]: "conn" #45: ABORT at /tmp/tmp.pCVGT6/libreswan-3.11/programs/pluto/crypt_dh.c:877
ipsec__plutorun: !pluto failure!:  exited with error status 134 (signal 6)

After that, our maintenance window ran out, so we could do no more tests.

I hope this is useful. Please let me know if I can provide any  
more information, or if there is anything I should test.

Thanks,

Roel
-------------- next part --------------
diff -ruN a/nss/lib/pk11wrap/pk11skey.c b/nss/lib/pk11wrap/pk11skey.c
--- a/nss/lib/pk11wrap/pk11skey.c	2014-10-10 18:56:55.000000000 +0200
+++ b/nss/lib/pk11wrap/pk11skey.c	2014-11-04 10:42:30.426403959 +0100
@@ -20,13 +20,13 @@
 
 static void
 pk11_EnterKeyMonitor(PK11SymKey *symKey) {
-    if (!symKey->sessionOwner || !(symKey->slot->isThreadSafe)) 
+    if ((symKey->slot) && (!symKey->sessionOwner || !(symKey->slot->isThreadSafe))) 
 	PK11_EnterSlotMonitor(symKey->slot);
 }
 
 static void
 pk11_ExitKeyMonitor(PK11SymKey *symKey) {
-    if (!symKey->sessionOwner || !(symKey->slot->isThreadSafe)) 
+    if ((symKey->slot) && (!symKey->sessionOwner || !(symKey->slot->isThreadSafe))) 
     	PK11_ExitSlotMonitor(symKey->slot);
 }
 
@@ -1489,7 +1489,7 @@
     PR_ASSERT(templateCount <= sizeof(keyTemplate)/sizeof(CK_ATTRIBUTE));
 
     /* move the key to a slot that can do the function */
-    if (!PK11_DoesMechanism(slot,derive)) {
+    if (slot == NULL || !PK11_DoesMechanism(slot,derive)) {
 	/* get a new base key & slot */
 	PK11SlotInfo *newSlot = PK11_GetBestSlot(derive, baseKey->cx);
 
diff -ruN a/nss/lib/pk11wrap/pk11slot.c b/nss/lib/pk11wrap/pk11slot.c
--- a/nss/lib/pk11wrap/pk11slot.c	2014-10-10 18:56:55.000000000 +0200
+++ b/nss/lib/pk11wrap/pk11slot.c	2014-11-04 10:50:41.415893864 +0100
@@ -1815,6 +1815,10 @@
 {
     int i;
 
+    if (slot == NULL) {
+	return PR_FALSE;
+    }
+
     /* CKM_FAKE_RANDOM is not a real PKCS mechanism. It's a marker to
      * tell us we're looking form someone that has implemented get
      * random bits */