Re: [Freeipa-devel] [PATCHSET] Replica promotion patches

[Martin Basti]

On 09/22/2015 10:45 AM, Jan Cholasta wrote:

Hi,

On 9.9.2015 20:25, Simo Sorce wrote:

On Wed, 2015-08-26 at 17:27 -0400, Simo Sorce wrote:

This patchset implements https://fedorahosted.org/freeipa/ticket/2888
and introduces a number of required  changes and dependencies to 
achieve

this goal.
This work requires the custodia project to securely transfer keys
between ipa servers.

This work is not 100% complete, it still misses the ability to install
kra instances and the ability to install a CA (via ipa-ca-install) with
externally signed certs.

However it is massive enough that warrants review and pushing, the 
resat

of the changes can be applied later as this work should not disrupt the
classic install methods.

In order to build my previous patches (530-533) are needed as well as a
number of updated components.

I used the following coprs for testing:
simo/jwcrypto
simo/custodia
abbra/sssd-kkdcproxy (for sssd 1.13.1)
lkrispen/389-ds-current (for 389 > 1.3.4.4)
vakwetu/dogtag_10.2.7_test_builds (for dogtag 10.2.7)
mkosek/freeipa-4.2-fedora-22 (misc)
fedora/updates-testing (python-gssapi 1.1.2)

Ludwig's copr is necessary to have a functional DNA plugin in replicas,
eventually his patches should be committed in 389-ds-base 1.3.4.4 when
it will be released.

We are aware of a dogtag bug https://fedorahosted.org/pki/ticket/1580
that may cause installation issues in some case (re-install of a
replica).

The domain must be raised to level 1 in order to use replica promotion.

In order to promote a replica the server must be first joined as a
regular client to the domain.

This is the flow I usually use for testing:

# ipa-client-install
# kinit admin
# ipa-replica-install --promote --setup-ca


These patches are also available in this git tree rebnase on current
master:
https://fedorapeople.org/cgit/simo/public_git/freeipa.git/log/?h=custodia-review 



FYI: I rebased this branch on top of master and applied minor changes to
one of the DNS patches. I also added the missing support to install KRA.

DS 1.3.4.4 is also in Fedora updates testing, so ludwig's copr is not
needed anymore.

Dogtag's ticket is not fixed yet so running both --setup-ca and
--setup-kra at the same time will still yield an error and install will
fail.

Please let me know if there are any major issues with this patchset, I'd
like to push it to master and attack the remaining issues as add ons
(install with external certs not supported yet for example)


So far I have only read through the code without running it (mostly).


"Remove unused arguments": ACK


"Simplify the install_replica_ca function": ACK


"IPA Custodia Daemon":

1) Instead of putting the code in "ipakeys" package, could you put it 
in "ipapython.keys"? This way it would be consistent with DNSSEC, 
which has binaries in daemons/dnssec/ and modules in ipapython/dnssec/.


2) Is it safe to create cn=custodia in update file only? Updates are 
executed late in ipa-server-install. Is is guaranteed that nothing 
will try to access cn=custodia before the updates are run?


(Nevermind, it is added to bootstrap-template.ldif 2 commits below.)

3) Shouldn't cn=custodia be created only when domain level >= 1?

4) pylint fails with:

daemons/ipa-custodia/ipakeys/kem.py:156: [E1002(super-on-old-class), 
IPAKEMKeys.__init__] Use of super on an old style class)
daemons/ipa-custodia/ipakeys/kem.py:178: [E1101(no-member), 
IPAKEMKeys.generate_server_keys] Instance of 'IPAKEMKeys' has no 
'config' member)
daemons/ipa-custodia/ipakeys/kem.py:187: [E1101(no-member), 
IPAKEMKeys.server_keys] Instance of 'IPAKEMKeys' has no 'config' member)


5) There are some PEP8 transgressions:

./daemons/ipa-custodia/ipakeys/kem.py:202:80: E501 line too long (82 > 
79 characters)
./daemons/ipa-custodia/ipakeys/kem.py:203:80: E501 line too long (82 > 
79 characters)
./daemons/ipa-custodia/ipakeys/store.py:33:1: E302 expected 2 blank 
lines, found 1
./daemons/ipa-custodia/setup.py:8:9: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:8:11: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:9:12: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:9:14: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:10:12: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:10:14: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:11:15: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:11:17: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:12:21: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:12:23: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ipa-custodia/setup.py:14:13: E251 unexpected spaces around 
keyword / parameter equals
./daemons/ip

Re: [Freeipa-devel] [PATCHSET] Replica promotion patches

[Simo Sorce]

On Wed, 2015-09-23 at 08:35 +0200, Jan Cholasta wrote:
> What I mean is that installing a replica using an already existing 
> replica file should be prevented at level 1 as well:
> 
> root@ipa1# ipa-server-install --domain-level=0
> root@ipa1# ipa-replica-prepare ipa2.example.com
> root@ipa1# ipa domainlevel-set 1
> 
> root@ipa2# ipa-replica-install replica-info-ipa2.example.com.gpg
> ERROR: Can't install replica from a replica file at domain level > 0

Ok I rebased the patchset with a modification to assume promotion if no
file was provided, and then raise appropriate RuntimeErrors if
conditions about the domain level are not met.

This change also prevents installing with a replica file if domain level
is currently at 1.

They are in the usual custodia-review branch.

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] [PATCHES 0370-0371] winsync-migrate: Handle invalid characters in the names of IPA entities

[Tomas Babej]

On 09/23/2015 04:56 PM, Jan Cholasta wrote:
> On 23.9.2015 16:52, Martin Babinsky wrote:
>> On 09/23/2015 01:39 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> this fixes https://fedorahosted.org/freeipa/ticket/5319.
>>>
>>> Details in the commit messages.
>>>
>>> Tomas
>>>
>>>
>>
>> ACK
>>
> 
> The patches need to be rebased on top of ipa-4-2.
> 

Attached.
From 109709e4fc448d4bed38a008916c2c3fa8d302b1 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 23 Sep 2015 13:27:35 +0200
Subject: [PATCH] winsync-migrate: Convert entity names to posix friendly
 strings

During the migration from winsync replicated users to their
trusted identities, memberships are being preserved. However,
trusted users are external and as such cannot be added as
direct members to the IPA entities. External groups which
encapsulate the migrated users are added as members to those
entities instead.

The name of the external group is generated from the type
of the entity and its name. However, the entity's name can
contain characters which are invalid for use in the group
name.

Adds a helper function to convert a given string to a string
which would be valid for such use and leverages it in the
winsync-migrate tool.

https://fedorahosted.org/freeipa/ticket/5319
---
 ipapython/ipautil.py | 23 +++
 ipaserver/install/ipa_winsync_migrate.py | 15 ---
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 88e89706b8e2aa6dea80809510d88bceaa836e85..64fe9bc27e58c8ecfcfabe69690db0493a10c3b1 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1318,6 +1318,29 @@ def restore_hostname(statestore):
 except CalledProcessError, e:
 print >>sys.stderr, "Failed to set this machine hostname back to %s: %s" % (old_hostname, str(e))
 
+def posixify(string):
+"""
+Convert a string to a more strict alpha-numeric representation.
+
+- Alpha-numeric, underscore, dot and dash characters are accepted
+- Space is converted to underscore
+- Other characters are omitted
+- Leading dash is stripped
+
+Note: This mapping is not one-to-one and may map different input to the
+same result. When using posixify, make sure the you do not map two different
+entities to one unintentionally.
+"""
+
+def valid_char(char):
+return char.isalnum() or char in ('_', '.', '-')
+
+# First replace space characters
+replaced = string.replace(' ','_')
+omitted = ''.join(filter(valid_char, replaced))
+
+# Leading dash is not allowed
+return omitted.lstrip('-')
 
 @contextmanager
 def private_ccache(path=None):
diff --git a/ipaserver/install/ipa_winsync_migrate.py b/ipaserver/install/ipa_winsync_migrate.py
index c327e502e6bfb6e402931e1962fe2410570b2bc2..4dacde3f27ead341fd4d7d2a744d28f74d5c5b95 100644
--- a/ipaserver/install/ipa_winsync_migrate.py
+++ b/ipaserver/install/ipa_winsync_migrate.py
@@ -24,7 +24,7 @@ from ipalib import api
 from ipalib import errors
 from ipapython import admintool
 from ipapython.dn import DN
-from ipapython.ipautil import realm_to_suffix
+from ipapython.ipautil import realm_to_suffix, posixify
 from ipapython.ipa_log_manager import log_mgr
 from ipaserver.plugins.ldap2 import ldap2
 from ipaserver.install import replication
@@ -214,12 +214,21 @@ class WinsyncMigrate(admintool.AdminTool):
 
 def winsync_group_name(object_entry):
 """
-Returns the generated name of group containing migrated external users
+Returns the generated name of group containing migrated external
+users.
+
+The group name is of the form:
+ "__winsync_external"
+
+Object name is converted to posix-friendly string by omitting
+and/or replacing characters. This may lead to collisions, i.e.
+if both 'trust_admins' and 'trust admin' groups have winsync
+users being migrated.
 """
 
 return u"{0}_{1}_winsync_external".format(
 winsync_group_prefix,
-object_entry['cn'][0]
+posixify(object_entry['cn'][0])
 )
 
 def create_winsync_group(object_entry):
-- 
2.1.0

From df33cc4d7b1cb4a86dd11043a89979a48a9dca7b Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 23 Sep 2015 13:28:33 +0200
Subject: [PATCH] winsync-migrate: Properly handle collisions in the names of
 external groups

Since the names of the external groups containing the migrated users
must be stripped of characters which are not valid for use in group names,
two different groups might be mapped to one during this process.

Properly handle collisions in the names by adding an incremental
numeric suffix.

https://fedorahosted.org/freeipa/ticket/5319
---
 ipaserver/install/ipa_winsync_migrate.py | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/ipa_win

Re: [Freeipa-devel] [PATCHES 0370-0371] winsync-migrate: Handle invalid characters in the names of IPA entities

[Tomas Babej]

On 09/23/2015 05:06 PM, Tomas Babej wrote:
> 
> 
> On 09/23/2015 04:56 PM, Jan Cholasta wrote:
>> On 23.9.2015 16:52, Martin Babinsky wrote:
>>> On 09/23/2015 01:39 PM, Tomas Babej wrote:
 Hi,

 this fixes https://fedorahosted.org/freeipa/ticket/5319.

 Details in the commit messages.

 Tomas


>>>
>>> ACK
>>>
>>
>> The patches need to be rebased on top of ipa-4-2.
>>
> 
> Attached.
> 

Ah, I was race conditioned!

-- 
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] [PATCHES 0370-0371] winsync-migrate: Handle invalid characters in the names of IPA entities

[Jan Cholasta]

On 23.9.2015 17:03, Martin Babinsky wrote:

On 09/23/2015 04:56 PM, Jan Cholasta wrote:

On 23.9.2015 16:52, Martin Babinsky wrote:

On 09/23/2015 01:39 PM, Tomas Babej wrote:

Hi,

this fixes https://fedorahosted.org/freeipa/ticket/5319.

Details in the commit messages.

Tomas




ACK



The patches need to be rebased on top of ipa-4-2.



Attaching rebased patches on Tomas's behalf.



Thanks.

Pushed to:
master: 75cba4e8bfe0479078ba112d99628ed517e010a2
ipa-4-2: d639e932e248866e7a5993f899f025778860bc95

--
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] [PATCHES 0370-0371] winsync-migrate: Handle invalid characters in the names of IPA entities

[Martin Babinsky]

On 09/23/2015 04:56 PM, Jan Cholasta wrote:

On 23.9.2015 16:52, Martin Babinsky wrote:

On 09/23/2015 01:39 PM, Tomas Babej wrote:

Hi,

this fixes https://fedorahosted.org/freeipa/ticket/5319.

Details in the commit messages.

Tomas




ACK



The patches need to be rebased on top of ipa-4-2.



Attaching rebased patches on Tomas's behalf.

--
Martin^3 Babinsky
From b09a852446309c794fadb2a300194bc57c4cbe8c Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 23 Sep 2015 13:28:33 +0200
Subject: [PATCH 2/2] winsync-migrate: Properly handle collisions in the names
 of external groups

Since the names of the external groups containing the migrated users
must be stripped of characters which are not valid for use in group names,
two different groups might be mapped to one during this process.

Properly handle collisions in the names by adding an incremental
numeric suffix.

https://fedorahosted.org/freeipa/ticket/5319
---
 ipaserver/install/ipa_winsync_migrate.py | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/ipa_winsync_migrate.py b/ipaserver/install/ipa_winsync_migrate.py
index 4dacde3f27ead341fd4d7d2a744d28f74d5c5b95..13c5ddef383204451cbc4bb662c8a1befc1d5f93 100644
--- a/ipaserver/install/ipa_winsync_migrate.py
+++ b/ipaserver/install/ipa_winsync_migrate.py
@@ -231,15 +231,26 @@ class WinsyncMigrate(admintool.AdminTool):
 posixify(object_entry['cn'][0])
 )
 
-def create_winsync_group(object_entry):
+def create_winsync_group(object_entry, suffix=0):
 """
 Creates the group containing migrated external users that were
 previously available via winsync.
 """
 
 name = winsync_group_name(object_entry)
-api.Command['group_add'](name, external=True)
-api.Command[object_membership_command](object_entry['cn'][0], group=[name])
+
+# Only non-trivial suffix is appended at the end
+if suffix != 0:
+name += str(suffix)
+
+try:
+api.Command['group_add'](name, external=True)
+except errors.DuplicateEntry:
+# If there is a collision, let's try again with a higher suffix
+create_winsync_group(object_entry, suffix=suffix+1)
+else:
+# In case of no collision, add the membership
+api.Command[object_membership_command](object_entry['cn'][0], group=[name])
 
 # Search for all objects containing the given user as a direct member
 member_filter = self.ldap.make_filter_from_attr(user_dn_attribute,
-- 
2.4.3

From eb59d73f459775f0677fde20adecf2ec22aa0058 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 23 Sep 2015 13:27:35 +0200
Subject: [PATCH 1/2] winsync-migrate: Convert entity names to posix friendly
 strings

During the migration from winsync replicated users to their
trusted identities, memberships are being preserved. However,
trusted users are external and as such cannot be added as
direct members to the IPA entities. External groups which
encapsulate the migrated users are added as members to those
entities instead.

The name of the external group is generated from the type
of the entity and its name. However, the entity's name can
contain characters which are invalid for use in the group
name.

Adds a helper function to convert a given string to a string
which would be valid for such use and leverages it in the
winsync-migrate tool.

https://fedorahosted.org/freeipa/ticket/5319
---
 ipapython/ipautil.py | 23 +++
 ipaserver/install/ipa_winsync_migrate.py | 15 ---
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 88e89706b8e2aa6dea80809510d88bceaa836e85..64fe9bc27e58c8ecfcfabe69690db0493a10c3b1 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1318,6 +1318,29 @@ def restore_hostname(statestore):
 except CalledProcessError, e:
 print >>sys.stderr, "Failed to set this machine hostname back to %s: %s" % (old_hostname, str(e))
 
+def posixify(string):
+"""
+Convert a string to a more strict alpha-numeric representation.
+
+- Alpha-numeric, underscore, dot and dash characters are accepted
+- Space is converted to underscore
+- Other characters are omitted
+- Leading dash is stripped
+
+Note: This mapping is not one-to-one and may map different input to the
+same result. When using posixify, make sure the you do not map two different
+entities to one unintentionally.
+"""
+
+def valid_char(char):
+return char.isalnum() or char in ('_', '.', '-')
+
+# First replace space characters
+replaced = string.replace(' ','_')
+omitted = ''.join(filter(valid_char, replaced))
+
+# Leading dash is not allowed
+return omitted.lstrip('-')
 
 @contextmanager
 def pr

Re: [Freeipa-devel] [PATCHES 466-468, 0316] install: Add common base class for server and replica install

[Martin Basti]

On 09/22/2015 12:10 PM, Jan Cholasta wrote:

On 22.9.2015 10:29, Martin Babinsky wrote:

On 09/16/2015 10:44 AM, Jan Cholasta wrote:

On 16.9.2015 08:11, Jan Cholasta wrote:

On 15.9.2015 07:22, Jan Cholasta wrote:

On 10.8.2015 16:58, Martin Babinsky wrote:

On 08/06/2015 08:22 AM, Jan Cholasta wrote:

Hi,

the attached patch fixes part of
.

See also Martin Babinsky's patch 51:
. 








Honza



Sorry but NACK, see below:

1.) it seems that passing kwargs to Server components doesn't 
work as

expected. See these logs (install on fresh F22 VM):

http://fpaste.org/253416/21363814/
http://fpaste.org/253419/43921374/


Fixed.



2.) the following code blows up in BaseServers' __init__:
(http://fpaste.org/253400/21225314/)

392 if not self.dns.setup_dns:
393 if self.dns.forwarders:
394 raise RuntimeError(
395 "You cannot specify a --forwarder option
without
the "
396 "--setup-dns option")


I think that the check should be:

392 if not self.setup_dns:
393 if self.dns.forwarders:


Fixed.



IMHO BaseServerDNS class shouldn't have setup_dns knob, that 
should be

set in the parent class (BaseServer)


Fixed.



3.) Is there any reason why BaseServer doesn't have 
'master_password',
'idmax' and 'idstart' knobs? I know that these are then brought 
in by

the derived Server class, but the check for them is in parent's
__init__() method and it is IMHO a bit confusing


The check should be in Server, fixed.



4.) please add license header to the beginning of
'ipaserver/install/server/common.py' file


Added.

Updated patches attached.


Self-NACK, I broke ipa-server-install --uninstall.


Fixed.



ACK to all three patches.



Thanks.

Pushed to:
master: 86edd6abeb9749e159a529b83cfce6443fff4ba5
ipa-4-2: 42d16b02cd153ac89ebd8ae07c98611dc3b6e471


These patches introduced regression.
ipa-replica-install in unattended mode requires to specify -a, -p and -r 
options.


Attached patch fixes it.

From ff5c1247520ad2d97047d28cc3f94e76faf55301 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 23 Sep 2015 15:48:30 +0200
Subject: [PATCH] Replica inst. fix: do not require -r, -a, -p options in
 unattended mode

Previous patches for this ticket introduced error, that replica install
requires to specify -r, -p and -a option in unattended mode.
This options are not needed on replica side.

https://fedorahosted.org/freeipa/ticket/4517
---
 ipaserver/install/server/common.py  | 7 ---
 ipaserver/install/server/install.py | 6 ++
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py
index e7fb2acfc0eb36547f175fef51fcb5c7f5764269..0648b40e550309195f7bc88e2f5f29af4139d5d5 100644
--- a/ipaserver/install/server/common.py
+++ b/ipaserver/install/server/common.py
@@ -348,13 +348,6 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
 
 #pylint: disable=no-member
 
-if not self.uninstalling and not self.interactive:
-if (not self.realm_name or not self.dm_password or
-not self.admin_password):
-raise RuntimeError(
-"In unattended mode you need to provide at least -r, -p "
-"and -a options")
-
 # If any of the key file options are selected, all are required.
 cert_file_req = (self.ca.dirsrv_cert_files, self.ca.http_cert_files)
 cert_file_opt = (self.ca.pkinit_cert_files,)
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 9137e1a9b0cd77803fefaf60db82606eafa42d76..4fe1ed9f25206e7c014e544fcc3e71243e685f86 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -1263,6 +1263,12 @@ class Server(BaseServer):
 self.master_password):
 raise RuntimeError(
 "In uninstall mode, -a, -r and -P options are not allowed")
+elif not self.interactive:
+if (not self.realm_name or not self.dm_password or
+not self.admin_password):
+raise RuntimeError(
+"In unattended mode you need to provide at least -r, -p "
+"and -a options")
 
 if self.idmax < self.idstart:
 raise RuntimeError(
-- 
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] [PATCHES 0370-0371] winsync-migrate: Handle invalid characters in the names of IPA entities

[Jan Cholasta]

On 23.9.2015 16:52, Martin Babinsky wrote:

On 09/23/2015 01:39 PM, Tomas Babej wrote:

Hi,

this fixes https://fedorahosted.org/freeipa/ticket/5319.

Details in the commit messages.

Tomas




ACK



The patches need to be rebased on top of ipa-4-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

Re: [Freeipa-devel] [PATCHES 0370-0371] winsync-migrate: Handle invalid characters in the names of IPA entities

[Martin Babinsky]

On 09/23/2015 01:39 PM, Tomas Babej wrote:

Hi,

this fixes https://fedorahosted.org/freeipa/ticket/5319.

Details in the commit messages.

Tomas




ACK

--
Martin^3 Babinsky

--
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 494] install: create kdcproxy user during server install

[Jan Cholasta]

On 23.9.2015 16:25, Martin Babinsky wrote:

On 09/23/2015 01:37 PM, Jan Cholasta wrote:

On 23.9.2015 12:49, Christian Heimes wrote:

On 2015-09-23 12:40, Jan Cholasta wrote:

On 23.9.2015 11:44, Christian Heimes wrote:

On 2015-09-23 10:54, Jan Cholasta wrote:

Correction, the HTTP server works, but it spits lots of errors in
error_log about /var/lib/kdcproxy not existing.

Is the KDCProxy supposed to be installked/enabled on upgrade ?
If not, why not ?
Even if it is not enabled, shouldn't the user be created just in
case ?


Fixed, patch attached.


I haven't tested the patch yet. It looks like the kdcproxy user
doesn't
own its home directory. Please chown /var/lib/kdcproxy.


I can't chown it because the user may not exist at RPM install time. It
doesn't matter anyway, since nothing is ever stored in the directory
and
KDC proxy works just fine. The same thing is done for the DS user and
nobody complained so far, so I assumed it should be OK for KDC proxy as
well.


I think we have a slight misunderstanding here. :) Of course you can't
set the owner at RPM install time. I wasn't talking about chown-ing the
directory in RPM, but chown-ing the directory after or inside the
tasks.create_system_user() call. Sorry for the confusion!

AFAIK neither mod_wsgi nor python-kdcproxy need a writeable home
directory. It's not guaranteed for eternity, though.


OK. Updated patch attached. Added patch 496, please apply before 495.




ACK to both patches.



Thanks.

Pushed to:
master: 4c39561261e79fe1cfdef916eafbcb9c204e77e8
ipa-4-2: 091b119580f7bbd534e7643e09fd33a85d8c010b

--
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 494] install: create kdcproxy user during server install

[Martin Babinsky]

On 09/23/2015 01:37 PM, Jan Cholasta wrote:

On 23.9.2015 12:49, Christian Heimes wrote:

On 2015-09-23 12:40, Jan Cholasta wrote:

On 23.9.2015 11:44, Christian Heimes wrote:

On 2015-09-23 10:54, Jan Cholasta wrote:

Correction, the HTTP server works, but it spits lots of errors in
error_log about /var/lib/kdcproxy not existing.

Is the KDCProxy supposed to be installked/enabled on upgrade ?
If not, why not ?
Even if it is not enabled, shouldn't the user be created just in
case ?


Fixed, patch attached.


I haven't tested the patch yet. It looks like the kdcproxy user doesn't
own its home directory. Please chown /var/lib/kdcproxy.


I can't chown it because the user may not exist at RPM install time. It
doesn't matter anyway, since nothing is ever stored in the directory and
KDC proxy works just fine. The same thing is done for the DS user and
nobody complained so far, so I assumed it should be OK for KDC proxy as
well.


I think we have a slight misunderstanding here. :) Of course you can't
set the owner at RPM install time. I wasn't talking about chown-ing the
directory in RPM, but chown-ing the directory after or inside the
tasks.create_system_user() call. Sorry for the confusion!

AFAIK neither mod_wsgi nor python-kdcproxy need a writeable home
directory. It's not guaranteed for eternity, though.


OK. Updated patch attached. Added patch 496, please apply before 495.




ACK to both patches.

--
Martin^3 Babinsky

--
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 494] install: create kdcproxy user during server install

[Simo Sorce]

On Wed, 2015-09-23 at 13:37 +0200, Jan Cholasta wrote:
> On 23.9.2015 12:49, Christian Heimes wrote:
> > On 2015-09-23 12:40, Jan Cholasta wrote:
> >> On 23.9.2015 11:44, Christian Heimes wrote:
> >>> On 2015-09-23 10:54, Jan Cholasta wrote:
> > Correction, the HTTP server works, but it spits lots of errors in
> > error_log about /var/lib/kdcproxy not existing.
> >
> > Is the KDCProxy supposed to be installked/enabled on upgrade ?
> > If not, why not ?
> > Even if it is not enabled, shouldn't the user be created just in case ?
> 
>  Fixed, patch attached.
> >>>
> >>> I haven't tested the patch yet. It looks like the kdcproxy user doesn't
> >>> own its home directory. Please chown /var/lib/kdcproxy.
> >>
> >> I can't chown it because the user may not exist at RPM install time. It
> >> doesn't matter anyway, since nothing is ever stored in the directory and
> >> KDC proxy works just fine. The same thing is done for the DS user and
> >> nobody complained so far, so I assumed it should be OK for KDC proxy as
> >> well.
> >
> > I think we have a slight misunderstanding here. :) Of course you can't
> > set the owner at RPM install time. I wasn't talking about chown-ing the
> > directory in RPM, but chown-ing the directory after or inside the
> > tasks.create_system_user() call. Sorry for the confusion!
> >
> > AFAIK neither mod_wsgi nor python-kdcproxy need a writeable home
> > directory. It's not guaranteed for eternity, though.
> 
> OK. Updated patch attached. Added patch 496, please apply before 495.

We have 2 options:
1. Home is created and chowned at user creation time
2. Home is owned by RPM packages.

The option we do *not* have is to have RPM own the directory and then
chown it later.

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 0055] dnssec options missing in ipa-dns-install man page

[Gabe Alford]

Thanks. Updated patch attached.

On Wed, Sep 23, 2015 at 7:14 AM, Martin Basti  wrote:

>
>
> On 09/23/2015 03:12 PM, Gabe Alford wrote:
>
> Odd and done. Updated patch attached.
>
> Gabe
>
> On Wed, Sep 23, 2015 at 5:20 AM, Martin Basti  wrote:
>
>>
>>
>> On 09/22/2015 03:32 PM, Gabe Alford wrote:
>>
>>> create mode 100644
>>> install/tools/man/freeipa-rga-0055-dnssec-options-missing-in-ipa-dns-install-man-page.patch
>>>
>> Hello,
>>
>> your patch created new patch :-)
>>
>> Also there were 3 white space errors, please remove them.
>>
>> Martin
>>
>
> Thank you, but there is still missing update in ipa-server-install manpage
>
> Martin
>
From a47fa4db2b8b757dbaa1e189fe9b37a0983b Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Wed, 23 Sep 2015 07:32:13 -0600
Subject: [PATCH] dnssec option missing in ipa-dns-install man page

- Add DNSSEC option ipa-replica-install and ipa-server-install man page as well

https://fedorahosted.org/freeipa/ticket/5300
---
 install/tools/man/ipa-dns-install.1 | 12 
 install/tools/man/ipa-replica-install.1 |  3 +++
 install/tools/man/ipa-server-install.1  |  3 +++
 3 files changed, 18 insertions(+)

diff --git a/install/tools/man/ipa-dns-install.1 b/install/tools/man/ipa-dns-install.1
index 23427b1b15ddf21ff1aba5617adab395d2f25112..66afe7fae5e82f48c7dc4d7c763f0483a41ecda1 100644
--- a/install/tools/man/ipa-dns-install.1
+++ b/install/tools/man/ipa-dns-install.1
@@ -44,6 +44,18 @@ The reverse DNS zone to use. This option can be used multiple times to specify m
 \fB\-\-no\-reverse\fR
 Do not create new reverse DNS zone. If used on a replica and a reverse DNS zone already exists for the subnet, it will be used.
 .TP
+\fB\-\-no\-dnssec\-validation\fR
+Disable DNSSEC validation on this server.
+.TP
+\fB\-\-dnssec\-master\fR
+Setup server to be DNSSEC key master.
+.TP
+\fB\-\-disable\-dnssec\-master\fR
+Disable the DNSSEC master on this server.
+.TP
+\fB\-\-kasp\-db\fR=\fIKASP_DB\fR
+Copy OpenDNSSEC metadata from the specified kasp.db file. This will not create a new kasp.db file.
+.TP
 \fB\-\-zonemgr\fR
 The e\-mail address of the DNS zone manager. Defaults to hostmaster@DOMAIN
 .TP
diff --git a/install/tools/man/ipa-replica-install.1 b/install/tools/man/ipa-replica-install.1
index 780febf9d597d7d36b6104c0fc1be8f3d1f8fdee..ff4d7d1c09a875bff6a49070fbba3d13fb63 100644
--- a/install/tools/man/ipa-replica-install.1
+++ b/install/tools/man/ipa-replica-install.1
@@ -109,6 +109,9 @@ Do not use DNS for hostname lookup during installation
 .TP
 \fB\-\-no\-dns\-sshfp\fR
 Do not automatically create DNS SSHFP records.
+.TP
+\fB\-\-no\-dnssec\-validation\fR
+Disable DNSSEC validation on this server.
 
 .SH "EXIT STATUS"
 0 if the command was successful
diff --git a/install/tools/man/ipa-server-install.1 b/install/tools/man/ipa-server-install.1
index 1eaed72119a9cd2f9876d3dc3c4a662782c18a36..2e0ff803c1b185d699f6f15dfb487e455404932e 100644
--- a/install/tools/man/ipa-server-install.1
+++ b/install/tools/man/ipa-server-install.1
@@ -164,6 +164,9 @@ Do not use DNS for hostname lookup during installation
 .TP
 \fB\-\-no\-dns\-sshfp\fR
 Do not automatically create DNS SSHFP records.
+.TP
+\fB\-\-no\-dnssec\-validation\fR
+Disable DNSSEC validation on this server.
 
 .SS "UNINSTALL OPTIONS"
 .TP
-- 
1.8.3.1

-- 
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 0055] dnssec options missing in ipa-dns-install man page

[Martin Basti]

On 09/23/2015 03:12 PM, Gabe Alford wrote:

Odd and done. Updated patch attached.

Gabe

On Wed, Sep 23, 2015 at 5:20 AM, Martin Basti > wrote:




On 09/22/2015 03:32 PM, Gabe Alford wrote:

create mode 100644

install/tools/man/freeipa-rga-0055-dnssec-options-missing-in-ipa-dns-install-man-page.patch

Hello,

your patch created new patch :-)

Also there were 3 white space errors, please remove them.

Martin



Thank you, but there is still missing update in ipa-server-install manpage

Martin
-- 
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 0055] dnssec options missing in ipa-dns-install man page

[Gabe Alford]

Odd and done. Updated patch attached.

Gabe

On Wed, Sep 23, 2015 at 5:20 AM, Martin Basti  wrote:

>
>
> On 09/22/2015 03:32 PM, Gabe Alford wrote:
>
>> create mode 100644
>> install/tools/man/freeipa-rga-0055-dnssec-options-missing-in-ipa-dns-install-man-page.patch
>>
> Hello,
>
> your patch created new patch :-)
>
> Also there were 3 white space errors, please remove them.
>
> Martin
>
From 8b2e7a7ab20a5fd5c8b6d0be05c0b30539d36cfa Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Wed, 23 Sep 2015 06:50:04 -0600
Subject: [PATCH] dnssec option missing in ipa-dns-install man page

- Add DNSSEC option ipa-replica-install man page as well

https://fedorahosted.org/freeipa/ticket/5300
---
 install/tools/man/ipa-dns-install.1 | 12 
 install/tools/man/ipa-replica-install.1 |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/install/tools/man/ipa-dns-install.1 b/install/tools/man/ipa-dns-install.1
index 23427b1b15ddf21ff1aba5617adab395d2f25112..66afe7fae5e82f48c7dc4d7c763f0483a41ecda1 100644
--- a/install/tools/man/ipa-dns-install.1
+++ b/install/tools/man/ipa-dns-install.1
@@ -44,6 +44,18 @@ The reverse DNS zone to use. This option can be used multiple times to specify m
 \fB\-\-no\-reverse\fR
 Do not create new reverse DNS zone. If used on a replica and a reverse DNS zone already exists for the subnet, it will be used.
 .TP
+\fB\-\-no\-dnssec\-validation\fR
+Disable DNSSEC validation on this server.
+.TP
+\fB\-\-dnssec\-master\fR
+Setup server to be DNSSEC key master.
+.TP
+\fB\-\-disable\-dnssec\-master\fR
+Disable the DNSSEC master on this server.
+.TP
+\fB\-\-kasp\-db\fR=\fIKASP_DB\fR
+Copy OpenDNSSEC metadata from the specified kasp.db file. This will not create a new kasp.db file.
+.TP
 \fB\-\-zonemgr\fR
 The e\-mail address of the DNS zone manager. Defaults to hostmaster@DOMAIN
 .TP
diff --git a/install/tools/man/ipa-replica-install.1 b/install/tools/man/ipa-replica-install.1
index 780febf9d597d7d36b6104c0fc1be8f3d1f8fdee..ff4d7d1c09a875bff6a49070fbba3d13fb63 100644
--- a/install/tools/man/ipa-replica-install.1
+++ b/install/tools/man/ipa-replica-install.1
@@ -109,6 +109,9 @@ Do not use DNS for hostname lookup during installation
 .TP
 \fB\-\-no\-dns\-sshfp\fR
 Do not automatically create DNS SSHFP records.
+.TP
+\fB\-\-no\-dnssec\-validation\fR
+Disable DNSSEC validation on this server.
 
 .SH "EXIT STATUS"
 0 if the command was successful
-- 
1.8.3.1

-- 
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] [PATCHES 362-366] Realmdomains handling improvements

[Martin Basti]

On 09/22/2015 02:23 PM, Tomas Babej wrote:

On 09/03/2015 04:34 PM, Alexander Bokovoy wrote:

On Thu, 03 Sep 2015, Tomas Babej wrote:

Hi,

this couple of patches fix https://fedorahosted.org/freeipa/ticket/5278
and improve our handling of realmdomains in general.

The code looks good to me. I haven't tested it yet, though.


Rebased on top of current master.


Please fix tests too.

[root@vm-065 ~]# ipa-run-tests test_xmlrpc/test_realmdomains_plugin.py 
--verbose
=== 
test session starts 
===
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4 -- 
/usr/bin/python

plugins: multihost, sourceorder
collected 13 items

test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[: 
realmdomains_show: Retrieve realm domains] PASSED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0001: 
realmdomains_show: Retrieve realm domains - print all attributes] PASSED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0002: 
realmdomains_mod: Replace list of realm domains with 
"[u'abc.idm.lab.eng.brq.redhat.com', u'example1.com']"] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0003: 
realmdomains_mod: Add domain "example2.com" to list] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0004: 
realmdomains_mod: Delete domain "example2.com" from list] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0005: 
realmdomains_mod: Add domain "example2.com" and delete domain 
"example1.com"] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0006: 
realmdomains_mod: Try to specify --domain and --add-domain options 
together] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0007: 
realmdomains_mod: Try to replace list of realm domains with a list 
without our domain] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0008: 
realmdomains_mod: Try to replace list of realm domains with a list with 
an invalid domain "doesnotexist.test"] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0009: 
realmdomains_mod: Try to add an invalid domain "doesnotexist.test"] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0010: 
realmdomains_mod: Try to delete our domain] FAILED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0011: 
realmdomains_mod: Try to delete domain which is not in list] PASSED
test_xmlrpc/test_realmdomains_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_realmdomains::test_command[0012: 
realmdomains_mod: Add an invalid domain "doesnotexist.test" with --force 
option] FAILED


--
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] [PATCHES 0370-0371] winsync-migrate: Handle invalid characters in the names of IPA entities

[Tomas Babej]

Hi,

this fixes https://fedorahosted.org/freeipa/ticket/5319.

Details in the commit messages.

Tomas
From 7daf849d776696575f9ce7a374995845616341b8 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 23 Sep 2015 13:27:35 +0200
Subject: [PATCH] winsync-migrate: Convert entity names to posix friendly
 strings

During the migration from winsync replicated users to their
trusted identities, memberships are being preserved. However,
trusted users are external and as such cannot be added as
direct members to the IPA entities. External groups which
encapsulate the migrated users are added as members to those
entities instead.

The name of the external group is generated from the type
of the entity and its name. However, the entity's name can
contain characters which are invalid for use in the group
name.

Adds a helper function to convert a given string to a string
which would be valid for such use and leverages it in the
winsync-migrate tool.

https://fedorahosted.org/freeipa/ticket/5319
---
 ipapython/ipautil.py | 23 +++
 ipaserver/install/ipa_winsync_migrate.py | 15 ---
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 2402689ccc1980117166969944e09e1ab5063ec2..573e6040c167ab8e10290cf3924d27cb102aa623 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1330,6 +1330,29 @@ def restore_hostname(statestore):
 except CalledProcessError as e:
 print("Failed to set this machine hostname back to %s: %s" % (old_hostname, str(e)), file=sys.stderr)
 
+def posixify(string):
+"""
+Convert a string to a more strict alpha-numeric representation.
+
+- Alpha-numeric, underscore, dot and dash characters are accepted
+- Space is converted to underscore
+- Other characters are omitted
+- Leading dash is stripped
+
+Note: This mapping is not one-to-one and may map different input to the
+same result. When using posixify, make sure the you do not map two different
+entities to one unintentionally.
+"""
+
+def valid_char(char):
+return char.isalnum() or char in ('_', '.', '-')
+
+# First replace space characters
+replaced = string.replace(' ','_')
+omitted = ''.join(filter(valid_char, replaced))
+
+# Leading dash is not allowed
+return omitted.lstrip('-')
 
 @contextmanager
 def private_ccache(path=None):
diff --git a/ipaserver/install/ipa_winsync_migrate.py b/ipaserver/install/ipa_winsync_migrate.py
index cafec9d188dbf2f1be07edea031d76f5e88dccbf..d0c6cc80c6e5c4455afe506a568cd5bf68975215 100644
--- a/ipaserver/install/ipa_winsync_migrate.py
+++ b/ipaserver/install/ipa_winsync_migrate.py
@@ -26,7 +26,7 @@ from ipalib import api
 from ipalib import errors
 from ipapython import admintool
 from ipapython.dn import DN
-from ipapython.ipautil import realm_to_suffix
+from ipapython.ipautil import realm_to_suffix, posixify
 from ipapython.ipa_log_manager import log_mgr
 from ipaserver.plugins.ldap2 import ldap2
 from ipaserver.install import replication
@@ -219,12 +219,21 @@ class WinsyncMigrate(admintool.AdminTool):
 
 def winsync_group_name(object_entry):
 """
-Returns the generated name of group containing migrated external users
+Returns the generated name of group containing migrated external
+users.
+
+The group name is of the form:
+ "__winsync_external"
+
+Object name is converted to posix-friendly string by omitting
+and/or replacing characters. This may lead to collisions, i.e.
+if both 'trust_admins' and 'trust admin' groups have winsync
+users being migrated.
 """
 
 return u"{0}_{1}_winsync_external".format(
 winsync_group_prefix,
-object_entry['cn'][0]
+posixify(object_entry['cn'][0])
 )
 
 def create_winsync_group(object_entry):
-- 
2.1.0

From 058723703ccf39e32c187d920b8d4e5f0c02e785 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 23 Sep 2015 13:28:33 +0200
Subject: [PATCH] winsync-migrate: Properly handle collisions in the names of
 external groups

Since the names of the external groups containing the migrated users
must be stripped of characters which are not valid for use in group names,
two different groups might be mapped to one during this process.

Properly handle collisions in the names by adding an incremental
numeric suffix.

https://fedorahosted.org/freeipa/ticket/5319
---
 ipaserver/install/ipa_winsync_migrate.py | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/ipa_winsync_migrate.py b/ipaserver/install/ipa_winsync_migrate.py
index d0c6cc80c6e5c4455afe506a568cd5bf68975215..87e23fb3698bac0a0371a198d95994ed921ee011 100644
--- a/ipaserver/install/ipa_winsync_migrate.py
+++ b/ipaserver/install/ipa_winsync_migrate.

Re: [Freeipa-devel] [PATCH 494] install: create kdcproxy user during server install

[Jan Cholasta]

On 23.9.2015 12:49, Christian Heimes wrote:

On 2015-09-23 12:40, Jan Cholasta wrote:

On 23.9.2015 11:44, Christian Heimes wrote:

On 2015-09-23 10:54, Jan Cholasta wrote:

Correction, the HTTP server works, but it spits lots of errors in
error_log about /var/lib/kdcproxy not existing.

Is the KDCProxy supposed to be installked/enabled on upgrade ?
If not, why not ?
Even if it is not enabled, shouldn't the user be created just in case ?


Fixed, patch attached.


I haven't tested the patch yet. It looks like the kdcproxy user doesn't
own its home directory. Please chown /var/lib/kdcproxy.


I can't chown it because the user may not exist at RPM install time. It
doesn't matter anyway, since nothing is ever stored in the directory and
KDC proxy works just fine. The same thing is done for the DS user and
nobody complained so far, so I assumed it should be OK for KDC proxy as
well.


I think we have a slight misunderstanding here. :) Of course you can't
set the owner at RPM install time. I wasn't talking about chown-ing the
directory in RPM, but chown-ing the directory after or inside the
tasks.create_system_user() call. Sorry for the confusion!

AFAIK neither mod_wsgi nor python-kdcproxy need a writeable home
directory. It's not guaranteed for eternity, though.


OK. Updated patch attached. Added patch 496, please apply before 495.

--
Jan Cholasta
From 2877e9a98423fd4b66834f2c71dd47c32a6d4f45 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 23 Sep 2015 13:09:44 +0200
Subject: [PATCH 1/2] platform: add option to create home directory when adding
 user

https://fedorahosted.org/freeipa/ticket/5314
---
 ipaplatform/base/tasks.py   | 8 ++--
 ipaplatform/redhat/tasks.py | 4 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index 6571514..573287c 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -184,7 +184,7 @@ class BaseTaskNamespace(object):
 
 return
 
-def create_system_user(self, name, group, homedir, shell, uid = None, gid = None, comment = None):
+def create_system_user(self, name, group, homedir, shell, uid=None, gid=None, comment=None, create_homedir=False):
 """Create a system user with a corresponding group"""
 try:
 grp.getgrnam(group)
@@ -211,12 +211,16 @@ class BaseTaskNamespace(object):
 '-g', group,
 '-d', homedir,
 '-s', shell,
-'-M', '-r', name,
+'-r', name,
 ]
 if uid:
 args += ['-u', str(uid)]
 if comment:
 args += ['-c', comment]
+if create_homedir:
+args += ['-m']
+else:
+args += ['-M']
 try:
 ipautil.run(args)
 log.debug('Done adding user')
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 3b522b0..dd614c9 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -400,7 +400,7 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 
 return True
 
-def create_system_user(self, name, group, homedir, shell, uid = None, gid = None, comment = None):
+def create_system_user(self, name, group, homedir, shell, uid=None, gid=None, comment=None, create_homedir=False):
 """
 Create a system user with a corresponding group
 
@@ -421,7 +421,7 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 comment = 'DS System User'
 
 super(RedHatTaskNamespace, self).create_system_user(name, group,
-homedir, shell, uid, gid, comment)
+homedir, shell, uid, gid, comment, create_homedir)
 
 
 tasks = RedHatTaskNamespace()
-- 
2.4.3

From e87cb5acc9556ab7ead897c8d112da576be848ed Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 23 Sep 2015 10:35:06 +0200
Subject: [PATCH 2/2] install: fix kdcproxy user home directory

https://fedorahosted.org/freeipa/ticket/5314
---
 freeipa.spec.in   | 2 +-
 ipaplatform/base/paths.py | 1 +
 ipaserver/install/httpinstance.py | 4 +++-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 7a199a5..36179c5 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -482,7 +482,6 @@ install daemons/dnssec/ipa-ods-exporter %{buildroot}%{_libexecdir}/ipa/ipa-ods-e
 mkdir -p %{buildroot}%{_usr}/share/ipa/ui/js/plugins
 
 # KDC proxy config (Apache config sets KDCPROXY_CONFIG to load this file)
-mkdir -p %{buildroot}%{kdcproxy_home}
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/kdcproxy/
 install -m 644 install/share/kdcproxy.conf %{buildroot}%{_sysconfdir}/ipa/kdcproxy/kdcproxy.conf
 
@@ -714,6 +713,7 @@ fi
 %{_libexecdir}/ipa/ipa-dnskeysync-replica
 %{_libexecdir}/ipa/ipa-ods-exporter
 %{_libexecdir}/ipa/ipa-httpd-kdcproxy
+%ghost %verify(not owner group) %dir %{_sharedstatedir}/kdcproxy
 %dir %attr(0755,root,roo

Re: [Freeipa-devel] [PATCH 0055] dnssec options missing in ipa-dns-install man page

[Martin Basti]

On 09/22/2015 03:32 PM, Gabe Alford wrote:

create mode 100644 
install/tools/man/freeipa-rga-0055-dnssec-options-missing-in-ipa-dns-install-man-page.patch

Hello,

your patch created new patch :-)

Also there were 3 white space errors, please remove them.

Martin

--
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 0064] destroy httpd ccache after stopping the service

[Martin Basti]

On 09/22/2015 01:44 PM, Martin Babinsky wrote:
This patch fixes https://fedorahosted.org/freeipa/ticket/5296 and 
generally makes cleaning up of httpd ccache more thorough.





ACK

Pushed to:
master: 93d080d726359db16749104c8bc20d14a5455dc0
ipa-4-2: 23f1d4ed605f9f4b22d6ad93c4110fff7358682c
-- 
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 0065] CI test for IPA install/backup/uninstall/install/restore scenario

[Martin Babinsky]

On 09/23/2015 12:53 PM, Martin Babinsky wrote:

CI test for full IPA restore into a running IPA server

self-NACK

--
Martin^3 Babinsky

--
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 0065] CI test for IPA install/backup/uninstall/install/restore scenario

[Martin Babinsky]

Should help to catch bugs like https://fedorahosted.org/freeipa/ticket/5296

--
Martin^3 Babinsky
From e515bc3aff47ac83807f2ae0b625e0ef8291b7c9 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 21 Sep 2015 09:58:38 +0200
Subject: [PATCH] CI test for full IPA restore into a running IPA server

Tests the following scenario:

1.) install IPA master
2.) perform backup
3.) uninstall IPA master
4.) install IPA master again
5.) restore backup from 2.)

https://fedorahosted.org/freeipa/ticket/5296
---
 ipatests/test_integration/test_backup_and_restore.py | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/ipatests/test_integration/test_backup_and_restore.py b/ipatests/test_integration/test_backup_and_restore.py
index 93f5d131c7de2b40d9be9434a372477d5924c1b9..df74fd954d0aa8ba93ca507fa41054ca11faa0e0 100644
--- a/ipatests/test_integration/test_backup_and_restore.py
+++ b/ipatests/test_integration/test_backup_and_restore.py
@@ -153,6 +153,25 @@ class TestBackupAndRestore(IntegrationTest):
 self.master.run_command(['ipa-restore', backup_path],
 stdin_text=dirman_password + '\nyes')
 
+def test_backup_and_restore_with_installed_ipa(self):
+with restore_checker(self.master):
+backup_path = backup(self.master)
+self.log.info('Backup path for %s is %s', self.master, backup_path)
+
+self.log.info('Uninstalling master')
+self.master.run_command(['ipa-server-install',
+ '--uninstall',
+ '-U'])
+
+self.log.info('Reinstalling IPA master')
+tasks.install_master(self.master)
+
+self.log.info('Backing up previous install')
+dirman_password = self.master.config.dirman_password
+self.master.run_command(['ipa-restore', backup_path],
+stdin_text=dirman_password + '\nyes')
+tasks.kinit_admin(self.master)
+
 def test_full_backup_and_restore_with_removed_users(self):
 """regression test for https://fedorahosted.org/freeipa/ticket/3866""";
 with restore_checker(self.master):
-- 
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 494] install: create kdcproxy user during server install

[Christian Heimes]

On 2015-09-23 12:40, Jan Cholasta wrote:
> On 23.9.2015 11:44, Christian Heimes wrote:
>> On 2015-09-23 10:54, Jan Cholasta wrote:
 Correction, the HTTP server works, but it spits lots of errors in
 error_log about /var/lib/kdcproxy not existing.

 Is the KDCProxy supposed to be installked/enabled on upgrade ?
 If not, why not ?
 Even if it is not enabled, shouldn't the user be created just in case ?
>>>
>>> Fixed, patch attached.
>>
>> I haven't tested the patch yet. It looks like the kdcproxy user doesn't
>> own its home directory. Please chown /var/lib/kdcproxy.
> 
> I can't chown it because the user may not exist at RPM install time. It
> doesn't matter anyway, since nothing is ever stored in the directory and
> KDC proxy works just fine. The same thing is done for the DS user and
> nobody complained so far, so I assumed it should be OK for KDC proxy as
> well.

I think we have a slight misunderstanding here. :) Of course you can't
set the owner at RPM install time. I wasn't talking about chown-ing the
directory in RPM, but chown-ing the directory after or inside the
tasks.create_system_user() call. Sorry for the confusion!

AFAIK neither mod_wsgi nor python-kdcproxy need a writeable home
directory. It's not guaranteed for eternity, though.

Christian



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] [PATCH 494] install: create kdcproxy user during server install

[Jan Cholasta]

On 23.9.2015 11:44, Christian Heimes wrote:

On 2015-09-23 10:54, Jan Cholasta wrote:

Correction, the HTTP server works, but it spits lots of errors in
error_log about /var/lib/kdcproxy not existing.

Is the KDCProxy supposed to be installked/enabled on upgrade ?
If not, why not ?
Even if it is not enabled, shouldn't the user be created just in case ?


Fixed, patch attached.


I haven't tested the patch yet. It looks like the kdcproxy user doesn't
own its home directory. Please chown /var/lib/kdcproxy.


I can't chown it because the user may not exist at RPM install time. It 
doesn't matter anyway, since nothing is ever stored in the directory and 
KDC proxy works just fine. The same thing is done for the DS user and 
nobody complained so far, so I assumed it should be OK for KDC proxy as 
well.


--
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 494] install: create kdcproxy user during server install

[Christian Heimes]

On 2015-09-23 10:54, Jan Cholasta wrote:
>> Correction, the HTTP server works, but it spits lots of errors in
>> error_log about /var/lib/kdcproxy not existing.
>>
>> Is the KDCProxy supposed to be installked/enabled on upgrade ?
>> If not, why not ?
>> Even if it is not enabled, shouldn't the user be created just in case ?
> 
> Fixed, patch attached.

I haven't tested the patch yet. It looks like the kdcproxy user doesn't
own its home directory. Please chown /var/lib/kdcproxy.

Christian




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] [PATCH 494] install: create kdcproxy user during server install

[Jan Cholasta]

On 23.9.2015 02:22, Simo Sorce wrote:

On Tue, 2015-09-22 at 20:09 -0400, Simo Sorce wrote:

On Tue, 2015-09-22 at 16:35 +0200, Jan Cholasta wrote:

On 22.9.2015 15:11, Martin Babinsky wrote:

On 09/22/2015 01:33 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza




ACK



Thanks.

Pushed to:
master: 0de860318332114ca739a8dd45902f7cc9a3c722
ipa-4-2: 4663625bbb3456db7f13578e6cac0c3e5fae2591


This patch is somehow broken.

I see that %{kdcproxy_home} has been removed from the spec file but not
from everywhere, and it is simply undefined.

On upgrade of my server I have no kdcproxy user and http fails to
operate complaining that /var/lib/kdcproxy does not exist.


Correction, the HTTP server works, but it spits lots of errors in
error_log about /var/lib/kdcproxy not existing.

Is the KDCProxy supposed to be installked/enabled on upgrade ?
If not, why not ?
Even if it is not enabled, shouldn't the user be created just in case ?


Fixed, patch attached.

--
Jan Cholasta
From 45bc745849aade6e9f0495479e9df5d32d43274b Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 23 Sep 2015 10:35:06 +0200
Subject: [PATCH] install: fix kdcproxy user home directory

https://fedorahosted.org/freeipa/ticket/5314
---
 freeipa.spec.in   | 3 ++-
 ipaplatform/base/paths.py | 1 +
 ipaserver/install/httpinstance.py | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 7a199a5..782eefc 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -482,7 +482,7 @@ install daemons/dnssec/ipa-ods-exporter %{buildroot}%{_libexecdir}/ipa/ipa-ods-e
 mkdir -p %{buildroot}%{_usr}/share/ipa/ui/js/plugins
 
 # KDC proxy config (Apache config sets KDCPROXY_CONFIG to load this file)
-mkdir -p %{buildroot}%{kdcproxy_home}
+mkdir -p %{buildroot}%{_sharedstatedir}/kdcproxy
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/kdcproxy/
 install -m 644 install/share/kdcproxy.conf %{buildroot}%{_sysconfdir}/ipa/kdcproxy/kdcproxy.conf
 
@@ -714,6 +714,7 @@ fi
 %{_libexecdir}/ipa/ipa-dnskeysync-replica
 %{_libexecdir}/ipa/ipa-ods-exporter
 %{_libexecdir}/ipa/ipa-httpd-kdcproxy
+%dir %{_sharedstatedir}/kdcproxy
 %dir %attr(0755,root,root) %{_sysconfdir}/ipa/kdcproxy
 %config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
 %config(noreplace) %{_sysconfdir}/sysconfig/ipa-dnskeysyncd
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 97c330c..215caf9 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -286,6 +286,7 @@ class BasePathNamespace(object):
 REPLICA_INFO_GPG_TEMPLATE = "/var/lib/ipa/replica-info-%s.gpg"
 SYSRESTORE = "/var/lib/ipa/sysrestore"
 STATEFILE_DIR = "/var/lib/ipa/sysupgrade"
+VAR_LIB_KDCPROXY = "/var/lib/kdcproxy"
 VAR_LIB_PKI_DIR = "/var/lib/pki"
 VAR_LIB_PKI_CA_DIR = "/var/lib/pki-ca"
 PKI_ALIAS_CA_P12 = "/var/lib/pki-ca/alias/ca.p12"
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 7358511..ab84780 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -85,8 +85,9 @@ def create_kdcproxy_user():
 tasks.create_system_user(
 name=KDCPROXY_USER,
 group=KDCPROXY_USER,
-homedir=paths.VAR_LIB,
+homedir=paths.VAR_LIB_KDCPROXY,
 shell=paths.NOLOGIN,
+comment="IPA KDC Proxy User",
 )
 
 
-- 
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] improved logging in dnssec tests

[Petr Spacek]

On 22.9.2015 11:21, Oleg Fayans wrote:
> Hi all,
> 
> I've noticed that in some tests some low-level functions can return False in a
> number of different conditions, which severely complicates test debugging.
> This patch implements the approach widely used in the Go language (and maybe,
> some other): The function returns not only a boolean, but a boolean plus any
> error message caught during the execution. This error message may be used by
> the higher level code for logging. For example, compare these outputs:
> 1. AssertionError: Zone example.test. is not signed (master): request timed 
> out
> 2. AssertionError: Zone example.test. is not signed (master)
> 
> What do you think?

I'm not against this patch in principle but honestly, it seems as waste of
time to me. E.g. a timeout can be caused by several different problems (LDAP
down, KDC down, named down, firewall ...), the fact that the zone is not
signed can be caused by different problems (time skew between replica and
master, one of dnssec helpers does not work, LDAP replication is broken ...),
and the error you are going to display can be very misleading.

In all cases you have to try it using dig or drill and read logs to see what
happened and why.

(I did not test the patch because it would take too much time I do not have 
now.)

-- 
Petr^2 Spacek

-- 
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] Proper fix for ticket 5306

[Petr Spacek]

On 22.9.2015 10:42, Oleg Fayans wrote:
> +++ b/ipatests/test_integration/tasks.py
> @@ -58,6 +58,14 @@ def check_arguments_are(slice, instanceof):
>  return wrapped
>  return wrapper
>  
> +def prepare_reverse_zone(host, ip):
> +nums = ip.split('.')[:-1]
> +zone = ".".join(reversed(nums)) + ".in-addr.arpa."
> +host.run_command(["ipa",
> +  "dnszone-add",
> +  zone,
> +  "--name-from-ip=%s" % ip], raiseonerr=False)
> +

NACK:
- this will break IPv6-only hosts
- you should use DNSName class or other functions from python-dns for DNS name
manipulation

I hope this helps.

-- 
Petr^2 Spacek

-- 
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