Merge pull request #300 from trhoden/RM-11694-admin-perms

Fix CVE-2015-4053 world-readable permissions on client.admin key

Reviewed-by: Dan Mick <dmick@redhat.com>
Reviewed-by: Sage Weil <sweil@redhat.com>

ceph_deploy/admin.py

@@ -27,7 +27,6 @@ def admin(args):
         LOG.debug('Pushing admin keys and conf to %s', hostname)
         try:
             distro = hosts.get(hostname, username=args.username)
-            hostname = distro.conn.remote_module.shortname()
 
             distro.conn.remote_module.write_conf(
                 args.cluster,
@@ -37,7 +36,8 @@ def admin(args):
 
             distro.conn.remote_module.write_file(
                 '/etc/ceph/%s.client.admin.keyring' % args.cluster,
-                keyring
+                keyring,
+                0600,
             )
 
             distro.conn.exit()
@@ -58,7 +58,7 @@ def make(parser):
     parser.add_argument(
         'client',
         metavar='HOST',
-        nargs='*',
+        nargs='+',
         help='host to configure for ceph administration',
         )
     parser.set_defaults(

ceph_deploy/hosts/remotes.py

@@ -200,8 +200,12 @@ def write_monitor_keyring(keyring, monitor_keyring):
     write_file(keyring, monitor_keyring)
 
 
-def write_file(path, content):
-    with file(path, 'w') as f:
+def write_file(path, content, mode=0644, directory=None):
+    if directory:
+        if path.startswith("/"):
+            path = path[1:]
+        path = os.path.join(directory, path)
+    with os.fdopen(os.open(path, os.O_WRONLY | os.O_CREAT, mode), 'w') as f:
         f.write(content)
 
 

ceph_deploy/tests/test_cli_admin.py

@@ -0,0 +1,82 @@
+import os
+import subprocess
+
+import pytest
+from mock import patch, MagicMock, Mock
+
+from ceph_deploy.cli import _main as main
+from ceph_deploy.hosts import remotes
+from ceph_deploy.tests.directory import directory
+
+def test_help(tmpdir, cli):
+    with cli(
+        args=['ceph-deploy', 'admin', '--help'],
+        stdout=subprocess.PIPE,
+        ) as p:
+        result = p.stdout.read()
+    assert 'usage: ceph-deploy admin' in result
+    assert 'positional arguments' in result
+    assert 'optional arguments' in result
+
+
+def test_bad_no_hosts(tmpdir, cli):
+    with pytest.raises(cli.Failed) as err:
+        with cli(
+            args=['ceph-deploy', 'admin'],
+            stderr=subprocess.PIPE,
+            ) as p:
+            result = p.stderr.read()
+    assert 'usage: ceph-deploy admin' in result
+    assert 'too few arguments' in result
+    assert err.value.status == 2
+
+
+def test_bad_no_conf(tmpdir, cli):
+    with pytest.raises(cli.Failed) as err:
+        with cli(
+            args=['ceph-deploy', 'admin', 'host1'],
+            stderr=subprocess.PIPE,
+            ) as p:
+            result = p.stderr.read()
+    assert 'No such file or directory: \'ceph.conf\'' in result
+    assert err.value.status == 1
+
+
+def test_bad_no_key(tmpdir, cli):
+    with tmpdir.join('ceph.conf').open('w'):
+        pass
+    with pytest.raises(cli.Failed) as err:
+        with cli(
+            args=['ceph-deploy', 'admin', 'host1'],
+            stderr=subprocess.PIPE,
+            ) as p:
+            result = p.stderr.read()
+    assert 'ceph.client.admin.keyring not found' in result
+    assert err.value.status == 1
+
+
+def test_write_keyring(tmpdir):
+    with tmpdir.join('ceph.conf').open('w'):
+        pass
+    with tmpdir.join('ceph.client.admin.keyring').open('w'):
+        pass
+
+    etc_ceph = os.path.join(str(tmpdir), 'etc', 'ceph')
+    os.makedirs(etc_ceph)
+
+    distro = MagicMock()
+    distro.conn = MagicMock()
+    remotes.write_file.func_defaults = (str(tmpdir),)
+    distro.conn.remote_module = remotes
+    distro.conn.remote_module.write_conf = Mock()
+
+    with patch('ceph_deploy.admin.hosts'):
+        with patch('ceph_deploy.admin.hosts.get', MagicMock(return_value=distro)):
+            with directory(str(tmpdir)):
+                main(args=['admin', 'host1'])
+
+    keyring_file = os.path.join(etc_ceph, 'ceph.client.admin.keyring')
+    assert os.path.exists(keyring_file)
+
+    file_mode = oct(os.stat(keyring_file).st_mode & 0777)
+    assert file_mode == oct(0600)