Bug 1245842 - With openstack-neutron VPNaaS, pluto shutdown does not remove pluto.ctl/pluto.pid
Summary: With openstack-neutron VPNaaS, pluto shutdown does not remove pluto.ctl/pluto...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat OpenStack
Classification: Red Hat
Component: openstack-neutron-vpnaas
Version: 7.0 (Kilo)
Hardware: Unspecified
OS: Unspecified
medium
unspecified
Target Milestone: z5
: 7.0 (Kilo)
Assignee: Brent Eagles
QA Contact: Eran Kuris
URL:
Whiteboard:
Depends On: 1268444
Blocks: 1077162 1266519
TreeView+ depends on / blocked
 
Reported: 2015-07-22 23:11 UTC by Terry Wilson
Modified: 2023-02-22 23:02 UTC (History)
14 users (show)

Fixed In Version: openstack-neutron-vpnaas-2015.1.2-2.el7ost
Doc Type: Bug Fix
Doc Text:
Prior to this update, the LibreSwan IPsec implementation enforced strict security controls on child processes, which prevented it from removing root owned files when connections were restarted or removed. In addition, when VPNaaS updated a connection it restarted the pluto daemon to ensure changes take affect. However, a check in the daemon prohibited the startup if the associated control files existed (to prevent a multiple access scenario). Since the files could not be cleaned up beforehand, the restart would fail. This update addresses this issue, with the VPNaaS agent now removing the control files before attempting to update the connection. As a result, the start checks on the daemon will now pass.
Clone Of:
: 1266519 (view as bug list)
Environment:
Last Closed: 2017-01-19 13:31:55 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Launchpad 1331502 0 None None None Never
OpenStack gerrit 212673 0 None None None Never

Description Terry Wilson 2015-07-22 23:11:00 UTC
When run from neutron's vpnaas module via:

sudo ip netns exec qrouter-3b1b8181-593c-4f55-a30b-df23518b8c46 /usr/libexec/ipsec/pluto --ctlbase /opt/stack/data/neutron/ipsec/3b1b8181-593c-4f55-a30b-df23518b8c46/var/run/pluto --ipsecdir /opt/stack/data/neutron/ipsec/3b1b8181-593c-4f55-a30b-df23518b8c46/etc --use-netkey --uniqueids --nat_traversal --secretsfile /opt/stack/data/neutron/ipsec/3b1b8181-593c-4f55-a30b-df23518b8c46/etc/ipsec.secrets --virtual_private %v4:10.2.0.0/24,%v4:10.1.0.0/24

when neutron later runs:

sudo ip netns exec qrouter-3b1b8181-593c-4f55-a30b-df23518b8c46 /usr/libexec/ipsec/whack --shutdown --ctlbase /opt/stack/data/neutron/ipsec/3b1b8181-593c-4f55-a30b-df23518b8c46/var/run/pluto

the pluto.ctl and pluto.pid files are not deleted.

With strace on the pluto process:

   unlink("/opt/stack/data/neutron/ipsec/3b1b8181-593c-4f55-a30b-df23518b8c46/var/run/pluto.ctl") = -1 EACCES (Permission denied)
   unlink("/opt/stack/data/neutron/ipsec/3b1b8181-593c-4f55-a30b-df23518b8c46/var/run/pluto.pid") = -1 EACCES (Permission denied)

pluto is running as root and the files are:
srwx------. 1 root  root     0 Jul 22 00:51 pluto.ctl
-r--r--r--. 1 root  root     6 Jul 22 00:51 pluto.pid


Version-Release number of selected component (if applicable):
3.13

How reproducible:
always

Comment 2 Paul Wouters 2015-07-28 08:47:14 UTC
My guess is that this happens because we dropped capabilities and the files are in a non-root owned directory?

Is it possible to let the scripts that create these directories to also delete these?

Comment 3 Terry Wilson 2015-07-28 16:40:33 UTC
Paul: Sort of, but that means giving the script that creates the directories sudo rights to using 'rm' which I'm not a big fan of. We could probably find a way to restrict those rights to a certain subdirectory as a workaround, but I'm sure there edge cases where maybe we don't know that pluto has exited and go to try to start it and it fails. We could probably work around that as well if we *had* to.

This is the second issue we've run into re: CAP_DAC_OVERRIDE. I'm still not sure I completely understand the benefit to dropping the privilege. pluto is running as root, and we drop the privilege to access directories that aren't owned by root...when large portions of the sensitive material that we wouldn't want people to be able to access through some exploit are...owned by root. If someone can manage to write arbitrary data to a file...but only if it and its directory are owned by root, it can still modify passwd, shadow, etc. and open up all kinds of other vulnerabilities, right? It just doesn't seem like dropping the capability adds real value. What am I missing?

Comment 4 Paul Wouters 2015-07-31 09:25:24 UTC
I don't think you need to be root to delete root owned from your own owned dir? can you make the directory owned by your user or group of the user with group write?

The idea is that if pluto runs as root, it cannot read or write non-root files, like /home/ data. That might not very appropriate for you, but in other deployments it might matter more.

Comment 5 Terry Wilson 2015-08-05 15:58:46 UTC
> I don't think you need to be root to delete root owned from your own owned dir? can you make the directory owned by your user or group of the user with group write?

True. So that takes care of the sudo issue. We should be able to check the pid file's existence, read the pid, check for the process with that pid, and delete the .ctl and .pid file if pluto isn't running with that pid.

> The idea is that if pluto runs as root, it cannot read or write non-root files, like /home/ data. That might not very appropriate for you, but in other deployments it might matter more.

That sounds like putting a lock on a door that you can just walk around to me. The number of ways to get complete access to the system (including files not owned by root) by being able to modify files owned by root seems like it would make whether or not you could overwrite non-root files inconsequential.

Comment 6 Paul Wouters 2015-08-07 10:03:07 UTC
well, we are dropping as much capabilities as root as we can, so that we are left mostly with the root powers for making network/crypto changes which is the job of libreswan/openswan. But other than that, we try to drop as much root power as we can.

Comment 7 Brent Eagles 2015-08-14 19:08:23 UTC
I understand the rationale behind restricting things for security's sake - particularly in something like VPN software. It does seem a little odd to create a situation where a bit of software isn't restartable "on it's own", but in my opinion the security implications are too risky to change this at this time - especially considering that it seems only to be impacting OpenStack at the moment. I'm looking into working around this in VPNaaS, so I'm going to move the BZ to the appropriate component and re-assign to myself.

Comment 10 Brent Eagles 2015-09-23 18:48:52 UTC
There currently isn't adequate test coverage to verify this in our functional/system level tests, so manual verification is necessary for now. I did this by configuring a devstack environment and running this script:

https://github.com/beagles/oddsnends/blob/master/openstack/vpnaas/test_vpn.sh

And checking the logs for errors. One important caveat is that the /etc/neutron/vpn_agent.ini file needs to have the vpn_device_driver set as follows:

vpn_device_driver=neutron_vpnaas.services.vpn.device_drivers.libreswan_ipsec.LibreSwanDriver

Which it should be already in our packaging - if not, bug. It should also be the only vpn_device_driver line specified.

The error string to look for is described in this BZ.

Comment 11 Ihar Hrachyshka 2015-09-25 13:04:48 UTC
Scott, how do we get rhos-7.0 flag set? We can't merge the fix without it.

Comment 13 Ihar Hrachyshka 2015-09-25 13:18:57 UTC
Yes, Scott, there is a patch. I will move the bug to POST to reflect that.

Note that we don't have OSP8 yet, so targeting the bug to it seems wrong. I would better change target of this bug instead of having a clone.

Comment 14 Scott Lewis 2015-09-25 13:32:33 UTC
(In reply to Ihar Hrachyshka from comment #13)
> Yes, Scott, there is a patch. I will move the bug to POST to reflect that.
> 
> Note that we don't have OSP8 yet, so targeting the bug to it seems wrong. I
> would better change target of this bug instead of having a clone.

Clone created for OSP-8, I changed this bug to OSP-7

Comment 15 Ihar Hrachyshka 2015-09-25 13:35:17 UTC
Scott, we still lack the needed flag for OSP7. Or should we expect the bot to set it now?

Comment 17 Eran Kuris 2015-12-09 09:32:56 UTC
blocked by Bug 1268444

Comment 19 Brent Eagles 2016-07-29 18:02:36 UTC
This moved from ON_QA to  ASSIGNED because of a failure in a dependency that has since been resolved. I've updated the "fixed in" to the current package and setting this back to modified.

Comment 20 Lon Hohberger 2016-08-05 17:39:27 UTC
According to our records, this should be resolved by openstack-neutron-vpnaas-2015.1.2-2.el7ost.  This build is available now.


Note You need to log in before you can comment on or make changes to this bug.