On 07/22/2014 08:55 AM, Martin Kosek wrote:
On 07/21/2014 04:08 PM, David Kupka wrote:
On 07/18/2014 12:52 PM, Martin Kosek wrote:
On 07/18/2014 12:33 PM, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/2796


1) Would it be easier/more convenient to just implement following simple check
instead of bad_prefix/bad_suffix?

if password.strip() != password:
     raise ValueError('Password must not start or end with whitespace')


Yes it would. Edited patch attached.


2) The main goal of the ticket 2796 was not fixed yet. It sometimes happen that
when installation crashes somewhere right after pkicreate, it does not record
and and does not uninstall the PKI component during "ipa-server-install
--uninstall".

You may artificially invoke some crash in cainstance.py after pkicreate to test
it. When fixing it, check how is_configured() in Service object works an how
self.backup_state is called in other service modules (like dsinstance.py) where
the detection works correctly.

You're completely right, Martin. I was unable to reproduce the bug (to force
pkicreate/pkispawn to fail) so I thought that it was fixed by the password
restriction.
Then I discovered that most of the banned characters for password are no longer
causing troubles a focused on this. But it's yet another issue.

1) Whitespace error:

$ git am /tmp/freeipa-dkupka-0002-2-Improve-password-validity-check.patch
Applying: Improve password validity check.
/home/mkosek/freeipa/.git/rebase-apply/patch:25: trailing whitespace.
     # Disallow leading/trailing whaitespaces
warning: 1 line adds whitespace errors.

Git is highlighting these errors I was probably temporary blind.

2) The new admin validator is not applied to "-a" command line option and you
can pass any garbage to it. You need to replace this section:

     if options.admin_password is not None and len(options.admin_password) < 8:
         parser.error("Admin user password must be at least 8 characters long")

... with the new validator just like we validate DM password.
Added.


Martin


--
David Kupka
From c11f52766646a2ee597009bb14f15c3633c18591 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 24 Jul 2014 13:32:37 +0200
Subject: [PATCH] Improve password validity check.

Allow use of characters that no longer cause troubles. Check for
leading and trailing characters in case of 389 Direcory Manager password.
---
 install/tools/ipa-server-install | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 671a226d625ab9e8168c569a6d83c35dfae52115..fc9cef06076110a969a3db6640f0288e3de2ce45 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -121,7 +121,31 @@ def validate_dm_password(password):
         raise ValueError("Password must only contain ASCII characters")
 
     # Disallow characters that pkisilent doesn't process properly:
-    bad_characters = ' &\\<%'
+    bad_characters = '\\'
+    if any(c in bad_characters for c in password):
+        raise ValueError('Password must not contain these characters: %s' %
+            ', '.join('"%s"' % c for c in bad_characters))
+
+    # TODO: Check https://fedorahosted.org/389/ticket/47849
+    # Actual behavior of setup-ds.pl is that it does not accept white
+    # space characters in password when called interactively but does when
+    # provided such password in INF file. But it ignores leading and trailing
+    # white spaces in INF file.
+
+    # Disallow leading/trailing whaitespaces
+    if password.strip() != password:
+        raise ValueError('Password must not start or end with whitespace.')
+
+def validate_admin_password(password):
+    if len(password) < 8:
+        raise ValueError("Password must be at least 8 characters long")
+    if any(ord(c) < 0x20 for c in password):
+        raise ValueError("Password must not contain control characters")
+    if any(ord(c) >= 0x7F for c in password):
+        raise ValueError("Password must only contain ASCII characters")
+
+    # Disallow characters that pkisilent doesn't process properly:
+    bad_characters = '\\'
     if any(c in bad_characters for c in password):
         raise ValueError('Password must not contain these characters: %s' %
             ', '.join('"%s"' % c for c in bad_characters))
@@ -239,8 +263,11 @@ def parse_options():
             validate_dm_password(options.dm_password)
         except ValueError, e:
             parser.error("DS admin password: " + str(e))
-    if options.admin_password is not None and len(options.admin_password) < 8:
-        parser.error("Admin user password must be at least 8 characters long")
+    if options.admin_password is not None:
+        try:
+            validate_admin_password(options.admin_password)
+        except ValueError, e:
+            parser.error("Admin user password: " + str(e))
 
     if options.domain_name is not None:
         try:
@@ -450,7 +477,7 @@ def read_admin_password():
     print "This user is a regular system account used for IPA server administration."
     print ""
     #TODO: provide the option of generating a random password
-    admin_password = read_password("IPA admin")
+    admin_password = read_password("IPA admin", validator=validate_admin_password)
     return admin_password
 
 def check_dirsrv(unattended):
-- 
1.9.3

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

Reply via email to