Did vpn.inc commit cf0a2714c2 break IPsec transport mode?
-
Hi all,
I've been running IPsec in tunnel mode between two pfSense 2.1 snapshot hosts (most recently the Aug 24 release) linking two private ipv4 networks. This setup has been working perfectly fine but now I'm trying to convert this into a GRE tunnel protected by IPsec in transport mode instead because of the more flexible routing options. While the GRE tunnel itself works just as intended no IPsec Security Policy entries are being added. After looking into the code (/etc/inc/vpn.inc) it turns out that when it encounters the (only) Phase 2 entry it's being silently rejected because of the is_subnet check added by commit cf0a2714c2 ( https://github.com/bsdperimeter/pfsense/commit/cf0a2714c23c642d4119b2bb0e6ac887538967eb#etc/inc/vpn.inc ). Should this check really be done for transport mode IPsec?
Commenting out the check will allow the SPs to be added and the communication will now be secured, but the check should most likely only be omitted if the Phase 2 in question is in transport mode. Also there seems to be no way to add SPs only for specific protocols (like GRE)? Should I create a ticket for this?
/wj
-
Hmm that shouldn't have broken it, but I'm not sure I tested that (it has been several months…)
I thought it still used x.x.x.x/32 for a single IP in those cases though - what does your /var/etc/spd.conf look like when it does work?
-
Yes, the commit is 7 months old, so I completely understand that. I only discovered this when trying to use the feature, long after the commit made it in. The is_subnet check that fails just before $spdconf is generated was what I had to comment out, and that $localid is not used to create spd.conf in the transport case (it fetches $localid_data from Phase1 instead).
When it works my spd.conf looks like this: (when vpn.inc is left unpatched all I get are the first four anti-lockout lines)
spdadd -4 10.xxx.yyy.1/32 10.xxx.yyy.0/24 any -P out none;
spdadd -4 10.xxx.yyy.0/24 10.xxx.yyy.1/32 any -P in none;
spdadd -6 2601:9:zzzz:zzzz::1/128 2601:9:zzzz:zzzz:0:0:0:0/64 any -P out none;
spdadd -6 2601:9:zzzz:zzzz:0:0:0:0/64 2601:9:zzzz:zzzz::1/128 any -P in none;
spdadd 24.23.xxx.yyy 77.105.xxx.yyy any -P out ipsec esp/transport//require;
spdadd 77.105.xxx.yyy 24.23.xxx.yyy any -P in ipsec esp/transport//require;To only encrypt GRE traffic the vpn.inc file could be manually patched to output "gre" instead of "any" for the actual Phase2 entry. I noticed that grangej created a nice patch for that some time ago here: https://github.com/bsdperimeter/pfsense/pull/118.
Worth noting though is that the IPsec status in transport mode shows up as errored state (white X on yellow background), but with the SAD and SPD tabs populated (although Source and Destination fields are completely blank in the SPD tab when only GRE is protected instead of ANY).
/wj
-
Can you see if this works:
if (!is_ipaddr($localid_data) && !is_subnet($localid_data)) {If it's just an IP it would fail that subnet test but an IP is valid there.
-
By that I mean changing both instances of that test to be:
(!is_ipaddr($localid_data) && !is_subnet($localid_data))Not just the first one.
-
By that I mean changing both instances of that test to be:
(!is_ipaddr($localid_data) && !is_subnet($localid_data))Not just the first one.
That unfortunately did not work. The second test (which was the one I had to comment out) still won't pass. In the code it tests $localid_data in the first test and $localid in the second test. I tried replacing both conditions the one above and that did not work. I also tried replacing $localid_data with the original $localid for the second test, still no go. $localid_data is overridden on line 897 for transport mode, so the second test doesn't seem to make sense to me (in transport mode specifically).
/wj
-
Yeah I meant to mention the $localid in the second… So this didn't work:
if (!is_ipaddr($localid_data) && !is_subnet($localid_data)) { log_error("Invalid IPsec Phase 2 \"{$ph2ent['descr']}\" - {$ph2ent['localid']['type']} has no subnet."); continue; }…
// Error will be logged above, no need to log this twice. #2201 if ((!is_ipaddr($localid) && !is_subnet($localid))) continue;?
-
OK, if that didn't work, try this:
First use this top test:
// Don't let an empty subnet into racoon.conf, it can cause parse errors. Ticket #2201. if (!is_ipaddr($localid_data) && !is_subnet($localid_data)) { log_error("Invalid IPsec Phase 2 \"{$ph2ent['descr']}\" - {$ph2ent['localid']['type']} has no subnet."); continue; }Then move the second test down into the test that is only for tunnel modes:
if(($ph2ent['mode'] == "tunnel") or ($ph2ent['mode'] == 'tunnel6')) { // Error will be logged above, no need to log this twice. #2201 if (!is_subnet($localid)) continue; -
OK, if that didn't work, try this:
First use this top test:
// Don't let an empty subnet into racoon.conf, it can cause parse errors. Ticket #2201. if (!is_ipaddr($localid_data) && !is_subnet($localid_data)) { log_error("Invalid IPsec Phase 2 \"{$ph2ent['descr']}\" - {$ph2ent['localid']['type']} has no subnet."); continue; }Then move the second test down into the test that is only for tunnel modes:
if(($ph2ent['mode'] == "tunnel") or ($ph2ent['mode'] == 'tunnel6')) { // Error will be logged above, no need to log this twice. #2201 if (!is_subnet($localid)) continue;That works for me! By the way, where is the IPsec status check performed? I was thinking about looking into why it shows up as error when it clearly works (as shown by external packet capture showing only isakmp and esp packets between the two hosts).
/wj
-
OK, I committed that, should be in snaps later today or tomorrow.
Unfortunately that same code was in 2.0.2, so I'll have to rebuild those images also…
-
The IPsec status check is all in diag_ipsec.php I thought.
What it was supposed to check was for the presence of both a p1 and matching p2. I suppose transport mode is just slightly different enough that the current checks don't let it line up.
-
The IPsec status check is all in diag_ipsec.php I thought.
What it was supposed to check was for the presence of both a p1 and matching p2. I suppose transport mode is just slightly different enough that the current checks don't let it line up.
You're right. It's definitely the ipsec_phase2_status call in diag_ipsec.php that returns false, and the ipsec_phase2_status implementation in /etc/inc/ipsec.inc isn't matching the output from setkey. The two SPs I currently have looks like this:
77.105.xxx.yyy 24.23.xxx.yyy gre in ipsec esp/transport//require spid=36 seq=3 pid=22013 refcnt=1 24.23.xxx.yyy 77.105.xxx.yyy gre out ipsec esp/transport//require spid=35 seq=0 pid=22013 refcnt=1(substitute gre for any if not using the "Cisco compatibility")
There are no square brackets at all on the address line in transport mode, which definitely confuses the current code.
The corresponding SAs looks like this:
24.23.xxx.yyy 77.105.xxx.yyy esp mode=any spi=1965845(0x001dff15) reqid=0(0x00000000) E: blowfish-cbc zzzz A: hmac-sha256 zzzz seq=0x00000000 replay=4 flags=0x00000000 state=mature created: Aug 27 15:32:04 2012 current: Aug 27 15:35:28 2012 diff: 204(s) hard: 3600(s) soft: 2880(s) last: hard: 0(s) soft: 0(s) current: 0(bytes) hard: 0(bytes) soft: 0(bytes) allocated: 0 hard: 0 soft: 0 sadb_seq=3 pid=43976 refcnt=1 77.105.xxx.yyy 24.23.xxx.yyy esp mode=transport spi=173094859(0x0a5137cb) reqid=0(0x00000000) E: blowfish-cbc zzzz A: hmac-sha256 zzzz seq=0x00000000 replay=4 flags=0x00000000 state=mature created: Aug 27 15:32:04 2012 current: Aug 27 15:35:28 2012 diff: 204(s) hard: 3600(s) soft: 2880(s) last: hard: 0(s) soft: 0(s) current: 0(bytes) hard: 0(bytes) soft: 0(bytes) allocated: 0 hard: 0 soft: 0 sadb_seq=1 pid=43976 refcnt=1I'll dig into the ipsec.inc code some more to see if there could be an easy fix to allow the code to match the correct data for transport mode.
/wj
