Re: [Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS

2013-04-16 Thread Martin Kosek
On 04/15/2013 11:21 PM, Rob Crittenden wrote:
 Ana Krivokapic wrote:
 On 04/15/2013 07:06 PM, Martin Kosek wrote:
 On 04/15/2013 06:53 PM, Ana Krivokapic wrote:
 On 04/15/2013 06:30 PM, Martin Kosek wrote:
 On 04/12/2013 08:45 PM, Ana Krivokapic wrote:
 On 04/12/2013 01:26 PM, Ana Krivokapic wrote:
 On 04/12/2013 12:44 PM, Martin Kosek wrote:
 On 04/12/2013 12:20 PM, Ana Krivokapic wrote:
 On 04/11/2013 03:03 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Ana Krivokapic wrote:
 On 04/11/2013 01:43 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:24, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:09, Ana Krivokapic wrote:
 Integrate realmdomains with IPA DNS

 Add an entry to realmdomains when a DNS zone is added to IPA.
 Delete the
 related entry from  realmdomains when the DNS zone is deleted 
 from
 IPA.

 https://fedorahosted.org/freeipa/ticket/3544
 I would add a TXT record as I described in
 https://fedorahosted.org/freeipa/ticket/3544#comment:8

 This integration probably should go to both commands,
 realmdomains-*
 dnszone-*.

 Any objections? AB?
 Adding TXT record is probably harmless.

 I would actually add the TXT record creation only to
 realmdomains-* and
 trigger it only in case we manage our DNS and DNS zone is there.
 This way a hook from dnszone-add will trigger adding TXT record 
 back
 (via call to
 realmdomains-mod --add and then TXT record addition from there).
 Also
 the fact that admin added manually some domain to realmdomains
 mapping
 means that it is implied to be used in obtaining TGTs, so TXT
 record is
 helpful there as well.
 Okay, it makes sense. We will see how it will work in reality.
 One more thing to check is that we don't do this for our own 
 domain.

 Our own domain is already in realmdomains by default, and it cannot 
 be
 removed from there. So I don't think any check related to our 
 domain is
 necessary.
 We shouldn't start creating TXT records for our own domain, that's 
 what
 I'm asking for here.

 Think about server install stage -- we start creating our own domain 
 and
 the hook then causes to create realmdomains entry for the domain,
 causing realmdomains-mod code to raise ValidationError which is not
 handled in dnszone-add code with this patch.

 Same for TXT record creation starting from realmdomains-mod side -- 
 it
 simply should avoid calling dnsrecord-add for the case we know 
 wouldn't
 work.

 I just realized that this ticket was not marked as RFE although it
 obviously is
 one. I fixed the ticket summary and wrote the design page for this
 enhancement:

 http://www.freeipa.org/page/V3/DNS_realmdomains_integration

 Right, that was a good thing to do. I just have comment for the UPN
 enumeration
 image which you linked in the RFE - can you please process it, upload
 to the
 wiki and include in the overview? This will make the RFE page more
 appealing
 and it will also prevent us from having a broken link when Alexander
 removes
 the file from his temporary directory.

 Thanks,
 Martin
 Sure, done.

 I added the functionality to create TXT record to realmdomains-mod, and 
 also
 made sure that the case of our own domain is handled properly. Unit tests
 have
 been added to cover the new functionality. One unit test of the dns 
 plugin
 needed adjusting, but it still fails due to the bug in the testing
 framework[1]. It should pass after the bug is fixed.

 Updated patch is attached.

 [1] https://fedorahosted.org/freeipa/ticket/3562

 This looks nice, thanks for the new test cases.

 I experienced an issue with dnsrecord-find test in
 tests/test_xmlrpc/test_dns_plugin.py, but I see you already have an open
 ticket
 to fix that (https://fedorahosted.org/freeipa/ticket/3562) so it is not a
 show-stopper.

 This is a nitpick, but could you update
 tests/test_xmlrpc/test_dns_realmdomains_integration.py to use the same
 domains
 for testing as tests/test_xmlrpc/test_dns_plugin.py does?

 I often use example*.com zones in my testing and we also advertise test
 commands with these zones in ipa help dns too, so I (and maybe others)
 could
 get surprised that these zones are deleted after running the test suite.
 I.e. I
 would prefer to have dnszone*.test used for test.

 Thanks,
 Martin
 Sure. Updated patch attached.

 One more nitpick (sorry for not spotting it earlier). In realmdomains-mod, 
 you
 do checks for zone/record before you do the dnsrecord-add/dnsrecord-del
 command.

 I think this will unnecessarily make the command slower. You can just try
 add/delete a record and catch also a NotFound error - these commands already
 check for zone/record existence, so we do not need to do the checks twice.

 Relevant sections:

 +try:
 +api.Command['dnszone_show'](unicode(d))
 +except errors.NotFound:
 +pass
 +else:
 +try:
 +api.Command['dnsrecord_add'](
 +   

Re: [Freeipa-devel] [PATCHES] 126-127 Use A/AAAA records instead of CNAME records in ipa-ca

2013-04-16 Thread Jan Cholasta

On 15.4.2013 21:16, Martin Kosek wrote:

Jan, can you please rebase the patch also for ipa-3-1? We will need to
fix this issue also for 3.1.


Yep.

--
Jan Cholasta
From eb97cc1e5fa6b1d2953b78c72e601e17a87be208 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 15 Apr 2013 12:19:11 +0200
Subject: [PATCH 1/2] Use A/ records instead of CNAME records in ipa-ca.

https://fedorahosted.org/freeipa/ticket/3547
---
 install/share/bind.zone.db.template |   2 +-
 install/tools/ipa-ca-install|   2 +-
 install/tools/ipa-replica-manage|   1 +
 install/tools/ipa-upgradeconfig |  54 ---
 ipaserver/install/bindinstance.py   | 130 +---
 ipaserver/install/cainstance.py |   6 +-
 6 files changed, 142 insertions(+), 53 deletions(-)

diff --git a/install/share/bind.zone.db.template b/install/share/bind.zone.db.template
index 5ee71d6..6795bb0 100644
--- a/install/share/bind.zone.db.template
+++ b/install/share/bind.zone.db.template
@@ -26,4 +26,4 @@ _kpasswd._udp		IN SRV 0 100 464	$HOST
 $OPTIONAL_NTP
 
 ; CNAME for IPA CA replicas (used for CRL, OCSP)
-$IPA_CA_CNAME		IN CNAME		$HOST
+$IPA_CA_RECORD
diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index f8f7e1d..2ebce60 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -85,7 +85,7 @@ def install_dns_records(config, options):
 try:
 api.Backend.ldap2.connect(bind_dn=DN(('cn', 'Directory Manager')),
   bind_pw=config.dirman_password)
-bind.add_ipa_ca_cname(config.host_name, config.domain_name)
+bind.add_ipa_ca_dns_records(config.host_name, config.domain_name)
 finally:
 if api.Backend.ldap2.isconnected():
  api.Backend.ldap2.disconnect()
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index e78f9d2..a339a50 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -697,6 +697,7 @@ def del_master(realm, hostname, options):
 api.Backend.ldap2.connect(ccache=ccache)
 bind = bindinstance.BindInstance()
 bind.remove_master_dns_records(hostname, realm, realm.lower())
+bind.remove_ipa_ca_dns_records(hostname, realm.lower())
 except Exception, e:
 print Failed to cleanup %s DNS entries: %s % (hostname, convert_error(e))
 print You may need to manually remove them from the tree
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index f71d834..8bd7ff2 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -628,31 +628,35 @@ def migrate_crl_publish_dir(ca):
  'request pki-ca restart')
 return True
 
-def add_server_cname_records():
-root_logger.info('[Add missing server CNAME records]')
+def add_ca_dns_records():
+root_logger.info('[Add missing CA DNS records]')
 
-if not sysupgrade.get_upgrade_state('dns', 'ipa_ca_cname'):
-try:
-api.Backend.ldap2.connect(autobind=True)
-except ipalib.errors.PublicError, e:
-root_logger.error(Cannot connect to LDAP to add DNS records: %s, e)
-else:
-ret = api.Command['dns_is_enabled']()
-if not ret['result']:
-root_logger.info('DNS is not configured')
-sysupgrade.set_upgrade_state('dns', 'ipa_ca_cname', True)
-return
-
-bind = bindinstance.BindInstance()
-# DNS is enabled, so let bindinstance find out if CA is enabled
-# and let it add the CNAME in that case
-bind.add_ipa_ca_cname(api.env.host, api.env.domain, ca_configured=None)
-sysupgrade.set_upgrade_state('dns', 'ipa_ca_cname', True)
-finally:
-if api.Backend.ldap2.isconnected():
- api.Backend.ldap2.disconnect()
-else:
-root_logger.info('IPA CA CNAME already processed')
+if sysupgrade.get_upgrade_state('dns', 'ipa_ca_records'):
+root_logger.info('IPA CA DNS records already processed')
+return
+
+try:
+api.Backend.ldap2.connect(autobind=True)
+except ipalib.errors.PublicError, e:
+root_logger.error(Cannot connect to LDAP to add DNS records: %s, e)
+return
+
+ret = api.Command['dns_is_enabled']()
+if not ret['result']:
+root_logger.info('DNS is not configured')
+sysupgrade.set_upgrade_state('dns', 'ipa_ca_records', True)
+return
+
+bind = bindinstance.BindInstance()
+
+bind.convert_ipa_ca_cnames(api.env.domain)
+
+# DNS is enabled, so let bindinstance find out if CA is enabled
+# and let it add the record in that case
+bind.add_ipa_ca_dns_records(api.env.host, api.env.domain,
+ca_configured=None)
+
+sysupgrade.set_upgrade_state('dns', 'ipa_ca_records', True)
 
 def main():
 
@@ -746,7 

[Freeipa-devel] [PATCH] 128 Do not use new LDAP API in old code

2013-04-16 Thread Jan Cholasta

Hi,

while rebasing patches 126  127 on top of ipa-3-1, I have noticed that 
commit bceccbd6 uses new LDAP API, which is not available in 3.1. The 
attached patch fixes this.


Honza

--
Jan Cholasta
From 19090a990a48934459ae66d7ce1443ce2c6fdbfd Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 16 Apr 2013 09:45:34 +0200
Subject: [PATCH] Do not use new LDAP API in old code.

---
 ipaserver/install/adtrustinstance.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index c174351..2c9c90c 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -630,13 +630,13 @@ class ADTRUSTInstance(service.Service):
 try:
 cifs_services = DN(api.env.container_service, self.suffix)
 # Search for cifs services which also belong to adtrust agents, these are our DCs
-res = self.admin_conn.get_entries(cifs_services,
+res = self.admin_conn.getList(cifs_services,
 ldap.SCOPE_ONELEVEL,
 ((krbprincipalname=cifs/*@%s)(memberof=%s)) % (self.realm, str(self.smb_dn)))
 if len(res)  1:
 # there are other CIFS services defined, we are not alone
 for entry in res:
-managedBy = entry.single_value('managedBy', None)
+managedBy = entry.getValue('managedBy')
 if managedBy:
 fqdn = DN(managedBy)['fqdn']
 if fqdn != unicode(self.fqdn):
-- 
1.8.1.4

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

Re: [Freeipa-devel] [PATCH 0044] Update only selected attributes for winsync agreement

2013-04-16 Thread Tomas Babej

On 04/15/2013 11:58 PM, Rob Crittenden wrote:

Tomas Babej wrote:

On 04/09/2013 11:47 PM, Rob Crittenden wrote:

Tomas Babej wrote:

Hi,

Trying to insert nsDS5ReplicatedAttributeListTotal and
nsds5ReplicaStripAttrs to winsync agreements caused upgrade errors.
With this patch, these attributes are skipped for winsync agreements.

Made find_ipa_replication_agreements() in replication.py more
corresponding to find_replication_agreements. It returns list of
entries instead of unicode strings now.

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

Tomas


This will still do some work against a winsync agreement. Do we need
to do that at all? I'm not sure we do.

rob


I removed the nsds5replicahost attribute update for winsync agreements
after discussion.

Updated patch attached.

Tomas


This looks ok. The backup/restore patch added two more calls to 
find_ipa_replication_agreements so a rebase is needed. I think these 
are the required changes:


diff --git a/ipaserver/install/ipa_restore.py 
b/ipaserver/install/ipa_restore.py

index 04d4210..760da0b 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -373,7 +373,10 @@ class Restore(admintool.AdminTool):

 services_cns = [s.single_value('cn') for s in services]

-hosts = repl.find_ipa_replication_agreements()
+host_entries = repl.find_ipa_replication_agreements()
+hosts = [rep.single_value('nsds5replicahost', None)
+ for rep in host_entries]
+
 for host in hosts:
 self.log.info('Disabling replication agreement on %s 
to %s' % (

master, host))
 repl.disable_agreement(host)
@@ -385,7 +388,9 @@ class Restore(admintool.AdminTool):
 except Exception, e:
 self.log.critical(Unable to disable agreement on 
%s: %s %

 (master, e))

-hosts = repl.find_ipa_replication_agreements()
+host_entries = repl.find_ipa_replication_agreements()
+hosts = [rep.single_value('nsds5replicahost', None)
+ for rep in host_entries]
 for host in hosts:
 self.log.info('Disabling CA replication agreement 
on %s to

%s' % (master, host))
 repl.hostnames = [master, host]


I added the calls and rebased the patch. I also found one missed call in 
ipa-replica-csmanage.


Updated patch attached.
From ae0bddafc68b5a8ed501eaa36042aadabdd5ad16 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 8 Apr 2013 14:09:16 +0200
Subject: [PATCH] Update only selected attributes for winsync agreement

Trying to insert nsDS5ReplicatedAttributeListTotal and
nsds5ReplicaStripAttrs to winsync agreements caused upgrade errors.
With this patch, these attributes are skipped for winsync agreements.

Made find_ipa_replication_agreements() in replication.py more
corresponding to find_replication_agreements. It returns list of
entries instead of unicode strings now.

https://fedorahosted.org/freeipa/ticket/3522
---
 install/tools/ipa-csreplica-manage  |  4 +++-
 install/tools/ipa-replica-manage| 15 ---
 ipaserver/install/ipa_restore.py|  9 +++--
 ipaserver/install/plugins/fix_replica_agreements.py | 13 +
 ipaserver/install/replication.py|  9 ++---
 5 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/install/tools/ipa-csreplica-manage b/install/tools/ipa-csreplica-manage
index d70f24e26ef74e965299a172722c80385f70c8e0..4e11ffdefd5c38fc1ab090f38472e3675b700e54 100755
--- a/install/tools/ipa-csreplica-manage
+++ b/install/tools/ipa-csreplica-manage
@@ -262,7 +262,9 @@ def del_master(realm, hostname, options):
 # server not up, just remove it from this server
 replica_names = [options.host]
 else:
-replica_names = delrepl.find_ipa_replication_agreements()
+replica_entries = delrepl.find_ipa_replication_agreements()
+replica_names = [rep.single_value('nsds5replicahost', None)
+ for rep in replica_entries]
 
 # 5. Remove each agreement
 for r in replica_names:
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 8ab4dc5474752713c7cf9c089e2439a30d8d917e..ecb263813a5ce0e6d5a1e2033e16494a6bc7ce09 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -518,7 +518,10 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
 
 returns: hostname of orphaned server or None
 
-replica_names = delrepl.find_ipa_replication_agreements()
+replica_entries = delrepl.find_ipa_replication_agreements()
+
+replica_names = [rep.single_value('nsds5replicahost', None)
+ for rep in replica_entries]
 
 orphaned = []
 # Connect to each remote server and see what agreements it has
@@ -531,7 +534,11 @@ def 

Re: [Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS

2013-04-16 Thread Ana Krivokapic
On 04/16/2013 09:14 AM, Martin Kosek wrote:
 On 04/15/2013 11:21 PM, Rob Crittenden wrote:
 Ana Krivokapic wrote:
 On 04/15/2013 07:06 PM, Martin Kosek wrote:
 On 04/15/2013 06:53 PM, Ana Krivokapic wrote:
 On 04/15/2013 06:30 PM, Martin Kosek wrote:
 On 04/12/2013 08:45 PM, Ana Krivokapic wrote:
 On 04/12/2013 01:26 PM, Ana Krivokapic wrote:
 On 04/12/2013 12:44 PM, Martin Kosek wrote:
 On 04/12/2013 12:20 PM, Ana Krivokapic wrote:
 On 04/11/2013 03:03 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Ana Krivokapic wrote:
 On 04/11/2013 01:43 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:24, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:09, Ana Krivokapic wrote:
 Integrate realmdomains with IPA DNS

 Add an entry to realmdomains when a DNS zone is added to IPA.
 Delete the
 related entry from  realmdomains when the DNS zone is deleted 
 from
 IPA.

 https://fedorahosted.org/freeipa/ticket/3544
 I would add a TXT record as I described in
 https://fedorahosted.org/freeipa/ticket/3544#comment:8

 This integration probably should go to both commands,
 realmdomains-*
 dnszone-*.

 Any objections? AB?
 Adding TXT record is probably harmless.

 I would actually add the TXT record creation only to
 realmdomains-* and
 trigger it only in case we manage our DNS and DNS zone is there.
 This way a hook from dnszone-add will trigger adding TXT record 
 back
 (via call to
 realmdomains-mod --add and then TXT record addition from there).
 Also
 the fact that admin added manually some domain to realmdomains
 mapping
 means that it is implied to be used in obtaining TGTs, so TXT
 record is
 helpful there as well.
 Okay, it makes sense. We will see how it will work in reality.
 One more thing to check is that we don't do this for our own 
 domain.

 Our own domain is already in realmdomains by default, and it 
 cannot be
 removed from there. So I don't think any check related to our 
 domain is
 necessary.
 We shouldn't start creating TXT records for our own domain, that's 
 what
 I'm asking for here.

 Think about server install stage -- we start creating our own 
 domain and
 the hook then causes to create realmdomains entry for the domain,
 causing realmdomains-mod code to raise ValidationError which is not
 handled in dnszone-add code with this patch.

 Same for TXT record creation starting from realmdomains-mod side -- 
 it
 simply should avoid calling dnsrecord-add for the case we know 
 wouldn't
 work.

 I just realized that this ticket was not marked as RFE although it
 obviously is
 one. I fixed the ticket summary and wrote the design page for this
 enhancement:

 http://www.freeipa.org/page/V3/DNS_realmdomains_integration

 Right, that was a good thing to do. I just have comment for the UPN
 enumeration
 image which you linked in the RFE - can you please process it, upload
 to the
 wiki and include in the overview? This will make the RFE page more
 appealing
 and it will also prevent us from having a broken link when Alexander
 removes
 the file from his temporary directory.

 Thanks,
 Martin
 Sure, done.

 I added the functionality to create TXT record to realmdomains-mod, and 
 also
 made sure that the case of our own domain is handled properly. Unit 
 tests
 have
 been added to cover the new functionality. One unit test of the dns 
 plugin
 needed adjusting, but it still fails due to the bug in the testing
 framework[1]. It should pass after the bug is fixed.

 Updated patch is attached.

 [1] https://fedorahosted.org/freeipa/ticket/3562

 This looks nice, thanks for the new test cases.

 I experienced an issue with dnsrecord-find test in
 tests/test_xmlrpc/test_dns_plugin.py, but I see you already have an open
 ticket
 to fix that (https://fedorahosted.org/freeipa/ticket/3562) so it is not a
 show-stopper.

 This is a nitpick, but could you update
 tests/test_xmlrpc/test_dns_realmdomains_integration.py to use the same
 domains
 for testing as tests/test_xmlrpc/test_dns_plugin.py does?

 I often use example*.com zones in my testing and we also advertise test
 commands with these zones in ipa help dns too, so I (and maybe others)
 could
 get surprised that these zones are deleted after running the test suite.
 I.e. I
 would prefer to have dnszone*.test used for test.

 Thanks,
 Martin
 Sure. Updated patch attached.

 One more nitpick (sorry for not spotting it earlier). In realmdomains-mod, 
 you
 do checks for zone/record before you do the dnsrecord-add/dnsrecord-del
 command.

 I think this will unnecessarily make the command slower. You can just try
 add/delete a record and catch also a NotFound error - these commands 
 already
 check for zone/record existence, so we do not need to do the checks twice.

 Relevant sections:

 +try:
 +api.Command['dnszone_show'](unicode(d))
 +except errors.NotFound:
 +pass
 +else:
 +try:
 +

[Freeipa-devel] [PATCH 0147] Improve error logging for zones with idnsAllowDynUpdate == FALSE.

2013-04-16 Thread Petr Spacek

Hello,

Improve error logging for zones with idnsAllowDynUpdate == FALSE.

Zones with dynamic updates disabled are re-configured with empty
update policy string, so the update is refused by BIND and
an error is logged.

--
Petr Spacek
From 88a472349aec5216467aa1e30a35b8689b1cd439 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Apr 2013 10:49:11 +0200
Subject: [PATCH] Improve error logging for zones with idnsAllowDynUpdate ==
 FALSE.

Zones with dynamic updates disabled are re-configured with empty
update policy string, so the update is refused by BIND and
an error is logged.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/ldap_helper.c | 48 ++--
 src/settings.c|  7 +++
 2 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 70833b2e25ef15f89a9e5ffc4ec776ebbade1d86..d6061f247db625326ce09e75b1c7ca5c1f259ba5 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1242,6 +1242,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 	isc_boolean_t unlock = ISC_FALSE;
 	isc_boolean_t publish = ISC_FALSE;
 	isc_boolean_t published = ISC_FALSE;
+	isc_boolean_t ssu_changed;
 	isc_task_t *task = inst-task;
 	isc_uint32_t ldap_serial;
 	isc_uint32_t zr_serial;	/* SOA serial value from in-memory zone register */
@@ -1311,25 +1312,35 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
    idnsAllowDynUpdate, entry, inst-task);
 	if (result != ISC_R_SUCCESS  result != ISC_R_IGNORE)
 		goto cleanup;
+	ssu_changed = (result == ISC_R_SUCCESS);
 
 	result = setting_update_from_ldap_entry(sync_ptr, zone_settings,
    idnsAllowSyncPTR, entry, inst-task);
 	if (result != ISC_R_SUCCESS  result != ISC_R_IGNORE)
 		goto cleanup;
 
-	log_debug(2, Setting SSU table for %p: %s, zone, dn);
-	/* Get the update policy and update the zone with it. */
-	result = ldap_entry_getvalues(entry, idnsUpdatePolicy, values);
-	if (result == ISC_R_SUCCESS)
-		CHECK(configure_zone_ssutable(zone, HEAD(values)-value));
-	else
-		/* We need to declare zone as 'dynamic'
-		 * for dns_zone_isdynamic() to prevent unwanted
-		 * zone_postload() calls and warnings about serial and so on.
-		 *
-		 * Created SSU table contains no rules =
-		 * dns_ssutable_checkrules() will return deny. */
-		CHECK(configure_zone_ssutable(zone, ));
+	result = setting_update_from_ldap_entry(update_policy, zone_settings,
+   idnsUpdatePolicy, entry, inst-task);
+	if (result != ISC_R_SUCCESS  result != ISC_R_IGNORE)
+		goto cleanup;
+
+	if (result == ISC_R_SUCCESS || ssu_changed) {
+		isc_boolean_t ssu_enabled;
+		const char *ssu_policy = NULL;
+
+		log_debug(2, Setting SSU table for %p: %s, zone, dn);
+		CHECK(setting_get_bool(dyn_update, zone_settings, ssu_enabled));
+		if (ssu_enabled) {
+			/* Get the update policy and update the zone with it. */
+			CHECK(setting_get_str(update_policy, zone_settings,
+	  ssu_policy));
+			CHECK(configure_zone_ssutable(zone, ssu_policy));
+		} else {
+			/* Empty policy will prevent the update from reaching
+			 * LDAP driver and error will be logged. */
+			CHECK(configure_zone_ssutable(zone, ));
+		}
+	}
 
 	/* Fetch allow-query and allow-transfer ACLs */
 	log_debug(2, Setting allow-query for %p: %s, zone, dn);
@@ -2869,13 +2880,6 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
 		CLEANUP_WITH(DNS_R_NOTAUTH);
 	}
 
-	CHECK(setting_get_bool(dyn_update, zone_settings, zone_dyn_update));
-	if (!zone_dyn_update) {
-		log_debug(3, dynamic update is not allowed in zone '%s',
-			  zone_dn);
-		CLEANUP_WITH(DNS_R_REFUSED);
-	}
-
 	if (rdlist-type == dns_rdatatype_soa  mod_op == LDAP_MOD_DELETE)
 		CLEANUP_WITH(ISC_R_SUCCESS);
 
@@ -2991,8 +2995,8 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
 
 		CHECK(setting_get_bool(dyn_update, zone_settings, zone_dyn_update));
 		if (!zone_dyn_update) {
-			log_debug(3, dynamic update is not allowed in zone 
- '%s', zone_dn);
+			log_error(dynamic update is not allowed in zone 
+  '%s', zone_dn);
 			CLEANUP_WITH(ISC_R_NOPERM);
 		}
 
diff --git a/src/settings.c b/src/settings.c
index 8ced5fe9bff4beb70e8a6f5e8880aa34eae7c045..1f718d82788ed307dd0ecdb43570d7ce32395941 100644
--- a/src/settings.c
+++ b/src/settings.c
@@ -67,6 +67,13 @@ static const setting_t settings_default[] = {
 	{ ldap_hostname,		default_string()		},
 	{ sync_ptr,			default_boolean(ISC_FALSE)	},
 	{ dyn_update,			default_boolean(ISC_FALSE)	},
+	/* Empty string as default update_policy declares zone as 'dynamic'
+	 * for dns_zone_isdynamic() to prevent unwanted
+	 * zone_postload() calls and warnings about serial and so on.
+	 *
+	 * SSU table defined by empty string contains no rules =
+	 * dns_ssutable_checkrules() will return deny. */
+	{ update_policy,		default_string()		},
 	{ serial_autoincrement,	default_boolean(ISC_FALSE)	},
 	{ verbose_checks,		

[Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.

2013-04-16 Thread Petr Spacek

Hello,

Explicitly return SERVFAIL if PTR synchronization is misconfigured.

SERVFAIL will be returned if PTR synchronization is enabled
in forward zone but reverse zone has dynamic updates disabled.

--
Petr Spacek
From fe5f710b212b2cbe144f8132de5df3672ad479a5 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Apr 2013 11:00:04 +0200
Subject: [PATCH] Explicitly return SERVFAIL if PTR synchronization is
 misconfigured.

SERVFAIL will be returned if PTR synchronization is enabled
in forward zone but reverse zone has dynamic updates disabled.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/ldap_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index d6061f247db625326ce09e75b1c7ca5c1f259ba5..21dbed1b81af74cd3c8f28e3a78db7a878cfdeb4 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2990,7 +2990,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
 		if (result != ISC_R_SUCCESS) {
 			if (result == ISC_R_NOTFOUND)
 log_debug(3, active zone '%s' not found, zone_dn);
-			goto cleanup;
+			CLEANUP_WITH(DNS_R_SERVFAIL);
 		}
 
 		CHECK(setting_get_bool(dyn_update, zone_settings, zone_dyn_update));
-- 
1.7.11.7

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

Re: [Freeipa-devel] [PATCHES] 126-127 Use A/AAAA records instead of CNAME records in ipa-ca

2013-04-16 Thread Martin Kosek
On 04/16/2013 09:40 AM, Jan Cholasta wrote:
 On 15.4.2013 21:16, Martin Kosek wrote:
 Jan, can you please rebase the patch also for ipa-3-1? We will need to
 fix this issue also for 3.1.
 
 Yep.
 

Thanks. Pushed both to ipa-3-1.

Martin

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


Re: [Freeipa-devel] [PATCH] 128 Do not use new LDAP API in old code

2013-04-16 Thread Martin Kosek
On 04/16/2013 10:05 AM, Jan Cholasta wrote:
 Hi,
 
 while rebasing patches 126  127 on top of ipa-3-1, I have noticed that commit
 bceccbd6 uses new LDAP API, which is not available in 3.1. The attached patch
 fixes this.
 
 Honza

ACK, pushed to ipa-3-1.

Martin

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


[Freeipa-devel] [PATCH] 401 Require new samba and krb5

2013-04-16 Thread Martin Kosek
Require samba 4.0.5 (passdb API changed). Make sure that we use the
right epoch number with samba so that the Requires is correctly
enforced.

Require krb5 1.11.2-1 to fix missing PAC issue.

---

This patch makes sure we have the right dependencies in Fedora 19 (and Fedora
18 too for the samba one).

Martin
From 75a8ba7eb560007c69f5df937daa554093fce75b Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 16 Apr 2013 09:44:28 +0200
Subject: [PATCH] Require new samba and krb5

Require samba 4.0.5 (passdb API changed). Make sure that we use the
right epoch number with samba so that the Requires is correctly
enforced.

Require krb5 1.11.2-1 to fix missing PAC issue.
---
 freeipa.spec.in | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 7e9e994052bf32a7e2349adfbb932401bfd7f93e..3e71aa638cb1518a132a429518f3f1ad040cfadc 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -23,7 +23,7 @@ BuildRequires:  /usr/share/selinux/devel/Makefile
 BuildRequires:  policycoreutils = %{POLICYCOREUTILSVER}
 BuildRequires:  systemd-units
 %if 0%{?fedora} = 18
-BuildRequires:  samba-devel = 4.0.0-150
+BuildRequires:  samba-devel = 2:4.0.5-1
 BuildRequires:  samba-python
 BuildRequires:  libwbclient-devel
 %else
@@ -98,7 +98,7 @@ Requires: openldap-clients
 Requires: nss
 Requires: nss-tools
 %if 0%{?krb5_dal_version} = 4
-Requires: krb5-server = 1.11
+Requires: krb5-server = 1.11.2-1
 %else
 %if 0%{krb5_dal_version} == 3
 # krb5 1.11 bumped DAL interface major version, a rebuild is needed
@@ -194,7 +194,7 @@ Requires: %{name}-server = %version-%release
 Requires: m2crypto
 %if 0%{?fedora} = 18
 Requires: samba-python
-Requires: samba
+Requires: samba = 2:4.0.5-1
 Requires: samba-winbind
 %else
 Requires: samba4-python
@@ -794,6 +794,10 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
 
 %changelog
+* Mon Apr 15 2013 Martin Kosek mko...@redhat.com - 3.1.99-6
+- Require samba 4.0.5, includes new passdb API
+- Require krb5 1.11.2-1, fixes missing PAC issue
+
 * Fri Apr  5 2013 Rob Crittenden rcrit...@redhat.com - 3.1.99-5
 - Add backup and restore
 - Own /var/lib/ipa/backup
-- 
1.8.1.4

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

Re: [Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS

2013-04-16 Thread Alexander Bokovoy

On Tue, 16 Apr 2013, Martin Kosek wrote:

On 04/15/2013 11:21 PM, Rob Crittenden wrote:

Ana Krivokapic wrote:

On 04/15/2013 07:06 PM, Martin Kosek wrote:

On 04/15/2013 06:53 PM, Ana Krivokapic wrote:

On 04/15/2013 06:30 PM, Martin Kosek wrote:

On 04/12/2013 08:45 PM, Ana Krivokapic wrote:

On 04/12/2013 01:26 PM, Ana Krivokapic wrote:

On 04/12/2013 12:44 PM, Martin Kosek wrote:

On 04/12/2013 12:20 PM, Ana Krivokapic wrote:

On 04/11/2013 03:03 PM, Alexander Bokovoy wrote:

On Thu, 11 Apr 2013, Ana Krivokapic wrote:

On 04/11/2013 01:43 PM, Alexander Bokovoy wrote:

On Thu, 11 Apr 2013, Petr Spacek wrote:

On 11.4.2013 13:24, Alexander Bokovoy wrote:

On Thu, 11 Apr 2013, Petr Spacek wrote:

On 11.4.2013 13:09, Ana Krivokapic wrote:

Integrate realmdomains with IPA DNS

Add an entry to realmdomains when a DNS zone is added to IPA.
Delete the
related entry from  realmdomains when the DNS zone is deleted from
IPA.

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

I would add a TXT record as I described in
https://fedorahosted.org/freeipa/ticket/3544#comment:8

This integration probably should go to both commands,
realmdomains-*
dnszone-*.

Any objections? AB?

Adding TXT record is probably harmless.

I would actually add the TXT record creation only to
realmdomains-* and
trigger it only in case we manage our DNS and DNS zone is there.
This way a hook from dnszone-add will trigger adding TXT record back
(via call to
realmdomains-mod --add and then TXT record addition from there).
Also
the fact that admin added manually some domain to realmdomains
mapping
means that it is implied to be used in obtaining TGTs, so TXT
record is
helpful there as well.

Okay, it makes sense. We will see how it will work in reality.

One more thing to check is that we don't do this for our own domain.


Our own domain is already in realmdomains by default, and it cannot be
removed from there. So I don't think any check related to our domain is
necessary.

We shouldn't start creating TXT records for our own domain, that's what
I'm asking for here.

Think about server install stage -- we start creating our own domain and
the hook then causes to create realmdomains entry for the domain,
causing realmdomains-mod code to raise ValidationError which is not
handled in dnszone-add code with this patch.

Same for TXT record creation starting from realmdomains-mod side -- it
simply should avoid calling dnsrecord-add for the case we know wouldn't
work.


I just realized that this ticket was not marked as RFE although it
obviously is
one. I fixed the ticket summary and wrote the design page for this
enhancement:

http://www.freeipa.org/page/V3/DNS_realmdomains_integration


Right, that was a good thing to do. I just have comment for the UPN
enumeration
image which you linked in the RFE - can you please process it, upload
to the
wiki and include in the overview? This will make the RFE page more
appealing
and it will also prevent us from having a broken link when Alexander
removes
the file from his temporary directory.

Thanks,
Martin

Sure, done.


I added the functionality to create TXT record to realmdomains-mod, and also
made sure that the case of our own domain is handled properly. Unit tests
have
been added to cover the new functionality. One unit test of the dns plugin
needed adjusting, but it still fails due to the bug in the testing
framework[1]. It should pass after the bug is fixed.

Updated patch is attached.

[1] https://fedorahosted.org/freeipa/ticket/3562


This looks nice, thanks for the new test cases.

I experienced an issue with dnsrecord-find test in
tests/test_xmlrpc/test_dns_plugin.py, but I see you already have an open
ticket
to fix that (https://fedorahosted.org/freeipa/ticket/3562) so it is not a
show-stopper.

This is a nitpick, but could you update
tests/test_xmlrpc/test_dns_realmdomains_integration.py to use the same
domains
for testing as tests/test_xmlrpc/test_dns_plugin.py does?

I often use example*.com zones in my testing and we also advertise test
commands with these zones in ipa help dns too, so I (and maybe others)
could
get surprised that these zones are deleted after running the test suite.
I.e. I
would prefer to have dnszone*.test used for test.

Thanks,
Martin

Sure. Updated patch attached.


One more nitpick (sorry for not spotting it earlier). In realmdomains-mod, you
do checks for zone/record before you do the dnsrecord-add/dnsrecord-del
command.

I think this will unnecessarily make the command slower. You can just try
add/delete a record and catch also a NotFound error - these commands already
check for zone/record existence, so we do not need to do the checks twice.

Relevant sections:

+try:
+api.Command['dnszone_show'](unicode(d))
+except errors.NotFound:
+pass
+else:
+try:
+api.Command['dnsrecord_add'](
+unicode(d),
+u'_kerberos',
+

Re: [Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS

2013-04-16 Thread Martin Kosek
On 04/16/2013 12:31 PM, Ana Krivokapic wrote:
 On 04/16/2013 09:14 AM, Martin Kosek wrote:
 On 04/15/2013 11:21 PM, Rob Crittenden wrote:
 Ana Krivokapic wrote:
 On 04/15/2013 07:06 PM, Martin Kosek wrote:
 On 04/15/2013 06:53 PM, Ana Krivokapic wrote:
 On 04/15/2013 06:30 PM, Martin Kosek wrote:
 On 04/12/2013 08:45 PM, Ana Krivokapic wrote:
 On 04/12/2013 01:26 PM, Ana Krivokapic wrote:
 On 04/12/2013 12:44 PM, Martin Kosek wrote:
 On 04/12/2013 12:20 PM, Ana Krivokapic wrote:
 On 04/11/2013 03:03 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Ana Krivokapic wrote:
 On 04/11/2013 01:43 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:24, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:09, Ana Krivokapic wrote:
 Integrate realmdomains with IPA DNS

 Add an entry to realmdomains when a DNS zone is added to IPA.
 Delete the
 related entry from  realmdomains when the DNS zone is 
 deleted from
 IPA.

 https://fedorahosted.org/freeipa/ticket/3544
 I would add a TXT record as I described in
 https://fedorahosted.org/freeipa/ticket/3544#comment:8

 This integration probably should go to both commands,
 realmdomains-*
 dnszone-*.

 Any objections? AB?
 Adding TXT record is probably harmless.

 I would actually add the TXT record creation only to
 realmdomains-* and
 trigger it only in case we manage our DNS and DNS zone is 
 there.
 This way a hook from dnszone-add will trigger adding TXT 
 record back
 (via call to
 realmdomains-mod --add and then TXT record addition from 
 there).
 Also
 the fact that admin added manually some domain to realmdomains
 mapping
 means that it is implied to be used in obtaining TGTs, so TXT
 record is
 helpful there as well.
 Okay, it makes sense. We will see how it will work in reality.
 One more thing to check is that we don't do this for our own 
 domain.

 Our own domain is already in realmdomains by default, and it 
 cannot be
 removed from there. So I don't think any check related to our 
 domain is
 necessary.
 We shouldn't start creating TXT records for our own domain, that's 
 what
 I'm asking for here.

 Think about server install stage -- we start creating our own 
 domain and
 the hook then causes to create realmdomains entry for the domain,
 causing realmdomains-mod code to raise ValidationError which is not
 handled in dnszone-add code with this patch.

 Same for TXT record creation starting from realmdomains-mod side 
 -- it
 simply should avoid calling dnsrecord-add for the case we know 
 wouldn't
 work.

 I just realized that this ticket was not marked as RFE although it
 obviously is
 one. I fixed the ticket summary and wrote the design page for this
 enhancement:

 http://www.freeipa.org/page/V3/DNS_realmdomains_integration

 Right, that was a good thing to do. I just have comment for the UPN
 enumeration
 image which you linked in the RFE - can you please process it, upload
 to the
 wiki and include in the overview? This will make the RFE page more
 appealing
 and it will also prevent us from having a broken link when Alexander
 removes
 the file from his temporary directory.

 Thanks,
 Martin
 Sure, done.

 I added the functionality to create TXT record to realmdomains-mod, 
 and also
 made sure that the case of our own domain is handled properly. Unit 
 tests
 have
 been added to cover the new functionality. One unit test of the dns 
 plugin
 needed adjusting, but it still fails due to the bug in the testing
 framework[1]. It should pass after the bug is fixed.

 Updated patch is attached.

 [1] https://fedorahosted.org/freeipa/ticket/3562

 This looks nice, thanks for the new test cases.

 I experienced an issue with dnsrecord-find test in
 tests/test_xmlrpc/test_dns_plugin.py, but I see you already have an open
 ticket
 to fix that (https://fedorahosted.org/freeipa/ticket/3562) so it is not 
 a
 show-stopper.

 This is a nitpick, but could you update
 tests/test_xmlrpc/test_dns_realmdomains_integration.py to use the same
 domains
 for testing as tests/test_xmlrpc/test_dns_plugin.py does?

 I often use example*.com zones in my testing and we also advertise test
 commands with these zones in ipa help dns too, so I (and maybe others)
 could
 get surprised that these zones are deleted after running the test suite.
 I.e. I
 would prefer to have dnszone*.test used for test.

 Thanks,
 Martin
 Sure. Updated patch attached.

 One more nitpick (sorry for not spotting it earlier). In 
 realmdomains-mod, you
 do checks for zone/record before you do the dnsrecord-add/dnsrecord-del
 command.

 I think this will unnecessarily make the command slower. You can just try
 add/delete a record and catch also a NotFound error - these commands 
 already
 check for zone/record existence, so we do not need to do the checks twice.

 Relevant sections:

 +try:
 +api.Command['dnszone_show'](unicode(d))
 +except errors.NotFound:
 +pass
 +

Re: [Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS

2013-04-16 Thread Ana Krivokapic
On 04/16/2013 01:55 PM, Martin Kosek wrote:
 On 04/16/2013 12:31 PM, Ana Krivokapic wrote:
 On 04/16/2013 09:14 AM, Martin Kosek wrote:
 On 04/15/2013 11:21 PM, Rob Crittenden wrote:
 Ana Krivokapic wrote:
 On 04/15/2013 07:06 PM, Martin Kosek wrote:
 On 04/15/2013 06:53 PM, Ana Krivokapic wrote:
 On 04/15/2013 06:30 PM, Martin Kosek wrote:
 On 04/12/2013 08:45 PM, Ana Krivokapic wrote:
 On 04/12/2013 01:26 PM, Ana Krivokapic wrote:
 On 04/12/2013 12:44 PM, Martin Kosek wrote:
 On 04/12/2013 12:20 PM, Ana Krivokapic wrote:
 On 04/11/2013 03:03 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Ana Krivokapic wrote:
 On 04/11/2013 01:43 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:24, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:09, Ana Krivokapic wrote:
 Integrate realmdomains with IPA DNS

 Add an entry to realmdomains when a DNS zone is added to 
 IPA.
 Delete the
 related entry from  realmdomains when the DNS zone is 
 deleted from
 IPA.

 https://fedorahosted.org/freeipa/ticket/3544
 I would add a TXT record as I described in
 https://fedorahosted.org/freeipa/ticket/3544#comment:8

 This integration probably should go to both commands,
 realmdomains-*
 dnszone-*.

 Any objections? AB?
 Adding TXT record is probably harmless.

 I would actually add the TXT record creation only to
 realmdomains-* and
 trigger it only in case we manage our DNS and DNS zone is 
 there.
 This way a hook from dnszone-add will trigger adding TXT 
 record back
 (via call to
 realmdomains-mod --add and then TXT record addition from 
 there).
 Also
 the fact that admin added manually some domain to realmdomains
 mapping
 means that it is implied to be used in obtaining TGTs, so TXT
 record is
 helpful there as well.
 Okay, it makes sense. We will see how it will work in reality.
 One more thing to check is that we don't do this for our own 
 domain.

 Our own domain is already in realmdomains by default, and it 
 cannot be
 removed from there. So I don't think any check related to our 
 domain is
 necessary.
 We shouldn't start creating TXT records for our own domain, 
 that's what
 I'm asking for here.

 Think about server install stage -- we start creating our own 
 domain and
 the hook then causes to create realmdomains entry for the domain,
 causing realmdomains-mod code to raise ValidationError which is 
 not
 handled in dnszone-add code with this patch.

 Same for TXT record creation starting from realmdomains-mod side 
 -- it
 simply should avoid calling dnsrecord-add for the case we know 
 wouldn't
 work.

 I just realized that this ticket was not marked as RFE although it
 obviously is
 one. I fixed the ticket summary and wrote the design page for this
 enhancement:

 http://www.freeipa.org/page/V3/DNS_realmdomains_integration

 Right, that was a good thing to do. I just have comment for the UPN
 enumeration
 image which you linked in the RFE - can you please process it, 
 upload
 to the
 wiki and include in the overview? This will make the RFE page more
 appealing
 and it will also prevent us from having a broken link when Alexander
 removes
 the file from his temporary directory.

 Thanks,
 Martin
 Sure, done.

 I added the functionality to create TXT record to realmdomains-mod, 
 and also
 made sure that the case of our own domain is handled properly. Unit 
 tests
 have
 been added to cover the new functionality. One unit test of the dns 
 plugin
 needed adjusting, but it still fails due to the bug in the testing
 framework[1]. It should pass after the bug is fixed.

 Updated patch is attached.

 [1] https://fedorahosted.org/freeipa/ticket/3562

 This looks nice, thanks for the new test cases.

 I experienced an issue with dnsrecord-find test in
 tests/test_xmlrpc/test_dns_plugin.py, but I see you already have an 
 open
 ticket
 to fix that (https://fedorahosted.org/freeipa/ticket/3562) so it is 
 not a
 show-stopper.

 This is a nitpick, but could you update
 tests/test_xmlrpc/test_dns_realmdomains_integration.py to use the same
 domains
 for testing as tests/test_xmlrpc/test_dns_plugin.py does?

 I often use example*.com zones in my testing and we also advertise test
 commands with these zones in ipa help dns too, so I (and maybe 
 others)
 could
 get surprised that these zones are deleted after running the test 
 suite.
 I.e. I
 would prefer to have dnszone*.test used for test.

 Thanks,
 Martin
 Sure. Updated patch attached.

 One more nitpick (sorry for not spotting it earlier). In 
 realmdomains-mod, you
 do checks for zone/record before you do the dnsrecord-add/dnsrecord-del
 command.

 I think this will unnecessarily make the command slower. You can just try
 add/delete a record and catch also a NotFound error - these commands 
 already
 check for zone/record existence, so we do not need to do the checks 
 twice.

 Relevant sections:

 +try:
 +api.Command['dnszone_show'](unicode(d))
 +

Re: [Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS

2013-04-16 Thread Martin Kosek
On 04/16/2013 03:03 PM, Ana Krivokapic wrote:
 On 04/16/2013 01:55 PM, Martin Kosek wrote:
 On 04/16/2013 12:31 PM, Ana Krivokapic wrote:
 On 04/16/2013 09:14 AM, Martin Kosek wrote:
 On 04/15/2013 11:21 PM, Rob Crittenden wrote:
 Ana Krivokapic wrote:
 On 04/15/2013 07:06 PM, Martin Kosek wrote:
 On 04/15/2013 06:53 PM, Ana Krivokapic wrote:
 On 04/15/2013 06:30 PM, Martin Kosek wrote:
 On 04/12/2013 08:45 PM, Ana Krivokapic wrote:
 On 04/12/2013 01:26 PM, Ana Krivokapic wrote:
 On 04/12/2013 12:44 PM, Martin Kosek wrote:
 On 04/12/2013 12:20 PM, Ana Krivokapic wrote:
 On 04/11/2013 03:03 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Ana Krivokapic wrote:
 On 04/11/2013 01:43 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:24, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:09, Ana Krivokapic wrote:
 Integrate realmdomains with IPA DNS

 Add an entry to realmdomains when a DNS zone is added to 
 IPA.
 Delete the
 related entry from  realmdomains when the DNS zone is 
 deleted from
 IPA.

 https://fedorahosted.org/freeipa/ticket/3544
 I would add a TXT record as I described in
 https://fedorahosted.org/freeipa/ticket/3544#comment:8

 This integration probably should go to both commands,
 realmdomains-*
 dnszone-*.

 Any objections? AB?
 Adding TXT record is probably harmless.

 I would actually add the TXT record creation only to
 realmdomains-* and
 trigger it only in case we manage our DNS and DNS zone is 
 there.
 This way a hook from dnszone-add will trigger adding TXT 
 record back
 (via call to
 realmdomains-mod --add and then TXT record addition from 
 there).
 Also
 the fact that admin added manually some domain to 
 realmdomains
 mapping
 means that it is implied to be used in obtaining TGTs, so TXT
 record is
 helpful there as well.
 Okay, it makes sense. We will see how it will work in reality.
 One more thing to check is that we don't do this for our own 
 domain.

 Our own domain is already in realmdomains by default, and it 
 cannot be
 removed from there. So I don't think any check related to our 
 domain is
 necessary.
 We shouldn't start creating TXT records for our own domain, 
 that's what
 I'm asking for here.

 Think about server install stage -- we start creating our own 
 domain and
 the hook then causes to create realmdomains entry for the domain,
 causing realmdomains-mod code to raise ValidationError which is 
 not
 handled in dnszone-add code with this patch.

 Same for TXT record creation starting from realmdomains-mod side 
 -- it
 simply should avoid calling dnsrecord-add for the case we know 
 wouldn't
 work.

 I just realized that this ticket was not marked as RFE although it
 obviously is
 one. I fixed the ticket summary and wrote the design page for this
 enhancement:

 http://www.freeipa.org/page/V3/DNS_realmdomains_integration

 Right, that was a good thing to do. I just have comment for the UPN
 enumeration
 image which you linked in the RFE - can you please process it, 
 upload
 to the
 wiki and include in the overview? This will make the RFE page more
 appealing
 and it will also prevent us from having a broken link when 
 Alexander
 removes
 the file from his temporary directory.

 Thanks,
 Martin
 Sure, done.

 I added the functionality to create TXT record to realmdomains-mod, 
 and also
 made sure that the case of our own domain is handled properly. Unit 
 tests
 have
 been added to cover the new functionality. One unit test of the dns 
 plugin
 needed adjusting, but it still fails due to the bug in the testing
 framework[1]. It should pass after the bug is fixed.

 Updated patch is attached.

 [1] https://fedorahosted.org/freeipa/ticket/3562

 This looks nice, thanks for the new test cases.

 I experienced an issue with dnsrecord-find test in
 tests/test_xmlrpc/test_dns_plugin.py, but I see you already have an 
 open
 ticket
 to fix that (https://fedorahosted.org/freeipa/ticket/3562) so it is 
 not a
 show-stopper.

 This is a nitpick, but could you update
 tests/test_xmlrpc/test_dns_realmdomains_integration.py to use the same
 domains
 for testing as tests/test_xmlrpc/test_dns_plugin.py does?

 I often use example*.com zones in my testing and we also advertise 
 test
 commands with these zones in ipa help dns too, so I (and maybe 
 others)
 could
 get surprised that these zones are deleted after running the test 
 suite.
 I.e. I
 would prefer to have dnszone*.test used for test.

 Thanks,
 Martin
 Sure. Updated patch attached.

 One more nitpick (sorry for not spotting it earlier). In 
 realmdomains-mod, you
 do checks for zone/record before you do the dnsrecord-add/dnsrecord-del
 command.

 I think this will unnecessarily make the command slower. You can just 
 try
 add/delete a record and catch also a NotFound error - these commands 
 already
 check for zone/record existence, so we do not need to do the checks 
 twice.

 Relevant sections:

 +try:
 +

Re: [Freeipa-devel] [PATCH] 401 Require new samba and krb5

2013-04-16 Thread Martin Kosek
On 04/16/2013 01:16 PM, Martin Kosek wrote:
 Require samba 4.0.5 (passdb API changed). Make sure that we use the
 right epoch number with samba so that the Requires is correctly
 enforced.
 
 Require krb5 1.11.2-1 to fix missing PAC issue.
 
 ---
 
 This patch makes sure we have the right dependencies in Fedora 19 (and Fedora
 18 too for the samba one).
 
 Martin
 

Squashing a fix for backup dir permissions.

Martin

From 442543a4d76b00bf5a58c6817363c24cc70ea27f Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 16 Apr 2013 09:44:28 +0200
Subject: [PATCH] Require new samba and krb5

Require samba 4.0.5 (passdb API changed). Make sure that we use the
right epoch number with samba so that the Requires is correctly
enforced.

Require krb5 1.11.2-1 to fix missing PAC issue.

Also fix backup dir permissions.
---
 freeipa.spec.in | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 7e9e994052bf32a7e2349adfbb932401bfd7f93e..9448a4b485f4bd638b6e080926febbf772839ca8 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -23,7 +23,7 @@ BuildRequires:  /usr/share/selinux/devel/Makefile
 BuildRequires:  policycoreutils = %{POLICYCOREUTILSVER}
 BuildRequires:  systemd-units
 %if 0%{?fedora} = 18
-BuildRequires:  samba-devel = 4.0.0-150
+BuildRequires:  samba-devel = 2:4.0.5-1
 BuildRequires:  samba-python
 BuildRequires:  libwbclient-devel
 %else
@@ -98,7 +98,7 @@ Requires: openldap-clients
 Requires: nss
 Requires: nss-tools
 %if 0%{?krb5_dal_version} = 4
-Requires: krb5-server = 1.11
+Requires: krb5-server = 1.11.2-1
 %else
 %if 0%{krb5_dal_version} == 3
 # krb5 1.11 bumped DAL interface major version, a rebuild is needed
@@ -194,7 +194,7 @@ Requires: %{name}-server = %version-%release
 Requires: m2crypto
 %if 0%{?fedora} = 18
 Requires: samba-python
-Requires: samba
+Requires: samba = 2:4.0.5-1
 Requires: samba-winbind
 %else
 Requires: samba4-python
@@ -691,7 +691,7 @@ fi
 %attr(755,root,root) %{plugin_dir}/libipa_dns.so
 %attr(755,root,root) %{plugin_dir}/libipa_range_check.so
 %dir %{_localstatedir}/lib/ipa
-%attr(755,root,root) %dir %{_localstatedir}/lib/ipa/backup
+%attr(700,root,root) %dir %{_localstatedir}/lib/ipa/backup
 %attr(700,root,root) %dir %{_localstatedir}/lib/ipa/sysrestore
 %attr(700,root,root) %dir %{_localstatedir}/lib/ipa/sysupgrade
 %attr(755,root,root) %dir %{_localstatedir}/lib/ipa/pki-ca
@@ -794,6 +794,11 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
 
 %changelog
+* Mon Apr 15 2013 Martin Kosek mko...@redhat.com - 3.1.99-6
+- Require samba 4.0.5, includes new passdb API
+- Require krb5 1.11.2-1, fixes missing PAC issue
+- Change permissions on backup dir to 700
+
 * Fri Apr  5 2013 Rob Crittenden rcrit...@redhat.com - 3.1.99-5
 - Add backup and restore
 - Own /var/lib/ipa/backup
-- 
1.8.1.4

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

Re: [Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS

2013-04-16 Thread Ana Krivokapic
On 04/16/2013 03:54 PM, Martin Kosek wrote:
 On 04/16/2013 03:03 PM, Ana Krivokapic wrote:
 On 04/16/2013 01:55 PM, Martin Kosek wrote:
 On 04/16/2013 12:31 PM, Ana Krivokapic wrote:
 On 04/16/2013 09:14 AM, Martin Kosek wrote:
 On 04/15/2013 11:21 PM, Rob Crittenden wrote:
 Ana Krivokapic wrote:
 On 04/15/2013 07:06 PM, Martin Kosek wrote:
 On 04/15/2013 06:53 PM, Ana Krivokapic wrote:
 On 04/15/2013 06:30 PM, Martin Kosek wrote:
 On 04/12/2013 08:45 PM, Ana Krivokapic wrote:
 On 04/12/2013 01:26 PM, Ana Krivokapic wrote:
 On 04/12/2013 12:44 PM, Martin Kosek wrote:
 On 04/12/2013 12:20 PM, Ana Krivokapic wrote:
 On 04/11/2013 03:03 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Ana Krivokapic wrote:
 On 04/11/2013 01:43 PM, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:24, Alexander Bokovoy wrote:
 On Thu, 11 Apr 2013, Petr Spacek wrote:
 On 11.4.2013 13:09, Ana Krivokapic wrote:
 Integrate realmdomains with IPA DNS

 Add an entry to realmdomains when a DNS zone is added to 
 IPA.
 Delete the
 related entry from  realmdomains when the DNS zone is 
 deleted from
 IPA.

 https://fedorahosted.org/freeipa/ticket/3544
 I would add a TXT record as I described in
 https://fedorahosted.org/freeipa/ticket/3544#comment:8

 This integration probably should go to both commands,
 realmdomains-*
 dnszone-*.

 Any objections? AB?
 Adding TXT record is probably harmless.

 I would actually add the TXT record creation only to
 realmdomains-* and
 trigger it only in case we manage our DNS and DNS zone is 
 there.
 This way a hook from dnszone-add will trigger adding TXT 
 record back
 (via call to
 realmdomains-mod --add and then TXT record addition from 
 there).
 Also
 the fact that admin added manually some domain to 
 realmdomains
 mapping
 means that it is implied to be used in obtaining TGTs, so 
 TXT
 record is
 helpful there as well.
 Okay, it makes sense. We will see how it will work in 
 reality.
 One more thing to check is that we don't do this for our own 
 domain.

 Our own domain is already in realmdomains by default, and it 
 cannot be
 removed from there. So I don't think any check related to our 
 domain is
 necessary.
 We shouldn't start creating TXT records for our own domain, 
 that's what
 I'm asking for here.

 Think about server install stage -- we start creating our own 
 domain and
 the hook then causes to create realmdomains entry for the 
 domain,
 causing realmdomains-mod code to raise ValidationError which is 
 not
 handled in dnszone-add code with this patch.

 Same for TXT record creation starting from realmdomains-mod 
 side -- it
 simply should avoid calling dnsrecord-add for the case we know 
 wouldn't
 work.

 I just realized that this ticket was not marked as RFE although 
 it
 obviously is
 one. I fixed the ticket summary and wrote the design page for 
 this
 enhancement:

 http://www.freeipa.org/page/V3/DNS_realmdomains_integration

 Right, that was a good thing to do. I just have comment for the 
 UPN
 enumeration
 image which you linked in the RFE - can you please process it, 
 upload
 to the
 wiki and include in the overview? This will make the RFE page more
 appealing
 and it will also prevent us from having a broken link when 
 Alexander
 removes
 the file from his temporary directory.

 Thanks,
 Martin
 Sure, done.

 I added the functionality to create TXT record to realmdomains-mod, 
 and also
 made sure that the case of our own domain is handled properly. Unit 
 tests
 have
 been added to cover the new functionality. One unit test of the dns 
 plugin
 needed adjusting, but it still fails due to the bug in the testing
 framework[1]. It should pass after the bug is fixed.

 Updated patch is attached.

 [1] https://fedorahosted.org/freeipa/ticket/3562

 This looks nice, thanks for the new test cases.

 I experienced an issue with dnsrecord-find test in
 tests/test_xmlrpc/test_dns_plugin.py, but I see you already have an 
 open
 ticket
 to fix that (https://fedorahosted.org/freeipa/ticket/3562) so it is 
 not a
 show-stopper.

 This is a nitpick, but could you update
 tests/test_xmlrpc/test_dns_realmdomains_integration.py to use the 
 same
 domains
 for testing as tests/test_xmlrpc/test_dns_plugin.py does?

 I often use example*.com zones in my testing and we also advertise 
 test
 commands with these zones in ipa help dns too, so I (and maybe 
 others)
 could
 get surprised that these zones are deleted after running the test 
 suite.
 I.e. I
 would prefer to have dnszone*.test used for test.

 Thanks,
 Martin
 Sure. Updated patch attached.

 One more nitpick (sorry for not spotting it earlier). In 
 realmdomains-mod, you
 do checks for zone/record before you do the dnsrecord-add/dnsrecord-del
 command.

 I think this will unnecessarily make the command slower. You can just 
 try
 add/delete a record and catch also a NotFound error - these commands 
 already
 check for zone/record existence, so we do not need to do the checks 
 twice.

 

Re: [Freeipa-devel] [PATCH 0044] Update only selected attributes for winsync agreement

2013-04-16 Thread Rob Crittenden

Tomas Babej wrote:

On 04/15/2013 11:58 PM, Rob Crittenden wrote:

Tomas Babej wrote:

On 04/09/2013 11:47 PM, Rob Crittenden wrote:

Tomas Babej wrote:

Hi,

Trying to insert nsDS5ReplicatedAttributeListTotal and
nsds5ReplicaStripAttrs to winsync agreements caused upgrade errors.
With this patch, these attributes are skipped for winsync agreements.

Made find_ipa_replication_agreements() in replication.py more
corresponding to find_replication_agreements. It returns list of
entries instead of unicode strings now.

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

Tomas


This will still do some work against a winsync agreement. Do we need
to do that at all? I'm not sure we do.

rob


I removed the nsds5replicahost attribute update for winsync agreements
after discussion.

Updated patch attached.

Tomas


This looks ok. The backup/restore patch added two more calls to
find_ipa_replication_agreements so a rebase is needed. I think these
are the required changes:

diff --git a/ipaserver/install/ipa_restore.py
b/ipaserver/install/ipa_restore.py
index 04d4210..760da0b 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -373,7 +373,10 @@ class Restore(admintool.AdminTool):

 services_cns = [s.single_value('cn') for s in services]

-hosts = repl.find_ipa_replication_agreements()
+host_entries = repl.find_ipa_replication_agreements()
+hosts = [rep.single_value('nsds5replicahost', None)
+ for rep in host_entries]
+
 for host in hosts:
 self.log.info('Disabling replication agreement on %s
to %s' % (
master, host))
 repl.disable_agreement(host)
@@ -385,7 +388,9 @@ class Restore(admintool.AdminTool):
 except Exception, e:
 self.log.critical(Unable to disable agreement on
%s: %s %
 (master, e))

-hosts = repl.find_ipa_replication_agreements()
+host_entries = repl.find_ipa_replication_agreements()
+hosts = [rep.single_value('nsds5replicahost', None)
+ for rep in host_entries]
 for host in hosts:
 self.log.info('Disabling CA replication agreement
on %s to
%s' % (master, host))
 repl.hostnames = [master, host]



I added the calls and rebased the patch. I also found one missed call in
ipa-replica-csmanage.

Updated patch attached.


ACK, pushed to master

rob

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


Re: [Freeipa-devel] [PATCH] 401 Require new samba and krb5

2013-04-16 Thread Petr Viktorin

On 04/16/2013 01:16 PM, Martin Kosek wrote:

Require samba 4.0.5 (passdb API changed). Make sure that we use the
right epoch number with samba so that the Requires is correctly
enforced.

Require krb5 1.11.2-1 to fix missing PAC issue.

---

This patch makes sure we have the right dependencies in Fedora 19 (and Fedora
18 too for the samba one).

Martin


I've tested on f19 with Kerberos from Koji, and got the following test 
failure in test_cmdline/test_ipagetkeytab.py, test_2_run:


==
FAIL: Create a keytab with `ipa-getkeytab` for an existing service.
--
Traceback (most recent call last):
  File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in 
runTest

self.test(*self.arg)
  File 
/home/pviktori/freeipa/tests/test_cmdline/test_ipagetkeytab.py, line 
110, in test_2_run
assert err == 'Keytab successfully retrieved and stored in: %s\n' % 
self.keytabname

AssertionError


The command works, the test fails because it doesn't expect warnings 
about Camellia on stderr. I assume they're benign?


Failed to retrieve encryption type Camellia-128 CTS mode with CMAC (#25)
Failed to retrieve encryption type Camellia-256 CTS mode with CMAC (#26)
Keytab successfully retrieved and stored in: /tmp/tmpvLHm7l



On f18 my smoke testing is going fine.

--
PetrĀ³

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


Re: [Freeipa-devel] [PATCH] 401 Require new samba and krb5

2013-04-16 Thread Rob Crittenden

Petr Viktorin wrote:

On 04/16/2013 01:16 PM, Martin Kosek wrote:

Require samba 4.0.5 (passdb API changed). Make sure that we use the
right epoch number with samba so that the Requires is correctly
enforced.

Require krb5 1.11.2-1 to fix missing PAC issue.

---

This patch makes sure we have the right dependencies in Fedora 19 (and
Fedora
18 too for the samba one).

Martin


I've tested on f19 with Kerberos from Koji, and got the following test
failure in test_cmdline/test_ipagetkeytab.py, test_2_run:

==
FAIL: Create a keytab with `ipa-getkeytab` for an existing service.
--
Traceback (most recent call last):
   File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in
runTest
 self.test(*self.arg)
   File
/home/pviktori/freeipa/tests/test_cmdline/test_ipagetkeytab.py, line
110, in test_2_run
 assert err == 'Keytab successfully retrieved and stored in: %s\n' %
self.keytabname
AssertionError


The command works, the test fails because it doesn't expect warnings
about Camellia on stderr. I assume they're benign?

Failed to retrieve encryption type Camellia-128 CTS mode with CMAC (#25)
Failed to retrieve encryption type Camellia-256 CTS mode with CMAC (#26)
Keytab successfully retrieved and stored in: /tmp/tmpvLHm7l



On f18 my smoke testing is going fine.



Yes, looks like new ciphers were added that we don't have enabled by 
default in IPA.


The patch looks ok to me, ACK.

rob

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


Re: [Freeipa-devel] [PATCH] 401 Require new samba and krb5

2013-04-16 Thread Rob Crittenden

Rob Crittenden wrote:

Petr Viktorin wrote:

On 04/16/2013 01:16 PM, Martin Kosek wrote:

Require samba 4.0.5 (passdb API changed). Make sure that we use the
right epoch number with samba so that the Requires is correctly
enforced.

Require krb5 1.11.2-1 to fix missing PAC issue.

---

This patch makes sure we have the right dependencies in Fedora 19 (and
Fedora
18 too for the samba one).

Martin


I've tested on f19 with Kerberos from Koji, and got the following test
failure in test_cmdline/test_ipagetkeytab.py, test_2_run:

==
FAIL: Create a keytab with `ipa-getkeytab` for an existing service.
--
Traceback (most recent call last):
   File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in
runTest
 self.test(*self.arg)
   File
/home/pviktori/freeipa/tests/test_cmdline/test_ipagetkeytab.py, line
110, in test_2_run
 assert err == 'Keytab successfully retrieved and stored in: %s\n' %
self.keytabname
AssertionError


The command works, the test fails because it doesn't expect warnings
about Camellia on stderr. I assume they're benign?

Failed to retrieve encryption type Camellia-128 CTS mode with CMAC (#25)
Failed to retrieve encryption type Camellia-256 CTS mode with CMAC (#26)
Keytab successfully retrieved and stored in: /tmp/tmpvLHm7l



On f18 my smoke testing is going fine.



Yes, looks like new ciphers were added that we don't have enabled by
default in IPA.

The patch looks ok to me, ACK.


pushed to master

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