From: <tim...@ik...> - 2007-11-30 08:11:07
|
Hi, I'm resending my updated patches against CVS. The split up is as follows: 01-ipsecdoi_prefix_host.diff - Adds the IPSECDOI_PREFIX_HOST macro which can be used to request host identity (instead of subnet) in an address family independent manner - Changed IPSECDOI_PREFIX_HOST to 0xff as ~0 was considered unportable 02-transport-ids-from-addrs.diff - Use sockaddrs as transport mode IDs; this allows transport mode wildcard SPDs to be used 03-quick_r1recv-memleak.diff - Fixes a memory leak in one error path 04-transport-natoa.diff - Implements NAT-OA sending and parsing - Modifies checking of received IDci/IDcr to accept also NAT-OA as alternative to proposed one - Reports NAT-OA using pfkey to kernel - Tested: ipsec-tools on Linux 2.6.20.3 as initiator against Solaris And again, even though I'm no lawyer, I do believe that the patents do not cover NAT-OA, so these patches should be safe. The patents seem to cover NAT-D, port floating and some sort of address mapping system. I've also prepared a single patch with all of the above against 0.7 release. You can get it from http://www.solidboot.com/~fabled/ipsec-tools-0.7-spd-natoa.diff Please consider for applying. Thanks, Timo |
From: Dan M. <da...@su...> - 2007-11-30 16:06:55
|
On Fri, Nov 30, 2007 at 10:11:10AM +0200, Timo Ters wrote: > > And again, even though I'm no lawyer, I do believe that the patents do not > cover NAT-OA, so these patches should be safe. The patents seem to cover > NAT-D, port floating and some sort of address mapping system. Nobody on the IESG would've let NAT-T go to Proposed Standard without an open-source friendly patent disposition. I don't think anyone here has to worry about IPR issues, seriously. I can double check for folks at the IETF next week if anyone wants, though. Dan |
From: <tim...@ik...> - 2007-12-04 13:07:18
|
Hi again, I'm resending my patches again. As Emmanuel commented there should be no problem with including the NAT-OA, I'm now hoping some could review these and comment on them. I'm still planning to implement more stuff, so getting some feedback now on needed changes could help me avoid extra work on merging the changes later to multiple patches. The first three patches are unchanged from previous send (consider committing): 01-ipsecdoi_prefix_host.diff 02-transport-ids-from-addrs.diff 03-quick_r1recv-memleak.diff The following patches are new/changed and still a bit experimental (for review): 04-transport-natoa.diff - removed one extra hunk adding empty line within a comment - modfied checking of IDcr/IDci: when checking ID against NAT-OA check only the address portion against NAT-OA as port is never used in NAT-OA 05-admin-establish-ipsec-sa.diff - implement establish-sa [esp|ah] in admin.c - make a new helper function isakmp_get_sainfo(); is the correct place in isakmp.c? - send phase2 up events - establish-sa -w option to wait until sa is established or an error happens 06-racoonctl-ipproto-gre.diff - add GRE protocol number to racoonctl and related manpage I'm still planning to improve the event reporting to racoonctl. Current event dispatching is based on polling. So each event is seen only by one racoonctl process. And e.g. running multiple establish-sa requests with -w simultaneously (or vpn-connect) is broken. Also I'm planning addition of "do not replace SA" type parameter for establish-sa. These would make integration of ipsec-tools with OpenNHRP [http://sourceforge.net/projects/opennhrp] very straight forward. Cheers, Timo |
From: Matthew G. <mg...@sh...> - 2007-12-04 20:01:21
|
Timo Teräs wrote: > Hi again, > > I'm resending my patches again. As Emmanuel commented there should be no problem with including the NAT-OA, I'm now hoping some could review these and comment on them. I'm still planning to implement more stuff, so getting some feedback now on needed changes could help me avoid extra work on merging the changes later to multiple patches. > > The first three patches are unchanged from previous send (consider committing): > 01-ipsecdoi_prefix_host.diff > 02-transport-ids-from-addrs.diff > 03-quick_r1recv-memleak.diff I committed 01-03 to head. If there are no objections I will commit 03 to the 7 branch as well. > > The following patches are new/changed and still a bit experimental (for review): > 04-transport-natoa.diff > - removed one extra hunk adding empty line within a comment > - modfied checking of IDcr/IDci: when checking ID against NAT-OA check only the address portion against NAT-OA as port is never used in NAT-OA > I will need to take a closer look at 04 but am swamped at the moment. Maybe Yvan or Manu will get to this before I do. > 05-admin-establish-ipsec-sa.diff > - implement establish-sa [esp|ah] in admin.c > - make a new helper function isakmp_get_sainfo(); is the correct place in isakmp.c? > - send phase2 up events > - establish-sa -w option to wait until sa is established or an error happens > > 06-racoonctl-ipproto-gre.diff > - add GRE protocol number to racoonctl and related manpage > > I'm still planning to improve the event reporting to racoonctl. Current event dispatching is based on polling. So each event is seen only by one racoonctl process. And e.g. running multiple establish-sa requests with -w simultaneously (or vpn-connect) is broken. Also I'm planning addition of "do not replace SA" type parameter for establish-sa. These would make integration of ipsec-tools with OpenNHRP [http://sourceforge.net/projects/opennhrp] very straight forward. > I believe Yvan was looking at 06 ( maybe 05 as well )? -Matthew |
From: <tim...@ik...> - 2007-12-07 06:15:19
Attachments:
01-transport-nat-oa.diff
02-main-transport-id-check.diff
|
Matthew Grooms wrote: > Timo Teräs wrote: >> The following patches are new/changed and still a bit experimental >> (for review): >> 04-transport-natoa.diff >> - removed one extra hunk adding empty line within a comment >> - modfied checking of IDcr/IDci: when checking ID against NAT-OA check >> only the address portion against NAT-OA as port is never used in NAT-OA > > I will need to take a closer look at 04 but am swamped at the moment. > Maybe Yvan or Manu will get to this before I do. There was bug in 04. The attached 01 patch is fixed. Problem was that isakmp_p2ph() expects vchar_t* to be initialized to NULL which wasn't done in the previous patch. This attached 01 patch has been tested in two scenarios: - racoon in natted box as initiator against Solaris, complex SPDs - WinXP SP2 in natted box as initiator against racoon, pre-shared keys and simple SPD for ICMPs Solaris worked okay with the 01 patch. The WinXP situation in my case required extra patching: windows sent main mode ids as fqdn. I'm not sure why racoon expects to get IP identities in main mode when using pre-shared key. But after removing that check everything worked perfectly well. Patch for that is attached as 02. Please, consider committing 01. >> 05-admin-establish-ipsec-sa.diff >> - implement establish-sa [esp|ah] in admin.c >> - make a new helper function isakmp_get_sainfo(); is the correct place >> in isakmp.c? >> - send phase2 up events >> - establish-sa -w option to wait until sa is established or an error >> happens Any comments on this? >> 06-racoonctl-ipproto-gre.diff >> - add GRE protocol number to racoonctl and related manpage This one is trivial patch. Please consider committing. It applies cleanly (with some hunks having offsets) against CVS even though it is the last of the patch series. >> I'm still planning to improve the event reporting to racoonctl. >> Current event dispatching is based on polling. So each event is seen >> only by one racoonctl process. And e.g. running multiple establish-sa >> requests with -w simultaneously (or vpn-connect) is broken. Also I'm >> planning addition of "do not replace SA" type parameter for >> establish-sa. These would make integration of ipsec-tools with >> OpenNHRP [http://sourceforge.net/projects/opennhrp] very straight >> forward. Any thoughts how to do these improvements is appreciated. My initial plan is to add a command to subscribe events (e.g. ADMIN_SUBSCRIBE_EVT 0x0004 to admin.h) after which all events will be broadcast to the subscribing socket. And that events would be queued in racoon only if no listeners are subscribed. How does this sound? Cheers, Timo |
From: <tim...@ik...> - 2007-12-15 08:02:02
|
Timo Teräs wrote: > I'll resend my patch about implementing phase 2 establish-sa later when > I've implemented the event handling changes first. Ok. Initial version of the event handling stuff is done. Works well on my test setup. > Patches are as follows: > 01-racoonctl-ipproto-gre.diff > - trivial: add GRE protocol number to racoonctl Reposted unchanged. Please commit. > 02-main-transport-id-check.diff > - remove check that transport mode, phase1 id has to be IP > - gives better windows xp compatibility > - please comment if this has additional side effects I dropped this for the time being as it might have some unwanted security consequences. And I think one should be able to configure windows to send IP identities. > 03-fix-spd-wildmatching.diff Reposted unchanged as 02-fix-spd-wildmatching.diff. Please commit. Here comes the new stuff: 03-admin-events-refactored.diff - push events instead of polling - can have multiple event listeners - fix some stuff that didn't work e.g. one is now actually able to get the "racoon quit" event And fourth patch to follow in next mail. Cheers, Timo |
From: Matthew G. <mg...@sh...> - 2007-12-18 19:03:04
|
Timo Teräs wrote: > Timo Teräs wrote: >> I'll resend my patch about implementing phase 2 establish-sa later when >> I've implemented the event handling changes first. > > Ok. Initial version of the event handling stuff is done. Works well on > my test setup. > >> Patches are as follows: >> 01-racoonctl-ipproto-gre.diff >> - trivial: add GRE protocol number to racoonctl > > Reposted unchanged. > Please commit. > >> 02-main-transport-id-check.diff >> - remove check that transport mode, phase1 id has to be IP >> - gives better windows xp compatibility >> - please comment if this has additional side effects > > I dropped this for the time being as it might have some unwanted security > consequences. And I think one should be able to configure windows to send > IP identities. > >> 03-fix-spd-wildmatching.diff > > Reposted unchanged as 02-fix-spd-wildmatching.diff. > Please commit. > > Here comes the new stuff: > > 03-admin-events-refactored.diff > - push events instead of polling > - can have multiple event listeners > - fix some stuff that didn't work e.g. one is now actually able to get the > "racoon quit" event > > And fourth patch to follow in next mail. > Timo, Sorry for the delayed response. I am swamped at the moment but should have some time free up early next week. Will try to get these reviewed. Thanks again for your submissions. -Matthew |
From: Matthew G. <mg...@sh...> - 2007-12-31 02:00:39
|
Timo Teräs wrote: > Timo Teräs wrote: >> I'll resend my patch about implementing phase 2 establish-sa later when >> I've implemented the event handling changes first. > > Ok. Initial version of the event handling stuff is done. Works well on > my test setup. > >> Patches are as follows: >> 01-racoonctl-ipproto-gre.diff >> - trivial: add GRE protocol number to racoonctl > > Reposted unchanged. > Please commit. > Committed. >> 03-fix-spd-wildmatching.diff > > Reposted unchanged as 02-fix-spd-wildmatching.diff. > Please commit. > Committed. > Here comes the new stuff: > > 03-admin-events-refactored.diff > - push events instead of polling > - can have multiple event listeners > - fix some stuff that didn't work e.g. one is now actually able to get the > "racoon quit" event > I haven't worked with the admin socket much. The benefits for racoonctl sound very good. Do we need to be concerned about other applications that may be using the admin interface as well? I wonder if software like ipsecuritas would be adversely effected. Yvan or Manu, do you have any comments? If not, I will go ahead and commit this to head. -Matthew |
From: <ma...@ne...> - 2007-12-31 04:47:43
|
Matthew Grooms <mg...@sh...> wrote: > I haven't worked with the admin socket much. The benefits for racoonctl > sound very good. Do we need to be concerned about other applications > that may be using the admin interface as well? I wonder if software like > ipsecuritas would be adversely effected. Yvan or Manu, do you have any > comments? If not, I will go ahead and commit this to head. I was not aware that another tool used the socket interface. IMO we should ensure that the protocol remain backward compatible anyway, so that when someone upgrades, it does not get a weird bug because the older racoonctl is still in the path. As I understand, the change is meant to maintain backward compatibility, so it seems fine to me. -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz ma...@ne... |
From: Matthew G. <mg...@sh...> - 2007-12-31 07:05:20
|
Emmanuel Dreyfus wrote: > Matthew Grooms <mg...@sh...> wrote: > >> I haven't worked with the admin socket much. The benefits for racoonctl >> sound very good. Do we need to be concerned about other applications >> that may be using the admin interface as well? I wonder if software like >> ipsecuritas would be adversely effected. Yvan or Manu, do you have any >> comments? If not, I will go ahead and commit this to head. > > I was not aware that another tool used the socket interface. IMO we > should ensure that the protocol remain backward compatible anyway, so > that when someone upgrades, it does not get a weird bug because the > older racoonctl is still in the path. > > As I understand, the change is meant to maintain backward compatibility, > so it seems fine to me. > As I said before, I haven't worked much with the admin socket. I suppose I just need to take a closer look at the patch. It seemed to me that the communications protocol itself had changed fundamentally. If there is a clean enough abstraction, maybe things still remain compatible at an API level. -Matthew |
From: <tim...@ik...> - 2007-12-31 07:23:08
|
Matthew Grooms wrote: > Emmanuel Dreyfus wrote: >> Matthew Grooms <mg...@sh...> wrote: >> I was not aware that another tool used the socket interface. IMO we >> should ensure that the protocol remain backward compatible anyway, so >> that when someone upgrades, it does not get a weird bug because the >> older racoonctl is still in the path. >> >> As I understand, the change is meant to maintain backward compatibility, >> so it seems fine to me. > > As I said before, I haven't worked much with the admin socket. I suppose > I just need to take a closer look at the patch. It seemed to me that the > communications protocol itself had changed fundamentally. If there is a > clean enough abstraction, maybe things still remain compatible at an API > level. It's not backward compatible. The event structure has changed and it also leaves the unix socket open so that events can be pushed. We could make it backward compatible, by adding some flag the client needs to sent to get the pushed events. Though then there needs to be quite a bit of unneeded code for to support the old style API. Cheers, Timo |
From: <ma...@ne...> - 2008-01-01 07:19:31
|
Timo Ter=E4s <tim...@ik...> wrote: > We could make it backward compatible, by adding some flag the client need= s > to sent to get the pushed events. Though then there needs to be quite a b= it > of unneeded code for to support the old style API. That's an extra work that needs to be balanced with the problems users will experience (and will report to the list) because of botched upgrades.=20 Full blown backward compatibility would be great. If that's too much work, then IMO we should detect the version mismatch and issue a meaningful error message. As I understand,detecting a version mismatch is easy: you just have to avoid reusing the same message types when you altered message semantics. When you enhance a command, just use a new message type. What I'm not sure about is the difficulty of keeping the obsoleted message types functionnal. Can it be done easily? If it does not, then we can go the meaningful error message path: racoon could log an error and exit, leaving no way for the admin to miss the problem. --=20 Emmanuel Dreyfus http://hcpnet.free.fr/pubz ma...@ne... |
From: <tim...@ik...> - 2008-01-02 09:20:08
|
Emmanuel Dreyfus wrote: > Timo Teräs <tim...@ik...> wrote: >> We could make it backward compatible, by adding some flag the client needs >> to sent to get the pushed events. Though then there needs to be quite a bit >> of unneeded code for to support the old style API. > > That's an extra work that needs to be balanced with the problems users > will experience (and will report to the list) because of botched > upgrades. > > Full blown backward compatibility would be great. If that's too much > work, then IMO we should detect the version mismatch and issue a > meaningful error message. I think the current partial compatibility should pretty ok. I could try to make it a bit more robust, though. Currently all the non-event related admin stuff should work as before. But requesting events works differently. The event structure has changed, so that is incompatible in multiple ways. I would say that this should be sufficient, since the old event dispatching was broken anyway. If we just make ADMIN_SHOW_EVT request return an error we should be pretty good. > As I understand,detecting a version mismatch is easy: you just have to > avoid reusing the same message types when you altered message semantics. > When you enhance a command, just use a new message type. > > What I'm not sure about is the difficulty of keeping the obsoleted > message types functionnal. Can it be done easily? If it does not, then > we can go the meaningful error message path: racoon could log an error > and exit, leaving no way for the admin to miss the problem. Making ADMIN_SHOW_EVT request to be logged by racoon would be sufficient then? IMHO, it does not make sense to implement broken API for backwards compatibility. It would just add code complexity, more code paths to do the same thing and encourage people not to upgrade to an API that actually might work. ;) Cheers, Timo |
From: <ma...@ne...> - 2008-01-02 09:57:06
|
Timo Ter=E4s <tim...@ik...> wrote: > Making ADMIN_SHOW_EVT request to be logged by racoon would be sufficient > then? I'd treat it as fatal: log and exit. That way, one cannot miss the problem, which is a botched upgrade. Anyone has an opinion about it? --=20 Emmanuel Dreyfus http://hcpnet.free.fr/pubz ma...@ne... |
From: <tim...@ik...> - 2008-01-04 07:10:14
|
Emmanuel Dreyfus wrote: > Timo Teräs <tim...@ik...> wrote: >> Making ADMIN_SHOW_EVT request to be logged by racoon would be sufficient >> then? > > I'd treat it as fatal: log and exit. That way, one cannot miss the > problem, which is a botched upgrade. > > Anyone has an opinion about it? I'm ok either way. Making it fatal would certainly make it noticeable. Though it might be a bit overkill to make racoon exit when the problem is in the client. Your call. I'll need to update the patch to log errors about old clients anyway. Just say if you want it to quit too and I'll repost the patch. Cheers, Timo |
From: Matthew G. <mg...@sh...> - 2007-12-12 05:12:52
|
Timo Teräs wrote: > Matthew Grooms wrote: >> Timo Teräs wrote: >>> The following patches are new/changed and still a bit experimental >>> (for review): >>> 04-transport-natoa.diff >>> - removed one extra hunk adding empty line within a comment >>> - modfied checking of IDcr/IDci: when checking ID against NAT-OA check >>> only the address portion against NAT-OA as port is never used in NAT-OA >> I will need to take a closer look at 04 but am swamped at the moment. >> Maybe Yvan or Manu will get to this before I do. > > There was bug in 04. The attached 01 patch is fixed. Problem was that isakmp_p2ph() expects vchar_t* to be initialized to NULL which wasn't done in the previous patch. > > This attached 01 patch has been tested in two scenarios: > - racoon in natted box as initiator against Solaris, complex SPDs > - WinXP SP2 in natted box as initiator against racoon, pre-shared keys and simple SPD for ICMPs > > Solaris worked okay with the 01 patch. > > The WinXP situation in my case required extra patching: windows sent main mode ids as fqdn. I'm not sure why racoon expects to get IP identities in main mode when using pre-shared key. But after removing that check everything worked perfectly well. Patch for that is attached as 02. > > Please, consider committing 01. > The NAT-T OA patch looked good to me and has been committed to head. Thanks for your submission, -Matthew |
From: <tim...@ik...> - 2007-12-12 07:56:06
|
Matthew Grooms wrote: > The NAT-T OA patch looked good to me and has been committed to head. Great. Thanks. I'm reposting two earlier sent patch and one new bug fix. Patches 01 and 03 are trivial and safe to commit. Patch 02 is trivial code change too, but might have some security related side effects so it needs a bit more careful review. I'll resend my patch about implementing phase 2 establish-sa later when I've implemented the event handling changes first. Patches are as follows: 01-racoonctl-ipproto-gre.diff - trivial: add GRE protocol number to racoonctl 02-main-transport-id-check.diff - remove check that transport mode, phase1 id has to be IP - gives better windows xp compatibility - please comment if this has additional side effects 03-fix-spd-wildmatching.diff - fix cmpspidxwild() to allow ul_proto wildcard only in the db entry; even the comment for cmpspidxwild says that only database entry can contain wildcards - this prevents matching wildcard address entries with specific protocol in db to proposals with specific address and wildcard protocol (and thus makes generate_policy work correctly) Cheers, Timo |
From: <tim...@ik...> - 2007-12-15 08:03:00
Attachments:
04-admin-establish-ipsec-sa.diff
|
04-admin-establish-ipsec-sa.diff - updated to work with the previous patch Please review. Cheers, Timo |