Comment 11 for bug 1482765

Revision history for this message
Seth Arnold (seth-arnold) wrote :

I reviewed neutron-vpnaas version 2:8.0.0~rc1-0ubuntu1 as checked into
xenial. This shouldn't be considered a full security audit but rather a
quick gauge of maintainability.

I had to enable universe for this build:

 builddeps:neutron-vpnaas : Depends: dh-systemd but it is not installable
                            Depends: python-hacking but it is not going to be installed

- neutron-vpnaas knows how to configure network namespaces and several
  ipsec daemons to provide VPNs as a servive to openstack
- Build-Depends: debhelper, dh-python, dh-systemd, openstack-pkg-tools,
  python-all, python-pbr, python-setuptools, python-sphinx
- Build-Depends-Indep: alembic, python-coverage, python-fixtures,
  python-hacking, python-jinja2, python-mock, python-netaddr,
  python-neutron, python-neutron-lib, python-oslo.concurrency,
  python-oslo.config, python-oslo.db, python-oslo.log,
  python-oslo.messaging, python-oslo.serialization, python-oslo.service,
  python-oslo.utils, python-oslosphinx, python-oslotest, python-requests,
  python-requests-mock, python-six, python-sqlalchemy,
  python-testresources, python-testscenarios, python-testtools,
  python-webob, python-webtest, subunit, testrepository,

- neutron-vpn-agent starts via initscripts, though I lose the flow of
  execution nearly immediately -- no idea if it daemonizes correctly, but
  there's no fork() or setsid() calls in the codebase
- pre/post inst/rm scripts automatically generated
- No dbus services
- No setuid
- /usr/bin/neutron-vpn-netns-wrapper and /usr/bin/neutron-vpn-agent
  binaries
- No sudo fragments
- No udev rules
- Decent-sized tests run during build but I assume they're mostly mocked
- No cron jobs

- Dependency injection makes it difficult to trace filesystem writes
- Some adapter classes know how to configure:
  - cisco csr client
  - cisco ipsec
  - fedora strongswan
  - ipsec
  - libreswan ipsec
  - strongswan ipsec
  - vyatta ipsec

- Only environment variable referenced was HUDSON_PUBLISH_DOCS in doc/
- There is a racy chmod() call in vpn_utils.py that is likely exploitable
  if the machine in question has untrusted users
- Extensive configuration of cryptography but none implemented here
- I did not investigate the ipsec configurations for sanity or safety; I
  did see that many were configured to use sha1 authentication. sha1 is
  being phased out nearly everywhere but its use as online integrity
  checks for tcp/ip streams may still be acceptable.
- Outgoing networking initiated via paramiko, may be more
- temp file handling looks poor, /tmp is used with what looks to be
  often-predictable names, sometimes files aren't cleaned up
- No webkit
- No policykit

- We will consider rootwrap is an _advisory_ service only, much like php's
  safe_open. Any issues with rootwrap will probably be treated as "low" or
  lower in our workflow.
- write_key_to_compute_node() doesn't check write_key_to_local_path()
  error return
- write_key_to_compute_node() doesn't clean up local_path
- write_key_to_local_path() race condition, creates local_key_file and then
  sets permissions on the file -- an attacker could open the file for
  reading between the two system calls and then read the private key after
  it has been written to the file. Are these machines dedicated to this
  task?
- VpnBase setup() has a gross temporary file handling hack:
  self.local_key_files = ['/tmp/' + x for x in self.remote_key_files]
  This may be exploitable if the remote_key_files can have '../etc/' in
  the pathnames. I did not find sanity checks to prevent this.

- vpn_utils.py extensive use of unquoted string-based shell executions;
  this is very difficult to audit. Probably whoever has privileges to
  modify SDN functionality should be considered to have full control over
  the entire openstack environment. Examples:

  - cmd = "sudo ip netns exec {} ip a".format(namespace)
  - interfaces = execute_cmd_over_ssh(controller, cmd, private_key)

  - cmd = "sudo ip netns exec {} ping -w {} -c {} {}".format(
        namespace, 2 * count, count, router_gw_ip)
    return ping(controller, cmd, private_key)

  - for key, ns_comp in zip(remote_key_files, ns_compute_tuples):
       cmd = "sudo rm -f {}".format(key)
       host = ns_comp[1]
       execute_cmd_over_ssh(host, cmd, private_key)

  - cmd = ("sudo ssh-keygen -f /root/.ssh/known_hosts -R"
           " {}".format(host))
    execute_cmd_over_ssh(compute_host, cmd, private_key)

  I filed https://bugs.launchpad.net/bugs/1570694 for this, OpenStack
  security team considers it a non-issue.

- The /etc/init.d/neutron-vpn-agent file looks corrupted, two #!/bin/sh
  lines, PATH is set twice.

Security team ACK for promoting neutron-vpnaas to main.

Thanks