From 0acd3c12a2c40d4ccf89e4d531cf8de7c9d69d2a Mon Sep 17 00:00:00 2001 From: "Sverrir A. Berg" Date: Tue, 3 May 2016 15:17:59 +0000 Subject: [PATCH 1/3] Convert patchviasocket to python (removes perl dependency for KVM agent) As requested here: https://github.com/apache/cloudstack/pull/1495 No scripts are using perl so that install requirement can be removed. The new scripts are using standard python packages only. Includes extensive unit test. --- .../resource/LibvirtComputingResource.java | 4 +- scripts/installer/windows/client.wxs | 2 +- scripts/vm/hypervisor/kvm/patchviasocket.pl | 58 ------- scripts/vm/hypervisor/kvm/patchviasocket.py | 80 ++++++++++ .../vm/hypervisor/kvm/test_patchviasocket.py | 142 ++++++++++++++++++ 5 files changed, 225 insertions(+), 61 deletions(-) delete mode 100755 scripts/vm/hypervisor/kvm/patchviasocket.pl create mode 100755 scripts/vm/hypervisor/kvm/patchviasocket.py create mode 100755 scripts/vm/hypervisor/kvm/test_patchviasocket.py diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 17b3fd586ab8..eea16ae9531a 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -633,9 +633,9 @@ public boolean configure(final String name, final Map params) th throw new ConfigurationException("Unable to find versions.sh"); } - _patchViaSocketPath = Script.findScript(kvmScriptsDir + "/patch/", "patchviasocket.pl"); + _patchViaSocketPath = Script.findScript(kvmScriptsDir + "/patch/", "patchviasocket.py"); if (_patchViaSocketPath == null) { - throw new ConfigurationException("Unable to find patchviasocket.pl"); + throw new ConfigurationException("Unable to find patchviasocket.py"); } _heartBeatPath = Script.findScript(kvmScriptsDir, "kvmheartbeat.sh"); diff --git a/scripts/installer/windows/client.wxs b/scripts/installer/windows/client.wxs index 414d81347599..f5aec48bde49 100644 --- a/scripts/installer/windows/client.wxs +++ b/scripts/installer/windows/client.wxs @@ -1055,7 +1055,7 @@ - + diff --git a/scripts/vm/hypervisor/kvm/patchviasocket.pl b/scripts/vm/hypervisor/kvm/patchviasocket.pl deleted file mode 100755 index 7bcd245bc382..000000000000 --- a/scripts/vm/hypervisor/kvm/patchviasocket.pl +++ /dev/null @@ -1,58 +0,0 @@ -#!/usr/bin/perl -w -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -############################################################# -# This script connects to the system vm socket and writes the -# authorized_keys and cmdline data to it. The system VM then -# reads it from /dev/vport0p1 in cloud_early_config -############################################################# - -use strict; -use Getopt::Std; -use IO::Socket; -$|=1; - -my $opts = {}; -getopt('pn',$opts); -my $name = $opts->{n}; -my $cmdline = $opts->{p}; -my $sockfile = "/var/lib/libvirt/qemu/$name.agent"; -my $pubkeyfile = "/root/.ssh/id_rsa.pub.cloud"; - -if (! -S $sockfile) { - print "ERROR: $sockfile socket not found\n"; - exit 1; -} - -if (! -f $pubkeyfile) { - print "ERROR: ssh public key not found on host at $pubkeyfile\n"; - exit 1; -} - -open(FILE,$pubkeyfile) or die "ERROR: unable to open $pubkeyfile - $^E"; -my $key = ; -close FILE; - -$cmdline =~ s/%/ /g; -my $msg = "pubkey:" . $key . "\ncmdline:" . $cmdline; - -my $socket = IO::Socket::UNIX->new(Peer=>$sockfile,Type=>SOCK_STREAM) - or die "ERROR: unable to connect to $sockfile - $^E\n"; -print $socket "$msg\n"; -close $socket; - diff --git a/scripts/vm/hypervisor/kvm/patchviasocket.py b/scripts/vm/hypervisor/kvm/patchviasocket.py new file mode 100755 index 000000000000..d9616c9e8b9e --- /dev/null +++ b/scripts/vm/hypervisor/kvm/patchviasocket.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# +# This script connects to the system vm socket and writes the +# authorized_keys and cmdline data to it. The system VM then +# reads it from /dev/vport0p1 in cloud_early_config +# + +import argparse +import os +import socket + +SOCK_FILE = "/var/lib/libvirt/qemu/{name}.agent" +PUB_KEY_FILE = "/root/.ssh/id_rsa.pub.cloud" +MESSAGE = "pubkey:{key}\ncmdline:{cmdline}\n" + + +def read_pub_key(key_file): + try: + if os.path.isfile(key_file): + with open(key_file, "r") as f: + return f.read() + except IOError: + return None + + +def send_to_socket(sock_file, key_file, cmdline): + pub_key = read_pub_key(key_file) + + if not pub_key: + print("ERROR: ssh public key not found on host at {0}".format(key_file)) + return 1 + + # Keep old substitution from perl code: + cmdline = cmdline.replace("%", " ") + + msg = MESSAGE.format(key=pub_key, cmdline=cmdline) + + if not os.path.exists(sock_file): + print("ERROR: {0} socket not found".format(sock_file)) + return 1 + + try: + s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + s.connect(sock_file) + s.sendall(msg) + s.close() + except IOError as e: + print("ERROR: unable to connect to {0} - {1}".format(sock_file, e.strerror)) + return 1 + + return 0 # Success + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description="Send configuration to system VM socket") + parser.add_argument("-n", "--name", required=True, help="Name of VM") + parser.add_argument("-p", "--cmdline", required=True, help="Command line") + + arguments = parser.parse_args() + + socket_file = SOCK_FILE.format(name=arguments.name) + + exit(send_to_socket(socket_file, PUB_KEY_FILE, arguments.cmdline)) diff --git a/scripts/vm/hypervisor/kvm/test_patchviasocket.py b/scripts/vm/hypervisor/kvm/test_patchviasocket.py new file mode 100755 index 000000000000..074b159a7a64 --- /dev/null +++ b/scripts/vm/hypervisor/kvm/test_patchviasocket.py @@ -0,0 +1,142 @@ +#!/usr/bin/env python +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import patchviasocket + +import getpass +import os +import socket +import tempfile +import time +import threading +import unittest + +KEY_DATA = "I luv\nCloudStack\n" +CMD_DATA = "/run/this-for-me --please=TRUE! very%quickly" +NON_EXISTING_FILE = "must-not-exist" + + +def write_key_file(): + tmpfile = tempfile.mktemp(".sck") + with open(tmpfile, "w") as f: + f.write(KEY_DATA) + return tmpfile + + +class SocketThread(threading.Thread): + def __init__(self): + super(SocketThread, self).__init__() + self._data = "" + self._file = tempfile.mktemp(".sck") + self._ready = False + + def data(self): + return self._data + + def file(self): + return self._file + + def wait_until_ready(self): + while not self._ready: + time.sleep(0.050) + + def run(self): + TIMEOUT = 0.314 # Very short time for tests that don't write to socket. + MAX_SIZE = 10 * 1024 + + s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + s.bind(self._file) + s.listen(1) + s.settimeout(TIMEOUT) + try: + self._ready = True + client, address = s.accept() + self._data = client.recv(MAX_SIZE) + client.close() + except socket.timeout: + pass + s.close() + os.remove(self._file) + + +class TestPatchViaSocket(unittest.TestCase): + def setUp(self): + self._key_file = write_key_file() + + self._unreadable = write_key_file() + os.chmod(self._unreadable, 0) + + self.assertFalse(os.path.exists(NON_EXISTING_FILE)) + self.assertNotEqual("root", getpass.getuser(), "must be non-root user (to test access denied errors)") + + def tearDown(self): + os.remove(self._key_file) + os.remove(self._unreadable) + + def test_read_file(self): + pub_key = patchviasocket.read_pub_key(self._key_file) + self.assertEqual(KEY_DATA, pub_key) + + def test_read_file_error(self): + self.assertIsNone(patchviasocket.read_pub_key(NON_EXISTING_FILE)) + self.assertIsNone(patchviasocket.read_pub_key(self._unreadable)) + self.assertIsNone(patchviasocket.read_pub_key("/tmp")) # folder is not a file + + def test_write_to_socket(self): + reader = SocketThread() + reader.start() + reader.wait_until_ready() + self.assertEquals(0, patchviasocket.send_to_socket(reader.file(), self._key_file, CMD_DATA)) + reader.join() + data = reader.data() + self.assertIn(KEY_DATA, data) + self.assertIn(CMD_DATA.replace("%", " "), data) + self.assertNotIn("LUV", data) + self.assertNotIn("very%quickly", data) # Testing substitution + + def test_host_key_error(self): + reader = SocketThread() + reader.start() + reader.wait_until_ready() + self.assertEquals(1, patchviasocket.send_to_socket(reader.file(), NON_EXISTING_FILE, CMD_DATA)) + reader.join() # timeout + + def test_nonexistant_socket_error(self): + reader = SocketThread() + reader.start() + reader.wait_until_ready() + self.assertEquals(1, patchviasocket.send_to_socket(NON_EXISTING_FILE, self._key_file, CMD_DATA)) + reader.join() # timeout + + def test_invalid_socket_error(self): + reader = SocketThread() + reader.start() + reader.wait_until_ready() + self.assertEquals(1, patchviasocket.send_to_socket(self._key_file, self._key_file, CMD_DATA)) + reader.join() # timeout + + def test_access_denied_socket_error(self): + reader = SocketThread() + reader.start() + reader.wait_until_ready() + self.assertEquals(1, patchviasocket.send_to_socket(self._unreadable, self._key_file, CMD_DATA)) + reader.join() # timeout + + +if __name__ == '__main__': + unittest.main() From 751d3552dc3e3514c51fb9038ab91a625470212f Mon Sep 17 00:00:00 2001 From: Sverrir Berg Date: Tue, 17 May 2016 15:06:35 +0000 Subject: [PATCH 2/3] patchviasocket improve error handling more detailed error if host file not found or cannot be opened using mkstemp and mkdtemp for improved security improve resource cleanup in error conditions in unit test --- scripts/vm/hypervisor/kvm/patchviasocket.py | 20 ++++---- .../vm/hypervisor/kvm/test_patchviasocket.py | 46 ++++++++++--------- 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/scripts/vm/hypervisor/kvm/patchviasocket.py b/scripts/vm/hypervisor/kvm/patchviasocket.py index d9616c9e8b9e..c971d5dcc581 100755 --- a/scripts/vm/hypervisor/kvm/patchviasocket.py +++ b/scripts/vm/hypervisor/kvm/patchviasocket.py @@ -31,22 +31,18 @@ MESSAGE = "pubkey:{key}\ncmdline:{cmdline}\n" -def read_pub_key(key_file): - try: - if os.path.isfile(key_file): - with open(key_file, "r") as f: - return f.read() - except IOError: - return None - - def send_to_socket(sock_file, key_file, cmdline): - pub_key = read_pub_key(key_file) - - if not pub_key: + if not os.path.exists(key_file): print("ERROR: ssh public key not found on host at {0}".format(key_file)) return 1 + try: + with open(key_file, "r") as f: + pub_key = f.read() + except IOError as e: + print("ERROR: unable to open {0} - {1}".format(key_file, e.strerror)) + return 1 + # Keep old substitution from perl code: cmdline = cmdline.replace("%", " ") diff --git a/scripts/vm/hypervisor/kvm/test_patchviasocket.py b/scripts/vm/hypervisor/kvm/test_patchviasocket.py index 074b159a7a64..6b411d322467 100755 --- a/scripts/vm/hypervisor/kvm/test_patchviasocket.py +++ b/scripts/vm/hypervisor/kvm/test_patchviasocket.py @@ -32,7 +32,7 @@ def write_key_file(): - tmpfile = tempfile.mktemp(".sck") + _, tmpfile = tempfile.mkstemp(".sck") with open(tmpfile, "w") as f: f.write(KEY_DATA) return tmpfile @@ -42,7 +42,8 @@ class SocketThread(threading.Thread): def __init__(self): super(SocketThread, self).__init__() self._data = "" - self._file = tempfile.mktemp(".sck") + self._folder = tempfile.mkdtemp(".sck") + self._file = os.path.join(self._folder, "socket") self._ready = False def data(self): @@ -60,18 +61,21 @@ def run(self): MAX_SIZE = 10 * 1024 s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - s.bind(self._file) - s.listen(1) - s.settimeout(TIMEOUT) try: - self._ready = True - client, address = s.accept() - self._data = client.recv(MAX_SIZE) - client.close() - except socket.timeout: - pass - s.close() - os.remove(self._file) + s.bind(self._file) + s.listen(1) + s.settimeout(TIMEOUT) + try: + self._ready = True + client, address = s.accept() + self._data = client.recv(MAX_SIZE) + client.close() + except socket.timeout: + pass + finally: + s.close() + os.remove(self._file) + os.rmdir(self._folder) class TestPatchViaSocket(unittest.TestCase): @@ -88,15 +92,6 @@ def tearDown(self): os.remove(self._key_file) os.remove(self._unreadable) - def test_read_file(self): - pub_key = patchviasocket.read_pub_key(self._key_file) - self.assertEqual(KEY_DATA, pub_key) - - def test_read_file_error(self): - self.assertIsNone(patchviasocket.read_pub_key(NON_EXISTING_FILE)) - self.assertIsNone(patchviasocket.read_pub_key(self._unreadable)) - self.assertIsNone(patchviasocket.read_pub_key("/tmp")) # folder is not a file - def test_write_to_socket(self): reader = SocketThread() reader.start() @@ -116,6 +111,13 @@ def test_host_key_error(self): self.assertEquals(1, patchviasocket.send_to_socket(reader.file(), NON_EXISTING_FILE, CMD_DATA)) reader.join() # timeout + def test_host_key_access_denied(self): + reader = SocketThread() + reader.start() + reader.wait_until_ready() + self.assertEquals(1, patchviasocket.send_to_socket(reader.file(), self._unreadable, CMD_DATA)) + reader.join() # timeout + def test_nonexistant_socket_error(self): reader = SocketThread() reader.start() From 15da0c2b33bd52eff60d96fdd42f49d61481b5aa Mon Sep 17 00:00:00 2001 From: Sverrir Berg Date: Fri, 20 May 2016 15:43:55 +0000 Subject: [PATCH 3/3] Revert "Add perl-modules as install dependency for cloudstack-agent" perl-modules are no longer required. See: https://github.com/apache/cloudstack/pull/1533 This reverts commit 64b72a5c5a410f41bd869cc9d40807d888e05055. --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 728f6ff530e6..6452c03c5d73 100644 --- a/debian/control +++ b/debian/control @@ -22,7 +22,7 @@ Description: CloudStack server library Package: cloudstack-agent Architecture: all -Depends: ${misc:Depends}, ${python:Depends}, openjdk-8-jre-headless | openjdk-7-jre-headless, cloudstack-common (= ${source:Version}), lsb-base (>= 4.0), libcommons-daemon-java, openssh-client, qemu-kvm (>= 1.0), libvirt-bin (>= 0.9.8), uuid-runtime, iproute, ebtables, vlan, jsvc, ipset, python-libvirt, ethtool, iptables, perl-modules +Depends: ${misc:Depends}, ${python:Depends}, openjdk-8-jre-headless | openjdk-7-jre-headless, cloudstack-common (= ${source:Version}), lsb-base (>= 4.0), libcommons-daemon-java, openssh-client, qemu-kvm (>= 1.0), libvirt-bin (>= 0.9.8), uuid-runtime, iproute, ebtables, vlan, jsvc, ipset, python-libvirt, ethtool, iptables Conflicts: cloud-agent, cloud-agent-libs, cloud-agent-deps, cloud-agent-scripts Description: CloudStack agent The CloudStack agent is in charge of managing shared computing resources in