Re: [Freeipa-devel] [PATCHES] 149-151 Ask for PKCS#12 password interactively

2013-07-12 Thread Jan Cholasta

On 11.7.2013 20:51, Rob Crittenden wrote:

Jan Cholasta wrote:

Hi,

the attached patches fix https://fedorahosted.org/freeipa/ticket/3717.

Also added a small patch to fix a formatting issue with
installutils.read_password.

Honza


Functionally ok but I found it very jarring the way the passwords were
prompted for. I think they should be moved after the realm question and
the text should be more than just the path to the filename.

rob



Moved the prompt and changed the text to Enter file unlock password.

Updated patches attached.

Honza

--
Jan Cholasta
From 992080e556036afe35c39092a5326c3267d8edf1 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 9 Jul 2013 10:23:47 +
Subject: [PATCH 1/3] Ask for PKCS#12 password interactively in
 ipa-server-install.

https://fedorahosted.org/freeipa/ticket/3717
---
 install/tools/ipa-server-install | 73 ++--
 1 file changed, 48 insertions(+), 25 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index cc88a0b..fed2004 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -276,14 +276,20 @@ def parse_options():
 if not options.forwarders and not options.no_forwarders:
 parser.error(You must specify at least one --forwarder option or --no-forwarders option)
 
-# If any of the PKCS#12 options are selected, all are required. Create a
-# list of the options and count it to enforce that all are required without
-# having a huge set of it blocks.
-pkcs12 = [options.dirsrv_pkcs12, options.http_pkcs12, options.dirsrv_pin, options.http_pin]
-cnt = pkcs12.count(None)
-if cnt  0 and cnt  4:
+# If any of the PKCS#12 options are selected, all are required.
+pkcs12_req = (options.dirsrv_pkcs12, options.http_pkcs12)
+pkcs12_opt = (options.pkinit_pkcs12,)
+if any(pkcs12_req + pkcs12_opt) and not all(pkcs12_req):
 parser.error(All PKCS#12 options are required if any are used.)
 
+if options.unattended:
+if options.dirsrv_pkcs12 and not options.dirsrv_pin:
+parser.error(You must specify --dirsrv_pin with --dirsrv_pkcs12)
+if options.http_pkcs12 and not options.http_pin:
+parser.error(You must specify --http_pin with --http_pkcs12)
+if options.pkinit_pkcs12 and not options.pkinit_pin:
+parser.error(You must specify --pkinit_pin with --pkinit_pkcs12)
+
 if options.dirsrv_pkcs12 and not options.root_ca_file:
 parser.error(
 --root-ca-file must be given with the PKCS#12 options.)
@@ -704,18 +710,6 @@ def main():
 sys.exit(1)
 cert = certdict[certissuer]
 
-if options.http_pkcs12:
-http_pin_file = ipautil.write_tmp_file(options.http_pin)
-http_pkcs12_info = (options.http_pkcs12, http_pin_file.name)
-
-if options.dirsrv_pkcs12:
-dirsrv_pin_file = ipautil.write_tmp_file(options.dirsrv_pin)
-dirsrv_pkcs12_info = (options.dirsrv_pkcs12, dirsrv_pin_file.name)
-
-if options.pkinit_pkcs12:
-pkinit_pin_file = ipautil.write_tmp_file(options.pkinit_pin)
-pkinit_pkcs12_info = (options.pkinit_pkcs12, pkinit_pin_file.name)
-
 # We only set up the CA if the PKCS#12 options are not given.
 if options.dirsrv_pkcs12:
 setup_ca = False
@@ -834,13 +828,6 @@ def main():
 else:
 domain_name = options.domain_name
 
-if options.http_pkcs12:
-# Check the given PKCS#12 files
-ca_file = options.root_ca_file
-check_pkcs12 = installutils.check_pkcs12
-http_cert_name = check_pkcs12(http_pkcs12_info, ca_file, host_name)
-dirsrv_cert_name = check_pkcs12(dirsrv_pkcs12_info, ca_file, host_name)
-
 domain_name = domain_name.lower()
 
 ip = get_server_ip_address(host_name, fstore, options.unattended, options)
@@ -858,6 +845,42 @@ def main():
 if not options.subject:
 options.subject = DN(('O', realm_name))
 
+ca_file = options.root_ca_file
+
+if options.http_pkcs12:
+if not options.http_pin:
+options.http_pin = installutils.read_password(
+Enter %s unlock % options.http_pkcs12,
+confirm=False, validate=False)
+if options.http_pin is None:
+sys.exit(%s unlock password required % options.http_pkcs12)
+http_pin_file = ipautil.write_tmp_file(options.http_pin)
+http_pkcs12_info = (options.http_pkcs12, http_pin_file.name)
+http_cert_name = installutils.check_pkcs12(
+http_pkcs12_info, ca_file, host_name)
+
+if options.dirsrv_pkcs12:
+if not options.dirsrv_pin:
+options.dirsrv_pin = installutils.read_password(
+Enter %s unlock % options.dirsrv_pkcs12,
+confirm=False, validate=False)
+if options.dirsrv_pin is None:
+sys.exit(%s unlock password 

Re: [Freeipa-devel] [PATCH 0173] Improve logging for cases where SOA serial autoincrementation failed

2013-07-12 Thread Tomas Hozza
On 07/11/2013 12:24 PM, Petr Spacek wrote:
 Hello,
 
 Improve logging for cases where SOA serial autoincrementation failed.
 

ACK

Regards,

Tomas Hozza

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


Re: [Freeipa-devel] [PATCHES] 149-151 Ask for PKCS#12 password interactively

2013-07-12 Thread Tomas Babej
 
  Functionally ok but I found it very jarring the way the passwords were
  prompted for. I think they should be moved after the realm question and
  the text should be more than just the path to the filename.
 
  rob
 
 
 Moved the prompt and changed the text to Enter file unlock password.
 
 Updated patches attached.
 
 Honza

Just a nitpick:

+# If any of the PKCS#12 options are selected, all are required.
+pkcs12_req = (options.dirsrv_pkcs12, options.http_pkcs12)
+pkcs12_opt = (options.pkinit_pkcs12,)
+if any(pkcs12_req + pkcs12_opt) and not all(pkcs12_req):
 parser.error(All PKCS#12 options are required if any are used.)

This error message is somewhat misleading, since --pkinit-pkcs12 options is not 
required.

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

[Freeipa-devel] [PATCH] 1103 Add Camellia ciphers

2013-07-12 Thread Rob Crittenden

Add the Camellia ciphers to the supported list in the KDC.

Also exclude some attributes from being displayed in LDAP updater logs.

rob
From 713dbff6566947ddec5bd41a04375012accfc8f6 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 12 Jul 2013 10:41:23 -0400
Subject: [PATCH] Add Camellia ciphers to allowed list, don't log some
 attributes in updater.

The LDAP updater prints the initial and final states of an entry, as well
as details on the changes made to attributes. This has the potential to expose
sensitive values so exclude those from logging.

https://fedorahosted.org/freeipa/ticket/3749
---
 install/share/kerberos.ldif   |  4 +++
 install/updates/50-krbenctypes.update |  5 +++
 install/updates/Makefile.am   |  1 +
 ipaserver/install/ldapupdate.py   | 66 +--
 4 files changed, 57 insertions(+), 19 deletions(-)
 create mode 100644 install/updates/50-krbenctypes.update

diff --git a/install/share/kerberos.ldif b/install/share/kerberos.ldif
index 4778a6b4df1eb5c52c4ef73f27b4409bb1c394fd..41e77952adafaf28bfaa96b4c1f1a81ef96348be 100644
--- a/install/share/kerberos.ldif
+++ b/install/share/kerberos.ldif
@@ -22,6 +22,10 @@ krbSupportedEncSaltTypes: des3-hmac-sha1:normal
 krbSupportedEncSaltTypes: des3-hmac-sha1:special
 krbSupportedEncSaltTypes: arcfour-hmac:normal
 krbSupportedEncSaltTypes: arcfour-hmac:special
+krbSupportedEncSaltTypes: camellia128-cts-cmac:normal
+krbSupportedEncSaltTypes: camellia128-cts-cmac:special
+krbSupportedEncSaltTypes: camellia256-cts-cmac:normal
+krbSupportedEncSaltTypes: camellia256-cts-cmac:special
 krbMaxTicketLife: 86400
 krbMaxRenewableAge: 604800
 krbDefaultEncSaltTypes: aes256-cts:special
diff --git a/install/updates/50-krbenctypes.update b/install/updates/50-krbenctypes.update
new file mode 100644
index ..ef419bc44051b289c774bce57db73b6d8c9f641c
--- /dev/null
+++ b/install/updates/50-krbenctypes.update
@@ -0,0 +1,5 @@
+dn: cn=$REALM,cn=kerberos,$SUFFIX
+add: krbSupportedEncSaltTypes: camellia128-cts-cmac:normal
+add: krbSupportedEncSaltTypes: camellia128-cts-cmac:special
+add: krbSupportedEncSaltTypes: camellia256-cts-cmac:normal
+add: krbSupportedEncSaltTypes: camellia256-cts-cmac:special
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index 5336f62ed97aba125ca8f1ae7c3e3505bb7ff3ea..40c3b3c8916faa267254a29d0f458ca53201950c 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -39,6 +39,7 @@ app_DATA =\
 	50-lockout-policy.update	\
 	50-groupuuid.update		\
 	50-hbacservice.update		\
+	50-krbenctypes.update		\
 	50-nis.update			\
 	50-ipaconfig.update		\
 	55-pbacmemberof.update		\
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 8f3e89249084aadfef27f63b65d6e74ffa0fc0ad..35191e7aeebde2c9a4f608a0e7d17f3da31e6cf8 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -58,6 +58,29 @@ class BadSyntax(installutils.ScriptError):
 def __str__(self):
 return repr(self.value)
 
+def safe_output(attr, values):
+
+Sanitizes values we do not want logged, like passwords.
+
+This should be called in all debug statements that output values.
+
+This list does not necessarily need to be exhaustive given the limited
+scope of types of values that the updater manages.
+
+This only supports lists, tuples and strings. If you pass a dict you may
+get a string back.
+
+sensitive_attributes = ['krbmkey', 'userpassword', 'passwordhistory', 'krbprincipalkey', 'sambalmpassword', 'sambantpassword', 'ipanthash']
+
+if attr.lower() in sensitive_attributes:
+if type(values) in (tuple, list):
+# try to still look a little like what is in LDAP
+return ['XXX'] * len(values)
+else:
+return ''
+else:
+return values
+
 class LDAPUpdate:
 action_keywords = [default, add, remove, only, onlyifexist, deleteentry, replace, addifnew, addifexist]
 
@@ -572,51 +595,51 @@ class LDAPUpdate:
 
 for update_value in update_values:
 if action == 'remove':
-self.debug(remove: '%s' from %s, current value %s, update_value, attr, entry_values)
+self.debug(remove: '%s' from %s, current value %s, safe_output(attr, update_value), attr, safe_output(attr,entry_values))
 try:
 entry_values.remove(update_value)
 except ValueError:
 self.warning(remove: '%s' not in %s, update_value, attr)
 pass
 entry[attr] = entry_values
-self.debug('remove: updated value %s', entry_values)
+self.debug('remove: updated value %s', safe_output(attr, entry_values))
 elif action == 'add':
-self.debug(add: '%s' to %s, 

[Freeipa-devel] [PATCH] 0043 Fix internal error in idrange-add

2013-07-12 Thread Ana Krivokapic
Hello,

This patch fixes https://fedorahosted.org/freeipa/ticket/3781

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 9c4d1df6592deb4c8b57907213486bee54c1ae1c Mon Sep 17 00:00:00 2001
From: Ana Krivokapic akriv...@redhat.com
Date: Fri, 12 Jul 2013 17:12:07 +0200
Subject: [PATCH] Fix internal error in idrange-add

Fix internal error in idrange-add, caused by a missing 'name' argument of
ValidationError.

https://fedorahosted.org/freeipa/ticket/3781
---
 ipalib/plugins/idrange.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 7f8c1ab7b265230c3dd5b0263f763ddf398c..06fcc4fc7232ea7080edb358c65948bfdd47289b 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -493,7 +493,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
 # TODO: can also be ipa-ad-winsync here?
 if entry_attrs['iparangetype'] in (u'ipa-ad-trust',
u'ipa-ad-trust-posix'):
-raise errors.ValidationError('ID Range setup',
+raise errors.ValidationError(name='ID Range setup',
 error=_('IPA Range type must not be one of ipa-ad-trust '
 'or ipa-ad-trust-posix when SID of the trusted '
 'domain is not specified.'))
-- 
1.8.1.4

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

Re: [Freeipa-devel] [PATCH] 1103 Add Camellia ciphers

2013-07-12 Thread Alexander Bokovoy

On Fri, 12 Jul 2013, Rob Crittenden wrote:

Add the Camellia ciphers to the supported list in the KDC.

Also exclude some attributes from being displayed in LDAP updater logs.

ACK if you split the patches into separate for logging and adding
Camelia cyphers, for maintenance purposes (easier to cherry-pick).

Thanks!
--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 0043 Fix internal error in idrange-add

2013-07-12 Thread Alexander Bokovoy

On Fri, 12 Jul 2013, Ana Krivokapic wrote:

Hello,

This patch fixes https://fedorahosted.org/freeipa/ticket/3781

ACK.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCH] 1103 Add Camellia ciphers

2013-07-12 Thread Rob Crittenden

Rob Crittenden wrote:

Add the Camellia ciphers to the supported list in the KDC.

Also exclude some attributes from being displayed in LDAP updater logs.

rob


I've split the Camellia patch out from the updater patch to clarify things.

rob

From 5a26ad3f0e76f507b6d31bca99efba85b356064e Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 12 Jul 2013 11:28:20 -0400
Subject: [PATCH] Add Camellia ciphers to allowed list.

https://fedorahosted.org/freeipa/ticket/3749
---
 install/share/kerberos.ldif   | 4 
 install/updates/50-krbenctypes.update | 5 +
 install/updates/Makefile.am   | 1 +
 3 files changed, 10 insertions(+)
 create mode 100644 install/updates/50-krbenctypes.update

diff --git a/install/share/kerberos.ldif b/install/share/kerberos.ldif
index 4778a6b4df1eb5c52c4ef73f27b4409bb1c394fd..41e77952adafaf28bfaa96b4c1f1a81ef96348be 100644
--- a/install/share/kerberos.ldif
+++ b/install/share/kerberos.ldif
@@ -22,6 +22,10 @@ krbSupportedEncSaltTypes: des3-hmac-sha1:normal
 krbSupportedEncSaltTypes: des3-hmac-sha1:special
 krbSupportedEncSaltTypes: arcfour-hmac:normal
 krbSupportedEncSaltTypes: arcfour-hmac:special
+krbSupportedEncSaltTypes: camellia128-cts-cmac:normal
+krbSupportedEncSaltTypes: camellia128-cts-cmac:special
+krbSupportedEncSaltTypes: camellia256-cts-cmac:normal
+krbSupportedEncSaltTypes: camellia256-cts-cmac:special
 krbMaxTicketLife: 86400
 krbMaxRenewableAge: 604800
 krbDefaultEncSaltTypes: aes256-cts:special
diff --git a/install/updates/50-krbenctypes.update b/install/updates/50-krbenctypes.update
new file mode 100644
index ..ef419bc44051b289c774bce57db73b6d8c9f641c
--- /dev/null
+++ b/install/updates/50-krbenctypes.update
@@ -0,0 +1,5 @@
+dn: cn=$REALM,cn=kerberos,$SUFFIX
+add: krbSupportedEncSaltTypes: camellia128-cts-cmac:normal
+add: krbSupportedEncSaltTypes: camellia128-cts-cmac:special
+add: krbSupportedEncSaltTypes: camellia256-cts-cmac:normal
+add: krbSupportedEncSaltTypes: camellia256-cts-cmac:special
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index 5336f62ed97aba125ca8f1ae7c3e3505bb7ff3ea..40c3b3c8916faa267254a29d0f458ca53201950c 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -39,6 +39,7 @@ app_DATA =\
 	50-lockout-policy.update	\
 	50-groupuuid.update		\
 	50-hbacservice.update		\
+	50-krbenctypes.update		\
 	50-nis.update			\
 	50-ipaconfig.update		\
 	55-pbacmemberof.update		\
-- 
1.8.3.1

From ba73e80e67984a4c8e5456a80cd085308043511d Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 12 Jul 2013 11:28:43 -0400
Subject: [PATCH] Hide sensitive attributes in LDAP updater logging and output

The LDAP updater prints the initial and final states of an entry, as well
as details on the changes made to attributes. This has the potential to
expose sensitive values so exclude those from logging.

https://fedorahosted.org/freeipa/ticket/3782
---
 ipaserver/install/ldapupdate.py | 66 +
 1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 8f3e89249084aadfef27f63b65d6e74ffa0fc0ad..35191e7aeebde2c9a4f608a0e7d17f3da31e6cf8 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -58,6 +58,29 @@ class BadSyntax(installutils.ScriptError):
 def __str__(self):
 return repr(self.value)
 
+def safe_output(attr, values):
+
+Sanitizes values we do not want logged, like passwords.
+
+This should be called in all debug statements that output values.
+
+This list does not necessarily need to be exhaustive given the limited
+scope of types of values that the updater manages.
+
+This only supports lists, tuples and strings. If you pass a dict you may
+get a string back.
+
+sensitive_attributes = ['krbmkey', 'userpassword', 'passwordhistory', 'krbprincipalkey', 'sambalmpassword', 'sambantpassword', 'ipanthash']
+
+if attr.lower() in sensitive_attributes:
+if type(values) in (tuple, list):
+# try to still look a little like what is in LDAP
+return ['XXX'] * len(values)
+else:
+return ''
+else:
+return values
+
 class LDAPUpdate:
 action_keywords = [default, add, remove, only, onlyifexist, deleteentry, replace, addifnew, addifexist]
 
@@ -572,51 +595,51 @@ class LDAPUpdate:
 
 for update_value in update_values:
 if action == 'remove':
-self.debug(remove: '%s' from %s, current value %s, update_value, attr, entry_values)
+self.debug(remove: '%s' from %s, current value %s, safe_output(attr, update_value), attr, safe_output(attr,entry_values))
 try:
 entry_values.remove(update_value)
 except ValueError: