Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLOUDSTACK-9296] Start ipsec for client VPN #1423

Merged
merged 1 commit into from May 27, 2016

Conversation

syed
Copy link
Contributor

@syed syed commented Feb 23, 2016

This fix starts the IPSEC daemon when enabling client side vpn

@cristofolini
Copy link
Contributor

This looks like a sensible fix to me. Code LGTM 👍

@kiwiflyer
Copy link
Contributor

@syed Can you squash the 2 commits please?
Can you provide some logs showing the fix works?

@syed
Copy link
Contributor Author

syed commented Mar 10, 2016

@kiwiflyer : Rebase done. Here is the log from the VR when I enable client side vpn

2016-03-10 17:01:25,876  merge.py load:60 Loading data bag type remoteaccessvpn
2016-03-10 17:01:25,876  configure.py process:624 {u'69.196.164.233': {u'local_ip': u'10.1.2.1', u'preshared_key': u'FTfwOHPZK8gBAXWHS4SzhsQN', u'create': True, u'vpn_server_ip': u'69.196.164.233', u'local_cidr': u'10.20.0.0/22', u'ip_range': u'10.1.2.2-10.1.2.254', u'public_interface': u'eth1', u'type': u'remoteaccessvpn'}, u'id': u'remoteaccessvpn'}
2016-03-10 17:01:25,876  configure.py process:632 Enabling  remote access vpn  on 69.196.164.233
2016-03-10 17:01:25,876  CsHelper.py execute2:181 Executing: service ipsec status
2016-03-10 17:01:25,907  CsHelper.py execute2:181 Executing: service ipsec start

earlier we were not starting IPSec

@ThoTischner
Copy link

This should be brought out quickly to community via 4.8.1 release.

@remibergsma
Copy link
Contributor

If there's a Marvin integration test for this feature, I'm wondering why it succeeded without this fix. I think we might need to improve the test and show it's broken, then also show this PR fixes it. Anyone feels like fixing the test as well?

@syed
Copy link
Contributor Author

syed commented Mar 14, 2016

@remibergsma I beleive there is a marvin test for this but I beleive it was failing. I want to test this test but I could not find enough documentaiton on how to do this cleanly. All the more reason for a better CI

@syed
Copy link
Contributor Author

syed commented Mar 14, 2016

@remibergsma Going through the VPN tests on Marvin, I see that it is not actually testing if VPN is working. It just check if we can enable VPN without any errors but doesn't go to test if the enabled VPN is connectable.

@swill
Copy link
Contributor

swill commented Apr 8, 2016

@syed I have not looked into this. Is it possible for us to improve the test so it actually tests that the VPN works?

@syed
Copy link
Contributor Author

syed commented Apr 10, 2016

@swill I beleieve we can do a better test. After the VPN is set up we can do something like
ike-scan -M 172.16.21.200 to see if the VPN opened up. I'll test this and update

@swill
Copy link
Contributor

swill commented Apr 10, 2016

thank you sir. :)

@syed
Copy link
Contributor Author

syed commented Apr 18, 2016

@swill I have added a marvin test to see if the VPN service starts correctly however I am having trouble getting marvin to work correctly. When I run the test I get the follwing error

/usr/lib/python2.6/site-packages/nose/plugins/manager.py:395: RuntimeWarning: Unable to load plugin marvinPlugin = marvin.marvinPlugin:MarvinPlugin: 'module' object has no attribute 'case'

Basically marvin is failing to start. Have you had this problem? Also, in my script I use an external dependency ike-scan. If you are running this on the jenkins server, this binary has to be present for the test to pass and I could not find an easy way via the marvin test to inform of this dependency so I am assuming that the test framework assumes all the external dependencies to already be present.

@swill
Copy link
Contributor

swill commented Apr 18, 2016

Introducing an external dependency is probably going to cause some problems. @bhaisaab can you review this and maybe give some feedback on the best way to handle this?

@rohityadavcloud
Copy link
Member

rohityadavcloud commented Apr 18, 2016

@swill we'll need to publish a new systemvm template (i.e. we can no longer use the 4.6 systemvm template) if we have a new systemvm dependency; for the marvin test this requires that the host where you're running the test has ike-scan.

@swill
Copy link
Contributor

swill commented Apr 18, 2016

@bhaisaab Ok thanks. Are there other things we want to get into the systemvm template if we do publish a new one? Probably not the best place for this discussion, but maybe this is something we should discuss on dev@?

@rohityadavcloud
Copy link
Member

rohityadavcloud commented Apr 18, 2016

@swill we can ask around on ML, afaik in addition to strongswan (provided the strongswan PR gets reviewed and merged before 4.9 freeze) I known of the ospf stuff by @abhinandanprateek which needs zebra and related packages for ospf. Abhi is on leave this week, but we can discuss this next week. In general, we should evaluate and build a new systemvm template based on what new features end up in master until the freeze date in May (as you had shared on dev@).

@swill
Copy link
Contributor

swill commented Apr 18, 2016

Ok, thanks @bhaisaab. So I need to get on the systemvm template stuff asap to try to get those PRs in a good place so we have a little breathing room before the RC date to work out any kinks. Thanks for the help on this. :)

@syed
Copy link
Contributor Author

syed commented Apr 19, 2016

@bhaisaab Do you have an idea of why Marvin is failing on my dev setup? I've followed the guide mentioned here. I get the following error

/usr/lib/python2.6/site-packages/nose/plugins/manager.py:395: RuntimeWarning: Unable to load plugin marvinPlugin = marvin.marvinPlugin:MarvinPlugin: 'module' object has no attribute 'case'

Appriciate your help

@syed syed closed this Apr 19, 2016
@rohityadavcloud
Copy link
Member

@syed looks like marvin plugin is missing; if you've both python 2.6/2.7 (looks like nose is using python 2.6) make sure to install it for the one you use (the default python). Alternatively, try to install using pip with --upgrade; for example, after building cloudstack run: sudo pip install --upgrade tools/marvin/dist/.tar.gz
See if this helps.

@syed syed reopened this Apr 20, 2016
@rohityadavcloud
Copy link
Member

LGTM
@syed can you do a rebase and push -f again thanks

tag:mergeready

rohityadavcloud added a commit to shapeblue/cloudstack that referenced this pull request May 5, 2016
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@syed
Copy link
Contributor Author

syed commented May 9, 2016

@swill Sqashed. I believe this is good to go.

@swill
Copy link
Contributor

swill commented May 9, 2016

Thanks @syed. I will get CI run against this. I see we have the required code review, so I just need to get CI run. Thx...

@syed syed closed this May 16, 2016
@syed syed reopened this May 16, 2016
@syed syed closed this May 18, 2016
@syed syed reopened this May 18, 2016
@syed syed closed this May 18, 2016
@syed syed reopened this May 18, 2016
@syed syed closed this May 18, 2016
@syed syed reopened this May 18, 2016
@swill
Copy link
Contributor

swill commented May 20, 2016

@syed please close and reopen or repush. we are close to freeze and I want to get some of these PRs in...

@swill
Copy link
Contributor

swill commented May 22, 2016

@syed can you provide details for how to add ike-scan in order to run the test and verify via CI? Not sure how I feel about adding a test that can't be run without modification to the setup. Is it the management server that needs ike-scan? If so, that is easier to handle...

Also, can you repush this or close and reopen?

@syed syed closed this May 24, 2016
@syed syed reopened this May 24, 2016
@swill
Copy link
Contributor

swill commented May 25, 2016

CI RESULTS

Tests Run: 85
  Skipped: 0
   Failed: 0
   Errors: 1
 Duration: 4h 14m 49s

Summary of the problem(s):

ERROR: Failure: SyntaxError (invalid syntax (test_vpn_service.py, line 209))
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/case.py", line 132, in run
    self.beforeTest(result)
  File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/case.py", line 74, in beforeTest
    beforeTest(self.test)
  File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/proxy.py", line 117, in beforeTest
    self.plugins.beforeTest(self.test)
  File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/plugins/manager.py", line 99, in __call__
    return self.call(*arg, **kw)
  File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/plugins/manager.py", line 167, in simple
    result = meth(*arg, **kw)
  File "/usr/lib/python2.7/site-packages/marvin/marvinPlugin.py", line 174, in beforeTest
    join([self.__identifier, self.__testName])
TypeError: sequence item 0: expected string, NoneType found
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/LW938D/results.txt

Associated Uploads

/tmp/MarvinLogs/DeployDataCenter__May_24_2016_19_30_24_G1GYB2:

/tmp/MarvinLogs/LW938D:

/tmp/MarvinLogs/test_network_9YH1EQ:

/tmp/MarvinLogs/test_vpc_routers_0J3G7J:

Uploads will be available until 2016-07-25 02:00:00 +0200 CEST

Comment created by upr comment.

@swill
Copy link
Contributor

swill commented May 25, 2016

@syed can you review this error? It seems to be related to your test. Thanks...

@syed syed closed this May 26, 2016
@syed syed reopened this May 26, 2016
@swill
Copy link
Contributor

swill commented May 27, 2016

CI RESULTS

Tests Run: 85
  Skipped: 0
   Failed: 0
   Errors: 0
 Duration: 4h 16m 13s

Associated Uploads

/tmp/MarvinLogs/DeployDataCenter__May_26_2016_22_16_14_FKVNL2:

/tmp/MarvinLogs/test_network_00QSMN:

/tmp/MarvinLogs/test_vpc_routers_8J7VCM:

Uploads will be available until 2016-07-27 02:00:00 +0200 CEST

Comment created by upr comment.

@swill
Copy link
Contributor

swill commented May 27, 2016

This one is ready...

@asfgit asfgit merged commit 2622776 into apache:master May 27, 2016
asfgit pushed a commit that referenced this pull request May 27, 2016
[CLOUDSTACK-9296] Start ipsec for client VPNThis fix starts the IPSEC daemon when enabling client side vpn

* pr/1423:
  [CLOUDSTACK-9296] Start ipsec for client VPN

Signed-off-by: Will Stevens <williamstevens@gmail.com>
Slair1 added a commit to Slair1/cloudstack that referenced this pull request Feb 24, 2017
After a vRouter is recreated (which happens when a reboot via CloudStack
UI for example) and Remote Access VPN enabled, VPN won't work anymore.
Here is the abbreviated output of "ipsec auto -status" while we were
having the issue:
root@r-10-VM:~# ipsec auto --status
000 using kernel interface: netkey
000 interface lo/lo 127.0.0.1
000 interface lo/lo 127.0.0.1
000 interface eth0/eth0 169.254.1.45
000 interface eth0/eth0 169.254.1.45
000 %myid = (none)
Notice that only eth0 is shown, not the public interface eth1. Because
of that ipsec is broken.
However, if we manually stopped and started ipsec, then issued a "ipsec
auto -status", the abbreviated output would be:
root@r-10-VM:~# ipsec auto --status
000 using kernel interface: netkey
000 interface lo/lo 127.0.0.1
000 interface lo/lo 127.0.0.1
000 interface eth0/eth0 169.254.1.45
000 interface eth0/eth0 169.254.1.45
000 interface eth1/eth1 xxx.xxx.xxx.172
000 interface eth1/eth1 xxx.xxx.xxx.172
000 interface eth2/eth2 192.168.1.1
000 interface eth2/eth2 192.168.1.1
000 %myid = (none)
eth1 interface IP is masked, but now ipsec sees all the interfaces and
VPN works.
Looks like this bug was introduced by Pull Request apache#1423
apache#1423
It added code to start ipsec
(cloudstack/systemvm/patches/debian/config/opt/cloud/bin/configure.py)
if vpnconfig['create']:
logging.debug("Enabling remote access vpn on "+ public_ip)
CsHelper.start_if_stopped("ipsec")
self.configure_l2tpIpsec(public_ip, self.dbag[public_ip])
The issue is that if a reboot is issued from the CloudStack UI (as
opposed to manually by logging into the vRouter), the NICS (except eth0)
are not added to the VM until the cloud service is running.
Since ipsec was started before the nics were added to the VM and before
the public IP address is added to the nic, ipsec is not listening on the
public IP address and all VPNs are broken.
This is not a problem with the Site2Site VPN section of configure.py,
because that section does not start ipsec if the public IP is not on the
system yet...
Slair1 added a commit to Slair1/cloudstack that referenced this pull request Feb 24, 2017
After a vRouter is recreated (which happens when a reboot via CloudStack
UI for example) and Remote Access VPN enabled, VPN won't work anymore.
Here is the abbreviated output of "ipsec auto -status" while we were
having the issue:
root@r-10-VM:~# ipsec auto --status
000 using kernel interface: netkey
000 interface lo/lo 127.0.0.1
000 interface lo/lo 127.0.0.1
000 interface eth0/eth0 169.254.1.45
000 interface eth0/eth0 169.254.1.45
000 %myid = (none)
Notice that only eth0 is shown, not the public interface eth1. Because
of that ipsec is broken.
However, if we manually stopped and started ipsec, then issued a "ipsec
auto -status", the abbreviated output would be:
root@r-10-VM:~# ipsec auto --status
000 using kernel interface: netkey
000 interface lo/lo 127.0.0.1
000 interface lo/lo 127.0.0.1
000 interface eth0/eth0 169.254.1.45
000 interface eth0/eth0 169.254.1.45
000 interface eth1/eth1 xxx.xxx.xxx.172
000 interface eth1/eth1 xxx.xxx.xxx.172
000 interface eth2/eth2 192.168.1.1
000 interface eth2/eth2 192.168.1.1
000 %myid = (none)
eth1 interface IP is masked, but now ipsec sees all the interfaces and
VPN works.
Looks like this bug was introduced by Pull Request apache#1423
apache#1423
It added code to start ipsec
(cloudstack/systemvm/patches/debian/config/opt/cloud/bin/configure.py)
if vpnconfig['create']:
logging.debug("Enabling remote access vpn on "+ public_ip)
CsHelper.start_if_stopped("ipsec")
self.configure_l2tpIpsec(public_ip, self.dbag[public_ip])
The issue is that if a reboot is issued from the CloudStack UI (as
opposed to manually by logging into the vRouter), the NICS (except eth0)
are not added to the VM until the cloud service is running.
Since ipsec was started before the nics were added to the VM and before
the public IP address is added to the nic, ipsec is not listening on the
public IP address and all VPNs are broken.
This is not a problem with the Site2Site VPN section of configure.py,
because that section does not start ipsec if the public IP is not on the
system yet...
rohityadavcloud pushed a commit that referenced this pull request Aug 4, 2017
…eate (#1966)

This makes sure IP address is active.

After a vRouter is recreated (e.g. reboot via CloudStack UI) and Remote Access VPN enabled, VPN won't work anymore. Here is the abbreviated output of "ipsec auto -status" while we were having the issue:

root@r-10-VM:~# ipsec auto --status
000 using kernel interface: netkey
000 interface lo/lo 127.0.0.1
000 interface lo/lo 127.0.0.1
000 interface eth0/eth0 169.254.1.45
000 interface eth0/eth0 169.254.1.45
000 %myid = (none)
After this commit, the following occurs and VPNs work:


root@r-10-VM:~# ipsec auto --status
000 using kernel interface: netkey
000 interface lo/lo 127.0.0.1
000 interface lo/lo 127.0.0.1
000 interface eth0/eth0 169.254.1.45
000 interface eth0/eth0 169.254.1.45
000 interface eth1/eth1 xxx.xxx.xxx.172
000 interface eth1/eth1 xxx.xxx.xxx.172
000 interface eth2/eth2 192.168.1.1
000 interface eth2/eth2 192.168.1.1
000 %myid = (none)

eth1 interface IP is masked, but now ipsec sees all the interfaces and VPN works.

Looks like this bug was introduced by Pull Request #1423

It added code to start ipsec (cloudstack/systemvm/patches/debian/config/opt/cloud/bin/configure.py)

if vpnconfig['create']:
    logging.debug("Enabling remote access vpn on "+ public_ip)
    CsHelper.start_if_stopped("ipsec")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants