nm-l2tp / NetworkManager-l2tp

L2TP and L2TP/IPsec support for NetworkManager
GNU General Public License v2.0
486 stars 84 forks source link

Support @ literal value for vpn.data[ipsec-gateway-id] (rightid) #85

Closed pdugas closed 6 years ago

pdugas commented 6 years ago

Trying to connect to an L2TP/IPSEC VPN appliance that's setting leftid=@remote.example.com and am getting errors on my end when I try to connect.

Peer ID is ID_FQDN: '@remote.example.com'
we require IKEv1 peer to have ID 'X.X.X.X', but peer declares '@remote.example.com'

The generated /var/run/nm-l2tp-ipsec-....conf file contains rightid=%any which I would expect to cause pluto to bypass validation of the peer ID. I guess that's my first issue.

I set ipsec-gateway-id to @remote.example.com in the vpn.data property of the NM connection profile. I did so using the Gnome UI, the Gateway ID field in the IPsec Properties dialog, and verified it with nmcli. When I try to connect, I now get

VPN connection: failed to connect: 'invalid ipsec-gateway-id 'ipsec-gateway-id''

It looks to me like there are issues in src/nm-l2tp-service.c.

I am currently unable to connect to my office's Sophos UTM9 firewall as a result. Any suggestions on a workaround would be appreciated.

pdugas commented 6 years ago

I'm getting further with this change. Still not working so I'm still digging...

diff --git a/src/nm-l2tp-service.c b/src/nm-l2tp-service.c
index 5a49b66..8254c2b 100644
--- a/src/nm-l2tp-service.c
+++ b/src/nm-l2tp-service.c
@@ -265,6 +265,9 @@ validate_gateway_id (const char *id)
        if (!id || !id[0])
                return FALSE;

+       if ('@' == id[0])
+               return TRUE;
+
        /* Ensure it's a valid IP address */
        return inet_aton (id, &addr);
 }
@@ -316,7 +319,7 @@ validate_one_property (const char *key, const char *value, gpointer user_data)
                                             NM_VPN_PLUGIN_ERROR,
                                             NM_VPN_PLUGIN_ERROR_BAD_ARGUMENTS,
                                             _("invalid ipsec-gateway-id '%s'"),
-                                            key);
+                                            value);
                        }
                        return;
                case G_TYPE_UINT:
pdugas commented 6 years ago

It's working for me now with those changes.

Would still like to understand why leaving it blank and getting rightid=%any isn't working. Seems like it should. Perhaps it would be better to just omit the rightid setting if that field is empty?

pdugas commented 6 years ago

Docs for strongswan don't appear to mention use of rightid=%any but there is an explanation that %any can be used for left (without id) and that it matches any IP address, not anything. That may explain my initial confusion.

I wonder if omitting rightid when that connection property isn't set is more appropriate than setting it to %any.

pdugas commented 6 years ago

Nevermind. Omitting rightid causes it to default to the IP, not just accept anything.

Given the commit above, I think this is resolved. Will close it now. Thanks much for the effort on this project.

dkosovic commented 6 years ago

Thanks for the fix! I liked the idea of users explicitly putting the @ at the start of the Gateway ID when required, in earlier versions at one stage, @ was being prepended .

Originally rightid wasn't set, then someone requested rightid=%any be added to the code for NAT'ed VPN servers, after that the number of Peer ID issues being submitted almost stopped.

It was after issue https://github.com/nm-l2tp/network-manager-l2tp/issues/35 when I started explicitly testing only for an IP address for the Gateway ID, I was wrong.

pdugas commented 6 years ago

I didn't look into it but if there is a tooltip for that field in the UI, it should probably be updated to add a blurb about supporting @. Same with any docs.

dkosovic commented 6 years ago

indeed, will do.