Thread: [Ipsec-tools-devel] Use-after-free subsequent to "flush-sa isakmp"
Brought to you by:
mit_warlord,
netbsd
From: John-Mark B. <jm...@ze...> - 2011-12-15 12:00:30
|
Hi, I've been experiencing racoon (ipsec-tools 0.8.0) crashing with SIGSEGV after requesting that the ISAKMP associations are flushed through the admin port. This appears to be the result of the phase 2 associations retaining a backlink to the phase1 associations, which is not cleaned up as a result of the call to flushph1() on line 286 of admin.c Sample backtrace: #0 0x000000000043e4f5 in eay_hmac_one (key=0x0, data=0x764690, type=0x7f6d48409680) at crypto_openssl.c:1816 #1 0x000000000043443c in oakley_prf (key=<value optimized out>, buf=<value optimized out>, iph1=<value optimized out>) at oakley.c:438 #2 0x0000000000436ba4 in oakley_compute_hash1 (iph1=0x759ee0, msgid=<value optimized out>, body=0x760050) at oakley.c:868 #3 0x000000000041b17e in quick_i1send (iph2=0x759ce0, msg=<value optimized out>) at isakmp_quick.c:360 #4 0x0000000000407899 in isakmp_post_getspi (iph2=0x7f6d48409680) at isakmp.c:2338 #5 0x0000000000426940 in pk_recvgetspi (mhp=<value optimized out>) at pfkey.c:1077 #6 0x0000000000427001 in pfkey_handler (ctx=<value optimized out>, fd=<value optimized out>) at pfkey.c:287 #7 0x0000000000407282 in session () at session.c:325 #8 0x0000000000406b14 in main (ac=4, av=<value optimized out>) at main.c:348 Should the admin port flush-sa request handler also be calling flushph2, much like every other call site for flushph1? Let me know if you need more information. Thanks, John-Mark. |
From: VANHULLEBUS Y. <va...@fr...> - 2011-12-16 13:48:56
|
On Thu, Dec 15, 2011 at 12:00:19PM +0000, John-Mark Bell wrote: > Hi, Hi. > I've been experiencing racoon (ipsec-tools 0.8.0) crashing with SIGSEGV > after requesting that the ISAKMP associations are flushed through the > admin port. This appears to be the result of the phase 2 associations > retaining a backlink to the phase1 associations, which is not cleaned > up as a result of the call to flushph1() on line 286 of admin.c > > Sample backtrace: [...] > Should the admin port flush-sa request handler also be calling flushph2, > much like every other call site for flushph1? I don't use adminport, but I guess it would be expected to do more or less the same stuff done in isakmp_ph1delete(): try to migrate existing phase 2 handlers, then purge them if no other phase 1 found. Yvan. |
From: John-Mark B. <jm...@ze...> - 2011-12-17 10:45:31
|
On Fri, Dec 16, 2011 at 02:48:41PM +0100, VANHULLEBUS Yvan wrote: > On Thu, Dec 15, 2011 at 12:00:19PM +0000, John-Mark Bell wrote: > > Sample backtrace: > [...] > > Should the admin port flush-sa request handler also be calling flushph2, > > much like every other call site for flushph1? > > I don't use adminport, but I guess it would be expected to do more or > less the same stuff done in isakmp_ph1delete(): try to migrate > existing phase 2 handlers, then purge them if no other phase 1 found. OK, but given that we're flushing all phase 1 entries, there's presumably no point in migrating the phase 2 handlers at all -- instead, just unilaterally purging them? John-Mark. |
From: Timo T. <tim...@ik...> - 2012-01-01 17:43:30
|
On 12/17/2011 12:45 PM, John-Mark Bell wrote: > On Fri, Dec 16, 2011 at 02:48:41PM +0100, VANHULLEBUS Yvan wrote: >> On Thu, Dec 15, 2011 at 12:00:19PM +0000, John-Mark Bell wrote: >>> Sample backtrace: >> [...] >>> Should the admin port flush-sa request handler also be calling flushph2, >>> much like every other call site for flushph1? >> >> I don't use adminport, but I guess it would be expected to do more or >> less the same stuff done in isakmp_ph1delete(): try to migrate >> existing phase 2 handlers, then purge them if no other phase 1 found. > > OK, but given that we're flushing all phase 1 entries, there's > presumably no point in migrating the phase 2 handlers at all -- instead, > just unilaterally purging them? It perfectly valid to have phase2 handlers without existing phase1. Though certain features would not work, e.g. DPD. The admin port explicitly passes in the type of SAs it wants to purge, so I'd rather just leave the phase2 handlers hanging and clear the ph1 pointer. If user wants both deleted, it will call flush-sa twice. - Timo |
From: John-Mark B. <jm...@ze...> - 2012-01-03 13:12:58
|
On Sun, Jan 01, 2012 at 07:43:24PM +0200, Timo Teräs wrote: > On 12/17/2011 12:45 PM, John-Mark Bell wrote: > > > > OK, but given that we're flushing all phase 1 entries, there's > > presumably no point in migrating the phase 2 handlers at all -- instead, > > just unilaterally purging them? > > It perfectly valid to have phase2 handlers without existing phase1. > Though certain features would not work, e.g. DPD. The admin port > explicitly passes in the type of SAs it wants to purge, so I'd rather > just leave the phase2 handlers hanging and clear the ph1 pointer. If > user wants both deleted, it will call flush-sa twice. Having inspected the call path of the stack trace I quoted, clearing the ph1 pointer in the phase2 handler would result in dereferencing NULL within isakmp_post_getspi. I imagine this isn't the only place where it's assumed that the ph1 pointer is valid. What should the behaviour be if there is no phase1 handler associated with a phase2 handler? John-Mark. |
From: VANHULLEBUS Y. <va...@fr...> - 2012-01-04 16:05:24
|
On Sun, Jan 01, 2012 at 07:43:24PM +0200, Timo Ters wrote: > On 12/17/2011 12:45 PM, John-Mark Bell wrote: > > On Fri, Dec 16, 2011 at 02:48:41PM +0100, VANHULLEBUS Yvan wrote: > >> On Thu, Dec 15, 2011 at 12:00:19PM +0000, John-Mark Bell wrote: > >>> Sample backtrace: > >> [...] > >>> Should the admin port flush-sa request handler also be calling flushph2, > >>> much like every other call site for flushph1? > >> > >> I don't use adminport, but I guess it would be expected to do more or > >> less the same stuff done in isakmp_ph1delete(): try to migrate > >> existing phase 2 handlers, then purge them if no other phase 1 found. > > > > OK, but given that we're flushing all phase 1 entries, there's > > presumably no point in migrating the phase 2 handlers at all -- instead, > > just unilaterally purging them? > > It perfectly valid to have phase2 handlers without existing phase1. > Though certain features would not work, e.g. DPD. The admin port > explicitly passes in the type of SAs it wants to purge, so I'd rather > just leave the phase2 handlers hanging and clear the ph1 pointer. If > user wants both deleted, it will call flush-sa twice. The actual code of racoon expects iph2->ph1 to be valid in various places, so having some code to put it to NULL will probably lead to various crashes.... So we should really NOT go back to such NULL iph2->ph1, unless we also check all the code to ensure they will be correctly handled ! Yvan. |