On Fri, 3 Jul 2015 at 11:26 Petr Pudlak <[email protected]> wrote:
On Wed, Jul 01, 2015 at 03:02:00PM +0000, 'Helga Velroyen' via
ganeti-devel wrote:
>Hi,
>
>I forgot the tests, so please consider this interdiff:
>diff --git a/test/py/ganeti.ssh_unittest.py b/test/py/
ganeti.ssh_unittest.py
>index b520a97..a121d42 100755
>--- a/test/py/ganeti.ssh_unittest.py
>+++ b/test/py/ganeti.ssh_unittest.py
>@@ -147,6 +147,9 @@ class TestGetUserFiles(unittest.TestCase):
> constants.SSHK_DSA:
> (os.path.join(self.tmpdir, ".ssh", "id_dsa"),
> os.path.join(self.tmpdir, ".ssh", "id_dsa.pub")),
>+ constants.SSHK_ECDSA:
>+ (os.path.join(self.tmpdir, ".ssh", "id_ecdsa"),
>+ os.path.join(self.tmpdir, ".ssh", "id_ecdsa.pub")),
> }))
> self.assertEqual(os.listdir(self.tmpdir), [])
>
>diff --git a/test/py/ganeti.tools.prepare_node_join_unittest.py
b/test/py/
>ganeti.tools.prepare_node_join_unittest.py
>index e0c60a4..409b7bb 100755
>--- a/test/py/ganeti.tools.prepare_node_join_unittest.py
>+++ b/test/py/ganeti.tools.prepare_node_join_unittest.py
>@@ -143,6 +143,9 @@ class TestUpdateSshDaemon(unittest.TestCase):
> constants.SSHK_DSA:
> (utils.PathJoin(self.tmpdir, "dsa.private"),
> utils.PathJoin(self.tmpdir, "dsa.public")),
>+ constants.SSHK_ECDSA:
>+ (utils.PathJoin(self.tmpdir, "ecdsa.private"),
>+ utils.PathJoin(self.tmpdir, "ecdsa.public")),
> }
>
> def tearDown(self):
Would it make sense to also add some more tests to the file, like a new
method 'testDryRunEcdsa' and to extend _TestUpdate?
Indeed, good point. I extended the tests and pasted an additional interdiff
below.
Rest (with the interdiff) LGTM, thanks
PS: Some interesting reading about ECDSA:
http://security.stackexchange.com/a/46781/12485
Yeah, I found that too. I guess we should still support ECDSA as it is
default in some distros. :/
>
>
>
>On Wed, 1 Jul 2015 at 14:26 Helga Velroyen <[email protected]> wrote:
>
>> So far, Ganeti did only care about DSA and RSA host
>> keys. With the rising popularity of ECDSA, we should
>> support this key type as well, as it is already
>> enabled by default in many common distributions.
>>
>> This fixes Issue 1098.
>>
>> Signed-off-by: Helga Velroyen <[email protected]>
>> ---
>> lib/ssh.py | 2 ++
>> src/Ganeti/Constants.hs | 12 +++++++++++-
>> 2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/ssh.py b/lib/ssh.py
>> index 599b7cc..06003a5 100644
>> --- a/lib/ssh.py
>> +++ b/lib/ssh.py
>> @@ -79,6 +79,8 @@ def GetUserFiles(user, mkdir=False, dircheck=True,
>> kind=constants.SSHK_DSA,
>> suffix = "dsa"
>> elif kind == constants.SSHK_RSA:
>> suffix = "rsa"
>> + elif kind == constants.SSHK_ECDSA:
>> + suffix = "ecdsa"
>> else:
>> raise errors.ProgrammerError("Unknown SSH key kind '%s'" % kind)
>>
>> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
>> index 97093df..106e4d4 100644
>> --- a/src/Ganeti/Constants.hs
>> +++ b/src/Ganeti/Constants.hs
>> @@ -4399,11 +4399,14 @@ cryptoOptionSerialNo = "serial_no"
>> sshkDsa :: String
>> sshkDsa = "dsa"
>>
>> +sshkEcdsa :: String
>> +sshkEcdsa = "ecdsa"
>> +
>> sshkRsa :: String
>> sshkRsa = "rsa"
>>
>> sshkAll :: FrozenSet String
>> -sshkAll = ConstantUtils.mkSet [sshkRsa, sshkDsa]
>> +sshkAll = ConstantUtils.mkSet [sshkRsa, sshkDsa, sshkEcdsa]
>>
>> -- * SSH authorized key types
>>
>> @@ -4438,6 +4441,12 @@ sshHostDsaPriv = sshConfigDir ++
"/ssh_host_dsa_key"
>> sshHostDsaPub :: String
>> sshHostDsaPub = sshHostDsaPriv ++ ".pub"
>>
>> +sshHostEcdsaPriv :: String
>> +sshHostEcdsaPriv = sshConfigDir ++ "/ssh_host_ecdsa_key"
>> +
>> +sshHostEcdsaPub :: String
>> +sshHostEcdsaPub = sshHostEcdsaPriv ++ ".pub"
>> +
>> sshHostRsaPriv :: String
>> sshHostRsaPriv = sshConfigDir ++ "/ssh_host_rsa_key"
>>
>> @@ -4448,6 +4457,7 @@ sshDaemonKeyfiles :: Map String (String, String)
>> sshDaemonKeyfiles =
>> Map.fromList [ (sshkRsa, (sshHostRsaPriv, sshHostRsaPub))
>> , (sshkDsa, (sshHostDsaPriv, sshHostDsaPub))
>> + , (sshkEcdsa, (sshHostEcdsaPriv, sshHostEcdsaPub))
>> ]
>>
>> -- * Node daemon setup
>> --
>> 2.4.3.573.g4eafbef
>>
>>
The interdiff:
diff --git a/test/py/ganeti.tools.prepare_node_join_unittest.py b/test/py/
ganeti.tools.prepare_node_join_unittest.py
index 409b7bb..09f8750 100755
--- a/test/py/ganeti.tools.prepare_node_join_unittest.py
+++ b/test/py/ganeti.tools.prepare_node_join_unittest.py
@@ -183,6 +183,13 @@ class TestUpdateSshDaemon(unittest.TestCase):
],
})
+ def testDryRunEcdsa(self):
+ self._TestDryRun({
+ constants.SSHS_SSH_HOST_KEY: [
+ (constants.SSHK_ECDSA, "ecdsapriv", "ecdsapub"),
+ ],
+ })
+
def _RunCmd(self, fail, cmd, interactive=NotImplemented):
self.assertTrue(interactive)
self.assertEqual(cmd, [pathutils.DAEMON_UTIL, "reload-ssh-keys"])
@@ -198,6 +205,7 @@ class TestUpdateSshDaemon(unittest.TestCase):
data = {
constants.SSHS_SSH_HOST_KEY: [
(constants.SSHK_DSA, "dsapriv", "dsapub"),
+ (constants.SSHK_ECDSA, "ecdsapriv", "ecdsapub"),
(constants.SSHK_RSA, "rsapriv", "rsapub"),
],
}
@@ -212,6 +220,7 @@ class TestUpdateSshDaemon(unittest.TestCase):
self.assertEqual(sorted(os.listdir(self.tmpdir)), sorted([
"rsa.public", "rsa.private",
"dsa.public", "dsa.private",
+ "ecdsa.public", "ecdsa.private",
]))
self.assertEqual(utils.ReadFile(utils.PathJoin(self.tmpdir,
"rsa.public")),
"rsapub")
@@ -221,6 +230,10 @@ class TestUpdateSshDaemon(unittest.TestCase):
"dsapub")
self.assertEqual(utils.ReadFile(utils.PathJoin(self.tmpdir,
"dsa.private")),
"dsapriv")
+ self.assertEqual(utils.ReadFile(utils.PathJoin(
+ self.tmpdir, "ecdsa.public")), "ecdsapub")
+ self.assertEqual(utils.ReadFile(utils.PathJoin(
+ self.tmpdir, "ecdsa.private")), "ecdsapriv")
def testSuccess(self):
self._TestUpdate(False)
(END)
