Re: [Freeipa-devel] [PATCH] 0044-0045 Add profiles and default CA ACL on migration

2015-11-24 Thread Jan Cholasta

On 24.11.2015 08:37, Fraser Tweedale wrote:

On Mon, Nov 23, 2015 at 10:05:32AM +0100, Jan Cholasta wrote:

On 23.11.2015 06:54, Fraser Tweedale wrote:

Hi all,

The attached patches fix #5459[1]: Default CA ACL rule is not
created during ipa-replica-install.

These patches apply on branch ipa-4-2.  There is a (trivial)
conflict in imports when applying to master.


When a patch does not apply cleanly on all the target branches, you should
attach a rebased patch as well.



I strongly recommend review / testing of these patches with patches
0042-0043[2] due to the prevalence of the other issue.

[1] https://fedorahosted.org/freeipa/ticket/5459
[2] https://www.redhat.com/archives/freeipa-devel/2015-November/msg00298.html


Patch 0044: ACK

Patch 0045:

1) The check in caacl_del could be better, please take a look at how the
admins group is handled in ipalib/plugins/group.py for an example. You
should at least raise ProtectedEntryError rather than ValidationError.

2) _add_default_caacl() should be located in
ipaserver/install/cainstance.py.

3) Rather than calling the cainstance functions in replicainstall.install(),
they should be called from CAInstance.configure_instance() to make them
effective in ipa-ca-install and replica promotion as well.

Honza


Updated patches for ipa-4-2 and master branches attached.

The new patch 0045 is somewhat more intrusive; I have tested server
install, replica install (with CA) from 3.0 and 4.2 master and
ipa-ca-install with replica file from 3.0 master... but more testing
would be no bad thing!


Works for me, ACK.

Pushed to:
master: 620036d26e98fdcefff00168e9e5463a8257d49c
ipa-4-2: a2371f38e4fb027aeacaf0ab6f2b35ae49fa41ea

--
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] 0044-0045 Add profiles and default CA ACL on migration

2015-11-23 Thread Fraser Tweedale
On Mon, Nov 23, 2015 at 10:05:32AM +0100, Jan Cholasta wrote:
> On 23.11.2015 06:54, Fraser Tweedale wrote:
> >Hi all,
> >
> >The attached patches fix #5459[1]: Default CA ACL rule is not
> >created during ipa-replica-install.
> >
> >These patches apply on branch ipa-4-2.  There is a (trivial)
> >conflict in imports when applying to master.
> 
> When a patch does not apply cleanly on all the target branches, you should
> attach a rebased patch as well.
> 
> >
> >I strongly recommend review / testing of these patches with patches
> >0042-0043[2] due to the prevalence of the other issue.
> >
> >[1] https://fedorahosted.org/freeipa/ticket/5459
> >[2] https://www.redhat.com/archives/freeipa-devel/2015-November/msg00298.html
> 
> Patch 0044: ACK
> 
> Patch 0045:
> 
> 1) The check in caacl_del could be better, please take a look at how the
> admins group is handled in ipalib/plugins/group.py for an example. You
> should at least raise ProtectedEntryError rather than ValidationError.
> 
> 2) _add_default_caacl() should be located in
> ipaserver/install/cainstance.py.
> 
> 3) Rather than calling the cainstance functions in replicainstall.install(),
> they should be called from CAInstance.configure_instance() to make them
> effective in ipa-ca-install and replica promotion as well.
> 
> Honza
> 
Updated patches for ipa-4-2 and master branches attached.

The new patch 0045 is somewhat more intrusive; I have tested server
install, replica install (with CA) from 3.0 and 4.2 master and
ipa-ca-install with replica file from 3.0 master... but more testing
would be no bad thing!

I'll be offline for a few hours but will check back later tonight;
see where things are at.

Thanks,
Fraser
From 354796196f029865e4f5a652900288e043d9c03a Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Mon, 23 Nov 2015 12:09:32 +1100
Subject: [PATCH 44/45] Do not erroneously reinit NSS in Dogtag interface

The Dogtag interface always attempts to (re)init NSS, which can fail
with SEC_ERROR_BUSY.  Do not reinitialise NSS when it has already
been initialised with the given dbdir.

Part of: https://fedorahosted.org/freeipa/ticket/5459
---
 ipapython/dogtag.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index 
71de96dc6889ca677db2cf7bb4f2d7b56df832f6..0436d5f464aac20c6b3c7db7b1c3a972d3533823
 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -265,7 +265,8 @@ def https_request(host, port, url, secdir, password, 
nickname,
 """
 
 def connection_factory(host, port):
-conn = nsslib.NSSConnection(host, port, dbdir=secdir,
+no_init = secdir == nsslib.current_dbdir
+conn = nsslib.NSSConnection(host, port, dbdir=secdir, no_init=no_init,
 tls_version_min=api.env.tls_version_min,
 tls_version_max=api.env.tls_version_max)
 conn.set_debuglevel(0)
-- 
2.4.3

From c0234a507fada45eb64a00e1d3e6ed39321564bc Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Mon, 23 Nov 2015 12:09:32 +1100
Subject: [PATCH 44/45] Do not erroneously reinit NSS in Dogtag interface

The Dogtag interface always attempts to (re)init NSS, which can fail
with SEC_ERROR_BUSY.  Do not reinitialise NSS when it has already
been initialised with the given dbdir.

Part of: https://fedorahosted.org/freeipa/ticket/5459
---
 ipapython/dogtag.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index 
26b2de6ca77202fa9ccc61ee16ed7623e10ecb5f..8996902ba92f0fdd6106e2650c2decde375c593b
 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -255,7 +255,8 @@ def https_request(host, port, url, secdir, password, 
nickname,
 """
 
 def connection_factory(host, port):
-conn = nsslib.NSSConnection(host, port, dbdir=secdir,
+no_init = secdir == nsslib.current_dbdir
+conn = nsslib.NSSConnection(host, port, dbdir=secdir, no_init=no_init,
 tls_version_min=api.env.tls_version_min,
 tls_version_max=api.env.tls_version_max)
 conn.set_debuglevel(0)
-- 
2.4.3

From 81d939ad6755ab80b7db2e592d08730d88f1e06b Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Mon, 23 Nov 2015 12:09:32 +1100
Subject: [PATCH 45/45] Add profiles and default CA ACL on migration

Profiles and the default CA ACL were not being added during replica
install from pre-4.2 servers.  Update ipa-replica-install to add
these if they are missing.

Also update the caacl plugin to prevent deletion of the default CA
ACL and instruct the administrator to disable it instead.

To ensure that the cainstance installation can add profiles, supply
the RA certificate as part of the instance configuration.
Certmonger renewal setup is avoided at this point because the NSSDB
gets reinitialised later in installation procedure.

Also move the addition of the default CA ACL from dsinstance
inst

Re: [Freeipa-devel] [PATCH] 0044-0045 Add profiles and default CA ACL on migration

2015-11-23 Thread Jan Cholasta

On 23.11.2015 06:54, Fraser Tweedale wrote:

Hi all,

The attached patches fix #5459[1]: Default CA ACL rule is not
created during ipa-replica-install.

These patches apply on branch ipa-4-2.  There is a (trivial)
conflict in imports when applying to master.


When a patch does not apply cleanly on all the target branches, you 
should attach a rebased patch as well.




I strongly recommend review / testing of these patches with patches
0042-0043[2] due to the prevalence of the other issue.

[1] https://fedorahosted.org/freeipa/ticket/5459
[2] https://www.redhat.com/archives/freeipa-devel/2015-November/msg00298.html


Patch 0044: ACK

Patch 0045:

1) The check in caacl_del could be better, please take a look at how the 
admins group is handled in ipalib/plugins/group.py for an example. You 
should at least raise ProtectedEntryError rather than ValidationError.


2) _add_default_caacl() should be located in 
ipaserver/install/cainstance.py.


3) Rather than calling the cainstance functions in 
replicainstall.install(), they should be called from 
CAInstance.configure_instance() to make them effective in ipa-ca-install 
and replica promotion as well.


Honza

--
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] [PATCH] 0044-0045 Add profiles and default CA ACL on migration

2015-11-22 Thread Fraser Tweedale
Hi all,

The attached patches fix #5459[1]: Default CA ACL rule is not
created during ipa-replica-install.

These patches apply on branch ipa-4-2.  There is a (trivial)
conflict in imports when applying to master.

I strongly recommend review / testing of these patches with patches
0042-0043[2] due to the prevalence of the other issue.

[1] https://fedorahosted.org/freeipa/ticket/5459
[2] https://www.redhat.com/archives/freeipa-devel/2015-November/msg00298.html

Thanks,
Fraser
From 8c3f2ce4a985e873277b7e84a8b95acca80c0348 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Mon, 23 Nov 2015 12:09:32 +1100
Subject: [PATCH 44/45] Do not erroneously reinit NSS in Dogtag interface

The Dogtag interface always attempts to (re)init NSS, which can fail
with SEC_ERROR_BUSY.  Do not reinitialise NSS when it has already
been initialised with the given dbdir.

Part of: https://fedorahosted.org/freeipa/ticket/5459
---
 ipapython/dogtag.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index 
3f0d08154d21a3072e344c311c3e70e414d9dee4..75c34db697ec5f7b1aac771de8517937fa09fbdd
 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -255,7 +255,8 @@ def https_request(host, port, url, secdir, password, 
nickname,
 """
 
 def connection_factory(host, port):
-conn = nsslib.NSSConnection(host, port, dbdir=secdir,
+no_init = secdir == nsslib.current_dbdir
+conn = nsslib.NSSConnection(host, port, dbdir=secdir, no_init=no_init,
 tls_version_min=api.env.tls_version_min,
 tls_version_max=api.env.tls_version_max)
 conn.set_debuglevel(0)
-- 
2.4.3

From 2a05260345627e5b636596a715333a20b5631cd1 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Mon, 23 Nov 2015 14:50:45 +1100
Subject: [PATCH 45/45] Add profiles and default CA ACL on migration

Profiles and the default CA ACL were not being added during replica
install from pre-4.2 servers.  Update ipa-replica-install to add
these if they are missing.

Also update the caacl plugin to prevent deletion of the default CA
ACL and instruct the administrator to disable it instead.

Fixes: https://fedorahosted.org/freeipa/ticket/5459
---
 install/updates/50-dogtag10-migration.update |  1 +
 ipalib/plugins/caacl.py  |  7 +++
 ipaserver/install/server/replicainstall.py   |  8 
 ipaserver/install/server/upgrade.py  | 28 
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/install/updates/50-dogtag10-migration.update 
b/install/updates/50-dogtag10-migration.update
index 
2ab9d15bd220540dbc6b3fcd7928fc15c42caf80..0070c308aefc39aa4c27a046d185ce6d268e6270
 100644
--- a/install/updates/50-dogtag10-migration.update
+++ b/install/updates/50-dogtag10-migration.update
@@ -16,3 +16,4 @@ addifexist:resourceACLS:certServer.ca.groups:execute:allow 
(execute) group="Admi
 addifexist:resourceACLS:certServer.ca.users:execute:allow (execute) 
group="Administrators":Admins may execute user operations
 replace:resourceACLS:certServer.securitydomain.domainxml:read,modify:allow 
(read) user="anybody";allow (modify) group="Subsystem Group":Anybody is allowed 
to read domain.xml but only Subsystem group is allowed to modify the 
domain.xml::certServer.securitydomain.domainxml:read,modify:allow (read) 
user="anybody";allow (modify) group="Subsystem Group" || group="Enterprise CA 
Administrators" || group="Enterprise KRA Administrators" || group="Enterprise 
RA Administrators" || group="Enterprise OCSP Administrators" || 
group="Enterprise TKS Administrators" || group="Enterprise TPS 
Administrators":Anybody is allowed to read domain.xml but only Subsystem group 
and Enterprise Administrators are allowed to modify the domain.xml
 replace:resourceACLS:certServer.ca.connectorInfo:read,modify:allow 
(modify,read) group="Enterprise KRA Administrators":Only Enterprise 
Administrators are allowed to update the connector 
information::certServer.ca.connectorInfo:read,modify:allow (read) 
group="Enterprise KRA Administrators";allow (modify) group="Enterprise KRA 
Administrators" || group="Subsystem Group":Only Enterprise Administrators and 
Subsystem Group are allowed to update the connector information
+addifexist:resourceACLS:certServer.profile.configuration:read,modify:allow 
(read,modify) group="Certificate Manager Agents":Certificate Manager agents may 
modify (create/update/delete) and read profiles
diff --git a/ipalib/plugins/caacl.py b/ipalib/plugins/caacl.py
index 
247d6df143aef1fba9f0ee74a9f7d8386bef5180..77bad38cd7e9f3cf10b7476acc7ac16fc6494bdf
 100644
--- a/ipalib/plugins/caacl.py
+++ b/ipalib/plugins/caacl.py
@@ -307,6 +307,13 @@ class caacl_del(LDAPDelete):
 
 msg_summary = _('Deleted CA ACL "%(value)s"')
 
+def pre_callback(self, ldap, dn, *keys, **options):
+if keys[0] == 'hosts_services_caIPAserviceCert':
+raise errors