On 06/21/2013 02:15 PM, Martin Kosek wrote:
On 06/21/2013 02:11 PM, Tomas Babej wrote:
On 06/20/2013 06:00 PM, Simo Sorce wrote:
On Thu, 2013-06-20 at 17:47 +0200, Martin Kosek wrote:
On 06/20/2013 05:44 PM, Simo Sorce wrote:
On Thu, 2013-06-20 at 17:33 +0200, Martin Kosek wrote:
On 06/20/2013 05:15 PM, Tomas Babej wrote:
Hi,

Spec file modified so that /var/lib/ipa/pki-ca/publish/ is owned
by pkiuser group.

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

Tomas

NACK. This won't fly. pkiuser is created by FreeIPA when server is installed,
thus you cannot just simply change ownership in our spec file because in the
time when package is installed or updated, pkiuser may not exist.

I think you need to delete the %attr from spec file and set the correct
ownership during ipa-{server,ca}-install. When CA is configured, we should
also
probably let ipa-upgradeconfig check this directory and amend when necessary
(to fix affected IPA CA instances).
Probably even better to not create the directory via rpm at all, but
make ipa-ca-install create it and remove it when --uninstall is run.

Simo.
This could also work, sure. Could we then at least mark this directory in our
spec file as %ghost? So that "rpm -qf /var/lib/ipa/pki-ca/publish/" gives some
information?
I guess so.

Simo.

Updated version attached.

Tomas
Looks good by reading (I did not test it), maybe just one nitpick:

+                root_logger.warning("Error while removing CRL publish "
+                                    "directory: %s" % str(e))

This should read:
+                root_logger.warning("Error while removing CRL publish "
+                                    "directory: %s", e)

We do not need to format the string before it is really logged and we also do
not need to convert it to "str" as we already requested the conversion to
string by "%s".

Martin
Fixed.

Tomas
From 21046b4c093568b8b89bcc976ce06801a5a00f36 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 20 Jun 2013 15:49:58 +0200
Subject: [PATCH] Change group ownership of CRL publish directory

Spec file modified so that /var/lib/ipa/pki-ca/publish/ is no
longer owned by created with package installation. The directory
is rather created/removed with the CA instance itself.

This ensures proper creation/removeal, group ownership
and SELinux context.

https://fedorahosted.org/freeipa/ticket/3727
---
 freeipa.spec.in                 |  6 ++++--
 install/Makefile.am             |  3 +--
 install/tools/ipa-upgradeconfig |  7 ++++---
 ipaserver/install/cainstance.py | 13 +++++++++++++
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 239811ac26aa84e1928cefb9c3adac58326ad9a7..d3463f4ef65ec48300bea8fe9773dfe4b3b298f5 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -381,7 +381,6 @@ rm %{buildroot}/%{_libdir}/samba/pdb/ipasam.la
 mkdir -p %{buildroot}/%{_sysconfdir}/ipa/html
 mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/sysrestore
 mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/sysupgrade
-mkdir -p %{buildroot}/%{_localstatedir}/cache/ipa/pki-ca/publish
 mkdir %{buildroot}%{_usr}/share/ipa/html/
 ln -s ../../../..%{_sysconfdir}/ipa/html/ffconfig.js \
     %{buildroot}%{_usr}/share/ipa/html/ffconfig.js
@@ -710,7 +709,7 @@ fi
 %attr(700,root,root) %dir %{_localstatedir}/lib/ipa/sysrestore
 %attr(700,root,root) %dir %{_localstatedir}/lib/ipa/sysupgrade
 %attr(755,root,root) %dir %{_localstatedir}/lib/ipa/pki-ca
-%attr(755,root,root) %dir %{_localstatedir}/lib/ipa/pki-ca/publish
+%ghost %{_localstatedir}/lib/ipa/pki-ca/publish
 %attr(755,root,root) %{_libdir}/krb5/plugins/kdb/ipadb.so
 %{_mandir}/man1/ipa-replica-conncheck.1.gz
 %{_mandir}/man1/ipa-replica-install.1.gz
@@ -819,6 +818,9 @@ fi
 %endif  # ! %{ONLY_CLIENT}
 
 %changelog
+* Fri Jun 21 2013 Tomas Babej <tba...@redhat.com> - 3.2.99-3
+- Do not create /var/lib/ipa/pki-ca/publish, retain reference as ghost
+
 * Mon Jun 17 2013 Petr Viktorin <pvikt...@redhat.com> - 3.2.99-2
 - Add the freeipa-tests subpackage
 
diff --git a/install/Makefile.am b/install/Makefile.am
index b2e6e9a65e1483be6f921280f13ebf7e0dd2469a..c07f571550af9651366234db86da7a0c3ff13480 100644
--- a/install/Makefile.am
+++ b/install/Makefile.am
@@ -24,9 +24,8 @@ install-exec-local:
 	chmod 700 $(DESTDIR)$(localstatedir)/lib/ipa/sysrestore
 	mkdir -p $(DESTDIR)$(localstatedir)/lib/ipa/sysupgrade
 	chmod 700 $(DESTDIR)$(localstatedir)/lib/ipa/sysupgrade
-	mkdir -p $(DESTDIR)$(localstatedir)/lib/ipa/pki-ca/publish
+	mkdir -p $(DESTDIR)$(localstatedir)/lib/ipa/pki-ca
 	chmod 755 $(DESTDIR)$(localstatedir)/lib/ipa/pki-ca
-	chmod 755 $(DESTDIR)$(localstatedir)/lib/ipa/pki-ca/publish
 
 uninstall-local:
 	-rmdir $(DESTDIR)$(localstatedir)/lib/ipa/sysrestore
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 4e9216964a045b5a87c22f6eb87bb1844f4adce9..4fbcdb6bf5092c12301f6ec76c5a329f14594fd6 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -690,15 +690,16 @@ def migrate_crl_publish_dir(ca):
                 caconfig.CS_CFG_PATH, e)
         return False
 
+    # Prepare target publish dir (creation, permissions, SELinux context)
+    # Run this every update to ensure proper values
+    publishdir = ca.prepare_crl_publish_dir()
+
     if old_publish_dir == caconfig.CRL_PUBLISH_PATH:
         # publish dir is already updated
         root_logger.info('Publish directory already set to new location')
         sysupgrade.set_upgrade_state('dogtag', 'moved_crl_publish_dir', True)
         return False
 
-    # Prepare target publish dir (permissions, SELinux context)
-    publishdir = ca.prepare_crl_publish_dir()
-
     # Copy all CRLs to new directory
     root_logger.info('Copy all CRLs to new publish directory')
     try:
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index d83fd7a1d4c45ed06c61c1d96ca9eb6b27341ccc..ca3ee69fbce8bea512ff402b196b457cedad86c8 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1107,6 +1107,10 @@ class CAInstance(service.Service):
         Returns a path to the CRL publishing directory
         """
         publishdir = self.dogtag_constants.CRL_PUBLISH_PATH
+
+        if not os.path.exists(publishdir):
+            os.mkdir(publishdir)
+
         os.chmod(publishdir, 0775)
         pent = pwd.getpwnam(PKI_USER)
         os.chown(publishdir, 0, pent.pw_gid)
@@ -1334,6 +1338,15 @@ class CAInstance(service.Service):
             root_logger.debug("Remove %s", f)
             installutils.remove_file(f)
 
+        # remove CRL directory
+        root_logger.info("Remove CRL directory")
+        if os.path.exists(self.dogtag_constants.CRL_PUBLISH_PATH):
+            try:
+                shutil.rmtree(self.dogtag_constants.CRL_PUBLISH_PATH)
+            except OSError, e:
+                root_logger.warning("Error while removing CRL publish "
+                                    "directory: %s" % e)
+
     def publish_ca_cert(self, location):
         args = ["-L", "-n", self.canickname, "-a"]
         (cert, err, returncode) = self.__run_certutil(args)
-- 
1.8.1.4

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

Reply via email to