On 08/01/2013 04:52 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
>> On 08/01/2013 02:58 PM, Martin Kosek wrote:
>>> On 08/01/2013 02:54 PM, Petr Viktorin wrote:
>>>> On 07/31/2013 11:51 AM, Ana Krivokapic wrote:
>>>>> On 07/30/2013 06:24 PM, Petr Viktorin wrote:
>>>>>> On 07/30/2013 10:27 AM, Ana Krivokapic wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> This patch addresses ticket
>>>>>>> https://fedorahosted.org/freeipa/ticket/3783.
>>>>>>>
>>>>>>
>>>>>> Thanks for the patch, I have a concern below:
>>>>>>
>>>>>>> freeipa-akrivoka-0051-Handle-subject-option-in-ipa-server-install.patch
>>>>>>>
>>>>>>> diff --git a/install/tools/ipa-upgradeconfig
>>>>>>> b/install/tools/ipa-upgradeconfig
>>>>>>> index
>>>>>>> de17c5b23d79f31e8571a3400d44397630cadada..a2625e6198bcff0811c482e479c8af10716dcea1
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 100644
>>>>>>> --- a/install/tools/ipa-upgradeconfig
>>>>>>> +++ b/install/tools/ipa-upgradeconfig
>>>>>>> @@ -894,6 +895,7 @@ def main():
>>>>>>>         configured_constants = dogtag.configured_constants()
>>>>>>>         sub_dict = dict(
>>>>>>>             REALM=api.env.realm,
>>>>>>> +        SUBJECT_BASE=str(DN(('O', api.env.realm))),
>>>>>>
>>>>>> When certmap.conf.template's version changes again, this will
>>>>>> rewrite the
>>>>>> subject to the default. Don't we want to use the subject base also
>>>>>> here?
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> You are right. The updated patch uses the current value of subject
>>>>> base from
>>>>> LDAP to update certmap.conf during upgrades.
>>>>
>>>> When ipa-upgradeconfig is run while the DS is down, this results in a
>>>> small
>>>> warning, and very bad configuration:
>>>>      certmap ipaca           CN=Certificate Authority,None
>>>>
>>>>
>>>> I'm not sure how this should be handled. I'm adding Rob to the loop.
>>>> Rob, can we start the DS in ipa-upgradeconfig? That sounds quite
>>>> heavy-handed
>>>> for a RPM upgrade script.
>>>>
>>>> Maybe if the DS is unavailable, we should use the old value from the
>>>> config
>>>> file itself.
>>>
>>> Values can be stored/restored using a sysupgrade module.
>>
>> The problem is that (AFAIK) old instances won't have this particular
>> value stored. Upgrading from 3.1 to a future version where certmap.conf
>> is modified again would fail.
>>
>>> I would not be so
>>> afraid of starting the DS module, we already do that exercise in
>>> ipa-ldap-updater, so adding it in ipa-upgradeconfig too does not
>>> change much.
>>>
>>> Question is, what should we do what DS cannot be started or cannot be
>>> read
>>> (e.g. when upgrade is run in fedup's chrooted environment) - we must
>>> make sure
>>> we don't mess the configuration up.
>>
>> Again AFAIK, if the server is down, certmap.conf is the only place where
>> this value is available. I didn't check thoroughly, though.
>
> I think that's right.
>
> So the big problem here is we have no way of alerting users to possible
> problems found during the upgrade unless they happen to sift through
> ipaupgrade.log, which tends to be huge, and most users don't even know about.
>
> Part of this is due to not displaying anything during an rpm upgrade. I think
> we should print any exceptional errors found during the upgrade. Sure,
> something may eat the output, but it's a best effort sort of thing.
>
> I don't see us finding a perfect solution here. What I'd propose is this:
>
> - Store the value in sysupgrade from now on, even in new installs
> - Look there first on upgrade
> - Start dirsrv if necessary get the value
> - If we can't get the value, skip changing file and log loudly
> - Find out exactly this will present to a user by doing an upgrade from an old
> version to master and create an FAQ entry.
>
> I think that's about the best we can do.
>
> rob
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Updated patch tries to find the subject base value by searching in this order:
1) sysupgrade
2) dirsrv
3) certmap.conf
If it cannot be found, an error is displayed and the file is not modified.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 4c0ef5301a963bb36520ed1f0e60116a55056d2e Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Mon, 29 Jul 2013 18:33:09 +0200
Subject: [PATCH] Handle --subject option in ipa-server-install

Properly handle --subject option of ipa-server-install, making sure this
value gets passed to certmap.conf. Introduce a new template variable
$SUBJECT_BASE for this purpose.

Also make sure that this value is preserved on upgrades.

https://fedorahosted.org/freeipa/ticket/3783
---
 install/share/certmap.conf.template |  4 +-
 install/tools/ipa-upgradeconfig     | 96 ++++++++++++++++++++++++++++++++++++-
 ipaserver/install/dsinstance.py     |  8 +++-
 3 files changed, 103 insertions(+), 5 deletions(-)

diff --git a/install/share/certmap.conf.template b/install/share/certmap.conf.template
index cff3a669b8946223b62e4fda00dbfa21d98245cd..7beb5070fbff2f7fe614eba8669d48578d52059c 100644
--- a/install/share/certmap.conf.template
+++ b/install/share/certmap.conf.template
@@ -1,4 +1,4 @@
-# VERSION 1 - DO NOT REMOVE THIS LINE
+# VERSION 2 - DO NOT REMOVE THIS LINE
 #
 # This file is managed by IPA and will be overwritten on upgrades.
 
@@ -84,6 +84,6 @@ certmap default         default
 #default:InitFn         <Init function's name>
 default:DNComps
 default:FilterComps     uid
-certmap ipaca           CN=Certificate Authority,O=$REALM
+certmap ipaca           CN=Certificate Authority,$SUBJECT_BASE
 ipaca:CmapLdapAttr      seeAlso
 ipaca:verifycert        on
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index de17c5b23d79f31e8571a3400d44397630cadada..ca1dcc789005f6b41c71e1baa809f365f016518e 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -760,6 +760,90 @@ def add_ca_dns_records():
 
     sysupgrade.set_upgrade_state('dns', 'ipa_ca_records', True)
 
+
+def find_subject_base():
+    """
+    Try to find the current value of certificate subject base.
+    1) Look in sysupgrade first
+    2) If no value is found there, look in DS (start DS if necessary)
+    3) Last resort, look in the certmap.conf itself
+    4) If all fails, log loudly and return None
+    """
+    root_logger.debug('Trying to find certificate subject base in sysupgrade')
+    subject_base = sysupgrade.get_upgrade_state('certmap.conf', 'subject_base')
+
+    if subject_base:
+        root_logger.debug(
+            'Found certificate subject base in sysupgrade: %s',
+            subject_base
+        )
+        return subject_base
+
+    root_logger.debug('Unable to find certificate subject base in sysupgrade')
+    root_logger.debug('Trying to find certificate subject base in DS')
+
+    ds_is_running = services.knownservices.dirsrv.is_running()
+    if not ds_is_running:
+        try:
+            services.knownservices.dirsrv.start()
+        except ipautil.CalledProcessError as e:
+            root_logger.error('Cannot start DS to find certificate '
+                              'subject base: %s', e)
+        else:
+            ds_is_running = True
+
+    if ds_is_running:
+        try:
+            api.Backend.ldap2.connect(autobind=True)
+        except ipalib.errors.PublicError, e:
+            root_logger.error('Cannot connect to DS to find certificate '
+                              'subject base: %s', e)
+        else:
+            ret = api.Command['config_show']()
+            api.Backend.ldap2.disconnect()
+            subject_base = str(ret['result']['ipacertificatesubjectbase'][0])
+            root_logger.debug(
+                'Found certificate subject base in DS: %s',
+                subject_base
+            )
+
+    if not subject_base:
+        root_logger.debug('Unable to find certificate subject base in DS')
+        root_logger.debug('Trying to find certificate subject base in '
+                          'certmap.conf')
+
+        certmap_dir = dsinstance.config_dirname(
+            dsinstance.realm_to_serverid(api.env.realm)
+        )
+        try:
+            with open(os.path.join(certmap_dir, 'certmap.conf')) as f:
+                for line in f:
+                    if line.startswith('certmap ipaca'):
+                        subject_base = line.strip().split(',')[-1]
+                        root_logger.debug(
+                            'Found certificate subject base in certmap.conf: '
+                            '%s',
+                            subject_base
+                        )
+
+        except IOError as e:
+            root_logger.error('Cannot open certmap.conf to find certificate '
+                              'subject base: %s', e.strerror)
+
+    if subject_base:
+        sysupgrade.set_upgrade_state(
+            'certmap.conf',
+            'subject_base',
+            subject_base
+        )
+        return subject_base
+
+    root_logger.debug('Unable to find certificate subject base in '
+                      'certmap.conf')
+    root_logger.error('Unable to determine certificate subject base. '
+                      'certmap.conf will not be updated.')
+
+
 def uninstall_selfsign(ds, http):
     root_logger.info('[Removing self-signed CA]')
     """Replace self-signed CA by a CA-less install"""
@@ -901,6 +985,10 @@ def main():
         CLONE='#'
     )
 
+    subject_base = find_subject_base()
+    if subject_base:
+        sub_dict['SUBJECT_BASE'] = subject_base
+
     ca = cainstance.CAInstance(api.env.realm, certs.NSS_DIR)
 
     # migrate CRL publish dir before the location in ipa.conf is updated
@@ -918,8 +1006,12 @@ def main():
     upgrade(sub_dict, "/etc/httpd/conf.d/ipa.conf", ipautil.SHARE_DIR + "ipa.conf")
     upgrade(sub_dict, "/etc/httpd/conf.d/ipa-rewrite.conf", ipautil.SHARE_DIR + "ipa-rewrite.conf")
     upgrade(sub_dict, "/etc/httpd/conf.d/ipa-pki-proxy.conf", ipautil.SHARE_DIR + "ipa-pki-proxy.conf", add=True)
-    upgrade(sub_dict, os.path.join(certmap_dir, "certmap.conf"),
-        os.path.join(ipautil.SHARE_DIR, "certmap.conf.template"))
+    if subject_base:
+        upgrade(
+            sub_dict,
+            os.path.join(certmap_dir, "certmap.conf"),
+            os.path.join(ipautil.SHARE_DIR, "certmap.conf.template")
+        )
     upgrade_pki(ca, fstore)
     update_dbmodules(api.env.realm)
     uninstall_ipa_kpasswd()
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index e48ced4b8653863f377debe206594e304a80d11e..8815757290efd0812bb551b4185a6afe91970211 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -37,6 +37,7 @@
 import ldap
 from ipaserver.install import ldapupdate
 from ipaserver.install import replication
+from ipaserver.install import sysupgrade
 from ipalib import errors
 from ipapython.dn import DN
 
@@ -653,7 +654,12 @@ def __certmap_conf(self):
         shutil.copyfile(ipautil.SHARE_DIR + "certmap.conf.template",
                         config_dirname(self.serverid) + "certmap.conf")
         installutils.update_file(config_dirname(self.serverid) + "certmap.conf",
-                                 '$REALM', self.realm_name)
+                                 '$SUBJECT_BASE', str(self.subject_base))
+        sysupgrade.set_upgrade_state(
+            'certmap.conf',
+            'subject_base',
+            str(self.subject_base)
+        )
 
     def __enable_ldapi(self):
         self._ldap_mod("ldapi.ldif", self.sub_dict)
-- 
1.8.1.4

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

Reply via email to