Re: [Freeipa-devel] [PATCH] 0105 FIX: LDAP_updater

2014-07-24 Thread Martin Basti

On 23/07/14 15:17, Martin Basti wrote:

This patch fixes ordering problem of schema updates

Martin should it be in IPA 4.0.x ? It requires rebased ldap_python 
(will be in Fedora 21)


Patch attached


I found a bug there, but before I send updated version, I need to decide 
how print modlist:


1. Print modlist before each LDAP update (if changes exist), show 
modlist per incremental update


2. Print modlist only at once with all updates

3. Use [1] for live_run, [2] for test

--
Martin Basti

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


Re: [Freeipa-devel] [PATCH] 0005 Verify otptoken timespan is valid

2014-07-24 Thread David Kupka



On 07/23/2014 05:07 PM, Jan Cholasta wrote:

Hi,

On 23.7.2014 15:46, David Kupka wrote:

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


1) Use "isinstance(X, Y)" instead of "type(X) is Y".


Thanks for advice, will try to remember.


2) When is "type(not_before) is str" or "type(not_after) is str" true?
The values coming from command options or LDAP should always be
datetime, never str.


Actually, it is never true. I don't  know why I thought that there is 
such option.


3) There are some misindentations:

+raise ValidationError(name='not_after',
+error='is before not_before!')

+raise ValidationError(name='not_after',
+error='is before not_before!')

+raise ValidationError(name='not_before',
+error='is after not_after!')

4) We don't do exclamation marks in errors messages.


You re right, it's probably better not to shout at customer :)


5) Generally, when you want to validate command options, you should look
into "options", not "entry_attrs".

6) This is not right:

+result = self.api.Command.otptoken_find(ipatokenuniqueid=
+entry_attrs.get('ipatokenuniqueid', None))['result']

This is:

+result = self.api.Command.otptoken_show(keys[-1])['result']


Both works, but Martin explained me why is otptoken_show better and how 
it actually works.


Honza



--
David Kupka
From 3f01bb641b9f659cb21ba852fd4fbbe57c29b78e Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 24 Jul 2014 09:42:04 +0200
Subject: [PATCH] Verify otptoken timespan is valid

When creating or modifying otptoken check that token validity start is not after
validity end.

https://fedorahosted.org/freeipa/ticket/4244
---
 ipalib/plugins/otptoken.py | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 2880ee660d5dcdb18c504f50d7b72f5b8fb43d48..53e3206987b719ca12d1f0850def3432cd269a5b 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -21,7 +21,7 @@ from ipalib.plugins.baseldap import DN, LDAPObject, LDAPAddMember, LDAPRemoveMem
 from ipalib.plugins.baseldap import LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, LDAPRetrieve
 from ipalib import api, Int, Str, Bool, DateTime, Flag, Bytes, IntEnum, StrEnum, Password, _, ngettext
 from ipalib.plugable import Registry
-from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound
+from ipalib.errors import PasswordMismatch, ConversionError, LastMemberError, NotFound, ValidationError
 from ipalib.request import context
 from ipalib.frontend import Local
 
@@ -103,6 +103,11 @@ def _normalize_owner(userobj, entry_attrs):
 if owner is not None:
 entry_attrs['ipatokenowner'] = userobj.get_dn(owner)
 
+def _check_interval(not_before, not_after):
+if not_before and not_after:
+if not_before > not_after:
+return False
+return True
 
 @register()
 class otptoken(LDAPObject):
@@ -254,6 +259,11 @@ class otptoken_add(LDAPCreate):
 entry_attrs['ipatokenuniqueid'] = str(uuid.uuid4())
 dn = DN("ipatokenuniqueid=%s" % entry_attrs['ipatokenuniqueid'], dn)
 
+if not _check_interval(options.get('ipatokennotbefore', None),
+   options.get('ipatokennotafter', None)):
+raise ValidationError(name='not_after',
+  error='is before not_before.')
+
 # Set the object class and defaults for specific token types
 entry_attrs['objectclass'] = otptoken.object_class + ['ipatoken' + options['type']]
 for ttype, tattrs in TOKEN_TYPES.items():
@@ -336,6 +346,26 @@ class otptoken_mod(LDAPUpdate):
 msg_summary = _('Modified OTP token "%(value)s"')
 
 def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
+notafter_set = True
+notbefore = options.get('ipatokennotbefore', None)
+notafter = options.get('ipatokennotafter', None)
+# notbefore xor notafter, exactly one of them is not None
+if bool(notbefore) ^ bool(notafter):
+result = self.api.Command.otptoken_show(keys[-1])['result']
+if result:
+if notbefore is None:
+notbefore = result['ipatokennotbefore'][0]
+if notafter is None:
+notafter_set = False
+notafter = result['ipatokennotafter'][0]
+
+if not _check_interval(notbefore, notafter):
+if notafter_set:
+raise ValidationError(name='not_after',
+  error='is before not_before.')
+else:
+raise ValidationError(name='not_before',
+  error='is after not_after.')
 _normalize_owner(self.api.Object.u

Re: [Freeipa-devel] [PATCH 0275] Add TLSARecord to idnsRecord object class

2014-07-24 Thread Tomas Hozza
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/23/2014 02:25 PM, Petr Spacek wrote:
> Hello,
> 
> Add TLSARecord to idnsRecord object class.
> 

ACK.

Tomas
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJT0MG0AAoJEMWIetUdnzwtdCcH/RqNYSs4x/rRh0RfrFnl01UG
ub7B7Er3BOgYwB1KeOcjFMBySI8DXZHPE40YshBw1fWfhatrc9DMPGZGJSebW2ti
4rvr6olfRHPwGJuzDxG7FLpd7CDt16m2Vc62a6191LlZeXYbeABimfUuz6Ffst+8
JejLkdKR2dpKOOYHWEZ1TMKG7WVd1qA3eOWdzWWVbiCxt7oAGZpJL+ftDh9UnJMS
PwwWlYKKiZI8d+bqX7fmZfsGkMGIliQxEnKoRV/j+G/dJjIIGeGJuHRMas/NGlSL
CUD8NuUTuJmLwYzZRkPrRoeOKiLhZhjlEuOhUMNqHNGaA91zi1bQYF9XeMPt16s=
=m1A3
-END PGP SIGNATURE-

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


Re: [Freeipa-devel] [PATCH 0276] Fix crash during reconnection to LDAP

2014-07-24 Thread Tomas Hozza
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/23/2014 02:26 PM, Petr Spacek wrote:
> Hello,
> 
> Fix crash during reconnection to LDAP.
> 

The fix works.

ACK.

Tomas
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJT0MHhAAoJEMWIetUdnzwtwBYH/1GyydKyhakuGp42PWLUHs55
cpl/VvHjQtDqlQPe5osxkMRhz32oJ/pn5B965lHZnRHsOySp9Cqtk/3dRiyxxvUA
rllLgunTrC4oVM4SsLateREOZPl+hvIy599e2ZiXyHnPqvmi1rXN/SX9BZEhLGoh
z6DAK6unkkcEG+8NUkt4SnPXwnQ6zY4yYicmAp73IjO9p09Xy8uV98xk6Od/UfSB
6VPfvre3OhEEndiCISnzti18LTKrmgCeH371bXb8gKcX8y3zNcnxAFE95Yp5N2W3
/qViRpHTt+iUABN4Bt5rk4fVMN95Mn9AndE2O7CKx42xdh9TMzYYPM5amZKZUz8=
=Ka0U
-END PGP SIGNATURE-

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


Re: [Freeipa-devel] [PATCH 0277] Bump NVR to 5.1

2014-07-24 Thread Tomas Hozza
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/23/2014 02:27 PM, Petr Spacek wrote:
> Hello,
> 
> Bump NVR to 5.1.
> 

ACK.

Tomas
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJT0MHvAAoJEMWIetUdnzwtwG4H/3yoylMft5vskUK5thelM+TH
19dLJ5LmDNuEhLQx2nsz7L4BzI9XLfESj1KYZiqFzR9ENqvwb9uKoEV6rZztwnNe
jT3A14AIRoE6S/hhaaXbDvOzTheyfQBbHh0WZxTY7Ge+QVv/1WfD1Ko8bLiPbNY3
m+CdXGGB+lnF6zBP+yNyyHwnyhI9Vda7JwVi5VfIVKPJCtafXloVTiHGKVBKyBj/
sojXcVOF2FSyhykBFbM24HnirWbHVUuv5NLiO1ciwKzJx3hL3feP8bTySCwr+FzH
E4vvaa3jVT9yBLzz7bnzsL4r2jLn+IYaT84nG8SGlMyqNioHqxrQPKXRnSGBuds=
=X41u
-END PGP SIGNATURE-

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


Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files

2014-07-24 Thread Petr Spacek

On 27.2.2014 15:19, Lukas Slebodnik wrote:

ehlo,

I did some reviews of bind-dyndb-ldap last week  and it was little bit annoying
to export special CFLAGS for bind9 header files. It can be automatically
detected in configure script using utility isc-config.

Attached patch should improve this and CFLAGS needn't be exported.


Kind NACK. It would be valuable to test if isc/errno2result.h header is 
present and throw appropriate error.


Current check with isc-config.sh only will pass if you have bind-devel package 
installed but you are missing bind-lite-devel package.



I have a question: How
+ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99
works?

Will it take user-defined CFLAGS into account? I would like to place 
user-defined flags at the end of the list so you can easily override settings 
given by autotools.


Thank you for clarification :-)


I will be really happy to commit complete fix. Thank you for cleaning this 
autotools mess!


--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0275] Add TLSARecord to idnsRecord object class

2014-07-24 Thread Petr Spacek

On 24.7.2014 10:20, Tomas Hozza wrote:

On 07/23/2014 02:25 PM, Petr Spacek wrote:

>Hello,
>
>Add TLSARecord to idnsRecord object class.
>

ACK.

Pushed to master: 2d358ccbc323ea6d4339f22b16d419195054e017

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0277] Bump NVR to 5.1

2014-07-24 Thread Petr Spacek

On 24.7.2014 10:21, Tomas Hozza wrote:

On 07/23/2014 02:27 PM, Petr Spacek wrote:

>Hello,
>
>Bump NVR to 5.1.
>

ACK.


Pushed to master: 1ac2fd5e1d7e5ad742739b4ec5d2c326dcc0f184

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0276] Fix crash during reconnection to LDAP

2014-07-24 Thread Petr Spacek

On 24.7.2014 10:20, Tomas Hozza wrote:

On 07/23/2014 02:26 PM, Petr Spacek wrote:

>Hello,
>
>Fix crash during reconnection to LDAP.
>

The fix works.

ACK.


Pushed to master: fb979d2f07be16f8cf441d393612504235ab26d8

--
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0246] baseldap: Fix undefined variable reference in

2014-07-24 Thread Tomas Babej
Hi,

on receiving a PublicError we fail with InternalError since msg is not
defined.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 


>From d81984981a2de72aa820feb710fec1ccb8e5 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Thu, 24 Jul 2014 12:33:35 +0200
Subject: [PATCH] baseldap: Fix undefined variable reference in
 LDAPAddReverseMember and LDAPRemoveReverseMember

---
 ipalib/plugins/baseldap.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 610b9b0f11660996a874fe6151e202f8241c5f27..f96665f82e5562b274e57043c01e8243692f1236 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -2108,6 +2108,8 @@ class LDAPAddReverseMember(LDAPModReverseMember):
 failed['member'][self.reverse_attr].append((attr, unicode(msg.strip(
 
 except errors.PublicError, e:
+msg = str(e)
+(attr, msg) = msg.split(':', 1)
 failed['member'][self.reverse_attr].append((attr, unicode(msg)))
 
 # Update the member data.
@@ -2209,6 +2211,8 @@ class LDAPRemoveReverseMember(LDAPModReverseMember):
 failed['member'][self.reverse_attr].append((attr, unicode(msg.strip(
 
 except errors.PublicError, e:
+msg = str(e)
+(attr, msg) = msg.split(':', 1)
 failed['member'][self.reverse_attr].append((attr, unicode(msg)))
 
 # Update the member data.
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH 0246] baseldap: Fix undefined variable reference in

2014-07-24 Thread Tomas Babej

On 07/24/2014 12:35 PM, Tomas Babej wrote:
> Hi,
>
> on receiving a PublicError we fail with InternalError since msg is not
> defined.
>
>
>
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

I also realized there's no need for the nested try-blocks, so added some
refactoring to the fix, which makes the code much more simple.



-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

>From 426e1be07982088b0cc71f705f91d20ad95feef3 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Thu, 24 Jul 2014 12:33:35 +0200
Subject: [PATCH] baseldap: Fix undefined variable reference in
 LDAPAddReverseMember and LDAPRemoveReverseMember

---
 ipalib/plugins/baseldap.py | 44 ++--
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 610b9b0f11660996a874fe6151e202f8241c5f27..5367b487e96f5d6d0daf00e72341c50ed5dfface 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -2096,19 +2096,15 @@ class LDAPAddReverseMember(LDAPModReverseMember):
 for attr in options.get(self.reverse_attr) or []:
 try:
 options = {'%s' % self.member_attr: keys[-1]}
-try:
-result = self._exc_wrapper(keys, options, self.api.Command[self.member_command])(attr, **options)
-if result['completed'] == 1:
-completed = completed + 1
-else:
-failed['member'][self.reverse_attr].append((attr, result['failed']['member'][self.member_attr][0][1]))
-except errors.NotFound, e:
-msg = str(e)
-(attr, msg) = msg.split(':', 1)
-failed['member'][self.reverse_attr].append((attr, unicode(msg.strip(
-
-except errors.PublicError, e:
-failed['member'][self.reverse_attr].append((attr, unicode(msg)))
+result = self._exc_wrapper(keys, options, self.api.Command[self.member_command])(attr, **options)
+if result['completed'] == 1:
+completed = completed + 1
+else:
+failed['member'][self.reverse_attr].append((attr, result['failed']['member'][self.member_attr][0][1]))
+except (errors.PublicError, errors.NotFound) as e:
+msg = str(e)
+(attr, msg) = msg.split(':', 1)
+failed['member'][self.reverse_attr].append((attr, unicode(msg.strip(
 
 # Update the member data.
 entry_attrs = ldap.get_entry(dn, ['*'])
@@ -2197,19 +2193,15 @@ class LDAPRemoveReverseMember(LDAPModReverseMember):
 for attr in options.get(self.reverse_attr) or []:
 try:
 options = {'%s' % self.member_attr: keys[-1]}
-try:
-result = self._exc_wrapper(keys, options, self.api.Command[self.member_command])(attr, **options)
-if result['completed'] == 1:
-completed = completed + 1
-else:
-failed['member'][self.reverse_attr].append((attr, result['failed']['member'][self.member_attr][0][1]))
-except errors.NotFound, e:
-msg = str(e)
-(attr, msg) = msg.split(':', 1)
-failed['member'][self.reverse_attr].append((attr, unicode(msg.strip(
-
-except errors.PublicError, e:
-failed['member'][self.reverse_attr].append((attr, unicode(msg)))
+result = self._exc_wrapper(keys, options, self.api.Command[self.member_command])(attr, **options)
+if result['completed'] == 1:
+completed = completed + 1
+else:
+failed['member'][self.reverse_attr].append((attr, result['failed']['member'][self.member_attr][0][1]))
+except (errors.PublicError, errors.NotFound) as e:
+msg = str(e)
+(attr, msg) = msg.split(':', 1)
+failed['member'][self.reverse_attr].append((attr, unicode(msg.strip(
 
 # Update the member data.
 entry_attrs = ldap.get_entry(dn, ['*'])
-- 
1.9.3

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

[Freeipa-devel] Announcing bind-dyndb-ldap version 5.1

2014-07-24 Thread Petr Spacek

The FreeIPA team is proud to announce bind-dyndb-ldap version 5.1.

It can be downloaded from https://fedorahosted.org/released/bind-dyndb-ldap/

The new version has also been built for Fedora 20+ and and is on its way to 
updates-testing:

https://admin.fedoraproject.org/updates/bind-dyndb-ldap-5.1-1.fc20

Release to Fedora 'updates' repo will be coordinated with FreeIPA 4.0 release 
to prevent breakages.


== Changes in 5.1 ==
[1] Fix crash during reconnection to LDAP.

== Changes in 5.0 ==
[1] Support for DNSSEC in-line signing was added. Now any LDAP zone can be
signed with keys provided by user.

[2] DNSKEY, RRSIG, NSEC and NSEC3 records are automatically managed
by BIND+bind-dyndb-ldap. Respective attributes in LDAP are ignored.

[3] Forwarder semantic was changed to match BIND's semantics:
- idnsZone object always represents master zone
- idnsForwardZone object (new) always represents forward zone

[4] Master root zone can be stored in LDAP.


== Upgrading ==
A server can be upgraded by installing updated RPM. BIND has to be restarted 
manually after the RPM installation.


!!! CAUTION !!!
idnsZone object class changed it's semantics in version 5.0. Please read
https://git.fedorahosted.org/cgit/bind-dyndb-ldap.git/plain/README
and update idnsForwarders and idnsForward policy attributes in your DNS zones 
accordingly.


Transition from idnsZone to idnsForwardZone object class can be made seamless 
if you change data in LDAP before you upgrade to version 5.x. All 
bind-dyndb-ldap versions >= 3.0 support the idnsForwardZone object class.



Users of FreeIPA < 4.0 should be careful when upgrading bind-dyndb-ldap to 
version >= 5.0 (if they do not upgrade to FreeIPA 4.x at the same time).


Configuration semantics related to conditional (per-zone) forwarding has 
changed and FreeIPA < 4.0 doesn't have appropriate user interface and API.


It is safe to upgrade if you use *only* global forwarders (shown by 'ipa 
dnsconfig-show') and *do not* use per-zone forwarders (shown by 'ipa 
dnszone-show').


Don't hesitate to ask freeipa-users mailing list if you need help with upgrade.
!!! CAUTION !!!

Downgrading back to any 4.x version is supported.


== Feedback ==
Please provide comments, report bugs and send any other feedback via the 
freeipa-users mailing list:

http://www.redhat.com/mailman/listinfo/freeipa-users

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH] 479 Do not require dogtag-pki-server-theme

2014-07-24 Thread Martin Kosek
On 07/23/2014 05:28 PM, Alexander Bokovoy wrote:
> On Wed, 23 Jul 2014, Martin Kosek wrote:
>> On 07/23/2014 05:21 PM, Alexander Bokovoy wrote:
>>> On Wed, 23 Jul 2014, Martin Kosek wrote:
 On 07/23/2014 05:07 PM, Alexander Bokovoy wrote:
> On Wed, 23 Jul 2014, Martin Kosek wrote:
>> Theme package is contains resources for PKI web interface. This interface
>> is not needed by FreeIPA as it rather utilizes it's API. As recommended 
>> in
>> https://bugzilla.redhat.com/show_bug.cgi?id=1068029#c5, remove this hard
>> dependency.
> I've seen several times that people continue using PKI web interfaces
> for things we do not support yet, like issuing user certificates and the
> rest of features supported in Dogtag.
>
> I think this change might be subtle but otherwise breaking for some
> users.

 Ah, I personally did not see that yet. However, as this is still quite 
 hacky
 and experimental use of FreeIPA PKI, it should not be in the list of hard
 requirements IMO. People who really need to use this interface can always
 install the package.
>>> At the very least this change has to be in the release notes.
>>
>> I think I see the deal - apply the patch only for FreeIPA 4.1 and master (and
>> not for 4.0.x) and add appropriate release notes for 4.1, when released.
> Agreed.

Pushed to:
master: 1026a6387cd392994ec996db53141d16dfcbee29
ipa-4-1: 1026a6387cd392994ec996db53141d16dfcbee29

Please remind me in case we forgot to mention that in our release notes :-)

Thanks,
Martin

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


Re: [Freeipa-devel] [PATCH] 0002 Improve password validity check

2014-07-24 Thread David Kupka

On 07/22/2014 08:55 AM, Martin Kosek wrote:

On 07/21/2014 04:08 PM, David Kupka wrote:

On 07/18/2014 12:52 PM, Martin Kosek wrote:

On 07/18/2014 12:33 PM, David Kupka wrote:

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


1) Would it be easier/more convenient to just implement following simple check
instead of bad_prefix/bad_suffix?

if password.strip() != password:
 raise ValueError('Password must not start or end with whitespace')



Yes it would. Edited patch attached.



2) The main goal of the ticket 2796 was not fixed yet. It sometimes happen that
when installation crashes somewhere right after pkicreate, it does not record
and and does not uninstall the PKI component during "ipa-server-install
--uninstall".

You may artificially invoke some crash in cainstance.py after pkicreate to test
it. When fixing it, check how is_configured() in Service object works an how
self.backup_state is called in other service modules (like dsinstance.py) where
the detection works correctly.


You're completely right, Martin. I was unable to reproduce the bug (to force
pkicreate/pkispawn to fail) so I thought that it was fixed by the password
restriction.
Then I discovered that most of the banned characters for password are no longer
causing troubles a focused on this. But it's yet another issue.


1) Whitespace error:

$ git am /tmp/freeipa-dkupka-0002-2-Improve-password-validity-check.patch
Applying: Improve password validity check.
/home/mkosek/freeipa/.git/rebase-apply/patch:25: trailing whitespace.
 # Disallow leading/trailing whaitespaces
warning: 1 line adds whitespace errors.


Git is highlighting these errors I was probably temporary blind.


2) The new admin validator is not applied to "-a" command line option and you
can pass any garbage to it. You need to replace this section:

 if options.admin_password is not None and len(options.admin_password) < 8:
 parser.error("Admin user password must be at least 8 characters long")

... with the new validator just like we validate DM password.

Added.



Martin



--
David Kupka
From c11f52766646a2ee597009bb14f15c3633c18591 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 24 Jul 2014 13:32:37 +0200
Subject: [PATCH] Improve password validity check.

Allow use of characters that no longer cause troubles. Check for
leading and trailing characters in case of 389 Direcory Manager password.
---
 install/tools/ipa-server-install | 35 +++
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 671a226d625ab9e8168c569a6d83c35dfae52115..fc9cef06076110a969a3db6640f0288e3de2ce45 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -121,7 +121,31 @@ def validate_dm_password(password):
 raise ValueError("Password must only contain ASCII characters")
 
 # Disallow characters that pkisilent doesn't process properly:
-bad_characters = ' &\\<%'
+bad_characters = '\\'
+if any(c in bad_characters for c in password):
+raise ValueError('Password must not contain these characters: %s' %
+', '.join('"%s"' % c for c in bad_characters))
+
+# TODO: Check https://fedorahosted.org/389/ticket/47849
+# Actual behavior of setup-ds.pl is that it does not accept white
+# space characters in password when called interactively but does when
+# provided such password in INF file. But it ignores leading and trailing
+# white spaces in INF file.
+
+# Disallow leading/trailing whaitespaces
+if password.strip() != password:
+raise ValueError('Password must not start or end with whitespace.')
+
+def validate_admin_password(password):
+if len(password) < 8:
+raise ValueError("Password must be at least 8 characters long")
+if any(ord(c) < 0x20 for c in password):
+raise ValueError("Password must not contain control characters")
+if any(ord(c) >= 0x7F for c in password):
+raise ValueError("Password must only contain ASCII characters")
+
+# Disallow characters that pkisilent doesn't process properly:
+bad_characters = '\\'
 if any(c in bad_characters for c in password):
 raise ValueError('Password must not contain these characters: %s' %
 ', '.join('"%s"' % c for c in bad_characters))
@@ -239,8 +263,11 @@ def parse_options():
 validate_dm_password(options.dm_password)
 except ValueError, e:
 parser.error("DS admin password: " + str(e))
-if options.admin_password is not None and len(options.admin_password) < 8:
-parser.error("Admin user password must be at least 8 characters long")
+if options.admin_password is not None:
+try:
+validate_admin_password(options.admin_password)
+except ValueError, e:
+parser.error("Admin user password: " + str(e))
 
 if options.domain_name is not None:
 try:
@@ -4

Re: [Freeipa-devel] [PATCH] 0002 Improve password validity check

2014-07-24 Thread Martin Kosek
On 07/24/2014 02:02 PM, David Kupka wrote:
> On 07/22/2014 08:55 AM, Martin Kosek wrote:
>> On 07/21/2014 04:08 PM, David Kupka wrote:
>>> On 07/18/2014 12:52 PM, Martin Kosek wrote:
 On 07/18/2014 12:33 PM, David Kupka wrote:
> https://fedorahosted.org/freeipa/ticket/2796

 1) Would it be easier/more convenient to just implement following simple 
 check
 instead of bad_prefix/bad_suffix?

 if password.strip() != password:
  raise ValueError('Password must not start or end with whitespace')

>>>
>>> Yes it would. Edited patch attached.
>>>

 2) The main goal of the ticket 2796 was not fixed yet. It sometimes happen
 that
 when installation crashes somewhere right after pkicreate, it does not 
 record
 and and does not uninstall the PKI component during "ipa-server-install
 --uninstall".

 You may artificially invoke some crash in cainstance.py after pkicreate to
 test
 it. When fixing it, check how is_configured() in Service object works an 
 how
 self.backup_state is called in other service modules (like dsinstance.py)
 where
 the detection works correctly.
>>>
>>> You're completely right, Martin. I was unable to reproduce the bug (to force
>>> pkicreate/pkispawn to fail) so I thought that it was fixed by the password
>>> restriction.
>>> Then I discovered that most of the banned characters for password are no 
>>> longer
>>> causing troubles a focused on this. But it's yet another issue.
>>
>> 1) Whitespace error:
>>
>> $ git am /tmp/freeipa-dkupka-0002-2-Improve-password-validity-check.patch
>> Applying: Improve password validity check.
>> /home/mkosek/freeipa/.git/rebase-apply/patch:25: trailing whitespace.
>>  # Disallow leading/trailing whaitespaces
>> warning: 1 line adds whitespace errors.
> 
> Git is highlighting these errors I was probably temporary blind.
>>
>> 2) The new admin validator is not applied to "-a" command line option and you
>> can pass any garbage to it. You need to replace this section:
>>
>>  if options.admin_password is not None and len(options.admin_password) < 
>> 8:
>>  parser.error("Admin user password must be at least 8 characters 
>> long")
>>
>> ... with the new validator just like we validate DM password.
> Added.

This one works fine. ACK.

Pushed to:
master: 603842867c65ae93d74a7c453c4301073c998441
ipa-4-1: 603842867c65ae93d74a7c453c4301073c998441

Martin

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


[Freeipa-devel] [PATCH] 0007 test group: remove group from protected group

2014-07-24 Thread David Kupka

Simple test scenario from ticket #4448.

Last test will fail until patch freeipa-dkupka-0006 gets accepted.

--
David Kupka
From 240f48865ebb93a9a4d71250f3bcef1c48c453bb Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 24 Jul 2014 14:45:50 +0200
Subject: [PATCH] test group: remove group from protected group.

Related to https://fedorahosted.org/freeipa/ticket/4448
---
 ipatests/test_xmlrpc/test_group_plugin.py | 63 +++
 1 file changed, 63 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py
index 71172893beb97d3373b0b9299f2bfa7bb642dba6..cc526277cfeca1a5122a704d72afb88b7d6250d7 100644
--- a/ipatests/test_xmlrpc/test_group_plugin.py
+++ b/ipatests/test_xmlrpc/test_group_plugin.py
@@ -31,6 +31,7 @@ from ipatests.test_xmlrpc.test_user_plugin import get_user_result
 group1 = u'testgroup1'
 group2 = u'testgroup2'
 group3 = u'testgroup3'
+group4 = u'testgroup4'
 renamedgroup1 = u'testgroup'
 user1 = u'tuser1'
 
@@ -50,6 +51,7 @@ class test_group(Declarative):
 ('group_del', [group1], {}),
 ('group_del', [group2], {}),
 ('group_del', [group3], {}),
+('group_del', [group4], {}),
 ('group_del', [renamedgroup1], {}),
 ('user_del', [user1], {}),
 ]
@@ -1010,4 +1012,65 @@ class test_group(Declarative):
 ),
 ),
 
+# Test scenario from ticket #4448
+# https://fedorahosted.org/freeipa/ticket/4448
+dict(
+desc='Add group %s' % group4,
+command=('group_add', [group4], dict(description=u'Test desc 4')),
+expected=dict(
+value=group4,
+summary=u'Added group "%s"' % group4,
+result=dict(
+cn=[group4],
+description=[u'Test desc 4'],
+objectclass=objectclasses.posixgroup,
+gidnumber=[fuzzy_digits],
+ipauniqueid=[fuzzy_uuid],
+dn=get_group_dn(group4),
+),
+),
+),
+
+dict(
+desc='Add %s group to admins group' % group4,
+command=('group_add_member', [u'admins'], dict(group=group4)),
+expected=dict(
+completed=1,
+failed=dict(
+member=dict(
+group=tuple(),
+user=tuple(),
+),
+),
+result=dict(
+dn=get_group_dn('admins'),
+member_user=[u'admin'],
+member_group=[group4],
+gidnumber=[fuzzy_digits],
+cn=[u'admins'],
+description=[u'Account administrators group'],
+),
+),
+),
+
+dict(
+desc='Remove %s group from admins group' % group4,
+command=('group_remove_member', [u'admins'], dict(group=group4)),
+expected=dict(
+completed=1,
+failed=dict(
+member=dict(
+group=tuple(),
+user=tuple(),
+),
+),
+result=dict(
+dn=get_group_dn(u'admins'),
+cn=[u'admins'],
+gidnumber=[fuzzy_digits],
+member_user=[u'admin'],
+description=[u'Account administrators group'],
+),
+),
+),
 ]
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH 0246] baseldap: Fix undefined variable reference in

2014-07-24 Thread Rob Crittenden
Tomas Babej wrote:
> 
> On 07/24/2014 12:35 PM, Tomas Babej wrote:
>> Hi,
>>
>> on receiving a PublicError we fail with InternalError since msg is not
>> defined.
>>
>>
>>
>> ___
>> Freeipa-devel mailing list
>> Freeipa-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
> 
> I also realized there's no need for the nested try-blocks, so added some
> refactoring to the fix, which makes the code much more simple.

Please open a ticket for this.

Also note that the exc_wrapper may raise an exception which I believe is
why I nested the exception originally. It may be no longer needed but
worth a second look.

rob

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


[Freeipa-devel] [PATCH 0060] Fix ipa-getkeytab for pre-4.0 servers

2014-07-24 Thread Nathaniel McCallum
Also, make the error messages for this fallback case less scary and
clean up some indentation issues in the nearby code which made this
code difficult to read.
From 7cfe668e116b60ab2e4149135110f32b165f4915 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Thu, 24 Jul 2014 09:50:57 -0400
Subject: [PATCH] Fix ipa-getkeytab for pre-4.0 servers

Also, make the error messages for this fallback case less scary and
clean up some indentation issues in the nearby code which made this
code difficult to read.
---
 ipa-client/ipa-getkeytab.c | 38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/ipa-client/ipa-getkeytab.c b/ipa-client/ipa-getkeytab.c
index c887cff9bb5e3688cc84b5c28f791eb922f4fe61..7861e4e508ce956a92d80d2e91294215854a2a32 100644
--- a/ipa-client/ipa-getkeytab.c
+++ b/ipa-client/ipa-getkeytab.c
@@ -258,10 +258,10 @@ static int ipa_ldap_extended_op(LDAP *ld, const char *reqoid,
 int msgid;
 int ret, rc;
 
-ret = ldap_extended_operation(ld, KEYTAB_GET_OID, control,
+ret = ldap_extended_operation(ld, reqoid, control,
   NULL, NULL, &msgid);
 if (ret != LDAP_SUCCESS) {
-fprintf(stderr, _("Operation failed! %s\n"), ldap_err2string(ret));
+fprintf(stderr, _("Operation failed: %s\n"), ldap_err2string(ret));
 return ret;
 }
 
@@ -270,20 +270,20 @@ static int ipa_ldap_extended_op(LDAP *ld, const char *reqoid,
 tv.tv_usec = 0;
 ret = ldap_result(ld, msgid, 1, &tv, &res);
 if (ret == -1) {
-fprintf(stderr, _("Failed to get result! %s\n"), ldap_err2string(ret));
+fprintf(stderr, _("Failed to get result: %s\n"), ldap_err2string(ret));
 goto done;
 }
 
 ret = ldap_parse_extended_result(ld, res, &retoid, &retdata, 0);
 if (ret != LDAP_SUCCESS) {
-fprintf(stderr, _("Failed to parse extended result! %s\n"),
+fprintf(stderr, _("Failed to parse extended result: %s\n"),
 ldap_err2string(ret));
 goto done;
 }
 
 ret = ldap_parse_result(ld, res, &rc, NULL, &err, NULL, srvctrl, 0);
 if (ret != LDAP_SUCCESS || rc != LDAP_SUCCESS) {
-fprintf(stderr, _("Failed to parse result! %s\n"),
+fprintf(stderr, _("Failed to parse result: %s\n"),
 err ? err : ldap_err2string(ret));
 if (ret == LDAP_SUCCESS) ret = rc;
 goto done;
@@ -917,20 +917,24 @@ int main(int argc, const char *argv[])
 }
 }
 
-if (password && (retrieve == 0) && (kvno == -1)) {
-if (!quiet) fprintf(stderr, _("Retrying with old method\n"));
+if (retrieve == 0 && kvno == -1) {
+if (!quiet) {
+fprintf(stderr,
+_("Retrying with pre-4.0 keytab retrieval method...\n"));
+}
 
-	/* create key material */
-	ret = create_keys(krbctx, sprinc, password, enctypes_string, &keys, &err_msg);
-	if (!ret) {
-		if (err_msg != NULL) {
-			fprintf(stderr, "%s", err_msg);
-		}
-		fprintf(stderr, _("Failed to create key material\n"));
-		exit(8);
-	}
+/* create key material */
+ret = create_keys(krbctx, sprinc, password, enctypes_string, &keys, &err_msg);
+if (!ret) {
+if (err_msg != NULL) {
+fprintf(stderr, "%s", err_msg);
+}
 
-	kvno = ldap_set_keytab(krbctx, server, principal, uprinc, binddn, bindpw, &keys);
+fprintf(stderr, _("Failed to create key material\n"));
+exit(8);
+}
+
+kvno = ldap_set_keytab(krbctx, server, principal, uprinc, binddn, bindpw, &keys);
 }
 
 if (kvno == -1) {
-- 
2.0.1

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

[Freeipa-devel] [PATCH] 308 Allow changing CA renewal master in ipa-csreplica-manage

2014-07-24 Thread Jan Cholasta

Hi,

the attached patch fixes .

Requires my patches 246 and 262 (current versions attached).

Honza

--
Jan Cholasta
>From 37deddbb4c80697460ef4af204e3a2e36dcbbe4e Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 24 Mar 2014 15:30:53 +0100
Subject: [PATCH 06/56] Add method for setting CA renewal master in LDAP to
 CAInstance.

Allow checking and setting CA renewal master for non-local CA instances.
---
 ipaserver/install/cainstance.py | 41 ++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index f0aef75..7e2572d 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1609,12 +1609,15 @@ class CAInstance(service.Service):
 return True
 return False
 
-def is_renewal_master(self):
+def is_renewal_master(self, fqdn=None):
+if fqdn is None:
+fqdn = api.env.host
+
 if not self.admin_conn:
 self.ldap_connect()
 
-dn = DN(('cn', 'CA'), ('cn', api.env.host), ('cn', 'masters'),
-('cn', 'ipa'), ('cn', 'etc'), api.env.basedn)
+dn = DN(('cn', 'CA'), ('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'),
+('cn', 'etc'), api.env.basedn)
 filter = '(ipaConfigString=caRenewalMaster)'
 try:
 self.admin_conn.get_entries(base_dn=dn, filter=filter,
@@ -1624,6 +1627,38 @@ class CAInstance(service.Service):
 
 return True
 
+def set_renewal_master(self, fqdn=None):
+if fqdn is None:
+fqdn = api.env.host
+
+if not self.admin_conn:
+self.ldap_connect()
+
+base_dn = DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'),
+ api.env.basedn)
+filter = '(&(cn=CA)(ipaConfigString=caRenewalMaster))'
+try:
+entries = self.admin_conn.get_entries(
+base_dn=base_dn, filter=filter, attrs_list=['ipaConfigString'])
+except errors.NotFound:
+entries = []
+
+dn = DN(('cn', 'CA'), ('cn', fqdn), base_dn)
+master_entry = self.admin_conn.get_entry(dn, ['ipaConfigString'])
+
+for entry in entries:
+if master_entry is not None and entry.dn == master_entry.dn:
+master_entry = None
+continue
+
+entry['ipaConfigString'] = [x for x in entry['ipaConfigString']
+if x.lower() != 'carenewalmaster']
+self.admin_conn.update_entry(entry)
+
+if master_entry is not None:
+master_entry['ipaConfigString'].append('caRenewalMaster')
+self.admin_conn.update_entry(master_entry)
+
 
 def replica_ca_install_check(config):
 if not config.setup_ca:
-- 
1.9.3

>From 8d9e84e14472d842d1b67452c9208131b76b2652 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 10 Apr 2014 14:14:10 +0200
Subject: [PATCH 14/56] Pick new CA renewal master when deleting a replica.

---
 install/tools/ipa-csreplica-manage | 10 --
 install/tools/ipa-replica-manage   | 13 -
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/install/tools/ipa-csreplica-manage b/install/tools/ipa-csreplica-manage
index eb589f3..cfcb354 100755
--- a/install/tools/ipa-csreplica-manage
+++ b/install/tools/ipa-csreplica-manage
@@ -25,7 +25,8 @@ import os
 import krbV
 from ipapython.ipa_log_manager import *
 
-from ipaserver.install import replication, installutils, bindinstance
+from ipaserver.install import (replication, installutils, bindinstance,
+cainstance, certs)
 from ipalib import api, errors, util
 from ipalib.constants import CACERT
 from ipapython import ipautil, ipaldap, version, dogtag
@@ -272,7 +273,12 @@ def del_master(realm, hostname, options):
 except Exception, e:
 sys.exit("There were issues removing a connection: %s" % e)
 
-# 6. And clean up the removed replica DNS entries if any.
+# 6. Pick CA renewal master
+ca = cainstance.CAInstance(api.env.realm, certs.NSS_DIR)
+if ca.is_renewal_master(hostname):
+ca.set_renewal_master(options.host)
+
+# 7. And clean up the removed replica DNS entries if any.
 try:
 if bindinstance.dns_container_exists(options.host, api.env.basedn,
  dm_password=options.dirman_passwd):
diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index d468850..aa71095 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -28,7 +28,7 @@ import socket
 
 from ipapython import ipautil
 from ipaserver.install import replication, dsinstance, installutils
-from ipaserver.install import bindinstance
+from ipaserver.install import bindinstance, cainstance, certs
 from ipaserver.plugins import ldap2
 from ipapython import version, ipaldap
 from ipalib im

Re: [Freeipa-devel] [PATCH 0060] Fix ipa-getkeytab for pre-4.0 servers

2014-07-24 Thread Alexander Bokovoy

On Thu, 24 Jul 2014, Nathaniel McCallum wrote:

Also, make the error messages for this fallback case less scary and
clean up some indentation issues in the nearby code which made this
code difficult to read.

ACK. Here is how it looks now in /var/log/ipaclient-install.log:

2014-07-24T14:15:36Z DEBUG Starting external process
2014-07-24T14:15:36Z DEBUG args='/usr/sbin/ipa-join' '-s' 'ipa-07-f20.t.vda.li' 
'-b' 'dc=t,dc=vda,dc=li' '-h' 'ipa-01.t.vda.li'
2014-07-24T14:15:38Z DEBUG Process finished, return code=0
2014-07-24T14:15:38Z DEBUG stdout=
2014-07-24T14:15:38Z DEBUG stderr=Failed to parse result: unsupported extended 
operation
Retrying with pre-4.0 keytab retrieval method...
Keytab successfully retrieved and stored in: /etc/krb5.keytab
Certificate subject base is: O=T.VDA.LI

2014-07-24T14:15:38Z INFO Enrolled in IPA realm T.VDA.LI


--
/ Alexander Bokovoy

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


[Freeipa-devel] [PATCH] 309 Check if /root/ipa.csr exists when installing server with external CA

2014-07-24 Thread Jan Cholasta

Hi,

the attached patch fixes .

Honza

--
Jan Cholasta
>From 94a7696c04b2d36da10bf6d64c94902d0dde0216 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 24 Jul 2014 16:32:00 +0200
Subject: [PATCH] Check if /root/ipa.csr exists when installing server with
 external CA.

Remove the file on uninstall.

https://fedorahosted.org/freeipa/ticket/4303
---
 install/tools/ipa-server-install | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 671a226..1f158a4 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -513,6 +513,10 @@ def uninstall():
 os.remove(ANSWER_CACHE)
 except Exception:
 pass
+try:
+os.remove(paths.ROOT_IPA_CSR)
+except Exception:
+pass
 
 # ipa-client-install removes /etc/ipa/default.conf
 
@@ -686,13 +690,21 @@ def main():
 
 if options.external_ca:
 if cainstance.is_step_one_done():
-print "CA is already installed.\nRun the installer with --external_cert_file and --external_ca_file."
+print ("CA is already installed.\nRun the installer with "
+   "--external_cert_file and --external_ca_file.")
+sys.exit(1)
+if ipautil.file_exists(paths.ROOT_IPA_CSR):
+print ("CA CSR file %s already exists.\nIn order to continue "
+   "remove the file and run the installer again." %
+   paths.ROOT_IPA_CSR)
 sys.exit(1)
 elif options.external_cert_file:
 if not cainstance.is_step_one_done():
 # This can happen if someone passes external_ca_file without
 # already having done the first stage of the CA install.
-print "CA is not installed yet. To install with an external CA is a two-stage process.\nFirst run the installer with --external-ca."
+print ("CA is not installed yet. To install with an external CA "
+   "is a two-stage process.\nFirst run the installer with "
+   "--external-ca.")
 sys.exit(1)
 
 # This will override any settings passed in on the cmdline
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH 0060] Fix ipa-getkeytab for pre-4.0 servers

2014-07-24 Thread Nathaniel McCallum
On Thu, 2014-07-24 at 17:19 +0300, Alexander Bokovoy wrote:
> On Thu, 24 Jul 2014, Nathaniel McCallum wrote:
> >Also, make the error messages for this fallback case less scary and
> >clean up some indentation issues in the nearby code which made this
> >code difficult to read.
> ACK. Here is how it looks now in /var/log/ipaclient-install.log:
> 
> 2014-07-24T14:15:36Z DEBUG Starting external process
> 2014-07-24T14:15:36Z DEBUG args='/usr/sbin/ipa-join' '-s' 
> 'ipa-07-f20.t.vda.li' '-b' 'dc=t,dc=vda,dc=li' '-h' 'ipa-01.t.vda.li'
> 2014-07-24T14:15:38Z DEBUG Process finished, return code=0
> 2014-07-24T14:15:38Z DEBUG stdout=
> 2014-07-24T14:15:38Z DEBUG stderr=Failed to parse result: unsupported 
> extended operation
> Retrying with pre-4.0 keytab retrieval method...
> Keytab successfully retrieved and stored in: /etc/krb5.keytab
> Certificate subject base is: O=T.VDA.LI
> 
> 2014-07-24T14:15:38Z INFO Enrolled in IPA realm T.VDA.LI

Attached is the same patch with the bug link in the commit message.
From 94d66c803e412d6415da0d62a6fa3d03d3ebd997 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum 
Date: Thu, 24 Jul 2014 09:50:57 -0400
Subject: [PATCH] Fix ipa-getkeytab for pre-4.0 servers

Also, make the error messages for this fallback case less scary and
clean up some indentation issues in the nearby code which made this
code difficult to read.

https://fedorahosted.org/freeipa/ticket/4446
---
 ipa-client/ipa-getkeytab.c | 38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/ipa-client/ipa-getkeytab.c b/ipa-client/ipa-getkeytab.c
index c887cff9bb5e3688cc84b5c28f791eb922f4fe61..7861e4e508ce956a92d80d2e91294215854a2a32 100644
--- a/ipa-client/ipa-getkeytab.c
+++ b/ipa-client/ipa-getkeytab.c
@@ -258,10 +258,10 @@ static int ipa_ldap_extended_op(LDAP *ld, const char *reqoid,
 int msgid;
 int ret, rc;
 
-ret = ldap_extended_operation(ld, KEYTAB_GET_OID, control,
+ret = ldap_extended_operation(ld, reqoid, control,
   NULL, NULL, &msgid);
 if (ret != LDAP_SUCCESS) {
-fprintf(stderr, _("Operation failed! %s\n"), ldap_err2string(ret));
+fprintf(stderr, _("Operation failed: %s\n"), ldap_err2string(ret));
 return ret;
 }
 
@@ -270,20 +270,20 @@ static int ipa_ldap_extended_op(LDAP *ld, const char *reqoid,
 tv.tv_usec = 0;
 ret = ldap_result(ld, msgid, 1, &tv, &res);
 if (ret == -1) {
-fprintf(stderr, _("Failed to get result! %s\n"), ldap_err2string(ret));
+fprintf(stderr, _("Failed to get result: %s\n"), ldap_err2string(ret));
 goto done;
 }
 
 ret = ldap_parse_extended_result(ld, res, &retoid, &retdata, 0);
 if (ret != LDAP_SUCCESS) {
-fprintf(stderr, _("Failed to parse extended result! %s\n"),
+fprintf(stderr, _("Failed to parse extended result: %s\n"),
 ldap_err2string(ret));
 goto done;
 }
 
 ret = ldap_parse_result(ld, res, &rc, NULL, &err, NULL, srvctrl, 0);
 if (ret != LDAP_SUCCESS || rc != LDAP_SUCCESS) {
-fprintf(stderr, _("Failed to parse result! %s\n"),
+fprintf(stderr, _("Failed to parse result: %s\n"),
 err ? err : ldap_err2string(ret));
 if (ret == LDAP_SUCCESS) ret = rc;
 goto done;
@@ -917,20 +917,24 @@ int main(int argc, const char *argv[])
 }
 }
 
-if (password && (retrieve == 0) && (kvno == -1)) {
-if (!quiet) fprintf(stderr, _("Retrying with old method\n"));
+if (retrieve == 0 && kvno == -1) {
+if (!quiet) {
+fprintf(stderr,
+_("Retrying with pre-4.0 keytab retrieval method...\n"));
+}
 
-	/* create key material */
-	ret = create_keys(krbctx, sprinc, password, enctypes_string, &keys, &err_msg);
-	if (!ret) {
-		if (err_msg != NULL) {
-			fprintf(stderr, "%s", err_msg);
-		}
-		fprintf(stderr, _("Failed to create key material\n"));
-		exit(8);
-	}
+/* create key material */
+ret = create_keys(krbctx, sprinc, password, enctypes_string, &keys, &err_msg);
+if (!ret) {
+if (err_msg != NULL) {
+fprintf(stderr, "%s", err_msg);
+}
 
-	kvno = ldap_set_keytab(krbctx, server, principal, uprinc, binddn, bindpw, &keys);
+fprintf(stderr, _("Failed to create key material\n"));
+exit(8);
+}
+
+kvno = ldap_set_keytab(krbctx, server, principal, uprinc, binddn, bindpw, &keys);
 }
 
 if (kvno == -1) {
-- 
2.0.1

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

[Freeipa-devel] LDAP updater with --test option

2014-07-24 Thread Martin Basti

Hi list,

maybe I missed something, but I expected, there are no modifications 
with this option.


With --test option the LDAP schema is not updated,  but update plugins 
don't care about --test option ('live_run' in code).


Update plugins use and IPA api directly to modify LDAP instead of return 
a required changes
(DNS, update_idranges, update_managed_permissions, update_pacs, 
update_services  plugins).


Am wrong, or it is bad behavior and plugin should be fixed to not 
execute any modifications in test mode?


Next Q: I have method which prepares IPA to support DNSSEC. The method  
requires both updating LDAP and creating directories/keytabs/etc.
Should I separate the LDAP part of update method, or can I use it all in 
ldap-updater?


--
Martin Basti

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


Re: [Freeipa-devel] LDAP updater with --test option

2014-07-24 Thread Jan Cholasta

Dne 24.7.2014 v 17:14 Martin Basti napsal(a):

Hi list,

maybe I missed something, but I expected, there are no modifications
with this option.

With --test option the LDAP schema is not updated,  but update plugins
don't care about --test option ('live_run' in code).


Most plugins should respect --test, only those that use ipaldap directly 
not, see .




Update plugins use and IPA api directly to modify LDAP instead of return
a required changes
(DNS, update_idranges, update_managed_permissions, update_pacs,
update_services  plugins).

Am wrong, or it is bad behavior and plugin should be fixed to not
execute any modifications in test mode?

Next Q: I have method which prepares IPA to support DNSSEC. The method
requires both updating LDAP and creating directories/keytabs/etc.
Should I separate the LDAP part of update method, or can I use it all in
ldap-updater?




--
Jan Cholasta

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


Re: [Freeipa-devel] LDAP updater with --test option

2014-07-24 Thread Rob Crittenden
Martin Basti wrote:
> Hi list,
> 
> maybe I missed something, but I expected, there are no modifications
> with this option.
> 
> With --test option the LDAP schema is not updated,  but update plugins
> don't care about --test option ('live_run' in code).
> 
> Update plugins use and IPA api directly to modify LDAP instead of return
> a required changes
> (DNS, update_idranges, update_managed_permissions, update_pacs,
> update_services  plugins).
> 
> Am wrong, or it is bad behavior and plugin should be fixed to not
> execute any modifications in test mode?

I seem to recall that Petr^3 saw this as well and filed a ticket though
I can't find it. IMHO yes, plugins should honor the test mode.

> Next Q: I have method which prepares IPA to support DNSSEC. The method 
> requires both updating LDAP and creating directories/keytabs/etc.
> Should I separate the LDAP part of update method, or can I use it all in
> ldap-updater?

The updater is intended for LDAP updates only. Probably best to split it
with the ipa-upgradeconfig script.

rob

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


[Freeipa-devel] [PATCH] 310 Exclude attributelevelrights from --raw result processing in baseldap

2014-07-24 Thread Jan Cholasta

Hi,

the attached patch fixes .

Honza

--
Jan Cholasta
>From 76204f58608ee9f85b704f71909449bed0173253 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 24 Jul 2014 17:17:48 +0200
Subject: [PATCH] Exclude attributelevelrights from --raw result processing in
 baseldap.

https://fedorahosted.org/freeipa/ticket/4371
---
 ipalib/plugins/baseldap.py | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index e5a23b9..b112c35 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -240,9 +240,11 @@ def entry_from_entry(entry, newentry):
 def entry_to_dict(entry, **options):
 if options.get('raw', False):
 result = {}
-for attr, value in entry.raw.iteritems():
-if entry.conn.get_type(attr) is not str:
-value = list(value)
+for attr in entry.iterkeys():
+if attr.lower() == 'attributelevelrights':
+value = entry[attr]
+elif entry.conn.get_type(attr) is not str:
+value = list(entry.raw[attr])
 for (i, v) in enumerate(value):
 try:
 value[i] = v.decode('utf-8')
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH] 711 webui: internet explorer fixes

2014-07-24 Thread Petr Vobornik

On 23.7.2014 15:17, Petr Vobornik wrote:

Fixed:
1. IE doesn't support value 'initial' in CSS rule.
2. setting innerHTML='' also destroys content of child nodes in
LoginScreen in IE -> reattached buttons have no text.

Should go into 4.0 Milestone



Found an issue in the implementation, new version attached.
--
Petr Vobornik
From 4697a76a171e8a64a3c118be77e1e818e196eaca Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Tue, 22 Jul 2014 14:15:11 +0200
Subject: [PATCH] webui: internet explorer fixes

Fixed:
1. IE doesn't support value 'initial' in CSS rule.
2. setting innerHTML='' also destroys content of child nodes in
LoginScreen in IE -> reattached buttons have no text.
---
 install/ui/less/widgets.less  | 2 +-
 install/ui/src/freeipa/widgets/LoginScreen.js | 8 +++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/install/ui/less/widgets.less b/install/ui/less/widgets.less
index c21a163a1061514061463060195a1adfff619752..7876307f1cdf44bdd548a89f6f49a7d83adbc2d5 100644
--- a/install/ui/less/widgets.less
+++ b/install/ui/less/widgets.less
@@ -4,7 +4,7 @@
 
 	.global-activity-indicator {
 
-		bottom: initial;
+		bottom: auto;
 		height: auto;
 		background-color: rgba(0, 0, 0, 0.3);
 		color: white;
diff --git a/install/ui/src/freeipa/widgets/LoginScreen.js b/install/ui/src/freeipa/widgets/LoginScreen.js
index 3e0986435bc80a82a626aa85c6f9fe1a73988b58..e7e1b029e890469874ba040e6d4142f50afcdd3a 100644
--- a/install/ui/src/freeipa/widgets/LoginScreen.js
+++ b/install/ui/src/freeipa/widgets/LoginScreen.js
@@ -231,7 +231,13 @@ define(['dojo/_base/declare',
 
 refresh: function() {
 if (this.buttons_node) {
-this.buttons_node.innerHTML = "";
+// detach button nodes politely
+// hard methods like `innerHTML=''` might have undesired
+// consequences, e.g., removal of children's content in IE
+var bn = this.buttons_node;
+while (bn.firstChild) {
+bn.removeChild(bn.firstChild);
+}
 }
 if (this.view === 'reset') {
 this.show_reset_view();
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH 0060] Fix ipa-getkeytab for pre-4.0 servers

2014-07-24 Thread Martin Kosek
On 07/24/2014 05:12 PM, Nathaniel McCallum wrote:
> On Thu, 2014-07-24 at 17:19 +0300, Alexander Bokovoy wrote:
>> On Thu, 24 Jul 2014, Nathaniel McCallum wrote:
>>> Also, make the error messages for this fallback case less scary and
>>> clean up some indentation issues in the nearby code which made this
>>> code difficult to read.
>> ACK. Here is how it looks now in /var/log/ipaclient-install.log:
>>
>> 2014-07-24T14:15:36Z DEBUG Starting external process
>> 2014-07-24T14:15:36Z DEBUG args='/usr/sbin/ipa-join' '-s' 
>> 'ipa-07-f20.t.vda.li' '-b' 'dc=t,dc=vda,dc=li' '-h' 'ipa-01.t.vda.li'
>> 2014-07-24T14:15:38Z DEBUG Process finished, return code=0
>> 2014-07-24T14:15:38Z DEBUG stdout=
>> 2014-07-24T14:15:38Z DEBUG stderr=Failed to parse result: unsupported 
>> extended operation
>> Retrying with pre-4.0 keytab retrieval method...
>> Keytab successfully retrieved and stored in: /etc/krb5.keytab
>> Certificate subject base is: O=T.VDA.LI
>>
>> 2014-07-24T14:15:38Z INFO Enrolled in IPA realm T.VDA.LI
> 
> Attached is the same patch with the bug link in the commit message.

Good! Thanks for fixing the scary error messages :-)

Pushed to:
master: 96986056f65beb120cd74a311524b6601383ee80
ipa-4-1: 96986056f65beb120cd74a311524b6601383ee80
ipa-4-0: 217aba77dcfc59c52ad565e33af341da06e76bcc

Martin

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