URL: https://github.com/freeipa/freeipa/pull/1347
Author: frasertweedale
 Title: #1347: Prevent set_directive from clobbering other keys
Action: opened

PR body:
"""
`set_directive` only looks for a prefix of the line matching the
given directive (key).  If a directive is encountered for which the
given key is prefix, it will be vanquished.

This occurs in the case of `{ca,kra}.sslserver.cert[req]`; the
`cert` directive gets updated after certificate renewal, and the
`certreq` directive gets clobbered.  This can cause failures later
on during KRA installation, and possibly cloning.

Match the whole directive to avoid this issue.

Fixes: https://pagure.io/freeipa/issue/7288

-----

Cause: corner case.

How to test:

1. ensure `ca.sslserver.certreq=<base64 CSR>` exists in `ca/CS.cfg`.
2. resubmit Certmonger tracking request for `Server-Cert cert-pki-ca` Dogtag 
system cert.
3. verify that `ca.sslserver.certreq=<base64 CSR>` still exists in `ca/CS.cfg`.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1347/head:pr1347
git checkout pr1347
From 6521d8704d9f43b95de9fae1ba6105c7e3270afb Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Sat, 21 Nov 2020 08:47:57 +1100
Subject: [PATCH] Prevent set_directive from clobbering other keys

`set_directive` only looks for a prefix of the line matching the
given directive (key).  If a directive is encountered for which the
given key is prefix, it will be vanquished.

This occurs in the case of `{ca,kra}.sslserver.cert[req]`; the
`cert` directive gets updated after certificate renewal, and the
`certreq` directive gets clobbered.  This can cause failures later
on during KRA installation, and possibly cloning.

Match the whole directive to avoid this issue.

Fixes: https://pagure.io/freeipa/issue/7288
---
 ipaserver/install/cainstance.py     | 2 +-
 ipaserver/install/dogtaginstance.py | 2 +-
 ipaserver/install/installutils.py   | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 8e0e69888a..859a7f901d 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -952,7 +952,7 @@ def __enable_crl_publish(self):
         installutils.set_directive(caconfig, 'ca.publish.rule.instance.FileCrlRule.enable', 'true', quotes=False, separator='=')
         installutils.set_directive(caconfig, 'ca.publish.rule.instance.FileCrlRule.mapper', 'NoMap', quotes=False, separator='=')
         installutils.set_directive(caconfig, 'ca.publish.rule.instance.FileCrlRule.pluginName', 'Rule', quotes=False, separator='=')
-        installutils.set_directive(caconfig, 'ca.publish.rule.instance.FileCrlRule.predicate=', '', quotes=False, separator='')
+        installutils.set_directive(caconfig, 'ca.publish.rule.instance.FileCrlRule.predicate', '', quotes=False, separator='=')
         installutils.set_directive(caconfig, 'ca.publish.rule.instance.FileCrlRule.publisher', 'FileBaseCRLPublisher', quotes=False, separator='=')
         installutils.set_directive(caconfig, 'ca.publish.rule.instance.FileCrlRule.type', 'crl', quotes=False, separator='=')
 
diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py
index bcc9265de9..67edaf511c 100644
--- a/ipaserver/install/dogtaginstance.py
+++ b/ipaserver/install/dogtaginstance.py
@@ -214,7 +214,7 @@ def enable_client_auth_to_db(self, config):
                 separator='=')
             # Remove internaldb password as is not needed anymore
             installutils.set_directive(paths.PKI_TOMCAT_PASSWORD_CONF,
-                                       'internaldb', None)
+                                       'internaldb', None, separator='=')
 
     def uninstall(self):
         if self.is_installed():
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 92e0d8fa8c..645493186b 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -441,7 +441,7 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
 
     A value of None means to drop the directive.
 
-    This has only been tested with nss.conf
+    Does not tolerate (or put) spaces around the separator.
 
     :param filename: input filename
     :param directive: directive name
@@ -450,7 +450,7 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
         any existing double quotes are first escaped to avoid
         unparseable directives.
     :param separator: character serving as separator between directive and
-        value
+        value.  Correct value required even when dropping a directive.
     """
 
     new_directive_value = ""
@@ -465,7 +465,7 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
     fd = open(filename)
     newfile = []
     for line in fd:
-        if line.lstrip().startswith(directive):
+        if re.match(r'\s*{}'.format(re.escape(directive + separator)), line):
             valueset = True
             if value is not None:
                 newfile.append(new_directive_value)
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to