Re: [Freeipa-devel] [PATCH] 0002 Improve password validity check

2014-07-24 Thread David Kupka

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 

Re: [Freeipa-devel] [PATCH] 0002 Improve password validity check

2014-07-24 Thread Martin Kosek
On 07/24/2014 02:02 PM, David Kupka wrote:
 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.

This one works fine. ACK.

Pushed to:
master: 603842867c65ae93d74a7c453c4301073c998441
ipa-4-1: 603842867c65ae93d74a7c453c4301073c998441

Martin

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


Re: [Freeipa-devel] [PATCH] 0002 Improve password validity check

2014-07-22 Thread Martin Kosek
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.

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.

Martin

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


Re: [Freeipa-devel] [PATCH] 0002 Improve password validity check

2014-07-21 Thread David Kupka

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.




Martin



--
David Kupka
From e9985196820757e61b07eb6470b6dec66502f497 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Mon, 21 Jul 2014 15:53:07 +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 | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 671a226d625ab9e8168c569a6d83c35dfae52115..e05b5fce7b77059cac2ad2318827c1df3ee5706b 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))
@@ -450,7 +474,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

[Freeipa-devel] [PATCH] 0002 Improve password validity check

2014-07-18 Thread David Kupka

https://fedorahosted.org/freeipa/ticket/2796
--
David Kupka
From c0fb9fe49a8b7eb190414571df211c87ba9c3166 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Fri, 18 Jul 2014 10:06:55 +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.

https://fedorahosted.org/freeipa/ticket/2796
---
 install/tools/ipa-server-install | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 671a226d625ab9e8168c569a6d83c35dfae52115..5b107c3ff3b61f87c30561a1aeed5ab65cf0bf27 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -121,7 +121,37 @@ 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 spaces (other white spaces are checked before)
+bad_prefix = ' '
+if password.startswith(bad_prefix):
+raise ValueError('Password must not start with %s.' % bad_prefix)
+
+# Disallow trailing spaces (other white spaces are checked before)
+bad_suffix = ' '
+if password.endswith(bad_suffix):
+raise ValueError('Password must not end with %s.' % bad_prefix)
+
+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))
@@ -450,7 +480,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

Re: [Freeipa-devel] [PATCH] 0002 Improve password validity check

2014-07-18 Thread Martin Kosek
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')


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.

Martin

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