Re: [Freeipa-devel] [PATCH] bind-dyndb-ldap: enable/disable PTR synchronization per zone

2011-12-07 Thread Jiri Kuncar
> On Thu, Dec 01, 2011 at 09:00:18AM -0500, Jiri Kuncar wrote:
> > I've added an attribute "idnsAllowSyncPTR" to "idnsZone" to enable
> > or disable synchronization of PTR records. However the
> > bind-dyndb-ldap plugin option "sync_ptr" has to be included in
> > /etc/named.conf to run synchronization feature.
> 
> Hello,
> 
> This doesn't seem too user-friendly for me. In my opinion better is
> to allow PTR
> synchronization when sync_ptr is "no" and idnsAllowSyncPTR is set to
> "TRUE". So
> admin can either set sync_ptr to allow updates for all zones or set
> per-zone
> idnsAllowSyncPTR attr.

I agree. Please check my corrections and comments inside the patch.

Regards, 
Jiri
From d52e4c9db6e9f6f75264295044d184ca3a768a2a Mon Sep 17 00:00:00 2001
From: Jiri Kuncar 
Date: Thu, 1 Dec 2011 08:32:34 -0500
Subject: [PATCH] Enable/disable PTR synchronization per zone. Class
 idnsRecord has new attribute idnsAllowSyncPTR (BOOLEAN).
 Changed sync_ptr option behavior as follows: - "yes" always
 sync PTR records; - "no" first check "idnsAllowSyncPTR"
 attribute in a zone.

Signed-off-by: Jiri Kuncar 
---
 README |9 ++---
 src/ldap_convert.c |4 
 src/ldap_helper.c  |   51 ++-
 3 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/README b/README
index b28fd32..3ee0b40 100644
--- a/README
+++ b/README
@@ -192,9 +192,12 @@ ldap_hostname (default "")
 	/bin/hostname output.
 
 sync_ptr (default no)
-	Keeps PTR record synchronized with coresponding A/ record.
-	When an A/ record is deleted the PTR record must point 
-	to the same hostname.
+	Set this option to "yes" if you would like to keep PTR record 
+	synchronized with coresponding A/ record for all zones.
+	If this option is set to "no", the LDAP driver will check
+	the idnsAllowSyncPTR attribute which specifies the synchronization
+	policy for PTR records in a zone. When an A/ record is deleted 
+	the PTR record must point to the same hostname. 
 
 5.2 Sample configuration
 
diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index 85b572e..b5cf4cf 100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -227,6 +227,10 @@ dnsname_to_dn(zone_register_t *zr, dns_name_t *name, ld_string_t *target)
 
 		CHECK(str_cat_char(target, "idnsName="));
 		CHECK(str_cat_isc_buffer(target, &buffer));
+		/* 
+		 * Modification of following line can affect modify_ldap_common().
+		 * See line with: char *zone_dn = strstr(str_buf(owner_dn),", ") + 1;  
+		 */
 		CHECK(str_cat_char(target, ", "));
 	}
 	CHECK(str_cat_char(target, zone_dn));
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index b60cf11..8a10068 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1,6 +1,7 @@
 /*
  * Authors: Martin Nagy 
  *  Adam Tkac 
+ *  Jiri Kuncar 
  *
  * Copyright (C) 2008, 2009  Red Hat
  * see file 'COPYING' for use and warranty information
@@ -1798,19 +1799,58 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
 	CHECK(ldap_modify_do(ldap_conn, str_buf(owner_dn), change, delete_node));
 
 	/* Keep the PTR of corresponding A/ record synchronized. */
-	if (ldap_inst->sync_ptr == ISC_TRUE && 
-	(rdlist->type == dns_rdatatype_a || rdlist->type == dns_rdatatype_)) {
+	if (rdlist->type == dns_rdatatype_a || rdlist->type == dns_rdatatype_) {
 		
+		/* Look for zone "idnsAllowSyncPTR" attribute when plugin 
+		 * option "sync_ptr" is set to "no" otherwise the synchronization
+		 * is always enabled for all zones. */
+		if (ldap_inst->sync_ptr == ISC_FALSE) {
+			/* 
+			 * Find parent zone entry.
+			 * @todo Try the cache first and improve split.
+			 */
+			char *zone_dn = strstr(str_buf(owner_dn),", ") + 1;
+			ldap_entry_t *entry;
+			ldap_valuelist_t values;
+			char *attrs[] = {"idnsAllowSyncPTR", NULL};
+			
+			CHECK(ldap_query(ldap_inst, ldap_conn, zone_dn,
+			 LDAP_SCOPE_BASE, attrs, 0,
+			 "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
+			
+			/* Search for zone entry with 'idnsAllowSyncPTR == "TRUE"'. */
+			for (entry = HEAD(ldap_conn->ldap_entries);
+ entry != NULL;
+ entry = NEXT(entry, link)) {
+result = ldap_entry_getvalues(entry, "idnsAllowSyncPTR", &values);
+if (result != ISC_R_SUCCESS) 
+	continue;
+
+if (strcmp(HEAD(values)->value, "TRUE") != 0) {
+	entry = NULL;
+}
+break;
+			}
+			/* Any valid zone was found. */
+			if (entry == NULL) {
+log_debug(3, "Sync PTR is not allowed in zone %s", zone_dn);
+goto cleanup;
+			}
+			log_debug(3, "Sync PTR is allowed for zone %s", zone_dn);
+		}
+
 		/* Get string with IP address from change request
 		 * and convert it to in_addr structure. */
 		in_addr_t ip;
 		if ((ip = inet_addr(change[0]->mod_values[0])) == 0) {
-			log_bug("Could not convert IP address from string '%s'.", change[0]->mod_values[0]);
+			log_bug("Could not convert IP address from string '%s'.",
+			change[0]->m

Re: [Freeipa-devel] [PATCH] bind-dyndb-ldap: enable/disable PTR synchronization per zone

2011-12-02 Thread Adam Tkac
On Thu, Dec 01, 2011 at 09:00:18AM -0500, Jiri Kuncar wrote:
> I've added an attribute "idnsAllowSyncPTR" to "idnsZone" to enable or disable 
> synchronization of PTR records. However the bind-dyndb-ldap plugin option 
> "sync_ptr" has to be included in /etc/named.conf to run synchronization 
> feature.

Hello,

This doesn't seem too user-friendly for me. In my opinion better is to allow PTR
synchronization when sync_ptr is "no" and idnsAllowSyncPTR is set to "TRUE". So
admin can either set sync_ptr to allow updates for all zones or set per-zone
idnsAllowSyncPTR attr.

Please check my comments inside the patch.

> My quick fix of LDAP schema in /usr/share/ipa/60basev2.ldif:
> -
> attributeTypes: (2.16.840.1.113730.3.8.5.11 NAME 'idnsAllowSyncPTR' DESC 
> 'permit synchronization of PTR records' EQUALITY booleanMatch SYNTAX 
> 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v2' )
> ...
> objectClasses: (2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' 
> SUP idnsRecord STRUCTURAL MUST ( idnsName $ idnsZoneActive $ idnsSOAmName $ 
> idnsSOArName $ idnsSOAserial $ idnsSOArefresh $ idnsSOAretry $ idnsSOAexpire 
> $ idnsSOAminimum ) MAY ( idnsUpdatePolicy $ idnsAllowSyncPTR ) )
> -
> 
> https://fedorahosted.org/bind-dyndb-ldap/ticket/39
> 
> Jiri

> From b1aaef1c05be1a16115eb30bb76a0d1795806bc7 Mon Sep 17 00:00:00 2001
> From: Jiri Kuncar 
> Date: Thu, 1 Dec 2011 08:32:34 -0500
> Subject: [PATCH] Enable/disable PTR synchronization per zone. Class
>  idnsRecord has new attribute idnsAllowSyncPTR (BOOLEAN).
> 
> Signed-off-by: Jiri Kuncar 
> ---
>  src/ldap_helper.c |   32 
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index b60cf11..4530155 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -1801,6 +1801,38 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t 
> *ldap_inst,
>   if (ldap_inst->sync_ptr == ISC_TRUE && 
>   (rdlist->type == dns_rdatatype_a || rdlist->type == 
> dns_rdatatype_)) {
>   
> + /* 
> +  * Find parent zone entry.
> +  * @todo Try the cache first and improve split.
> +  */
> + char *zone_dn = strstr(str_buf(owner_dn),", ") + 1;

Since this strstr() rule depends on current src/ldap_convert.c:dnsname_to_dn()
behavior, please add note to dnsname_to_dn() that modification of the

CHECK(str_cat_char(target, ", "));

line could affect this part of code.

> + ldap_entry_t *entry;
> + ldap_valuelist_t values;
> + char *attrs[] = {"idnsAllowSyncPTR", NULL};
> + 
> + CHECK(ldap_query(ldap_inst, ldap_conn, zone_dn,
> +  LDAP_SCOPE_BASE, attrs, 0,
> +  
> "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
> + 
> + /* Search for zone entry with 'idnsAllowSyncPTR == "TRUE"'. */
> + for (entry = HEAD(ldap_conn->ldap_entries);
> +  entry != NULL;
> +  entry = NEXT(entry, link)) {
> + result = ldap_entry_getvalues(entry, 
> "idnsAllowSyncPTR", &values);
> + if (result != ISC_R_SUCCESS) 
> + continue;
> +
> + if (strcmp(HEAD(values)->value, "TRUE") != 0) {
> + entry = NULL;
> + }
> + break;
> + }
> + /* Any valid zone was found. */
> + if (entry == NULL) {
> + log_error("Sync PTR is not allowed in zone %s", 
> zone_dn);

idnsAllowSyncPTR set to "FALSE" (or missing idnsAllowSyncPTR attr) shouldn't be
treated as error. Please degrade this msg to

log_debug(3, "...")

for example.

> + goto cleanup;
> + }

It would be nice to receive debug msg when idnsAllowSyncPTR is allowed. What
about

log_debug(3, "Sync PTR is allowed for zone %s", zone_dn);

Regards, Adam

-- 
Adam Tkac, Red Hat, Inc.

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


Re: [Freeipa-devel] [PATCH] bind-dyndb-ldap: enable/disable PTR synchronization per zone

2011-12-01 Thread Simo Sorce
On Thu, 2011-12-01 at 10:50 -0800, Nathan Kinder wrote:
> On 12/01/2011 06:27 AM, Simo Sorce wrote:
> > On Thu, 2011-12-01 at 09:00 -0500, Jiri Kuncar wrote:
> >> I've added an attribute "idnsAllowSyncPTR" to "idnsZone" to enable or
> >> disable synchronization of PTR records. However the bind-dyndb-ldap
> >> plugin option "sync_ptr" has to be included in /etc/named.conf to run
> >> synchronization feature.
> > We need an update script to run on ipa server at upgrade time then.
> >
> >> My quick fix of LDAP schema in /usr/share/ipa/60basev2.ldif:
> > The DNS schema objects are in 60ipadns.ldif
> >
> >> -
> >> attributeTypes: (2.16.840.1.113730.3.8.5.11 NAME 'idnsAllowSyncPTR'
> >> DESC 'permit synchronization of PTR records' EQUALITY booleanMatch
> >> SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v2' )
> > NACK.
> > 5.11 is reserved by idnsAllowQuery and 5.12 by idnsAllowTransfer. The
> > first available OID is 5.13

> Do you have a page for tracking OID allocation within the FreeIPA 
> namespace?  If so, we should be sure to consult it to choose the next 
> available OID and to update it once we have the final patch for this issue.

We have one place within Red Hat where we also keep track of all 389ds
OIDs that's how I know there is a conflict here.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

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


Re: [Freeipa-devel] [PATCH] bind-dyndb-ldap: enable/disable PTR synchronization per zone

2011-12-01 Thread Nathan Kinder

On 12/01/2011 06:27 AM, Simo Sorce wrote:

On Thu, 2011-12-01 at 09:00 -0500, Jiri Kuncar wrote:

I've added an attribute "idnsAllowSyncPTR" to "idnsZone" to enable or
disable synchronization of PTR records. However the bind-dyndb-ldap
plugin option "sync_ptr" has to be included in /etc/named.conf to run
synchronization feature.

We need an update script to run on ipa server at upgrade time then.


My quick fix of LDAP schema in /usr/share/ipa/60basev2.ldif:

The DNS schema objects are in 60ipadns.ldif


-
attributeTypes: (2.16.840.1.113730.3.8.5.11 NAME 'idnsAllowSyncPTR'
DESC 'permit synchronization of PTR records' EQUALITY booleanMatch
SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v2' )

NACK.
5.11 is reserved by idnsAllowQuery and 5.12 by idnsAllowTransfer. The
first available OID is 5.13
Do you have a page for tracking OID allocation within the FreeIPA 
namespace?  If so, we should be sure to consult it to choose the next 
available OID and to update it once we have the final patch for this issue.



objectClasses: (2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone
class' SUP idnsRecord STRUCTURAL MUST ( idnsName $ idnsZoneActive $
idnsSOAmName $ idnsSOArName $ idnsSOAserial $ idnsSOArefresh $
idnsSOAretry $ idnsSOAexpire $ idnsSOAminimum ) MAY ( idnsUpdatePolicy
$ idnsAllowSyncPTR ) )

These changes need to be committed to the right file and a .update is
also needed.


https://fedorahosted.org/bind-dyndb-ldap/ticket/39


Need some more work but looks promising.
Simo.



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


Re: [Freeipa-devel] [PATCH] bind-dyndb-ldap: enable/disable PTR synchronization per zone

2011-12-01 Thread Simo Sorce
On Thu, 2011-12-01 at 09:00 -0500, Jiri Kuncar wrote:
> I've added an attribute "idnsAllowSyncPTR" to "idnsZone" to enable or
> disable synchronization of PTR records. However the bind-dyndb-ldap
> plugin option "sync_ptr" has to be included in /etc/named.conf to run
> synchronization feature.

We need an update script to run on ipa server at upgrade time then.

> My quick fix of LDAP schema in /usr/share/ipa/60basev2.ldif:

The DNS schema objects are in 60ipadns.ldif

> -
> attributeTypes: (2.16.840.1.113730.3.8.5.11 NAME 'idnsAllowSyncPTR'
> DESC 'permit synchronization of PTR records' EQUALITY booleanMatch
> SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v2' )

NACK.
5.11 is reserved by idnsAllowQuery and 5.12 by idnsAllowTransfer. The
first available OID is 5.13

> objectClasses: (2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone
> class' SUP idnsRecord STRUCTURAL MUST ( idnsName $ idnsZoneActive $
> idnsSOAmName $ idnsSOArName $ idnsSOAserial $ idnsSOArefresh $
> idnsSOAretry $ idnsSOAexpire $ idnsSOAminimum ) MAY ( idnsUpdatePolicy
> $ idnsAllowSyncPTR ) )

These changes need to be committed to the right file and a .update is
also needed.

> https://fedorahosted.org/bind-dyndb-ldap/ticket/39
> 

Need some more work but looks promising.
Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

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


[Freeipa-devel] [PATCH] bind-dyndb-ldap: enable/disable PTR synchronization per zone

2011-12-01 Thread Jiri Kuncar
I've added an attribute "idnsAllowSyncPTR" to "idnsZone" to enable or disable 
synchronization of PTR records. However the bind-dyndb-ldap plugin option 
"sync_ptr" has to be included in /etc/named.conf to run synchronization feature.

My quick fix of LDAP schema in /usr/share/ipa/60basev2.ldif:
-
attributeTypes: (2.16.840.1.113730.3.8.5.11 NAME 'idnsAllowSyncPTR' DESC 
'permit synchronization of PTR records' EQUALITY booleanMatch SYNTAX 
1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v2' )
...
objectClasses: (2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' SUP 
idnsRecord STRUCTURAL MUST ( idnsName $ idnsZoneActive $ idnsSOAmName $ 
idnsSOArName $ idnsSOAserial $ idnsSOArefresh $ idnsSOAretry $ idnsSOAexpire $ 
idnsSOAminimum ) MAY ( idnsUpdatePolicy $ idnsAllowSyncPTR ) )
-

https://fedorahosted.org/bind-dyndb-ldap/ticket/39

JiriFrom b1aaef1c05be1a16115eb30bb76a0d1795806bc7 Mon Sep 17 00:00:00 2001
From: Jiri Kuncar 
Date: Thu, 1 Dec 2011 08:32:34 -0500
Subject: [PATCH] Enable/disable PTR synchronization per zone. Class
 idnsRecord has new attribute idnsAllowSyncPTR (BOOLEAN).

Signed-off-by: Jiri Kuncar 
---
 src/ldap_helper.c |   32 
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index b60cf11..4530155 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1801,6 +1801,38 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
 	if (ldap_inst->sync_ptr == ISC_TRUE && 
 	(rdlist->type == dns_rdatatype_a || rdlist->type == dns_rdatatype_)) {
 		
+		/* 
+		 * Find parent zone entry.
+		 * @todo Try the cache first and improve split.
+		 */
+		char *zone_dn = strstr(str_buf(owner_dn),", ") + 1;
+		ldap_entry_t *entry;
+		ldap_valuelist_t values;
+		char *attrs[] = {"idnsAllowSyncPTR", NULL};
+		
+		CHECK(ldap_query(ldap_inst, ldap_conn, zone_dn,
+		 LDAP_SCOPE_BASE, attrs, 0,
+		 "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
+		
+		/* Search for zone entry with 'idnsAllowSyncPTR == "TRUE"'. */
+		for (entry = HEAD(ldap_conn->ldap_entries);
+			 entry != NULL;
+			 entry = NEXT(entry, link)) {
+			result = ldap_entry_getvalues(entry, "idnsAllowSyncPTR", &values);
+			if (result != ISC_R_SUCCESS) 
+continue;
+
+			if (strcmp(HEAD(values)->value, "TRUE") != 0) {
+entry = NULL;
+			}
+			break;
+		}
+		/* Any valid zone was found. */
+		if (entry == NULL) {
+			log_error("Sync PTR is not allowed in zone %s", zone_dn);
+			goto cleanup;
+		}
+
 		/* Get string with IP address from change request
 		 * and convert it to in_addr structure. */
 		in_addr_t ip;
-- 
1.7.7.1

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