Re: [Freeipa-devel] [PATCH] 376-377 Use tkey-gssapi-keytab in named.conf

2013-03-11 Thread Martin Kosek

On 03/08/2013 09:49 AM, Petr Spacek wrote:

On 8.3.2013 00:14, Rob Crittenden wrote:

Martin Kosek wrote:

Remove obsolete BIND GSSAPI configuration options tkey-gssapi-credential
and tkey-domain and replace them with tkey-gssapi-keytab which avoids
unnecessary Kerberos checks on BIND startup and can cause issues when
KDC is not available.

Both new and current IPA installations are updated.

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



Still reviewing this but I noticed that after upgrading my 3.1.99 server
pre-patch to with with-patch version the connections argument in named.conf
got set to 4 (courtesy of ipa-upgradeconfig). Should we be setting that to 4
during the initial install too?


For 3.2 it doesn't matter. Anything >= 2 should be okay, but more connections
should not harm.

Higher value should allow higher level of parallelism, it is one of tuning
parameters. Value 4 was necessary to prevent deadlocks in some previous
versions of bind-dyndb-ldap.



Previously, when I implemented the upgrade script, I set connections arg only 
if it was present in named.conf and thus bind-dyndb-ldap could not use a 
reasonable default on its own decision.


This was changed in e578183ea25a40aedf6dcc3e1ee4bcb19b73e70f and connections is 
set always. Rob is correct, that in that case we might want to add it to 
named.conf by default to make it consistent... or we could also fix upgrade 
script to change connections only if it is present in named.conf.


Petr, what does make more sense bind-dyndb-ldap wise?

Thanks,
Martin

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


Re: [Freeipa-devel] [PATCH] 376-377 Use tkey-gssapi-keytab in named.conf

2013-03-11 Thread Petr Spacek

On 11.3.2013 09:09, Martin Kosek wrote:

On 03/08/2013 09:49 AM, Petr Spacek wrote:

On 8.3.2013 00:14, Rob Crittenden wrote:

Martin Kosek wrote:

Remove obsolete BIND GSSAPI configuration options tkey-gssapi-credential
and tkey-domain and replace them with tkey-gssapi-keytab which avoids
unnecessary Kerberos checks on BIND startup and can cause issues when
KDC is not available.

Both new and current IPA installations are updated.

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



Still reviewing this but I noticed that after upgrading my 3.1.99 server
pre-patch to with with-patch version the connections argument in named.conf
got set to 4 (courtesy of ipa-upgradeconfig). Should we be setting that to 4
during the initial install too?


For 3.2 it doesn't matter. Anything >= 2 should be okay, but more connections
should not harm.

Higher value should allow higher level of parallelism, it is one of tuning
parameters. Value 4 was necessary to prevent deadlocks in some previous
versions of bind-dyndb-ldap.



Previously, when I implemented the upgrade script, I set connections arg only
if it was present in named.conf and thus bind-dyndb-ldap could not use a
reasonable default on its own decision.

This was changed in e578183ea25a40aedf6dcc3e1ee4bcb19b73e70f and connections
is set always. Rob is correct, that in that case we might want to add it to
named.conf by default to make it consistent... or we could also fix upgrade
script to change connections only if it is present in named.conf.

Petr, what does make more sense bind-dyndb-ldap wise?


Default values should work. Personally I would include only "override" values 
in named.conf, but technically it doesn't matter.


Note: Latest bind-dyndb-ldap versions refuse to start if configuration is 
insane. Fatal error will point admin to errors in configuration and should 
prevent surprises from auto-magically changed values.


Invalid configurations - examples:
connections < 1
connections < 2 && psearch is enabled
serial_autoincrement enabled && psearch disabled

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH] 378-380 Improved CNAME and DNAME validation

2013-03-11 Thread Martin Kosek

On 03/06/2013 01:07 PM, Petr Spacek wrote:

On 6.3.2013 09:32, Martin Kosek wrote:

+error=u'CNAME record is not allowed to coexist with any
other record'),


Sorry for nitpicking again, but I would add note '(RFC 1034, section 3.6.2)'.

Thank you!



Fixed.

Martin
From f909d23b6121f9b5880ae1ccb8e9d81a1bfe5d56 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Mon, 4 Mar 2013 12:48:05 +0100
Subject: [PATCH 1/3] Change CNAME and DNAME attributes to single valued

These DNS attributeTypes are of a singleton type, update LDAP schema
to reflect it.

https://fedorahosted.org/freeipa/ticket/3440
https://fedorahosted.org/freeipa/ticket/3450
---
 install/share/60ipadns.ldif   | 4 ++--
 install/updates/10-bind-schema.update | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/install/share/60ipadns.ldif b/install/share/60ipadns.ldif
index 9697227fb7166b3711568ddea3e5c345277befa3..6293385d62ce10dd3020ad291a947ff0f0d67c6e 100644
--- a/install/share/60ipadns.ldif
+++ b/install/share/60ipadns.ldif
@@ -21,14 +21,14 @@ attributeTypes: (1.3.6.1.4.1.2428.20.1.35 NAME 'nAPTRRecord' DESC 'Naming Author
 attributeTypes: (1.3.6.1.4.1.2428.20.1.36 NAME 'kXRecord' DESC 'Key Exchange Delegation, RFC 2230' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.37 NAME 'certRecord' DESC 'certificate, RFC 2538' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.38 NAME 'a6Record' DESC 'A6 Record Type, RFC 2874' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
-attributeTypes: (1.3.6.1.4.1.2428.20.1.39 NAME 'dNameRecord' DESC 'Non-Terminal DNS Name Redirection, RFC 2672' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+attributeTypes: (1.3.6.1.4.1.2428.20.1.39 NAME 'dNameRecord' DESC 'Non-Terminal DNS Name Redirection, RFC 2672' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.43 NAME 'dSRecord' DESC 'Delegation Signer, RFC 3658' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.44 NAME 'sSHFPRecord' DESC 'SSH Key Fingerprint, draft-ietf-secsh-dns-05.txt' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.46 NAME 'rRSIGRecord' DESC 'RRSIG, RFC 3755' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (1.3.6.1.4.1.2428.20.1.47 NAME 'nSECRecord' DESC 'NSEC, RFC 3755' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (0.9.2342.19200300.100.1.26 NAME 'aRecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (0.9.2342.19200300.100.1.29 NAME 'nSRecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
-attributeTypes: (0.9.2342.19200300.100.1.31 NAME 'cNAMERecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+attributeTypes: (0.9.2342.19200300.100.1.31 NAME 'cNAMERecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE )
 attributeTypes: (0.9.2342.19200300.100.1.28 NAME 'mXRecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (0.9.2342.19200300.100.1.27 NAME 'mDRecord' EQUALITY caseIgnoreIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
 attributeTypes: (2.16.840.1.113730.3.8.5.0 NAME 'idnsName' DESC 'DNS FQDN' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE X-ORIGIN 'IPA v2' )
diff --git a/install/updates/10-bind-schema.update b/install/updates/10-bind-schema.update
index 3c43c8ec79fe6cb9830a27fb2880b6ed0cf0d8e4..cbe7a672b5300d5b945bf996a596909008dda5aa 100644
--- a/install/updates/10-bind-schema.update
+++ b/install/updates/10-bind-schema.update
@@ -78,3 +78,5 @@ add:objectClasses:
 
 dn: cn=schema
 replace:objectClasses:( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' SUP idnsRecord STRUCTURAL MUST ( idnsZoneActive $$ idnsSOAmName $$ idnsSOArName $$ idnsSOAserial $$ idnsSOArefresh $$ idnsSOAretry $$ idnsSOAexpire $$ idnsSOAminimum ) MAY idnsUpdatePolicy )::( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' SUP idnsRecordSTRUCTURAL MUST ( idnsName $$ idnsZoneActive $$ idnsSOAmName $$ idnsSOArName $$ idnsSOAserial $$ idnsSOArefresh $$ idnsSOAretry $$ idnsSOAexpire $$ idnsSOAminimum ) MAY ( idnsUpdatePolicy $$ idnsAllowQuery $$ idnsAllowTransfer $$ idnsAllowSyncPTR $$ idnsForwardPolicy $$ idnsForwarders ) )
+replace:attributeTypes:"(1.3.6.1.4.1.2428.20.1.39 NAME

Re: [Freeipa-devel] [PATCH] 381 Preserve order of servers in ipa-client-install

2013-03-11 Thread Martin Kosek

On 03/07/2013 03:07 PM, Petr Viktorin wrote:

On 03/07/2013 02:00 PM, Martin Kosek wrote:

When multiple servers are passed via --server option, ipadiscovery
module changed its order. Make sure that we preserve it.

Also make sure that user is always warned when a tested server is
not available as then the server will be excluded from the fixed
server list.


The message doesn't actually say that the server will be removed. It would be
nice if it did.

Otherwise, ACK.


Sending a patch with improved logging. User should now be more clear what 
server is failing to verify (and why).





--

When working on this ticket I was thinking - do we make the right thing we
deliberately remove a server from user-provided server list just because we
cannot connect to it at the moment if discovery? It may just be temporarily
down or something.

Maybe we should preserve the original --server list in this case and use this
list when writing krb5.conf or sssd.conf. Of course, for ipa-join or other
active configuration commands we would have to use only the valid servers so
that the we do not hit the server that is currently down.

Martin


Good point, this deserves a ticket.



Rob, do you think this deserves to be changed? Or is this behavior indeed 
intended?

Martin
From 2f33c8113e79311a28858bc55d3f7a534dfb920d Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Thu, 7 Mar 2013 13:54:11 +0100
Subject: [PATCH] Preserve order of servers in ipa-client-install

When multiple servers are passed via --server option, ipadiscovery
module changed its order. Make sure that we preserve it.

Also make sure that user is always warned when a tested server is
not available as then the server will be excluded from the fixed
server list. Log messages were made more informative so that user
knows which server is actually failing to be verified.

https://fedorahosted.org/freeipa/ticket/3418
---
 ipa-client/ipaclient/ipadiscovery.py | 33 -
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
index 7fc6aae88672e15a6bf3068ef8769e4cc80184a4..c4ec7323f54ae3d013c9e175c313f86b6f38ffc9 100644
--- a/ipa-client/ipaclient/ipadiscovery.py
+++ b/ipa-client/ipaclient/ipadiscovery.py
@@ -248,7 +248,7 @@ class IPADiscovery(object):
 self.realm = ldapret[2]
 self.server_source = self.realm_source = (
 'Discovered from LDAP DNS records in %s' % self.server)
-valid_servers.insert(0, server)
+valid_servers.append(server)
 # verified, we actually talked to the remote server and it
 # is definetely an IPA server
 verified_servers = True
@@ -258,7 +258,7 @@ class IPADiscovery(object):
 break
 elif ldapret[0] == NO_ACCESS_TO_LDAP or ldapret[0] == NO_TLS_LDAP:
 ldapaccess = False
-valid_servers.insert(0, server)
+valid_servers.append(server)
 # we may set verified_servers below, we don't have it yet
 if autodiscovered:
 # No need to keep verifying servers if we discovered them
@@ -266,11 +266,17 @@ class IPADiscovery(object):
 break
 elif ldapret[0] == NOT_IPA_SERVER:
 root_logger.warn(
-'%s (realm %s) is not an IPA server', server, self.realm)
+   '%s: not an IPA server. Removing from list of servers '
+   'to connect to', server)
 elif ldapret[0] == NO_LDAP_SERVER:
-root_logger.debug(
-'Unable to verify that %s (realm %s) is an IPA server',
-server, self.realm)
+root_logger.warn(
+   '%s: LDAP server is not responding, unable to verify if '
+   'this is an IPA server. Removing from list of servers '
+   'to connect to', server)
+else:
+root_logger.warn(
+   '%s: cannot verify if this is an IPA server. Removing '
+   'from list of servers to connect to', server)
 
 # If one of LDAP servers checked rejects access (maybe anonymous
 # bind is disabled), assume realm and basedn generated off domain.
@@ -344,7 +350,7 @@ class IPADiscovery(object):
 basedn = get_ipa_basedn(lh)
 
 if basedn is None:
-root_logger.debug("The server is not an IPA server")
+root_logger.debug("%s: The server is not an IPA server", thost)
 return [NOT_IPA_SERVER]
 
 self.basedn = basedn
@@ -384,25 +390,26 @@ class IPADiscovery(object):
 
 except LDAPError, err:
 if isinstance(err, ldap.TIMEOUT):
-root_logger.debug("LDAP Error: timeout")
+root_logger.debug("%

Re: [Freeipa-devel] [PATCH] 0006 Remove check for alphabetic only characters from domain name validation

2013-03-11 Thread Ana Krivokapic
On 02/27/2013 10:58 AM, Martin Kosek wrote:
> On 02/22/2013 04:02 PM, Ana Krivokapic wrote:
>> On 02/22/2013 10:19 AM, Petr Spacek wrote:
>>> On 20.2.2013 11:03, Ana Krivokapic wrote:
 On 02/18/2013 01:08 PM, Martin Kosek wrote:
> On 02/18/2013 12:47 PM, Sumit Bose wrote:
>> On Mon, Feb 18, 2013 at 12:27:35PM +0100, Petr Spacek wrote:
>>> On 15.2.2013 15:22, Ana Krivokapic wrote:
 Hello,

 The .isalpha() check in validate_domain_name() was too strict,
 causing some commands like ipa dnsrecord-add to fail.

 https://fedorahosted.org/freeipa/ticket/3385
>>> I would add --force option rather than removing whole check, if
>>> it's possible.
>>>
>>> Would it be possible to mention RFC in the error message? Something
>>> like _('top level domain label must be alphabetic (RFC 1123 section
>>> 2.1)')
>>> ?
>>>
>>> IMHO it is handy, because it educates users.
>> The problem is that this check is always done on the last component of
>> the domain_name even if it is just a sub-domain of the FreeIPA domain,
>> where e.g. numbers are valid characters.
>>
>> At the beginning of validate_domain_name() a trailing '.' is stripped
>> away. iirc the trailing '.' is an indication for a complete, fully
>> qualified name. Would it work if the presence of the trailing '.' is
>> saved and the check is only done if there was a '.'?
>>
>> bye,
>> Sumit
>>
> Sure. Though I am now not 100% sure that some IPA functions do not
> use this
> validator with a fqdn hostname without trailing dot. If not, I am
> for fixing
> this function as Sumit and Petr suggested.
>
> Martin
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
 After some thought, I decided to change the approach.

 As pointed out by Sumit, the problem was that the validate_domain_name()
 function did not distinguish between fqdn and non-fqdn domains
 (subdomains of the IPA domain). The trailing dot is not a clear
 indication either, because some IPA functions use this validator with an
 fqdn without the trailing dot.

 To fix this, I introduced an additional parameter to this function - a
 flag which indicates whether the domain name is an fqdn or not. The is
 .isalpha() check is then performed only in the case of an fqdn.

 I also improved the error message to mention the relevant RFC, as
 suggested by Petr.
>>> Please don't forget to add --force switch. It could be handy.
>>>
>> I added the --force switch to ipa dnsrecord-add and opened a new ticket
>> to handle the rest of the ipa commands that use domain name validation:
>> https://fedorahosted.org/freeipa/ticket/3455
>>
>> Updated patch is attached.
>>
> This patch fixed validation only partially. The --force flag you made 
> available
> will not allow admin to for example add a zone "example.zone1" which
> technically will be resolvable, it is just not a good practice:
>
> # ipa dnszone-add example.zone1 --name-server `hostname`. --force
> ipa: ERROR: invalid 'name': top level domain label must be alphabetic (RFC 
> 1123
> section 2.1)
>
> To enable this, I think you would need to not postpone the validation to DNS
> zone pre_callback as you could not check --force flag presence right in the
> idnsname parameter validator.
>
> We may also want to change --force flag label, it now talks only about NS
> record validation, but we now expanded it a bit, so the label would need to be
> more general.
>
> Martin

I added the fix for dnszone-add and edited the label of the --force flag
to make it more general.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 76360400f4eca52f27e27d5d79f0c1eaa4220c42 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Mon, 11 Mar 2013 07:04:05 -0400
Subject: [PATCH] Improve domain name validation

Check for alphabetic only characters is now performed only in the
case of fully qualified domain names. This fixes the bug that caused
some ipa commands (e.g. ipa dnsrecord-add) to fail due to the presence
of numbers in the domain name.

This patch also adds --force option to dnsrecord-add and dnszone-add
to allow these commands to skip domain name validation.

https://fedorahosted.org/freeipa/ticket/3385
---
 ipalib/plugins/dns.py |  9 -
 ipalib/util.py| 10 +-
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index a23d1b8233eec14825ac6b43f509de51ad0ff1f7..640ad05d836a9cc6f011148b1aa20a9b525ab649 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -410,7 +410,7 @@ def _validate_bind_forwarder(ugettext, forwarder):
 
 def _domain_name_validator(ugettext, value):
 try:
-validate_domain_name(va

Re: [Freeipa-devel] [PATCHES] 0191-0195 Use ipaldap in the client installer & password migration

2013-03-11 Thread Jan Cholasta

On 8.3.2013 14:14, Petr Viktorin wrote:

On 03/07/2013 05:42 PM, Jan Cholasta wrote:

Patch 191:

The patch is missing the ipapython/ipaldap.py file.


On 7.3.2013 18:29, Petr Viktorin wrote:
> It's there, it's just copied from ipaserver/ipaldap.py with a small
> change at the bottom.

There is no sign of the file, except in the patch header and the patch 
cannot be applied with git am nor with git apply. But perhaps I'm doing 
something wrong.




I think it should go into ipalib instead of ipapython.  It doesn't
make sense to keep ipapython and ipalib separate if they depend on each
other. We should either merge them or clean up the mess by removing
ipalib imports from ipapython. I'm not saying we should do it now, just
please don't add new modules to ipapython which import from ipalib.



This is a bigger problem.
Conceptually ipaldap should be in ipapython, it just needs the
problematic errors and text modules. I think we should move those rather
than ipaldap.


Moving test to ipapython makes sense. I think we should have a separate 
set of errors for ipaldap and translate them to framework errors in ipalib.





Also I am not very fond of the "ipa" prefix in "ipaldap". The module
lives in the namespace of our own package, so there's no need for it to
have such a prefix, is there?


It's nice to have a unique name for talking about it.
Since "ldap" is already a package we use, having another "ldap" would be
confusing, even if it is properly namespaced.


Since ipaldap should be the sole user of python-ldap after the 
refactoring is done, I don't think there would be any confusion. But 
whatever, I'm fine with both.





Patch 193:

+scope=conn.SCOPE_BASE,
+filter='objectclass=pkiCA',
+attrs_list=[ca_cert_attr],

Can we use a proper filter here please?

+:param conn: Bound LDAPConnection that will be used for searching


Fixed, thanks


LDAPClient

Patch 194:

-ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, True)

and

-lh.set_option(ldap.OPT_X_TLS_DEMAND, True)

Is removing these options safe?


I re-added them.

I also removed the forgotten debugging `raise`s Martin found.

Adding patch 0196, which disables downloading the schema for discovery.

Updated patches attached. They now depend on Honza's patches 116-119



ACK.

Honza

--
Jan Cholasta

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


Re: [Freeipa-devel] [PATCHES] 0191-0195 Use ipaldap in the client installer & password migration

2013-03-11 Thread Jan Cholasta

On 11.3.2013 13:43, Petr Viktorin wrote:

On 03/11/2013 01:13 PM, Jan Cholasta wrote:

On 8.3.2013 14:14, Petr Viktorin wrote:

On 03/07/2013 05:42 PM, Jan Cholasta wrote:

Patch 191:

The patch is missing the ipapython/ipaldap.py file.


On 7.3.2013 18:29, Petr Viktorin wrote:
 > It's there, it's just copied from ipaserver/ipaldap.py with a small
 > change at the bottom.

There is no sign of the file, except in the patch header and the patch
cannot be applied with git am nor with git apply. But perhaps I'm doing
something wrong.


Attaching a re-formatted version of the patch.

[...]

ACK.

Honza






ACK for real.

Honza

--
Jan Cholasta

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


Re: [Freeipa-devel] [PATCH 0038] Perform secondary rid range overlap check for local ranges

2013-03-11 Thread Martin Kosek

On 03/08/2013 04:41 PM, Tomas Babej wrote:

On 03/08/2013 12:10 PM, Martin Kosek wrote:

On 03/05/2013 12:59 PM, Tomas Babej wrote:

Hi,

Any of the following checks:
   - overlap between primary RID range and secondary RID range
   - overlap between secondary RID range and secondary RID range

is performed now only if both of the ranges involved are local
domain ranges.

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



I think the patch is functionally OK (I tested it), I would just change the
flow of the following:

@@ -194,19 +198,22 @@ static int ranges_overlap(struct range_info *r1, struct
range_info *r2)
 r1->id_range_size, r2->id_range_size))
 return 2;

-/* check if secondary rid range overlaps with existing secondary rid
range */
+/**
+ * The following 3 checks are relevant only if both ranges are local.
+ * Check if secondary rid range overlaps with existing secondary rid
+ * range. **/
 if (intervals_overlap(r1->secondary_base_rid, r2->secondary_base_rid,
-r1->id_range_size, r2->id_range_size))
+r1->id_range_size, r2->id_range_size) && local_ranges)
 return 3;
...


TO something like

...
/**
 * The following checks are relevant only if both ranges are local.
 * Check if secondary rid range overlaps with existing secondary rid
 * range. **/
if (local_ranges) {
... do the checks
}
...

Doing it your way, intervals_overlap() function is called 3 times when not
needed + it is not so obvious that these checks are only done with
"local_ranges" only.

Martin

Refactored.

Tomas


ACK. Pushed to master, ipa-3-1.

Martin

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


[Freeipa-devel] [PATCH] 266 Fixed Web UI build error caused by rhino changes in F19

2013-03-11 Thread Petr Vobornik
rhino-1.7R4-2.fc19.noarch dropped -main flag which made the build fail 
in rawhide (F19).


We can't use the same command for rhino-1.7R3-6 (F18) and rhino-1.7R4-2 
(F19).
This patch adds check if rhino supports '-require' option. If so it 
calls rhino with it if not it calls rhino with -main option.


https://fedorahosted.org/freeipa/ticket/3501
--
Petr Vobornik
From 792cd392ae2ec13c4d74afb9ef7f1262b9e06c1f Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Mon, 11 Mar 2013 14:15:46 +0100
Subject: [PATCH] Fixed Web UI build error caused by rhino changes in F19

rhino-1.7R4-2.fc19.noarch dropped -main flag which made the build fail in rawhide (F19).

We can't use the same command for rhino-1.7R3-6 (F18) and rhino-1.7R4-2 (F19).
This patch adds check if rhino supports '-require' option. If so it calls rhino with it if not it calls rhino with -main option.

https://fedorahosted.org/freeipa/ticket/3501
---
 install/ui/util/uglifyjs/uglify | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/install/ui/util/uglifyjs/uglify b/install/ui/util/uglifyjs/uglify
index 50dfb51d512b03a7eb72170d29ea701411934f92..7d25b38df19e465227f29b8b70ccf7ca140f725a 100755
--- a/install/ui/util/uglifyjs/uglify
+++ b/install/ui/util/uglifyjs/uglify
@@ -23,4 +23,10 @@
 
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 
-rhino -main $DIR/uglify-js.js $DIR/ug.js $1 $2 -v
\ No newline at end of file
+# rhino-1.7R4 doesn't have -main option to enable CommonJS support. It was
+# replaced by -require option.
+if [ `rhino --help | grep -e -require | wc -l` -gt 0 ] ; then
+rhino -require $DIR/uglify-js.js $@
+else
+rhino -main $DIR/uglify-js.js $DIR/ug.js $@
+fi
\ No newline at end of file
-- 
1.8.1.4

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

Re: [Freeipa-devel] Failed push to github

2013-03-11 Thread Rob Crittenden

Petr Viktorin wrote:

On 03/08/2013 12:38 AM, Nathaniel McCallum wrote:

I tried to push my branch of FreeIPA to github and it failed with the
following message. I don't know if anything can be done to fix it, but I
figured I'd mention it.

error: object 0b36ce6dcbfc8d7e6cda632e06a09c369428a2db:invalid
author/committer line - bad date
fatal: Error in object
error: pack-objects died of signal 13
error: failed to push some refs to
'g...@github.com:npmccallum/freeipa.git'

Nathaniel



Yes, we have some commits with invalid commit times.
If you fork https://github.com/encukou/freeipa you should be fine.
(I've asked Github staff to turn off the checks so I could push it
initially.)



Apparently when we migrated from mercurial to git a bunch of dates got 
horked. We've had requests to fix up the dates in our tree so github 
will work but apparently this would change all our commit ids so is not 
something we're interested in.


rob

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


Re: [Freeipa-devel] [PATCHES] 0191-0195 Use ipaldap in the client installer & password migration

2013-03-11 Thread Martin Kosek

On 03/11/2013 01:48 PM, Jan Cholasta wrote:

On 11.3.2013 13:43, Petr Viktorin wrote:

On 03/11/2013 01:13 PM, Jan Cholasta wrote:

On 8.3.2013 14:14, Petr Viktorin wrote:

On 03/07/2013 05:42 PM, Jan Cholasta wrote:

Patch 191:

The patch is missing the ipapython/ipaldap.py file.


On 7.3.2013 18:29, Petr Viktorin wrote:
 > It's there, it's just copied from ipaserver/ipaldap.py with a small
 > change at the bottom.

There is no sign of the file, except in the patch header and the patch
cannot be applied with git am nor with git apply. But perhaps I'm doing
something wrong.


Attaching a re-formatted version of the patch.

[...]

ACK.

Honza






ACK for real.

Honza



I would not want to rush this, I still see errors:

1) ipa-ldap-updater is broken:

# ipa-ldap-updater --upgrade
Upgrading IPA:
  [1/8]: stopping directory server
  [2/8]: saving configuration
  [3/8]: disabling listeners
  [4/8]: starting directory server
  [5/8]: upgrading server
Upgrade failed with 'NameSpace' object has no attribute 'ldap2'
  [6/8]: stopping directory server
  [7/8]: restoring configuration
  [8/8]: starting directory server
Done.
IPA upgrade failed.


2) What's the purpose of this new error?

+class DatabaseTimeout(DatabaseError):
+"""
+**4211** Raised when an LDAP call times out
+
+For example:
+
+>>> raise DatabaseTimeout()
+Traceback (most recent call last):
+  ...
+DatabaseTimeout: LDAP timeout
+"""
+
+errno = 4211
+format = _('LDAP timeout')

It is not raised anywhere (as far as I can see). BTW I assume it is not related 
to errors.LimitsExceeded in any way, right?



3) Client installation no longer works if the server has disabled anonymous 
authentication:


# ipa-client-install
Error checking LDAP: Inappropriate authentication: Anonymous access is not 
allowed.
DNS discovery failed to determine your DNS domain
Provide the domain name of your IPA server (ex: example.com): ^C


4) I suddenly cannot run some tests, looks like import loop:

# ./make-test tests/test_xmlrpc/test_host_plugin.py
/usr/bin/nosetests -v --with-doctest --doctest-tests --exclude=plugins 
tests/test_xmlrpc/test_host_plugin.py

Failure: ImportError (cannot import name ipautil) ... ERROR

==
ERROR: Failure: ImportError (cannot import name ipautil)
--
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/nose/loader.py", line 390, in 
loadTestsFromName

addr.filename, addr.module)
  File "/usr/lib/python2.7/site-packages/nose/importer.py", line 39, in 
importFromPath

return self.importFromDir(dir_path, fqname)
  File "/usr/lib/python2.7/site-packages/nose/importer.py", line 86, in 
importFromDir

mod = load_module(part_fqname, fh, filename, desc)
  File "/root/freeipa-master/tests/test_xmlrpc/test_host_plugin.py", line 27, 
in 

from ipapython import ipautil
  File "/root/freeipa-master/ipapython/ipautil.py", line 52, in 
from ipalib import errors
  File "/root/freeipa-master/ipalib/__init__.py", line 930, in 
api.finalize()
  File "/root/freeipa-master/ipalib/plugable.py", line 674, in finalize
self.__do_if_not_done('load_plugins')
  File "/root/freeipa-master/ipalib/plugable.py", line 454, in __do_if_not_done
getattr(self, name)()
  File "/root/freeipa-master/ipalib/plugable.py", line 613, in load_plugins
self.import_plugins('ipalib')
  File "/root/freeipa-master/ipalib/plugable.py", line 655, in import_plugins
__import__(fullname)
  File "/root/freeipa-master/ipalib/plugins/cert.py", line 30, in 
from ipalib import pkcs10
  File "/root/freeipa-master/ipalib/pkcs10.py", line 24, in 
from ipapython import ipautil
ImportError: cannot import name ipautil

--
Ran 1 test in 0.002s

FAILED (errors=1)
==
FAILED under '/usr/bin/python2.7'

** FAIL **


Martin

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


[Freeipa-devel] [PATCH 118] [WIP] Add 389 DS plugin for special idnsSOASerial attribute handling

2013-03-11 Thread Petr Spacek

Hello list!

My first patch for FreeIPA is attached :-)

I managed to add new 389 DS plugin to build system, but the LDAP magic in 
installer and updater is too much for my brain.


Could somebody show me how installer and updater should add new object to 
cn=config ? Plugin configuration is static (example is in comments in ipa_dns.c).


This patch implements minimal necessary support for idnsSOASerial replication. 
I investigating more advanced techniques, but I still see problems with 
locking and so on.


Anyway, this patch should be sufficient for now.

Commit message:

Add 389 DS plugin for special idnsSOASerial attribute handling

Default value "1" is added to replicated idnsZone objects
if idnsSOASerial attribute is missing.

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

--
Petr^2 Spacek
From f61f4396f88639f707e15696d730760c15e7bd94 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Fri, 8 Mar 2013 18:54:58 +0100
Subject: [PATCH] Add 389 DS plugin for special idnsSOASerial attribute
 handling

Default value "1" is added to replicated idnsZone objects
if idnsSOASerial attribute is missing.

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

Signed-off-by: Petr Spacek 
---
 daemons/configure.ac  |   1 +
 daemons/ipa-slapi-plugins/Makefile.am |   1 +
 daemons/ipa-slapi-plugins/ipa-dns/Makefile.am |  42 +
 daemons/ipa-slapi-plugins/ipa-dns/ipa_dns.c   | 211 ++
 freeipa.spec.in   |   2 +
 5 files changed, 257 insertions(+)
 create mode 100644 daemons/ipa-slapi-plugins/ipa-dns/Makefile.am
 create mode 100644 daemons/ipa-slapi-plugins/ipa-dns/ipa_dns.c

diff --git a/daemons/configure.ac b/daemons/configure.ac
index ebf625ebffd8a92e0a3b050955b9376e002ed6c9..2f6cfd5e2683167aa27625c39d2c6f0d99ca2769 100644
--- a/daemons/configure.ac
+++ b/daemons/configure.ac
@@ -334,6 +334,7 @@ AC_CONFIG_FILES([
 ipa-sam/Makefile
 ipa-slapi-plugins/Makefile
 ipa-slapi-plugins/ipa-cldap/Makefile
+ipa-slapi-plugins/ipa-dns/Makefile
 ipa-slapi-plugins/ipa-enrollment/Makefile
 ipa-slapi-plugins/ipa-lockout/Makefile
 ipa-slapi-plugins/ipa-pwd-extop/Makefile
diff --git a/daemons/ipa-slapi-plugins/Makefile.am b/daemons/ipa-slapi-plugins/Makefile.am
index c79e68db112c9d21bcbffba3d00442d2fd20ab3a..08c7558c812effc00ae940661e448779077fb409 100644
--- a/daemons/ipa-slapi-plugins/Makefile.am
+++ b/daemons/ipa-slapi-plugins/Makefile.am
@@ -2,6 +2,7 @@ NULL =
 
 SUBDIRS =			\
 	ipa-cldap		\
+	ipa-dns			\
 	ipa-enrollment		\
 	ipa-lockout		\
 	ipa-modrdn		\
diff --git a/daemons/ipa-slapi-plugins/ipa-dns/Makefile.am b/daemons/ipa-slapi-plugins/ipa-dns/Makefile.am
new file mode 100644
index ..a1528358e5e9c2fc32717700e292a86524b86382
--- /dev/null
+++ b/daemons/ipa-slapi-plugins/ipa-dns/Makefile.am
@@ -0,0 +1,42 @@
+NULL =
+
+PLUGIN_COMMON_DIR=../common
+
+INCLUDES =			\
+	-I.			\
+	-I$(srcdir)		\
+	-I$(PLUGIN_COMMON_DIR)	\
+	-I/usr/include/dirsrv	\
+	-DPREFIX=\""$(prefix)"\" \
+	-DBINDIR=\""$(bindir)"\"\
+	-DLIBDIR=\""$(libdir)"\" \
+	-DLIBEXECDIR=\""$(libexecdir)"\"			\
+	-DDATADIR=\""$(datadir)"\"\
+	$(AM_CFLAGS)		\
+	$(LDAP_CFLAGS)		\
+	$(WARN_CFLAGS)		\
+	$(NULL)
+
+plugindir = $(libdir)/dirsrv/plugins
+plugin_LTLIBRARIES = 		\
+	libipa_dns.la		\
+	$(NULL)
+
+libipa_dns_la_SOURCES = 	\
+	ipa_dns.c		\
+	$(NULL)
+
+libipa_dns_la_LDFLAGS = -avoid-version
+
+libipa_uuid_la_LIBADD = 	\
+	$(LDAP_LIBS)		\
+	$(UUID_LIBS)		\
+	$(NULL)
+
+EXTRA_DIST =			\
+	$(app_DATA)		\
+	$(NULL)
+
+MAINTAINERCLEANFILES =		\
+	*~			\
+	Makefile.in
diff --git a/daemons/ipa-slapi-plugins/ipa-dns/ipa_dns.c b/daemons/ipa-slapi-plugins/ipa-dns/ipa_dns.c
new file mode 100644
index ..0769a54aadc54a49909474cf86f8ebc0d228e759
--- /dev/null
+++ b/daemons/ipa-slapi-plugins/ipa-dns/ipa_dns.c
@@ -0,0 +1,211 @@
+/** BEGIN COPYRIGHT BLOCK
+ * This Program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation; version 2 of the License.
+ *
+ * This Program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this Program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * Copyright (C) 2001 Sun Microsystems, Inc. Used by permission.
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * Authors:
+ * original authors of 389 example ldap/servers/slapd/test-plugins/testpreop.c
+ * Petr Spacek 
+ *
+ * All rights reserved.
+ *
+ * END COPYRIGHT BLOCK **/
+
+/*
+ * This is 389 DS plug-in with supporting functi

Re: [Freeipa-devel] [PATCH] 1088 Recover DNA ranges when deleting a master

2013-03-11 Thread Rob Crittenden

Petr Viktorin wrote:

On 03/07/2013 08:27 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

On 03/06/2013 09:52 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

[...]

On new installs, the ACI on cn=Posix IDs,cn=Distributed Numeric
Assignment Plugin,cn=plugins,cn=config is added before the entry
itself.
I didn't test everything as I didn't get the access.


It shouldn't make a difference. What isn't working?


I get a CRITICAL message in the log:

add aci:
 (targetattr=dnaNextRange || dnaNextValue ||
dnaMaxValue)(version 3.0;acl "permission:Modify DNA Range";allow (write)
groupdn = "ldap:///cn=Modify DNA
Range,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com";)



modifying entry "cn=Posix IDs,cn=Distributed Numeric Assignment
Plugin,cn=plugins,cn=config"

2013-03-07T11:01:48Z DEBUG stderr=ldap_initialize(
ldap://vm-081.idm.lab.eng.brq.redhat.com:389/??base )
ldap_modify: No such object (32)

2013-03-07T11:01:48Z CRITICAL Failed to load replica-acis.ldif: Command
'/usr/bin/ldapmodify -v -f /tmp/tmpT55upM -H
ldap://vm-081.idm.lab.eng.brq.redhat.com:389 -x -D cn=Directory Manager
-y /tmp/tmplFeere' returned non-zero exit status 32



Gotcha. I moved where the replica acis are loaded.


Please attach the patch :)

[...]

+failed = 0
+for ent in entries:



This loops more than necessary and is somewhat hard to follow.
Consider
using for-else here:

for ...:
 ...
 if okay:
 break
else:
 raise error


I simplified things a bit but a for/else won't work here as we need to
check all ranges all the time. It is perfectly fine to not fit into a
range, as long as it fits into SOME range.


Well, that's how for's (not if's) else clause works -- it's executed
after all the looping's done if you didn't `break` out.
http://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops



Maybe I'm just used to it and it's too esoteric to the average reader,
though.


Thanks for the vote of confidence. Like I said, I wanted it to check all
the ranges. A for/else quits on the break, which I guess is really
probably ok assuming we trust that nothing else is going to stuff bad
ranges in. I can go ahead and make the change.


Your code also breaks out as soon as a good range is found.
Anyway this is a small nitpick; the loop works fine as it is.


[...]

Ok, I'll drop this since it doesn't affect things with the new LDAP
backend.


Please do, you left it in by mistake.


Yeah, there it is sitting unsquashed in my tree :-(

 >

I also added one change related to the LDAP core changes. In the
past if
you did not have a ticket it would prompt for DM password. This was
broken after the updates. I added an additional except in
test_connection().


This should also be fixed soon in ipaldap. Thanks for putting up with
the changes.



So should I drop this in my patch then? I don't really like having to
import ldap.


You can if you're fine with waiting until my patches are pushed.
Otherwise it's covered by https://fedorahosted.org/freeipa/ticket/3499



I left it in for now. Updated patch attached.

rob
>From ee189db2037b4a0ea3267b0ccdc9d8c7665ffbf3 Mon Sep 17 00:00:00 2001
From: Rob Crittenden 
Date: Fri, 1 Mar 2013 15:02:14 -0500
Subject: [PATCH] Extend ipa-replica-manage to be able to manage DNA ranges.

Attempt to automatically save DNA ranges when a master is removed.
This is done by trying to find a master that does not yet define
a DNA on-deck range. If one can be found then the range on the deleted
master is added.

If one cannot be found then it is reported as an error.

Some validation of the ranges are done to ensure that they do overlap
an IPA local range and do not overlap existing DNA ranges configured
on other masters.

http://freeipa.org/page/V3/Recover_DNA_Ranges

https://fedorahosted.org/freeipa/ticket/3321
---
 install/share/delegation.ldif  |   9 ++
 install/share/replica-acis.ldif|   5 +
 install/tools/ipa-replica-manage   | 285 -
 install/tools/man/ipa-replica-manage.1 |  45 +-
 install/updates/40-replication.update  |  12 ++
 ipaserver/install/dsinstance.py|   3 +-
 ipaserver/install/replication.py   |  98 
 ipaserver/ipaldap.py   |   2 +
 8 files changed, 450 insertions(+), 9 deletions(-)

diff --git a/install/share/delegation.ldif b/install/share/delegation.ldif
index f62062fe498634d56128ebf78874c3ba91d7d09b..14069586cf1f1021d281a3e86133de1535b62559 100644
--- a/install/share/delegation.ldif
+++ b/install/share/delegation.ldif
@@ -545,6 +545,15 @@ cn: Remove Replication Agreements
 ipapermissiontype: SYSTEM
 member: cn=Replication Administrators,cn=privileges,cn=pbac,$SUFFIX
 
+dn: cn=Modify DNA Range,cn=permissions,cn=pbac,$SUFFIX
+changetype: add
+objectClass: top
+objectClass: groupofnames
+objectClass: ipapermission
+cn: Modify DNA Range
+ipapermissiontype: SYSTEM
+member: cn=Replication Administrators,cn=privil

Re: [Freeipa-devel] [PATCH] 0186 Change DNA magic value to -1 to make UID 999 usable

2013-03-11 Thread Martin Kosek

On 02/22/2013 12:16 PM, Petr Viktorin wrote:

On 02/22/2013 11:16 AM, Petr Viktorin wrote:

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

This changes the DNA magic value to -1, and the corresponding IPA's
parameters (gidnumber, uidnumber) to be optional (instead of autofill).

Since the old clients still say "999" when they mean "pick one I don't
care", we need to detect them and change 999 to -1. For that there's a
new capability, optional_uid_params.


Behavior summary:

With --uid 999:
   old client, old server: sends 999, creates random UID
   old client, new server: sends 999, creates random UID
   new client, old server: incompatible
   new client, new server: sends 999, creates UID 999

Without --uid:
   old client, old server: sends 999, creates random UID
   old client, new server: sends 999, creates random UID
   new client, old server: incompatible
   new client, new server: doesn't send UID, creates random UID

Upgrade should work fine.

I didn't test winsync as I don't have a Windows machine.



The patch is here



Works like a charm for both old clients and the new ones.

ACK, pushed to master.

Martin

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


[Freeipa-devel] [PATCH] 1092 Fix LDAP lockout plugin

2013-03-11 Thread Rob Crittenden
Fixed a number of issues applying password policy against LDAP binds. 
See patch for details.


rob
>From 27b19a5fbf7ea999dd9e69732c61ac58668c29b0 Mon Sep 17 00:00:00 2001
From: Rob Crittenden 
Date: Fri, 15 Feb 2013 11:51:59 -0500
Subject: [PATCH] Fix lockout of LDAP bind.

There were several problems:

- A cut-n-paste error where the wrong value was being considered when
  an account was administratively unlocked.
- An off-by-one error where LDAP got one extra bind attempt.
- krbPwdPolicyReference wasn't being retrieved as a virtual attribute so
  only the global_policy was used.
- The lockout duration wasn't examined in the context of too many failed
  logins so was being applied properly.

https://fedorahosted.org/freeipa/ticket/3433
---
 .../ipa-slapi-plugins/ipa-lockout/ipa_lockout.c| 41 --
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c b/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
index fea997d79825e05a6ffe4bf65e22821948794ff3..5aa1f34fa55ee06034fd6f5547d2ea3552842637 100644
--- a/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
+++ b/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c
@@ -588,7 +588,7 @@ done:
 static int ipalockout_preop(Slapi_PBlock *pb)
 {
 char *dn = NULL;
-char *policy_dn = NULL;
+const char *policy_dn = NULL;
 Slapi_Entry *target_entry = NULL;
 Slapi_Entry *policy_entry = NULL;
 Slapi_DN *sdn = NULL;
@@ -605,6 +605,10 @@ static int ipalockout_preop(Slapi_PBlock *pb)
 time_t last_failed = 0;
 char *lastfail = NULL;
 char *unlock_time = NULL;
+int type_name_disposition = 0;
+char *actual_type_name = NULL;
+int attr_free_flags = 0;
+Slapi_ValueSet *values = NULL;
 
 LOG_TRACE("--in-->\n");
 
@@ -656,7 +660,21 @@ static int ipalockout_preop(Slapi_PBlock *pb)
 slapi_value_free(&objectclass);
 
 /* Only continue if there is a password policy */
-policy_dn = slapi_entry_attr_get_charptr(target_entry, "krbPwdPolicyReference");
+ldrc = slapi_vattr_values_get(target_entry, "krbPwdPolicyReference",
+&values,
+&type_name_disposition, &actual_type_name,
+SLAPI_VIRTUALATTRS_REQUEST_POINTERS,
+&attr_free_flags);
+if (ldrc == 0) {
+Slapi_Value *sv = NULL;
+
+slapi_valueset_first_value(values, &sv);
+
+if (values != NULL) {
+policy_dn = slapi_value_get_string(sv);
+}
+}
+
 if (policy_dn == NULL) {
 LOG_TRACE("No kerberos password policy\n");
 goto done;
@@ -705,7 +723,7 @@ static int ipalockout_preop(Slapi_PBlock *pb)
 time_t unlock;
 
 memset(&tm, 0, sizeof(struct tm));
-res = sscanf(lastfail,
+res = sscanf(unlock_time,
  "%04u%02u%02u%02u%02u%02u",
  &tm.tm_year, &tm.tm_mon, &tm.tm_mday,
  &tm.tm_hour, &tm.tm_min, &tm.tm_sec);
@@ -730,13 +748,14 @@ static int ipalockout_preop(Slapi_PBlock *pb)
 }
 
 lockout_duration = slapi_entry_attr_get_uint(policy_entry, "krbPwdLockoutDuration");
-if (lockout_duration == 0) {
-errstr = "Entry permanently locked.\n";
-ret = LDAP_UNWILLING_TO_PERFORM;
-goto done;
-}
 
-if (failedcount > max_fail) {
+if (failedcount >= max_fail) {
+if (lockout_duration == 0) {
+errstr = "Entry permanently locked.\n";
+ret = LDAP_UNWILLING_TO_PERFORM;
+goto done;
+}
+
 if (time_now < last_failed + lockout_duration) {
 /* Too many failures */
 LOG_TRACE("Too many failed logins. %lu out of %d\n", failedcount, max_fail);
@@ -748,7 +767,9 @@ static int ipalockout_preop(Slapi_PBlock *pb)
 done:
 slapi_entry_free(target_entry);
 slapi_entry_free(policy_entry);
-if (policy_dn) slapi_ch_free_string(&policy_dn);
+if (values != NULL) {
+slapi_vattr_values_free(&values, &actual_type_name, attr_free_flags);
+}
 if (sdn) slapi_sdn_free(&sdn);
 
 LOG("preop returning %d: %s\n", ret, errstr ? errstr : "success\n");
-- 
1.8.1

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