[Freeipa-devel] Move 4.1.5 tickets to 4.2.1

2015-07-28 Thread Martin Kosek
With FreeIPA 4.2.0 released, I think we can move all the 4.1.5 tickets to
FreeIPA 4.2.1 bucket and the fixes be based on that release.

I would only do exception if some of the fixes are critical for platforms that
did not adopt FreeIPA 4.2.0 yet (most of them ;-), but I am not aware of such
tickets.

-- 
Martin Kosek 
Supervisor, Software Engineering - Identity Management Team
Red Hat Inc.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] 4.2: ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.

2015-07-28 Thread Jan Pazdziora
On Tue, Jul 28, 2015 at 03:56:47PM +0200, Jan Pazdziora wrote:
> 
> INFO: Server startup in 5444 ms
> INFO: Server startup in 5936 ms
> INFO: Server startup in 5804 ms

Running netstat at the time when the tomcat should have restarted and
be ready shows

# /usr/bin/netstat -tln
Active Internet connections (only servers)
Proto Recv-Q Send-Q Local Address   Foreign Address State  
tcp6   0  0 127.0.0.1:8005  :::*LISTEN 
tcp6   0  0 :::389  :::*LISTEN 
tcp6   0  0 127.0.0.1:8009  :::*LISTEN 
tcp6   0  0 :::8443 :::*LISTEN 

The :::8080 is missing. Will try to figure out what causes 8443
listen to happen but not 8080.

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Replace stageuser-add --from-delete with user-undel --to-staged

2015-07-28 Thread Martin Basti

On 28/07/15 12:34, Jan Cholasta wrote:

Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a):



Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a):

Dne 27.7.2015 v 17:59 Martin Basti napsal(a):

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

Hello,

I tried to fix #5145 and I partially succeeded.

However, I cannot fix this part of ticket, where user is prompted to
write name and surname.

$ ipa stageuser-add tuser --from-delete
First name: this will be ignored
Last name: this will be also ignored

Added stage user "tuser"


As the first name and last name are mandatory attributes of
stageuser-add command, but they are not needed by when the
--from-delete option is used.
I would like to ask how to fix this issue, IMO this will be huge hack
in internal API. Or should we just document this bug as known issue
(thierry wrote that this is not use case that should be used often)?

The best solution would be separate command, but this idea was
rejected in thread "[Freeipa-devel] User life cycle: question
regarding the design"

Regards
Martin^2


Hello,

as was mentioned before, we have issue with current internal API 
and the

stageuser-add --from-delete command.

We discussed this today, and we did not find a nice way how to fix it,
so we propose this (which is IMO the best solution):

* stageuser-add --from-delete should be deprecated


+1


* create new option for user-undel: used-undel --to-staged  (or create
new command) that will handle moving deleted users to staged area as
--from-delete did.


Make it new command please.



Instead of stageuser-add and option --from-delete, which work totally
different, the command user-undel does similar operation than 
stage-user

--from-delete, it just uses different container.


NACK on stuffing everything into a single command just because it does
something similar.


How about making it a 'stageuser-undel'? The 'user-undel' moves
preserved user to active, so the 'stageuser-undel' would move preserved
to staged. The action is similar, but has slightly different specifics
(which attributes are preserved etc.), and for me the 'stageuser-undel'
feels more natural than 'user-undel --to-staged' since it's basically
the same as there is 'stageuser-add' for creating a staged user, not
'user-add --to-staged'. It would be in the same style as all the other
commands concerning operations with users in staged container.


Well, user-undel is the opposite of user-del, and stageuser-undel 
should be the opposite of stageuser-del. The stageuser-undel you are 
suggesting is not.


Also I'm not sure if we want to (always) remove the deleted user once 
a staged user is created from it, but -undel behaves like that.


I don't think the command should be limited to deleted users only. 
Active and deleted users share the same namespace, so it is an 
arbitrary limitation.


I think that what we are looking for is the opposite of 
stageuser-activate. So maybe user-stage?




Can we use stageuser-from-deleted ?

--
Martin Basti

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Replace stageuser-add --from-delete with user-undel --to-staged

2015-07-28 Thread Jan Cholasta

Dne 28.7.2015 v 16:54 Martin Basti napsal(a):

On 28/07/15 12:34, Jan Cholasta wrote:

Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a):



Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a):

Dne 27.7.2015 v 17:59 Martin Basti napsal(a):

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

Hello,

I tried to fix #5145 and I partially succeeded.

However, I cannot fix this part of ticket, where user is prompted to
write name and surname.

$ ipa stageuser-add tuser --from-delete
First name: this will be ignored
Last name: this will be also ignored

Added stage user "tuser"


As the first name and last name are mandatory attributes of
stageuser-add command, but they are not needed by when the
--from-delete option is used.
I would like to ask how to fix this issue, IMO this will be huge hack
in internal API. Or should we just document this bug as known issue
(thierry wrote that this is not use case that should be used often)?

The best solution would be separate command, but this idea was
rejected in thread "[Freeipa-devel] User life cycle: question
regarding the design"

Regards
Martin^2


Hello,

as was mentioned before, we have issue with current internal API
and the
stageuser-add --from-delete command.

We discussed this today, and we did not find a nice way how to fix it,
so we propose this (which is IMO the best solution):

* stageuser-add --from-delete should be deprecated


+1


* create new option for user-undel: used-undel --to-staged  (or create
new command) that will handle moving deleted users to staged area as
--from-delete did.


Make it new command please.



Instead of stageuser-add and option --from-delete, which work totally
different, the command user-undel does similar operation than
stage-user
--from-delete, it just uses different container.


NACK on stuffing everything into a single command just because it does
something similar.


How about making it a 'stageuser-undel'? The 'user-undel' moves
preserved user to active, so the 'stageuser-undel' would move preserved
to staged. The action is similar, but has slightly different specifics
(which attributes are preserved etc.), and for me the 'stageuser-undel'
feels more natural than 'user-undel --to-staged' since it's basically
the same as there is 'stageuser-add' for creating a staged user, not
'user-add --to-staged'. It would be in the same style as all the other
commands concerning operations with users in staged container.


Well, user-undel is the opposite of user-del, and stageuser-undel
should be the opposite of stageuser-del. The stageuser-undel you are
suggesting is not.

Also I'm not sure if we want to (always) remove the deleted user once
a staged user is created from it, but -undel behaves like that.

I don't think the command should be limited to deleted users only.
Active and deleted users share the same namespace, so it is an
arbitrary limitation.

I think that what we are looking for is the opposite of
stageuser-activate. So maybe user-stage?



Can we use stageuser-from-deleted ?



"from-deleted" is not a verb and like I said, restricting the command to 
deleted users only is rather arbitrary.


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Replace stageuser-add --from-delete with user-undel --to-staged

2015-07-28 Thread Petr Vobornik

On 07/28/2015 04:54 PM, Martin Basti wrote:

On 28/07/15 12:34, Jan Cholasta wrote:

Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a):



Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a):

Dne 27.7.2015 v 17:59 Martin Basti napsal(a):

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

Hello,

I tried to fix #5145 and I partially succeeded.

However, I cannot fix this part of ticket, where user is prompted to
write name and surname.

$ ipa stageuser-add tuser --from-delete
First name: this will be ignored
Last name: this will be also ignored

Added stage user "tuser"


As the first name and last name are mandatory attributes of
stageuser-add command, but they are not needed by when the
--from-delete option is used.
I would like to ask how to fix this issue, IMO this will be huge hack
in internal API. Or should we just document this bug as known issue
(thierry wrote that this is not use case that should be used often)?

The best solution would be separate command, but this idea was
rejected in thread "[Freeipa-devel] User life cycle: question
regarding the design"

Regards
Martin^2


Hello,

as was mentioned before, we have issue with current internal API
and the
stageuser-add --from-delete command.

We discussed this today, and we did not find a nice way how to fix it,
so we propose this (which is IMO the best solution):

* stageuser-add --from-delete should be deprecated


+1


* create new option for user-undel: used-undel --to-staged  (or create
new command) that will handle moving deleted users to staged area as
--from-delete did.


Make it new command please.



Instead of stageuser-add and option --from-delete, which work totally
different, the command user-undel does similar operation than
stage-user
--from-delete, it just uses different container.


NACK on stuffing everything into a single command just because it does
something similar.


How about making it a 'stageuser-undel'? The 'user-undel' moves
preserved user to active, so the 'stageuser-undel' would move preserved
to staged. The action is similar, but has slightly different specifics
(which attributes are preserved etc.), and for me the 'stageuser-undel'
feels more natural than 'user-undel --to-staged' since it's basically
the same as there is 'stageuser-add' for creating a staged user, not
'user-add --to-staged'. It would be in the same style as all the other
commands concerning operations with users in staged container.


Well, user-undel is the opposite of user-del, and stageuser-undel
should be the opposite of stageuser-del. The stageuser-undel you are
suggesting is not.

Also I'm not sure if we want to (always) remove the deleted user once
a staged user is created from it, but -undel behaves like that.

I don't think the command should be limited to deleted users only.
Active and deleted users share the same namespace, so it is an
arbitrary limitation.

I think that what we are looking for is the opposite of
stageuser-activate. So maybe user-stage?



Can we use stageuser-from-deleted ?



user-stage sounds better to me than stageuser-from-deleted

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 020] Change internal rsa_(public|private)_key variable names

2015-07-28 Thread Simo Sorce
On Tue, 2015-07-28 at 16:18 +0200, Christian Heimes wrote:
> In two places the vault plugin refers to rsa public or rsa private key
> although the code can handle just any kind of asymmetric algorithms,
> e.g. ECDSA. The patch just renames the occurences to avoid more
> confusion in the future.

LGTM.
Simo.

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

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] [PATCH 020] Change internal rsa_(public|private)_key variable names

2015-07-28 Thread Christian Heimes
In two places the vault plugin refers to rsa public or rsa private key
although the code can handle just any kind of asymmetric algorithms,
e.g. ECDSA. The patch just renames the occurences to avoid more
confusion in the future.
From 1b09967de50aa3c73a9fcab1ff11aa6d1800bae5 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Tue, 28 Jul 2015 16:12:40 +0200
Subject: [PATCH] Change internal rsa_(public|private)_key variable names

In two places the vault plugin refers to rsa public or rsa private key
although the code can handle just any kind of asymmetric algorithms,
e.g. ECDSA. The patch just renames the occurences to avoid more
confusion in the future.
---
 ipalib/plugins/vault.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index 81197f9328c7ed890fa336f464bfcda475ac6189..a2b78f4dec143524d81a1a006733c22db0f90847 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -469,11 +469,11 @@ class vault(LDAPObject):
 return fernet.encrypt(data)
 
 elif public_key:
-rsa_public_key = load_pem_public_key(
+public_key_obj = load_pem_public_key(
 data=public_key,
 backend=default_backend()
 )
-return rsa_public_key.encrypt(
+return public_key_obj.encrypt(
 data,
 padding.OAEP(
 mgf=padding.MGF1(algorithm=hashes.SHA1()),
@@ -496,12 +496,12 @@ class vault(LDAPObject):
 
 elif private_key:
 try:
-rsa_private_key = load_pem_private_key(
+private_key_obj = load_pem_private_key(
 data=private_key,
 password=None,
 backend=default_backend()
 )
-return rsa_private_key.decrypt(
+return private_key_obj.decrypt(
 data,
 padding.OAEP(
 mgf=padding.MGF1(algorithm=hashes.SHA1()),
-- 
2.4.3



signature.asc
Description: OpenPGP digital signature
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] 4.2: ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.

2015-07-28 Thread Jan Pazdziora
On Tue, Jul 28, 2015 at 03:25:50PM +0300, Alexander Bokovoy wrote:
> On Tue, 28 Jul 2015, Jan Pazdziora wrote:
> >
> >I do run it in container so it could be related, so I'm mostly looking
> >for blind hints about what might have changed in the installer or
> >in dogtag itself in 4.2 that could cause this. For example, did we make
> >the timeout shorter? 
> 
> The timeout is 300:
> >2015-07-28T11:15:42Z DEBUG wait_for_open_ports: localhost [8080, 8443] 
> >timeout 300
> 
> You can look at dogtag's catalina-.log, to see how long did it
> take:
> # grep 'Server startup' /var/log/pki/pki-tomcat/catalina.2015-07-24.log
> INFO: Server startup in 27159 ms
> INFO: Server startup in 11323 ms
> INFO: Server startup in 10472 ms
> INFO: Server startup in 11158 ms
> INFO: Server startup in 11194 ms

INFO: Server startup in 5444 ms
INFO: Server startup in 5936 ms
INFO: Server startup in 5804 ms

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm in AS request

2015-07-28 Thread Sumit Bose
On Tue, Jul 28, 2015 at 01:42:29PM +0200, Sumit Bose wrote:
> On Tue, Jul 28, 2015 at 02:26:34PM +0300, Alexander Bokovoy wrote:
> > On Tue, 28 Jul 2015, Simo Sorce wrote:
> > >On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote:
> > >>On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote:
> > >>> - Original Message -
> > >>> > From: "Sumit Bose" 
> > >>> > To: "freeipa-devel" 
> > >>> > Sent: Tuesday, July 21, 2015 7:41:14 AM
> > >>> > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive 
> > >>> > realm   in AS request
> > >>> >
> > >>> > Hi,
> > >>> >
> > >>> > this patch is my suggestion to solve
> > >>> > https://fedorahosted.org/freeipa/ticket/4844 .
> > >>> >
> > >>> > The original issue in the ticket has two part. One is a loop in 
> > >>> > libkrb5
> > >>> > which is already fixed. The other is to handle canonicalization 
> > >>> > better.
> > >>>
> > >>> Sorry Sumit,
> > >>> I see several issues with this patck.
> > >>>
> > >>> first of all you should really not change ipadb_get_principal(), that's 
> > >>> the
> > >>> wrong place to apply your logic.
> > >>>
> > >>> To support searching for the realm name case-insensitively all we 
> > >>> should do
> > >>> is to always forcibly upper case the realm name at the same time we 
> > >>> build the
> > >>> filter (in ipadb_fetch_principals(), if canonicalization was requested.
> > >>> Because we will never store (code to prevent that should probably be 
> > >>> dded with
> > >>> this patch) a realm name that is not all caps.
> > >>> Then the post search matches should be done straight within 
> > >>> ipadb_find_principal().
> > >>>
> > >>> > The general way to allow canonicalization on a principal is to add the
> > >>> > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together
> > >>> > with the objectclass 'ipaKrbPrincipal' to the user object.
> > >>>
> > >>> We have already a ticket open since long to remove krbprincipalalias, 
> > >>> it was
> > >>> a mistake to add it and any patch that depends on it will be nacked by 
> > >>> me.
> > >>> We need to use krbPrincipalName and krbCanonicalName.
> > >>>
> > >>> > Then the IPA
> > >>> > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive
> > >>> > matches and  the principal from 'krbcanonicalname' will be the 
> > >>> > canonical
> > >>> > principal used further on. The 'krbPrincipalName' is not suitable for
> > >>> > either because it has caseExact* matching rules and is a multivalue
> > >>> > attribute [2].
> > >>>
> > >>> Case-exact match is a problem only if we do not canonicalize names when 
> > >>> storing
> > >>> them, otherwise all you need to do is store a "search form" in 
> > >>> krbPrincipalName
> > >>> and always change searches to that form (forcibly upper case realm, 
> > >>> forcibly
> > >>> lowercase components) when canonicalization is requested.
> > >>>
> > >>> Additionally in the patch you are using stcasecmp(), that function is 
> > >>> not
> > >>> acceptable, look at ipadb_find_principal() and you'll see we use 
> > >>> ulc_casecmp()
> > >>> there.
> > >>> Also modyfing the principal before searching is done wrong (you use 
> > >>> strchr()
> > >>> to find the @ sign, but you could find an @ in the components this way, 
> > >>> you
> > >>> should use strrchr() at the very least), and is dangerous if done 
> > >>> outside of
> > >>> the inner functions because then we never have a way to know the 
> > >>> original
> > >>> form should it be needed. In any case as said above realm should be 
> > >>> forcibly
> > >>> uppercase, given a flag in the escape function instead.
> > >>
> > >>Thank for for the review and the comments.
> > >>
> > >>I changed the patch as you suggested to upper-case the realm in the
> > >>escape function if the flag is set.
> > >>
> > >>I didn't add any checks to make sure that the realm of newly added
> > >>principal attributes is always upper case. Since the attributes can be
> > >>added via various ways I think the check should happen on the DS level
> > >
> > >We should indeed intercept add/modify operations and see if they try to
> > >set krbPrincipalName/krbCanonicalName and then validate the name.
> > >Return unwilling to perform if the case of the realm is different (or
> > >fix it on the fly, up for discussion) from the default case as
> > >configured in the server.
> > Will break trusts -- ipasam does add these principals for krbtgt/IPA@AD.
> > 
> > >>but I see this more in the context of full canonicalization fix covered
> > >>by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a
> > >>requirement for the patch attached I would suggest to drop
> > >>https://fedorahosted.org/freeipa/ticket/4844 and solve it together with
> > >>#3864.
> > >
> > >We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5).
> > >I commented on #3864 about what we can do, and we can also avoid
> > >changing the schema.
> > Yep.
> > 
> > >So on the new patches, what does "unify" means ? I 

Re: [Freeipa-devel] 4.2: ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.

2015-07-28 Thread Alexander Bokovoy

On Tue, 28 Jul 2015, Jan Pazdziora wrote:


Hello,

ever since I started to run FreeIPA 4.2 installations (from upstream
copr repo on Fedora 22), I often (but not always) get

 [13/25]: setting audit signing renewal to 2 years
 [14/25]: restarting certificate server
ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the 
Dogtag instance.See the installation log for details.
 [15/25]: requesting RA certificate from CA
 [error] error: [Errno 111] Connection refused

In the ipaserver-install.log, there is

2015-07-28T11:15:42Z DEBUG Starting external process
2015-07-28T11:15:42Z DEBUG args='/bin/systemctl' 'is-active' 
'pki-tomcatd@pki-tomcat.service'
2015-07-28T11:15:42Z DEBUG Process finished, return code=0
2015-07-28T11:15:42Z DEBUG stdout=active

2015-07-28T11:15:42Z DEBUG stderr=
2015-07-28T11:15:42Z DEBUG wait_for_open_ports: localhost [8080, 8443] timeout 
300
2015-07-28T11:20:42Z DEBUG Traceback (most recent call last):
 File "/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py", 
line 183, in rest
art_instance
   self.restart(self.dogtag_constants.PKI_INSTANCE_NAME)
 File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line 
316, in restart
   self.service.restart(instance_name, capture_output=capture_output, wait=wait)
 File "/usr/lib/python2.7/site-packages/ipaplatform/redhat/services.py", line 
250, in restart
   instance_name, capture_output=capture_output, wait=wait)
 File "/usr/lib/python2.7/site-packages/ipaplatform/base/services.py", line 
317, in restart
   self.wait_for_open_ports(self.service_instance(instance_name))
 File "/usr/lib/python2.7/site-packages/ipaplatform/base/services.py", line 
272, in wait_for_op
en_ports
   self.api.env.startup_timeout)
 File "/usr/lib/python2.7/site-packages/ipapython/ipautil.py", line 1180, in 
wait_for_open_port
s
   raise socket.timeout("Timeout exceeded")
timeout: Timeout exceeded

I do run it in container so it could be related, so I'm mostly looking
for blind hints about what might have changed in the installer or
in dogtag itself in 4.2 that could cause this. For example, did we make
the timeout shorter?


The timeout is 300:

2015-07-28T11:15:42Z DEBUG wait_for_open_ports: localhost [8080, 8443] timeout 
300


You can look at dogtag's catalina-.log, to see how long did it
take:
# grep 'Server startup' /var/log/pki/pki-tomcat/catalina.2015-07-24.log 
INFO: Server startup in 27159 ms

INFO: Server startup in 11323 ms
INFO: Server startup in 10472 ms
INFO: Server startup in 11158 ms
INFO: Server startup in 11194 ms

--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] 4.2: ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the Dogtag instance.

2015-07-28 Thread Jan Pazdziora

Hello,

ever since I started to run FreeIPA 4.2 installations (from upstream
copr repo on Fedora 22), I often (but not always) get

  [13/25]: setting audit signing renewal to 2 years
  [14/25]: restarting certificate server
ipa.ipaserver.install.cainstance.CAInstance: CRITICAL Failed to restart the 
Dogtag instance.See the installation log for details.
  [15/25]: requesting RA certificate from CA
  [error] error: [Errno 111] Connection refused

In the ipaserver-install.log, there is

2015-07-28T11:15:42Z DEBUG Starting external process
2015-07-28T11:15:42Z DEBUG args='/bin/systemctl' 'is-active' 
'pki-tomcatd@pki-tomcat.service'
2015-07-28T11:15:42Z DEBUG Process finished, return code=0
2015-07-28T11:15:42Z DEBUG stdout=active

2015-07-28T11:15:42Z DEBUG stderr=
2015-07-28T11:15:42Z DEBUG wait_for_open_ports: localhost [8080, 8443] timeout 
300
2015-07-28T11:20:42Z DEBUG Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py", 
line 183, in rest
art_instance
self.restart(self.dogtag_constants.PKI_INSTANCE_NAME)
  File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line 
316, in restart
self.service.restart(instance_name, capture_output=capture_output, 
wait=wait)
  File "/usr/lib/python2.7/site-packages/ipaplatform/redhat/services.py", line 
250, in restart
instance_name, capture_output=capture_output, wait=wait)
  File "/usr/lib/python2.7/site-packages/ipaplatform/base/services.py", line 
317, in restart
self.wait_for_open_ports(self.service_instance(instance_name))
  File "/usr/lib/python2.7/site-packages/ipaplatform/base/services.py", line 
272, in wait_for_op
en_ports
self.api.env.startup_timeout)
  File "/usr/lib/python2.7/site-packages/ipapython/ipautil.py", line 1180, in 
wait_for_open_port
s
raise socket.timeout("Timeout exceeded")
timeout: Timeout exceeded

I do run it in container so it could be related, so I'm mostly looking
for blind hints about what might have changed in the installer or
in dogtag itself in 4.2 that could cause this. For example, did we make
the timeout shorter?

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0050] ACI plugin: correctly parse bind rules enclosed in parentheses

2015-07-28 Thread Martin Basti

On 28/07/15 13:33, Martin Babinsky wrote:

On 07/27/2015 05:10 PM, Martin Basti wrote:

On 23/07/15 16:06, Martin Babinsky wrote:

This is a quick fix for https://fedorahosted.org/freeipa/ticket/5037




NACK

I do not like your change in first regexp too much.

Can you try this instead?

PermPat = re.compile(r'(\w+)\s*\(([^()]*)\)\s*(.*)', re.UNICODE)

This just removes '(' and ') ' from pattern and accept all other 
characters.


--
Martin Basti



Attaching updated patch.


ACK

--
Martin Basti

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm in AS request

2015-07-28 Thread Simo Sorce
On Tue, 2015-07-28 at 14:26 +0300, Alexander Bokovoy wrote:
> On Tue, 28 Jul 2015, Simo Sorce wrote:
> >On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote:
> >> On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote:
> >> > - Original Message -
> >> > > From: "Sumit Bose" 
> >> > > To: "freeipa-devel" 
> >> > > Sent: Tuesday, July 21, 2015 7:41:14 AM
> >> > > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive 
> >> > > realmin AS request
> >> > >
> >> > > Hi,
> >> > >
> >> > > this patch is my suggestion to solve
> >> > > https://fedorahosted.org/freeipa/ticket/4844 .
> >> > >
> >> > > The original issue in the ticket has two part. One is a loop in libkrb5
> >> > > which is already fixed. The other is to handle canonicalization better.
> >> >
> >> > Sorry Sumit,
> >> > I see several issues with this patck.
> >> >
> >> > first of all you should really not change ipadb_get_principal(), that's 
> >> > the
> >> > wrong place to apply your logic.
> >> >
> >> > To support searching for the realm name case-insensitively all we should 
> >> > do
> >> > is to always forcibly upper case the realm name at the same time we 
> >> > build the
> >> > filter (in ipadb_fetch_principals(), if canonicalization was requested.
> >> > Because we will never store (code to prevent that should probably be 
> >> > dded with
> >> > this patch) a realm name that is not all caps.
> >> > Then the post search matches should be done straight within 
> >> > ipadb_find_principal().
> >> >
> >> > > The general way to allow canonicalization on a principal is to add the
> >> > > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together
> >> > > with the objectclass 'ipaKrbPrincipal' to the user object.
> >> >
> >> > We have already a ticket open since long to remove krbprincipalalias, it 
> >> > was
> >> > a mistake to add it and any patch that depends on it will be nacked by 
> >> > me.
> >> > We need to use krbPrincipalName and krbCanonicalName.
> >> >
> >> > > Then the IPA
> >> > > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive
> >> > > matches and  the principal from 'krbcanonicalname' will be the 
> >> > > canonical
> >> > > principal used further on. The 'krbPrincipalName' is not suitable for
> >> > > either because it has caseExact* matching rules and is a multivalue
> >> > > attribute [2].
> >> >
> >> > Case-exact match is a problem only if we do not canonicalize names when 
> >> > storing
> >> > them, otherwise all you need to do is store a "search form" in 
> >> > krbPrincipalName
> >> > and always change searches to that form (forcibly upper case realm, 
> >> > forcibly
> >> > lowercase components) when canonicalization is requested.
> >> >
> >> > Additionally in the patch you are using stcasecmp(), that function is not
> >> > acceptable, look at ipadb_find_principal() and you'll see we use 
> >> > ulc_casecmp()
> >> > there.
> >> > Also modyfing the principal before searching is done wrong (you use 
> >> > strchr()
> >> > to find the @ sign, but you could find an @ in the components this way, 
> >> > you
> >> > should use strrchr() at the very least), and is dangerous if done 
> >> > outside of
> >> > the inner functions because then we never have a way to know the original
> >> > form should it be needed. In any case as said above realm should be 
> >> > forcibly
> >> > uppercase, given a flag in the escape function instead.
> >>
> >> Thank for for the review and the comments.
> >>
> >> I changed the patch as you suggested to upper-case the realm in the
> >> escape function if the flag is set.
> >>
> >> I didn't add any checks to make sure that the realm of newly added
> >> principal attributes is always upper case. Since the attributes can be
> >> added via various ways I think the check should happen on the DS level
> >
> >We should indeed intercept add/modify operations and see if they try to
> >set krbPrincipalName/krbCanonicalName and then validate the name.
> >Return unwilling to perform if the case of the realm is different (or
> >fix it on the fly, up for discussion) from the default case as
> >configured in the server.
> Will break trusts -- ipasam does add these principals for krbtgt/IPA@AD.

You misunderstood, we compare case-insenstively and adjust the case (or
simply always uppercase).
We do not refuse to add realm names that are completely different, I
know about the cross realm principals :)

> >> but I see this more in the context of full canonicalization fix covered
> >> by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a
> >> requirement for the patch attached I would suggest to drop
> >> https://fedorahosted.org/freeipa/ticket/4844 and solve it together with
> >> #3864.
> >
> >We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5).
> >I commented on #3864 about what we can do, and we can also avoid
> >changing the schema.
> Yep.
> 
> >So on the new patches, what does "unify" means ? I do not get what it
> >mean

[Freeipa-devel] [PATCH] 906 webui: fix regressions failed auth messages

2015-07-28 Thread Petr Vobornik

1. after logout, krb auth no longer shows "session expired" but correct
"Authentication with Kerberos failed".

2. "The password or username you entered is incorrect." is showed on
failed forms-based auth.

https://fedorahosted.org/freeipa/ticket/5163
--
Petr Vobornik
From 88181b081dc696ec5c7709081b4b7abf72a5015a Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Tue, 28 Jul 2015 14:01:34 +0200
Subject: [PATCH] webui: fix regressions failed auth messages

1. after logout, krb auth no longer shows "session expired" but correct
"Authentication with Kerberos failed".

2. "The password or username you entered is incorrect." is showed on
failed forms-based auth.

https://fedorahosted.org/freeipa/ticket/5163
---
 install/ui/src/freeipa/ipa.js | 8 
 install/ui/src/freeipa/widgets/LoginScreen.js | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/install/ui/src/freeipa/ipa.js b/install/ui/src/freeipa/ipa.js
index 75dd73c379815a0e0e1dc2c4d786fdcf3be7c1b0..ef7fcfaee873d97d96630b72365ecffe6b08 100644
--- a/install/ui/src/freeipa/ipa.js
+++ b/install/ui/src/freeipa/ipa.js
@@ -32,6 +32,7 @@ define([
 './json2',
 './_base/i18n',
 './auth',
+'./config',
 './datetime',
 './metadata',
 './builder',
@@ -41,7 +42,8 @@ define([
 './util',
 'exports'
 ], function(declare, Deferred, Evented, keys, topic, $, JSON, i18n, auth,
-datetime, metadata_provider, builder, reg, rpc, text, util, exports) {
+config, datetime, metadata_provider, builder, reg, rpc, text,
+util, exports) {
 
 /**
  * @class
@@ -127,11 +129,9 @@ var IPA = function () {
 // if current path matches live server path, use live data
 if (that.url && window.location.pathname.substring(0, that.url.length) === that.url) {
 that.json_url = params.url || '/ipa/session/json';
-that.login_url = params.url || '/ipa/session/login_kerberos';
 
 } else { // otherwise use fixtures
 that.json_path = params.url || "test/data";
-// that.login_url is not needed for fixtures
 }
 
 $.ajaxSetup(that.ajax_options);
@@ -377,7 +377,7 @@ IPA.get_credentials = function() {
 }
 
 var request = {
-url: IPA.login_url,
+url: config.krb_login_url,
 cache: false,
 type: "GET",
 success: success_handler,
diff --git a/install/ui/src/freeipa/widgets/LoginScreen.js b/install/ui/src/freeipa/widgets/LoginScreen.js
index fb7c6c34d9c1c7115dd95809f3a39de488eb..eb95b9161f05eeac1ec9aed286c9730dada85d59 100644
--- a/install/ui/src/freeipa/widgets/LoginScreen.js
+++ b/install/ui/src/freeipa/widgets/LoginScreen.js
@@ -232,8 +232,8 @@ define(['dojo/_base/declare',
 this.set('view', 'reset');
 val_summary.add_info('login', this.password_expired);
 } else {
+password_f.set_value('');
 val_summary.add_error('login', this.form_auth_failed);
-password_f.set_value('');
 }
 }));
 },
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm in AS request

2015-07-28 Thread Sumit Bose
On Tue, Jul 28, 2015 at 02:26:34PM +0300, Alexander Bokovoy wrote:
> On Tue, 28 Jul 2015, Simo Sorce wrote:
> >On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote:
> >>On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote:
> >>> - Original Message -
> >>> > From: "Sumit Bose" 
> >>> > To: "freeipa-devel" 
> >>> > Sent: Tuesday, July 21, 2015 7:41:14 AM
> >>> > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive 
> >>> > realm in AS request
> >>> >
> >>> > Hi,
> >>> >
> >>> > this patch is my suggestion to solve
> >>> > https://fedorahosted.org/freeipa/ticket/4844 .
> >>> >
> >>> > The original issue in the ticket has two part. One is a loop in libkrb5
> >>> > which is already fixed. The other is to handle canonicalization better.
> >>>
> >>> Sorry Sumit,
> >>> I see several issues with this patck.
> >>>
> >>> first of all you should really not change ipadb_get_principal(), that's 
> >>> the
> >>> wrong place to apply your logic.
> >>>
> >>> To support searching for the realm name case-insensitively all we should 
> >>> do
> >>> is to always forcibly upper case the realm name at the same time we build 
> >>> the
> >>> filter (in ipadb_fetch_principals(), if canonicalization was requested.
> >>> Because we will never store (code to prevent that should probably be dded 
> >>> with
> >>> this patch) a realm name that is not all caps.
> >>> Then the post search matches should be done straight within 
> >>> ipadb_find_principal().
> >>>
> >>> > The general way to allow canonicalization on a principal is to add the
> >>> > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together
> >>> > with the objectclass 'ipaKrbPrincipal' to the user object.
> >>>
> >>> We have already a ticket open since long to remove krbprincipalalias, it 
> >>> was
> >>> a mistake to add it and any patch that depends on it will be nacked by me.
> >>> We need to use krbPrincipalName and krbCanonicalName.
> >>>
> >>> > Then the IPA
> >>> > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive
> >>> > matches and  the principal from 'krbcanonicalname' will be the canonical
> >>> > principal used further on. The 'krbPrincipalName' is not suitable for
> >>> > either because it has caseExact* matching rules and is a multivalue
> >>> > attribute [2].
> >>>
> >>> Case-exact match is a problem only if we do not canonicalize names when 
> >>> storing
> >>> them, otherwise all you need to do is store a "search form" in 
> >>> krbPrincipalName
> >>> and always change searches to that form (forcibly upper case realm, 
> >>> forcibly
> >>> lowercase components) when canonicalization is requested.
> >>>
> >>> Additionally in the patch you are using stcasecmp(), that function is not
> >>> acceptable, look at ipadb_find_principal() and you'll see we use 
> >>> ulc_casecmp()
> >>> there.
> >>> Also modyfing the principal before searching is done wrong (you use 
> >>> strchr()
> >>> to find the @ sign, but you could find an @ in the components this way, 
> >>> you
> >>> should use strrchr() at the very least), and is dangerous if done outside 
> >>> of
> >>> the inner functions because then we never have a way to know the original
> >>> form should it be needed. In any case as said above realm should be 
> >>> forcibly
> >>> uppercase, given a flag in the escape function instead.
> >>
> >>Thank for for the review and the comments.
> >>
> >>I changed the patch as you suggested to upper-case the realm in the
> >>escape function if the flag is set.
> >>
> >>I didn't add any checks to make sure that the realm of newly added
> >>principal attributes is always upper case. Since the attributes can be
> >>added via various ways I think the check should happen on the DS level
> >
> >We should indeed intercept add/modify operations and see if they try to
> >set krbPrincipalName/krbCanonicalName and then validate the name.
> >Return unwilling to perform if the case of the realm is different (or
> >fix it on the fly, up for discussion) from the default case as
> >configured in the server.
> Will break trusts -- ipasam does add these principals for krbtgt/IPA@AD.
> 
> >>but I see this more in the context of full canonicalization fix covered
> >>by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a
> >>requirement for the patch attached I would suggest to drop
> >>https://fedorahosted.org/freeipa/ticket/4844 and solve it together with
> >>#3864.
> >
> >We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5).
> >I commented on #3864 about what we can do, and we can also avoid
> >changing the schema.
> Yep.
> 
> >So on the new patches, what does "unify" means ? I do not get what it
> >means (so probably it is a poor name), I guess you may want to call it
> >"canonicalization" ? (or even 'canon' to shorten it a bit).
> I have same question. I tried to understand why it is called unify and
> failed.

I didn't want to use 'canonical' because the result will not be the
canonical name in 

Re: [Freeipa-devel] [PATCH 0050] ACI plugin: correctly parse bind rules enclosed in parentheses

2015-07-28 Thread Martin Babinsky

On 07/27/2015 05:10 PM, Martin Basti wrote:

On 23/07/15 16:06, Martin Babinsky wrote:

This is a quick fix for https://fedorahosted.org/freeipa/ticket/5037




NACK

I do not like your change in first regexp too much.

Can you try this instead?

PermPat = re.compile(r'(\w+)\s*\(([^()]*)\)\s*(.*)', re.UNICODE)

This just removes '(' and ') ' from pattern and accept all other characters.

--
Martin Basti



Attaching updated patch.

--
Martin^3 Babinsky
From af2b35c4971a7a13217307e52384edaf7255c7a4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 23 Jul 2015 15:45:35 +0200
Subject: [PATCH] ACI plugin: correctly parse bind rules enclosed in
 parentheses

Since bind rule such as `(userdn = "ldap:///anyone";)` is also a valid
statement, the ipalib ACI parser was updated to handle this case.

https://fedorahosted.org/freeipa/ticket/5037
---
 ipalib/aci.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ipalib/aci.py b/ipalib/aci.py
index a55732bf19e58d8a4b36fa18bee2725d5b6584da..f78c5327dbe659240f046ae15622e798c8552829 100755
--- a/ipalib/aci.py
+++ b/ipalib/aci.py
@@ -26,10 +26,11 @@ import re
 ACIPat = re.compile(r'\(version\s+3.0\s*;\s*ac[li]\s+\"([^\"]*)\"\s*;\s*([^;]*);\s*\)', re.UNICODE)
 
 # Break the permissions/bind_rules out
-PermPat = re.compile(r'(\w+)\s*\((.*)\)\s+(.*)', re.UNICODE)
+PermPat = re.compile(r'(\w+)\s*\(([^()]*)\)\s*(.*)', re.UNICODE)
 
 # Break the bind rule out
-BindPat = re.compile(r'([a-zA-Z0-9;\.]+)\s*(\!?=)\s*(.*)', re.UNICODE)
+BindPat = re.compile(r'\(?([a-zA-Z0-9;\.]+)\s*(\!?=)\s*\"(.*)\"\)?',
+ re.UNICODE)
 
 ACTIONS = ["allow", "deny"]
 
@@ -193,6 +194,9 @@ class ACI:
 self.target['target']['operator'] = operator
 
 def set_bindrule(self, bindrule):
+if bindrule.startswith('(') != bindrule.endswith(')'):
+raise SyntaxError("non-matching parentheses in bindrule")
+
 match = BindPat.match(bindrule)
 if not match or len(match.groups()) < 3:
 raise SyntaxError, "malformed bind rule"
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Is Backend.krb part of API?

2015-07-28 Thread Alexander Bokovoy

On Tue, 28 Jul 2015, Simo Sorce wrote:

On Tue, 2015-07-28 at 13:55 +0300, Alexander Bokovoy wrote:

On Tue, 28 Jul 2015, Petr Vobornik wrote:
>On 07/28/2015 10:57 AM, Michael Šimáček wrote:
>>Hi,
>>
>>I'm working on porting FreeIPA away from python-krbV. Backend.krb and
>>KRB5_CCache classes are mere wrappers around krbV bindings, so it would
>>make sense to remove them. But I found the former used in the example in
>>doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide
>>some partial compatibility layer for it? (only partial because some
>>methods can take krbV objects as arguments)
>>
>>Thank you,
>>Michael Simacek
>>
>
>Does the replacement offer API which has all the methods as the
>wrappers? If so we can remove them.
>
>Imho we can remove Backend.krb aka ipalib/plugins/kerberos.py. It's
>used only in 2 files, both are not in production. But I'm not sure
>about KRB5_CCache, the wrapper has some exception logic which might be
>wanted to be kept.
Backend.krb can go if you provide something similar to KRB5_CCache. We
need to be able to initialize ccache with that class -- either by using
existing ccache (we often marshall ccache content to memcached and then
unmarshall it when the same session comes back) or by using a keytab.
After ccache is provided, we need to be able to query default principal
of the existing ccache.


We should be able to do all this with python-gssapi and the store
extensions.

Yep. It would be good to have a helper, though.
--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm in AS request

2015-07-28 Thread Alexander Bokovoy

On Tue, 28 Jul 2015, Simo Sorce wrote:

On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote:

On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote:
> - Original Message -
> > From: "Sumit Bose" 
> > To: "freeipa-devel" 
> > Sent: Tuesday, July 21, 2015 7:41:14 AM
> > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm 
  in AS request
> >
> > Hi,
> >
> > this patch is my suggestion to solve
> > https://fedorahosted.org/freeipa/ticket/4844 .
> >
> > The original issue in the ticket has two part. One is a loop in libkrb5
> > which is already fixed. The other is to handle canonicalization better.
>
> Sorry Sumit,
> I see several issues with this patck.
>
> first of all you should really not change ipadb_get_principal(), that's the
> wrong place to apply your logic.
>
> To support searching for the realm name case-insensitively all we should do
> is to always forcibly upper case the realm name at the same time we build the
> filter (in ipadb_fetch_principals(), if canonicalization was requested.
> Because we will never store (code to prevent that should probably be dded with
> this patch) a realm name that is not all caps.
> Then the post search matches should be done straight within 
ipadb_find_principal().
>
> > The general way to allow canonicalization on a principal is to add the
> > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together
> > with the objectclass 'ipaKrbPrincipal' to the user object.
>
> We have already a ticket open since long to remove krbprincipalalias, it was
> a mistake to add it and any patch that depends on it will be nacked by me.
> We need to use krbPrincipalName and krbCanonicalName.
>
> > Then the IPA
> > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive
> > matches and  the principal from 'krbcanonicalname' will be the canonical
> > principal used further on. The 'krbPrincipalName' is not suitable for
> > either because it has caseExact* matching rules and is a multivalue
> > attribute [2].
>
> Case-exact match is a problem only if we do not canonicalize names when 
storing
> them, otherwise all you need to do is store a "search form" in 
krbPrincipalName
> and always change searches to that form (forcibly upper case realm, forcibly
> lowercase components) when canonicalization is requested.
>
> Additionally in the patch you are using stcasecmp(), that function is not
> acceptable, look at ipadb_find_principal() and you'll see we use ulc_casecmp()
> there.
> Also modyfing the principal before searching is done wrong (you use strchr()
> to find the @ sign, but you could find an @ in the components this way, you
> should use strrchr() at the very least), and is dangerous if done outside of
> the inner functions because then we never have a way to know the original
> form should it be needed. In any case as said above realm should be forcibly
> uppercase, given a flag in the escape function instead.

Thank for for the review and the comments.

I changed the patch as you suggested to upper-case the realm in the
escape function if the flag is set.

I didn't add any checks to make sure that the realm of newly added
principal attributes is always upper case. Since the attributes can be
added via various ways I think the check should happen on the DS level


We should indeed intercept add/modify operations and see if they try to
set krbPrincipalName/krbCanonicalName and then validate the name.
Return unwilling to perform if the case of the realm is different (or
fix it on the fly, up for discussion) from the default case as
configured in the server.

Will break trusts -- ipasam does add these principals for krbtgt/IPA@AD.


but I see this more in the context of full canonicalization fix covered
by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a
requirement for the patch attached I would suggest to drop
https://fedorahosted.org/freeipa/ticket/4844 and solve it together with
#3864.


We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5).
I commented on #3864 about what we can do, and we can also avoid
changing the schema.

Yep.


So on the new patches, what does "unify" means ? I do not get what it
means (so probably it is a poor name), I guess you may want to call it
"canonicalization" ? (or even 'canon' to shorten it a bit).

I have same question. I tried to understand why it is called unify and
failed.


I think the worst case for a utf8 string is more then length*2, probably
more like length*6, unless there is some guarantee around case changes
that I am not aware of, that said we could probably just allocate on the
stack a fixed size string of a KiB or so, the longest DNS name is 256
chars IIRC and a service name can't be that much longer, also usernames
can't be arbitrarily long. So 1/2 KiB should probably be fine for a full
principal name. (avoids a malloc too which is good).

Yes, sounds good. A hostname label can be up to 63 characters and full
domain name including dots wou

Re: [Freeipa-devel] [PATCH 0294] ULC: fix stageuser-add --from-delete command

2015-07-28 Thread David Kupka

On 23/07/15 13:46, Martin Basti wrote:

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

Patch attached.

This patch fixes only first part of problem -- the traceback.

Removing promt for name and surname requires too big hacks in internal
API, and I'm not sure if we will be able to do that.
IMO this should be separate command, I will open a discussion.





Works for me, ACK.
It would be better to leave the ticket open until the issue is fully 
resolved.


--
David Kupka

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Is Backend.krb part of API?

2015-07-28 Thread Simo Sorce
On Tue, 2015-07-28 at 13:55 +0300, Alexander Bokovoy wrote:
> On Tue, 28 Jul 2015, Petr Vobornik wrote:
> >On 07/28/2015 10:57 AM, Michael Šimáček wrote:
> >>Hi,
> >>
> >>I'm working on porting FreeIPA away from python-krbV. Backend.krb and
> >>KRB5_CCache classes are mere wrappers around krbV bindings, so it would
> >>make sense to remove them. But I found the former used in the example in
> >>doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide
> >>some partial compatibility layer for it? (only partial because some
> >>methods can take krbV objects as arguments)
> >>
> >>Thank you,
> >>Michael Simacek
> >>
> >
> >Does the replacement offer API which has all the methods as the 
> >wrappers? If so we can remove them.
> >
> >Imho we can remove Backend.krb aka ipalib/plugins/kerberos.py. It's 
> >used only in 2 files, both are not in production. But I'm not sure 
> >about KRB5_CCache, the wrapper has some exception logic which might be 
> >wanted to be kept.
> Backend.krb can go if you provide something similar to KRB5_CCache. We
> need to be able to initialize ccache with that class -- either by using
> existing ccache (we often marshall ccache content to memcached and then
> unmarshall it when the same session comes back) or by using a keytab.
> After ccache is provided, we need to be able to query default principal
> of the existing ccache. 

We should be able to do all this with python-gssapi and the store
extensions.

Simo.

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

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm in AS request

2015-07-28 Thread Simo Sorce
On Tue, 2015-07-28 at 12:15 +0200, Sumit Bose wrote:
> On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote:
> > - Original Message -
> > > From: "Sumit Bose" 
> > > To: "freeipa-devel" 
> > > Sent: Tuesday, July 21, 2015 7:41:14 AM
> > > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive 
> > > realm   in AS request
> > > 
> > > Hi,
> > > 
> > > this patch is my suggestion to solve
> > > https://fedorahosted.org/freeipa/ticket/4844 .
> > > 
> > > The original issue in the ticket has two part. One is a loop in libkrb5
> > > which is already fixed. The other is to handle canonicalization better.
> > 
> > Sorry Sumit,
> > I see several issues with this patck.
> > 
> > first of all you should really not change ipadb_get_principal(), that's the
> > wrong place to apply your logic.
> > 
> > To support searching for the realm name case-insensitively all we should do
> > is to always forcibly upper case the realm name at the same time we build 
> > the
> > filter (in ipadb_fetch_principals(), if canonicalization was requested. 
> > Because we will never store (code to prevent that should probably be dded 
> > with
> > this patch) a realm name that is not all caps.
> > Then the post search matches should be done straight within 
> > ipadb_find_principal().
> > 
> > > The general way to allow canonicalization on a principal is to add the
> > > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together
> > > with the objectclass 'ipaKrbPrincipal' to the user object.
> > 
> > We have already a ticket open since long to remove krbprincipalalias, it was
> > a mistake to add it and any patch that depends on it will be nacked by me.
> > We need to use krbPrincipalName and krbCanonicalName.
> > 
> > > Then the IPA
> > > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive
> > > matches and  the principal from 'krbcanonicalname' will be the canonical
> > > principal used further on. The 'krbPrincipalName' is not suitable for
> > > either because it has caseExact* matching rules and is a multivalue
> > > attribute [2].
> > 
> > Case-exact match is a problem only if we do not canonicalize names when 
> > storing
> > them, otherwise all you need to do is store a "search form" in 
> > krbPrincipalName
> > and always change searches to that form (forcibly upper case realm, forcibly
> > lowercase components) when canonicalization is requested.
> > 
> > Additionally in the patch you are using stcasecmp(), that function is not
> > acceptable, look at ipadb_find_principal() and you'll see we use 
> > ulc_casecmp()
> > there.
> > Also modyfing the principal before searching is done wrong (you use strchr()
> > to find the @ sign, but you could find an @ in the components this way, you
> > should use strrchr() at the very least), and is dangerous if done outside of
> > the inner functions because then we never have a way to know the original
> > form should it be needed. In any case as said above realm should be forcibly
> > uppercase, given a flag in the escape function instead.
> 
> Thank for for the review and the comments.
> 
> I changed the patch as you suggested to upper-case the realm in the
> escape function if the flag is set.
> 
> I didn't add any checks to make sure that the realm of newly added
> principal attributes is always upper case. Since the attributes can be
> added via various ways I think the check should happen on the DS level

We should indeed intercept add/modify operations and see if they try to
set krbPrincipalName/krbCanonicalName and then validate the name.
Return unwilling to perform if the case of the realm is different (or
fix it on the fly, up for discussion) from the default case as
configured in the server.

> but I see this more in the context of full canonicalization fix covered
> by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a
> requirement for the patch attached I would suggest to drop
> https://fedorahosted.org/freeipa/ticket/4844 and solve it together with
> #3864.

We should clsoe 4844 as fixed upstream (there *was* a bug in libkrb5).
I commented on #3864 about what we can do, and we can also avoid
changing the schema.

> I added a second patch which makes the unit test a bit more robust if
> the krb5.conf on the system running the tests is broken.

Ok.

So on the new patches, what does "unify" means ? I do not get what it
means (so probably it is a poor name), I guess you may want to call it
"canonicalization" ? (or even 'canon' to shorten it a bit).

I think the worst case for a utf8 string is more then length*2, probably
more like length*6, unless there is some guarantee around case changes
that I am not aware of, that said we could probably just allocate on the
stack a fixed size string of a KiB or so, the longest DNS name is 256
chars IIRC and a service name can't be that much longer, also usernames
can't be arbitrarily long. So 1/2 KiB should probably be fine for a full
principal name. (avoids a malloc t

Re: [Freeipa-devel] Is Backend.krb part of API?

2015-07-28 Thread Alexander Bokovoy

On Tue, 28 Jul 2015, Petr Vobornik wrote:

On 07/28/2015 10:57 AM, Michael Šimáček wrote:

Hi,

I'm working on porting FreeIPA away from python-krbV. Backend.krb and
KRB5_CCache classes are mere wrappers around krbV bindings, so it would
make sense to remove them. But I found the former used in the example in
doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide
some partial compatibility layer for it? (only partial because some
methods can take krbV objects as arguments)

Thank you,
Michael Simacek



Does the replacement offer API which has all the methods as the 
wrappers? If so we can remove them.


Imho we can remove Backend.krb aka ipalib/plugins/kerberos.py. It's 
used only in 2 files, both are not in production. But I'm not sure 
about KRB5_CCache, the wrapper has some exception logic which might be 
wanted to be kept.

Backend.krb can go if you provide something similar to KRB5_CCache. We
need to be able to initialize ccache with that class -- either by using
existing ccache (we often marshall ccache content to memcached and then
unmarshall it when the same session comes back) or by using a keytab.
After ccache is provided, we need to be able to query default principal
of the existing ccache. 
--

/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Is Backend.krb part of API?

2015-07-28 Thread Petr Vobornik

On 07/28/2015 10:57 AM, Michael Šimáček wrote:

Hi,

I'm working on porting FreeIPA away from python-krbV. Backend.krb and
KRB5_CCache classes are mere wrappers around krbV bindings, so it would
make sense to remove them. But I found the former used in the example in
doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide
some partial compatibility layer for it? (only partial because some
methods can take krbV objects as arguments)

Thank you,
Michael Simacek



Does the replacement offer API which has all the methods as the 
wrappers? If so we can remove them.


Imho we can remove Backend.krb aka ipalib/plugins/kerberos.py. It's used 
only in 2 files, both are not in production. But I'm not sure about 
KRB5_CCache, the wrapper has some exception logic which might be wanted 
to be kept.

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Replace stageuser-add --from-delete with user-undel --to-staged

2015-07-28 Thread Jan Cholasta

Dne 28.7.2015 v 11:36 Lenka Doudova napsal(a):



Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a):

Dne 27.7.2015 v 17:59 Martin Basti napsal(a):

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

Hello,

I tried to fix #5145 and I partially succeeded.

However, I cannot fix this part of ticket, where user is prompted to
write name and surname.

$ ipa stageuser-add tuser --from-delete
First name: this will be ignored
Last name: this will be also ignored

Added stage user "tuser"


As the first name and last name are mandatory attributes of
stageuser-add command, but they are not needed by when the
--from-delete option is used.
I would like to ask how to fix this issue, IMO this will be huge hack
in internal API. Or should we just document this bug as known issue
(thierry wrote that this is not use case that should be used often)?

The best solution would be separate command, but this idea was
rejected in thread "[Freeipa-devel] User life cycle: question
regarding the design"

Regards
Martin^2


Hello,

as was mentioned before, we have issue with current internal API and the
stageuser-add --from-delete command.

We discussed this today, and we did not find a nice way how to fix it,
so we propose this (which is IMO the best solution):

* stageuser-add --from-delete should be deprecated


+1


* create new option for user-undel: used-undel --to-staged  (or create
new command) that will handle moving deleted users to staged area as
--from-delete did.


Make it new command please.



Instead of stageuser-add and option --from-delete, which work totally
different, the command user-undel does similar operation than stage-user
--from-delete, it just uses different container.


NACK on stuffing everything into a single command just because it does
something similar.


How about making it a 'stageuser-undel'? The 'user-undel' moves
preserved user to active, so the 'stageuser-undel' would move preserved
to staged. The action is similar, but has slightly different specifics
(which attributes are preserved etc.), and for me the 'stageuser-undel'
feels more natural than 'user-undel --to-staged' since it's basically
the same as there is 'stageuser-add' for creating a staged user, not
'user-add --to-staged'. It would be in the same style as all the other
commands concerning operations with users in staged container.


Well, user-undel is the opposite of user-del, and stageuser-undel should 
be the opposite of stageuser-del. The stageuser-undel you are suggesting 
is not.


Also I'm not sure if we want to (always) remove the deleted user once a 
staged user is created from it, but -undel behaves like that.


I don't think the command should be limited to deleted users only. 
Active and deleted users share the same namespace, so it is an arbitrary 
limitation.


I think that what we are looking for is the opposite of 
stageuser-activate. So maybe user-stage?


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm in AS request

2015-07-28 Thread Sumit Bose
On Wed, Jul 22, 2015 at 09:41:51AM -0400, Simo Sorce wrote:
> - Original Message -
> > From: "Sumit Bose" 
> > To: "freeipa-devel" 
> > Sent: Tuesday, July 21, 2015 7:41:14 AM
> > Subject: [Freeipa-devel] [PATCH 149] IPA KDB: allow case in-sensitive realm 
> > in AS request
> > 
> > Hi,
> > 
> > this patch is my suggestion to solve
> > https://fedorahosted.org/freeipa/ticket/4844 .
> > 
> > The original issue in the ticket has two part. One is a loop in libkrb5
> > which is already fixed. The other is to handle canonicalization better.
> 
> Sorry Sumit,
> I see several issues with this patck.
> 
> first of all you should really not change ipadb_get_principal(), that's the
> wrong place to apply your logic.
> 
> To support searching for the realm name case-insensitively all we should do
> is to always forcibly upper case the realm name at the same time we build the
> filter (in ipadb_fetch_principals(), if canonicalization was requested. 
> Because we will never store (code to prevent that should probably be dded with
> this patch) a realm name that is not all caps.
> Then the post search matches should be done straight within 
> ipadb_find_principal().
> 
> > The general way to allow canonicalization on a principal is to add the
> > attributes 'krbcanonicalname'[1] and 'ipakrbprincipalalias' together
> > with the objectclass 'ipaKrbPrincipal' to the user object.
> 
> We have already a ticket open since long to remove krbprincipalalias, it was
> a mistake to add it and any patch that depends on it will be nacked by me.
> We need to use krbPrincipalName and krbCanonicalName.
> 
> > Then the IPA
> > KDB backend will use 'ipakrbprincipalalias' for case in-sensitive
> > matches and  the principal from 'krbcanonicalname' will be the canonical
> > principal used further on. The 'krbPrincipalName' is not suitable for
> > either because it has caseExact* matching rules and is a multivalue
> > attribute [2].
> 
> Case-exact match is a problem only if we do not canonicalize names when 
> storing
> them, otherwise all you need to do is store a "search form" in 
> krbPrincipalName
> and always change searches to that form (forcibly upper case realm, forcibly
> lowercase components) when canonicalization is requested.
> 
> Additionally in the patch you are using stcasecmp(), that function is not
> acceptable, look at ipadb_find_principal() and you'll see we use ulc_casecmp()
> there.
> Also modyfing the principal before searching is done wrong (you use strchr()
> to find the @ sign, but you could find an @ in the components this way, you
> should use strrchr() at the very least), and is dangerous if done outside of
> the inner functions because then we never have a way to know the original
> form should it be needed. In any case as said above realm should be forcibly
> uppercase, given a flag in the escape function instead.

Thank for for the review and the comments.

I changed the patch as you suggested to upper-case the realm in the
escape function if the flag is set.

I didn't add any checks to make sure that the realm of newly added
principal attributes is always upper case. Since the attributes can be
added via various ways I think the check should happen on the DS level
but I see this more in the context of full canonicalization fix covered
by https://fedorahosted.org/freeipa/ticket/3864 . If you think this is a
requirement for the patch attached I would suggest to drop
https://fedorahosted.org/freeipa/ticket/4844 and solve it together with
#3864.

I added a second patch which makes the unit test a bit more robust if
the krb5.conf on the system running the tests is broken.

bye,
Sumit

> 
> > What I got from the comments in the ticket and the related bugzilla
> > ticket is that it should be possible to get a TGT for a user even if the
> > realm is given in lower-case if canonicalization is enabled. Please note
> > that the client can only send such request because we have
> > 'dns_lookup_kdc = true' in krb.conf and DNS is case in-sensitive. If you
> > set 'dns_lookup_kdc = false' the client will fail immediately without
> > sending a request at all, because it is not able to find a KDC for the
> > lower-case realm.
> > 
> > On the server-side the request is processed because of
> > http://k5wiki.kerberos.org/wiki/Projects/Aliases which made parts of
> > processing case in-sensitive.
> > 
> > With the attached patch a second lookup is done if the lookup with the
> > original input returned no result, canonicalization is
> > enabled and the realm from the original input matches the IPA realm case
> > in-sensitive. For the second lookup the realm is replace with the IPA
> > realm. This approach adds a bit redundant code but does not add extra
> > processing requests which would be successful before.
> > 
> > Without the patch
> > kinitipauser@IPA.REALM -> success
> > kinit -C ipauser@IPA.REALM -> success
> > kinitipauser@ipa.realm -> failure
> > kinit -C ipauser@ipa.realm -> failure
> > 
> > With the 

Re: [Freeipa-devel] variable name 'rsa_public_key' in vault

2015-07-28 Thread Simo Sorce
On Fri, 2015-07-24 at 12:20 +0200, Christian Heimes wrote:
> Hello,
> 
> while I was working on https://fedorahosted.org/freeipa/ticket/5142 and
> patch 019, I noticed the variable names rsa_public_key and
> rsa_private_key in vault.py. load_pem_public_key() can load and return
> other key formats (DSA, ECDSA), too. Does vault mean to support the
> other algorithms?
> 
> In case vault should support any kind of asymmetric cipher, I'd like to
> change the variable names. It's confusing. Otherwise we should add a
> check for RSA and prevent DSA and ECDSA keys.
> 
> Christian
> 

We certainly want to support ECDSA keys eventually, so I'd rename the
vars.

Simo.

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

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Replace stageuser-add --from-delete with user-undel --to-staged

2015-07-28 Thread Lenka Doudova



Dne 28.7.2015 v 11:27 Jan Cholasta napsal(a):

Dne 27.7.2015 v 17:59 Martin Basti napsal(a):

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

Hello,

I tried to fix #5145 and I partially succeeded.

However, I cannot fix this part of ticket, where user is prompted to
write name and surname.

$ ipa stageuser-add tuser --from-delete
First name: this will be ignored
Last name: this will be also ignored

Added stage user "tuser"


As the first name and last name are mandatory attributes of
stageuser-add command, but they are not needed by when the
--from-delete option is used.
I would like to ask how to fix this issue, IMO this will be huge hack
in internal API. Or should we just document this bug as known issue
(thierry wrote that this is not use case that should be used often)?

The best solution would be separate command, but this idea was
rejected in thread "[Freeipa-devel] User life cycle: question
regarding the design"

Regards
Martin^2


Hello,

as was mentioned before, we have issue with current internal API and the
stageuser-add --from-delete command.

We discussed this today, and we did not find a nice way how to fix it,
so we propose this (which is IMO the best solution):

* stageuser-add --from-delete should be deprecated


+1


* create new option for user-undel: used-undel --to-staged  (or create
new command) that will handle moving deleted users to staged area as
--from-delete did.


Make it new command please.



Instead of stageuser-add and option --from-delete, which work totally
different, the command user-undel does similar operation than stage-user
--from-delete, it just uses different container.


NACK on stuffing everything into a single command just because it does 
something similar.


How about making it a 'stageuser-undel'? The 'user-undel' moves 
preserved user to active, so the 'stageuser-undel' would move preserved 
to staged. The action is similar, but has slightly different specifics 
(which attributes are preserved etc.), and for me the 'stageuser-undel' 
feels more natural than 'user-undel --to-staged' since it's basically 
the same as there is 'stageuser-add' for creating a staged user, not 
'user-add --to-staged'. It would be in the same style as all the other 
commands concerning operations with users in staged container.


Lenka





We need to do this in 4.2.1 to affect as least as possible users.

If you have any objections, please speak/write :)
Martin^2




--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Replace stageuser-add --from-delete with user-undel --to-staged

2015-07-28 Thread Jan Cholasta

Dne 27.7.2015 v 17:59 Martin Basti napsal(a):

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

Hello,

I tried to fix #5145 and I partially succeeded.

However, I cannot fix this part of ticket, where user is prompted to
write name and surname.

$ ipa stageuser-add tuser --from-delete
First name: this will be ignored
Last name: this will be also ignored

Added stage user "tuser"


As the first name and last name are mandatory attributes of
stageuser-add command, but they are not needed by when the
--from-delete option is used.
I would like to ask how to fix this issue, IMO this will be huge hack
in internal API. Or should we just document this bug as known issue
(thierry wrote that this is not use case that should be used often)?

The best solution would be separate command, but this idea was
rejected in thread "[Freeipa-devel] User life cycle: question
regarding the design"

Regards
Martin^2


Hello,

as was mentioned before, we have issue with current internal API and the
stageuser-add --from-delete command.

We discussed this today, and we did not find a nice way how to fix it,
so we propose this (which is IMO the best solution):

* stageuser-add --from-delete should be deprecated


+1


* create new option for user-undel: used-undel --to-staged  (or create
new command) that will handle moving deleted users to staged area as
--from-delete did.


Make it new command please.



Instead of stageuser-add and option --from-delete, which work totally
different, the command user-undel does similar operation than stage-user
--from-delete, it just uses different container.


NACK on stuffing everything into a single command just because it does 
something similar.




We need to do this in 4.2.1 to affect as least as possible users.

If you have any objections, please speak/write :)
Martin^2


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] Is Backend.krb part of API?

2015-07-28 Thread Michael Šimáček

Hi,

I'm working on porting FreeIPA away from python-krbV. Backend.krb and 
KRB5_CCache classes are mere wrappers around krbV bindings, so it would 
make sense to remove them. But I found the former used in the example in 
doc/examples/python-api.py. Is it part of FreeIPA's API? Shall I provide 
some partial compatibility layer for it? (only partial because some 
methods can take krbV objects as arguments)


Thank you,
Michael Simacek

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0035 client: Update DNS with all available local IP addresses.

2015-07-28 Thread David Kupka

On 27/07/15 16:45, David Kupka wrote:

On 15/01/15 17:13, David Kupka wrote:

On 01/15/2015 03:22 PM, David Kupka wrote:

On 01/15/2015 12:43 PM, David Kupka wrote:

On 01/12/2015 06:34 PM, Martin Basti wrote:

On 09/01/15 14:43, David Kupka wrote:

On 01/07/2015 04:15 PM, Martin Basti wrote:

On 07/01/15 12:27, David Kupka wrote:

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


Thank you for patch:

1)
-root_logger.error("Cannot update DNS records! "
-  "Failed to connect to server '%s'.",
server)
+ips = get_local_ipaddresses()
+except CalledProcessError as e:
+root_logger.error("Cannot update DNS records. %s" % e)

IMO the error message should be more specific,  add there something
like
"Unable to get local IP addresses". at least in log.debug()

2)
+lines = ipresult[0].replace('\\', '').split('\n')

.replace() is not needed

3)
+if len(ips) == 0:

if not ips:

is more pythonic by PEP8



Thanks for catching these. Updated patch attached.


merciful NACK

Thank you for the patch, unfortunately I hit one issue which needs
to be
resolved.

If "sync PTR" is activated in zone settings, and reverse zone doesn't
exists, nsupdate/BIND returns SERVFAIL and ipa-client-install print
Error message, 'DNS update failed'. In fact, all A/ records was
succesfully updated, only PTR records failed.

Bind log:
named-pkcs11[28652]: updating zone 'example.com/IN': adding an RR at
'vm-101.example.com' 

named-pkcs11[28652]: PTR record synchronization (addition) for A/
'vm-101.example.com.' refused: unable to find active reverse zone
for IP
address '2620:52:0:104c:21a:4aff:fe10:4eaa': not found

With IPv6 we have several addresses from different reverse zones and
this situation may happen often.
I suggest following:
1) Print list of addresses which will be updated. (Now if update
fails,
user needs to read log, which addresses installer tried to update)
2) Split nsupdates per A/ record.
3a) If failed, check with DNS query if A/ and PTR record are there
and print proper error message
3b) Just print A/ (or PTR) record may not be updated for
particular
IP address.

Any other suggestions are welcome.



After long discussion with DNS and UX guru I've implemented it this
way:
1. Call nsupdate only once with all updates.
2. Verify that the expected records are resolvable.
3. If no print list of missing A/, list of missing PTR records and
list to mismatched PTR record.

As this is running inside client we can't much more and it's up to user
to check what's rotten in his DNS setup.

Updated patch attached.


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




One more change to behave well in -crazy- exotic environments that
resolves more PTR records for single IP.



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



Yet another change to make language nerds and our UX guru happy :-)


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



Rebased patch attached.



Updated patch attached.

--
David Kupka
From 5c923daf7ce662e19b144e338557e1b8df7a061d Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Sun, 4 Jan 2015 15:04:18 -0500
Subject: [PATCH] client: Update DNS with all available local IP addresses.

Detect all usable IP addresses assigned to any interface and create
coresponding DNS records on server.

https://fedorahosted.org/freeipa/ticket/4249
---
 ipa-client/ipa-install/ipa-client-install | 174 +++---
 1 file changed, 113 insertions(+), 61 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 91323ae115a27d221bcbc43fee887c56d99c8635..947cb10d98e950498b9ea1e4a3b715de1ee33e3b 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -32,6 +32,7 @@ try:
 from optparse import SUPPRESS_HELP, OptionGroup, OptionValueError
 import shutil
 from krbV import Krb5Error
+import dns
 
 import nss.nss as nss
 import SSSDConfig
@@ -1285,6 +1286,7 @@ def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options, clie
 
 if options.dns_updates:
 domain.set_option('dyndns_update', True)
+domain.set_option('dyndns_iface', '*')
 if options.krb5_offline_passwords:
 domain.set_option('krb5_store_password_if_offline', True)
 
@@ -1500,40 +1502,22 @@ def unconfigure_nisdomain():
 if not enabled:
 services.knownservices.domainname.disable()
 
-
-def resolve_ipaddress(server):
-""" Connect to the server's LDAP port in order to determine what ip
-address this machine uses as "public" ip (relative to the server).
-
-Returns a