Re: [Freeipa-devel] [PATCH] 0002 Improve password validity check
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
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
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
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
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
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