On 06/15/2012 07:36 AM, Martin Kosek wrote:
On Thu, 2012-06-14 at 16:35 -0400, Rob Crittenden wrote:
Ondrej Hamada wrote:
Improved options checking so that host-mod operation is not changing
password for enrolled host when '--random' option is used.

https://fedorahosted.org/freeipa/ticket/2799

Updated set of characters that is used for generating random passwords
for ipa hosts. Following characters were removed from the set: '"`\$<>

https://fedorahosted.org/freeipa/ticket/2800
This works ok but it would be nice to have a test for both setting a
password and random on an enrolled host to prevent regressions. We have
some ipa-getkeytab tests already and these can be extended to test this
I think.

Might be nice to mention in the inline comment the set of characters
excluded and why.

rob

I've added new test class into test_host_plugin.py that takes care of that. Just there is a problem that the ipa-join command always fails on 'adding key into keytab'. But the attributes necessary for testing are set correctly, so the testing can continue.
We already generate passwords for users with this character set:
user_pwdchars = string.digits + string.ascii_letters + '_,.@+-='

Why would we want to generate passwords for host enrolling with a
different set? Additionally, I think the set of characters you chose is
too wide, try entering a passwords with ' ', !, (, ), &, or ; without
careful escaping or quoting...

Martin

Ok, I've used the same set of characters as for the user passwords.

--
Regards,

Ondrej Hamada
FreeIPA team
jabber: oh...@jabbim.cz
IRC: ohamada

From bc19f44023643ff726e6e36634fbcbcbd0859583 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada <oham...@redhat.com>
Date: Mon, 18 Jun 2012 15:25:05 +0200
Subject: [PATCH] Change random passwords behaviour

Improved options checking so that host-mod operation is not changing
password for enrolled host when '--random' option is used.

Unit tests added.

https://fedorahosted.org/freeipa/ticket/2799

Updated set of characters that is used for generating random passwords
for ipa hosts. All characters that might need escaping were removed.

https://fedorahosted.org/freeipa/ticket/2800
---
 ipalib/plugins/host.py                |   11 ++++-
 tests/test_xmlrpc/test_host_plugin.py |   75 ++++++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 96b73cc5594335ad02dd43f87e7e011ab84157a1..9680d7c024ea8976f92a71bf576d6712c44a2bcf 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -24,6 +24,7 @@ import sys
 from nss.error import NSPRError
 import nss.nss as nss
 import netaddr
+import string
 
 from ipalib import api, errors, util
 from ipalib import Str, Flag, Bytes
@@ -99,6 +100,10 @@ EXAMPLES:
    ipa host-add-managedby --hosts=test2 test
 """)
 
+# Characters to be used by random password generator
+# The set was chosen to avoid the need for escaping the characters by user
+host_pwd_chars=string.digits + string.ascii_letters + '_,.@+-='
+
 def remove_fwd_ptr(ipaddr, host, domain, recordtype):
     api.log.debug('deleting ipaddr %s' % ipaddr)
     try:
@@ -404,7 +409,7 @@ class host_add(LDAPCreate):
             if 'krbprincipal' in entry_attrs['objectclass']:
                 entry_attrs['objectclass'].remove('krbprincipal')
         if options.get('random'):
-            entry_attrs['userpassword'] = ipa_generate_password()
+            entry_attrs['userpassword'] = ipa_generate_password(characters=host_pwd_chars)
             # save the password so it can be displayed in post_callback
             setattr(context, 'randompassword', entry_attrs['userpassword'])
         cert = options.get('usercertificate')
@@ -596,7 +601,7 @@ class host_mod(LDAPUpdate):
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         # Allow an existing OTP to be reset but don't allow a OTP to be
         # added to an enrolled host.
-        if 'userpassword' in options:
+        if options.get('userpassword') or options.get('random'):
             entry = {}
             self.obj.get_password_attributes(ldap, dn, entry)
             if not entry['has_password'] and entry['has_keytab']:
@@ -649,7 +654,7 @@ class host_mod(LDAPUpdate):
             entry_attrs['usercertificate'] = cert
 
         if options.get('random'):
-            entry_attrs['userpassword'] = ipa_generate_password()
+            entry_attrs['userpassword'] = ipa_generate_password(characters=host_pwd_chars)
             setattr(context, 'randompassword', entry_attrs['userpassword'])
         if 'macaddress' in entry_attrs:
             if 'objectclass' in entry_attrs:
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 8798168afa71653b64870c77d11a7fa81ec4c952..fa1f2906f556af388499eac316c4b7c05c66ad85 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -22,9 +22,13 @@
 Test the `ipalib.plugins.host` module.
 """
 
+import os
+import tempfile
+from ipapython import ipautil
 from ipalib import api, errors, x509
 from ipalib.dn import *
-from tests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_digits
+from tests.test_xmlrpc.xmlrpc_test import Declarative, XMLRPC_test
+from tests.test_xmlrpc.xmlrpc_test import fuzzy_uuid, fuzzy_digits
 from tests.test_xmlrpc.xmlrpc_test import fuzzy_hash, fuzzy_date, fuzzy_issuer
 from tests.test_xmlrpc.xmlrpc_test import fuzzy_hex
 from tests.test_xmlrpc import objectclasses
@@ -740,3 +744,72 @@ class test_host(Declarative):
         ),
 
     ]
+
+class test_host_false_pwd_change(XMLRPC_test):
+
+    fqdn1 = u'testhost1.%s' % api.env.domain
+    short1 = u'testhost1'
+    new_pass = u'pass_123'
+
+    command = "ipa-client/ipa-join"
+    [keytabfd, keytabname] = tempfile.mkstemp()
+    os.close(keytabfd)
+
+    # auxiliary function for checking whether the join operation has set
+    # correct attributes
+    def keytab_exists(self):
+        ret = api.Command['host_show'](self.fqdn1,all=True)
+        assert (ret['result']['has_keytab'] == True)
+        assert (ret['result']['has_password'] == False)
+
+    def test_a_join_host(self):
+        """
+        Create a test host and join him into IPA.
+        """
+        try:
+           random_pass = api.Command['host_add'](self.fqdn1, random=True, force=True)['result']['randompassword']
+        except:
+            # new host must be created with the random password
+            assert (False)
+
+        new_args = [self.command,
+                    "-s", api.env.host,
+                    "-h", self.fqdn1,
+                    "-k", self.keytabname,
+                    "-w", random_pass,
+                    "-q",
+                   ]
+        try:
+            # join operation may fail on 'adding key into keytab', but
+            # the keytab is not necessary for further tests
+            (out, err, rc) = ipautil.run(new_args, None)
+            self.keytab_exists()
+        except ipautil.CalledProcessError, e:
+            self.keytab_exists()
+
+    def test_b_try_password(self):
+        """
+        Try to change the password of enrolled host with specified password
+        """
+        try:
+            api.Command['host_mod'](self.fqdn1,userpassword=self.new_pass)
+            assert (False)
+        except errors.ValidationError:
+            pass
+
+    def test_c_try_random(self):
+        """
+        Try to change the password of enrolled host with random password
+        """
+        try:
+            api.Command['host_mod'](self.fqdn1,random=True)
+            assert (False)
+        except errors.ValidationError:
+            pass
+
+    def test_d_cleanup(self):
+        """
+        Clean up test data
+        """
+        os.unlink(self.keytabname)
+        api.Command['host_del'](self.fqdn1)
-- 
1.7.6.5

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to