Hi Developers,

I caught a crash in racoon, and the following backtrace shows a problem isakmp_info_recv_d. I have proposed a fix for this at the end of this email. The version I am working on is a cehckout from March 16, 2012 of ipsec-tools-0.8.0.

#0  0x0000000000423c53 in isakmp_info_recv_d (iph1=0x2681440, delete=0x2679cac, msgid=4007099779, encrypted=0) at isakmp_inf.c:495
#1  0x000000000042357e in isakmp_info_recv (iph1=0x2681440, msg0=0x2680270) at isakmp_inf.c:299
#2  0x0000000000407e27 in isakmp_main (msg=0x2680270, remote=0x7fffae983550, local=0x7fffae9834d0) at isakmp.c:652
#3  0x00000000004073c1 in isakmp_handler (ctx=0x0, so_isakmp=8) at isakmp.c:377
#4  0x0000000000406603 in session () at session.c:325
#5  0x0000000000405b9b in main (ac=1, av=0x7fffae9847e8) at main.c:345

Scenario:
Racoon acts as a responder to and have received first phase 1 request and sent a response to this. The initiator now sends a ISAKMP request back to the server with a payload to delete the current SA. This request is unencrypted and racoon exposes a configuration option (weak_phase1_check on|off) to control if the real SA kept by racoon should be removed.

The code in context from isakmp_inf.c:495
495             if(!iph1->rmconf->weak_phase1_check && !encrypted) {
496                     plog(LLV_WARNING, LOCATION, iph1->remote,
497                             "Ignoring unencrypted delete payload "
498                             "(check the weak_phase1_check option)\n");
499                     return 0;

The iph1->rmconf is NULL and causes racoon to crash during initial phase1 exchange. The current status of the iph1 at the time of the incident looks like:

side:           1 RESPONDER
status:        3 PH1 MESSAGE SENT     (We have received the first PH1 msg)
etype:         2 Identity Protection
rmconf:       NULL
Retry:         2
NAT-T FL:    0x29
Vendor-ID:  131208
Encapsulation: UDP_ENCAP_ESPINUDP

Message Received: 12, Length 28
ISAKMP DELETE PAYLOAD

It looks to me that the rmconf is used before it is assigned. A simple NULL pointer check should suffice for this problem. However, in this particular case the NULL pointer check will cause it not to delete the specified SA which still seems to be active rendering the weak_phase1_check property useless.

Proposed Fix:
        // Eivind: Check if rmconf is NULL before dereferencing it.
        if(!(iph1->rmconf && iph1->rmconf->weak_phase1_check) && !encrypted) {
                plog(LLV_WARNING, LOCATION, iph1->remote,
                        "Ignoring unencrypted delete payload "
                        "(check the weak_phase1_check option)\n");
                return 0;
        }