URL: https://github.com/freeipa/freeipa/pull/1379
Author: tiran
Title: #1379: Prevent set_directive from clobbering other keys / safe
directive setter
Action: opened
PR body:
"""
This PR combines @frasertweedale PR #1347 with a safe directive setter
(https://pagure.io/freeipa/issue/7312)
## Original PR message
`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`.
## safe DirectiveSetter
The new context manager ``DirectiveSetter`` avoids several possible issues that
can lead to destroyed configuration files.
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1379/head:pr1379
git checkout pr1379
From 80fdfc8554196aeba406980fc129f711f6b92b35 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Sat, 21 Nov 2020 08:47:57 +1100
Subject: [PATCH 1/5] 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)
From 37209443dd39a399ac6538ea5d71dd304ea7454f Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Thu, 30 Nov 2017 12:00:53 +1100
Subject: [PATCH 2/5] pep8: reduce line lengths in
CAInstance.__enable_crl_publish
Part of: https://pagure.io/freeipa/issue/7288
---
ipaserver/install/cainstance.py | 71 ++++++++++++++++++++++++-----------------
1 file changed, 41 insertions(+), 30 deletions(-)
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 859a7f901d..49dfcd1373 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -928,52 +928,63 @@ def __enable_crl_publish(self):
https://access.redhat.com/knowledge/docs/en-US/Red_Hat_Certificate_System/8.0/html/Admin_Guide/Setting_up_Publishing.html
"""
- caconfig = paths.CA_CS_CFG_PATH
- publishdir = self.prepare_crl_publish_dir()
+ def put(k, v):
+ installutils.set_directive(
+ paths.CA_CS_CFG_PATH, k, v, quotes=False, separator='=')
# Enable file publishing, disable LDAP
- installutils.set_directive(caconfig, 'ca.publish.enable', 'true', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.ldappublish.enable', 'false', quotes=False, separator='=')
+ put('ca.publish.enable', 'true')
+ put('ca.publish.ldappublish.enable', 'false')
# Create the file publisher, der only, not b64
- installutils.set_directive(caconfig, 'ca.publish.publisher.impl.FileBasedPublisher.class','com.netscape.cms.publish.publishers.FileBasedPublisher', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.crlLinkExt', 'bin', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.directory', publishdir, quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.latestCrlLink', 'true', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.pluginName', 'FileBasedPublisher', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.timeStamp', 'LocalTime', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.zipCRLs', 'false', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.zipLevel', '9', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.Filename.b64', 'false', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.publisher.instance.FileBaseCRLPublisher.Filename.der', 'true', quotes=False, separator='=')
+ put('ca.publish.publisher.impl.FileBasedPublisher.class',
+ 'com.netscape.cms.publish.publishers.FileBasedPublisher')
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.crlLinkExt',
+ 'bin')
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.directory',
+ self.prepare_crl_publish_dir())
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.latestCrlLink',
+ 'true')
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.pluginName',
+ 'FileBasedPublisher')
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.timeStamp',
+ 'LocalTime')
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.zipCRLs',
+ 'false')
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.zipLevel', '9')
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.Filename.b64',
+ 'false')
+ put('ca.publish.publisher.instance.FileBaseCRLPublisher.Filename.der',
+ 'true')
# The publishing rule
- 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.publisher', 'FileBaseCRLPublisher', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.rule.instance.FileCrlRule.type', 'crl', quotes=False, separator='=')
+ put('ca.publish.rule.instance.FileCrlRule.enable', 'true')
+ put('ca.publish.rule.instance.FileCrlRule.mapper', 'NoMap')
+ put('ca.publish.rule.instance.FileCrlRule.pluginName', 'Rule')
+ put('ca.publish.rule.instance.FileCrlRule.predicate', '')
+ put('ca.publish.rule.instance.FileCrlRule.publisher',
+ 'FileBaseCRLPublisher')
+ put('ca.publish.rule.instance.FileCrlRule.type', 'crl')
# Now disable LDAP publishing
- installutils.set_directive(caconfig, 'ca.publish.rule.instance.LdapCaCertRule.enable', 'false', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.rule.instance.LdapCrlRule.enable', 'false', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.rule.instance.LdapUserCertRule.enable', 'false', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.publish.rule.instance.LdapXCertRule.enable', 'false', quotes=False, separator='=')
+ put('ca.publish.rule.instance.LdapCaCertRule.enable', 'false')
+ put('ca.publish.rule.instance.LdapCrlRule.enable', 'false')
+ put('ca.publish.rule.instance.LdapUserCertRule.enable', 'false')
+ put('ca.publish.rule.instance.LdapXCertRule.enable', 'false')
# If we are the initial master then we are the CRL generator, otherwise
# we point to that master for CRLs.
if not self.clone:
# These next two are defaults, but I want to be explicit that the
# initial master is the CRL generator.
- installutils.set_directive(caconfig, 'ca.crl.MasterCRL.enableCRLCache', 'true', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.crl.MasterCRL.enableCRLUpdates', 'true', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.listenToCloneModifications', 'true', quotes=False, separator='=')
+ put('ca.crl.MasterCRL.enableCRLCache', 'true')
+ put('ca.crl.MasterCRL.enableCRLUpdates', 'true')
+ put('ca.listenToCloneModifications', 'true')
else:
- installutils.set_directive(caconfig, 'ca.crl.MasterCRL.enableCRLCache', 'false', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.crl.MasterCRL.enableCRLUpdates', 'false', quotes=False, separator='=')
- installutils.set_directive(caconfig, 'ca.listenToCloneModifications', 'false', quotes=False, separator='=')
+ put('ca.crl.MasterCRL.enableCRLCache', 'false')
+ put('ca.crl.MasterCRL.enableCRLUpdates', 'false')
+ put('ca.listenToCloneModifications', 'false')
def uninstall(self):
# just eat state
From 8dc451afed33c68b1fa43ba638f85d592281c8b8 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Tue, 5 Dec 2017 13:43:04 +1100
Subject: [PATCH 3/5] installutils: refactor set_directive
To separate concerns and make it easier to test set_directive,
extract function ``set_directive_lines`` to do the line-wise
search/replace, leaving ``set_directive`` to deal with the file
handling.
Part of: https://pagure.io/freeipa/issue/7288
---
ipaserver/install/installutils.py | 56 +++++++++++++++++++++++----------------
1 file changed, 33 insertions(+), 23 deletions(-)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 645493186b..60d6c37b0d 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -452,34 +452,44 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
:param separator: character serving as separator between directive and
value. Correct value required even when dropping a directive.
"""
+ st = os.stat(filename)
+ with open(filename, 'r') as f:
+ lines = list(f) # read the whole file
+ new_lines = set_directive_lines(
+ quotes, separator, directive, value, lines)
+ with open(filename, 'w') as f:
+ # don't construct the whole string; write line-wise
+ for line in new_lines:
+ f.write(line)
+ os.chown(filename, st.st_uid, st.st_gid) # reset perms
- new_directive_value = ""
- if value is not None:
- value_to_set = quote_directive_value(value, '"') if quotes else value
- new_directive_value = "".join(
- [directive, separator, value_to_set, '\n'])
+def set_directive_lines(quotes, separator, k, v, lines):
+ """Set a name/value pair in a configuration (iterable of lines).
- valueset = False
- st = os.stat(filename)
- fd = open(filename)
- newfile = []
- for line in fd:
- if re.match(r'\s*{}'.format(re.escape(directive + separator)), line):
- valueset = True
- if value is not None:
- newfile.append(new_directive_value)
+ Replaces the value of the key if found, otherwise adds it at
+ end. If value is ``None``, remove the key if found.
+
+ Takes an iterable of lines (with trailing newline).
+ Yields lines (with trailing newline).
+
+ """
+ new_line = ""
+ if v is not None:
+ v_quoted = quote_directive_value(v, '"') if quotes else v
+ new_line = ''.join([k, separator, v_quoted, '\n'])
+
+ found = False
+ for line in lines:
+ if re.match(r'\s*{}'.format(re.escape(k + separator)), line):
+ found = True
+ if v is not None:
+ yield new_line
else:
- newfile.append(line)
- fd.close()
- if not valueset:
- if value is not None:
- newfile.append(new_directive_value)
+ yield line
- fd = open(filename, "w")
- fd.write("".join(newfile))
- fd.close()
- os.chown(filename, st.st_uid, st.st_gid) # reset perms
+ if not found and v is not None:
+ yield new_line
def get_directive(filename, directive, separator=' '):
From df7b4b2da744aebc4947359b81e1d6074582d55c Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Tue, 5 Dec 2017 15:00:18 +1100
Subject: [PATCH 4/5] Add tests for installutils.set_directive
Part of: https://pagure.io/freeipa/issue/7288
---
.../test_install/test_installutils.py | 57 ++++++++++++++++++++++
1 file changed, 57 insertions(+)
create mode 100644 ipatests/test_ipaserver/test_install/test_installutils.py
diff --git a/ipatests/test_ipaserver/test_install/test_installutils.py b/ipatests/test_ipaserver/test_install/test_installutils.py
new file mode 100644
index 0000000000..cc8fd3cf3f
--- /dev/null
+++ b/ipatests/test_ipaserver/test_install/test_installutils.py
@@ -0,0 +1,57 @@
+#
+# Copyright (C) 2017 FreeIPA Contributors. See COPYING for license
+#
+
+import os
+import tempfile
+
+from ipaserver.install import installutils
+
+EXAMPLE_CONFIG = [
+ 'foo=1\n',
+ 'foobar=2\n',
+]
+
+
+class test_set_directive_lines(object):
+ def test_remove_directive(self):
+ lines = installutils.set_directive_lines(
+ False, '=', 'foo', None, EXAMPLE_CONFIG)
+ assert list(lines) == ['foobar=2\n']
+
+ def test_add_directive(self):
+ lines = installutils.set_directive_lines(
+ False, '=', 'baz', '4', EXAMPLE_CONFIG)
+ assert list(lines) == ['foo=1\n', 'foobar=2\n', 'baz=4\n']
+
+ def test_set_directive_does_not_clobber_suffix_key(self):
+ lines = installutils.set_directive_lines(
+ False, '=', 'foo', '3', EXAMPLE_CONFIG)
+ assert list(lines) == ['foo=3\n', 'foobar=2\n']
+
+
+class test_set_directive(object):
+ def test_set_directive(self):
+ """Check that set_directive writes the new data and preserves mode."""
+ fd, filename = tempfile.mkstemp()
+ try:
+ os.close(fd)
+ stat_pre = os.stat(filename)
+
+ with open(filename, 'w') as f:
+ for line in EXAMPLE_CONFIG:
+ f.write(line)
+
+ installutils.set_directive(filename, 'foo', '3', False, '=')
+
+ stat_post = os.stat(filename)
+ with open(filename, 'r') as f:
+ lines = list(f)
+
+ assert lines == ['foo=3\n', 'foobar=2\n']
+ assert stat_pre.st_mode == stat_post.st_mode
+ assert stat_pre.st_uid == stat_post.st_uid
+ assert stat_pre.st_gid == stat_post.st_gid
+
+ finally:
+ os.remove(filename)
From 8f257bb84d3a916f79d236a52e8fad4ea77a5b9f Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 8 Dec 2017 12:38:41 +0100
Subject: [PATCH 5/5] Add safe DirectiveSetter context manager
installutils.set_directive() is both inefficient and potentially
dangerous. It does not ensure that the whole file is written and
properly synced to disk. In worst case it could lead to partially
written or destroyed config files.
The new DirectiveSetter context manager wraps everything under an easy
to use interface.
https://pagure.io/freeipa/issue/7312
Signed-off-by: Christian Heimes <chei...@redhat.com>
---
ipaserver/install/cainstance.py | 109 ++++++++++-----------
ipaserver/install/installutils.py | 65 +++++++++++-
.../test_install/test_installutils.py | 51 ++++++++++
3 files changed, 167 insertions(+), 58 deletions(-)
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 49dfcd1373..6af5ee90a9 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -929,62 +929,61 @@ def __enable_crl_publish(self):
https://access.redhat.com/knowledge/docs/en-US/Red_Hat_Certificate_System/8.0/html/Admin_Guide/Setting_up_Publishing.html
"""
- def put(k, v):
- installutils.set_directive(
- paths.CA_CS_CFG_PATH, k, v, quotes=False, separator='=')
+ with installutils.DirectiveSetter(paths.CA_CS_CFG_PATH,
+ quotes=False, separator='=') as ds:
- # Enable file publishing, disable LDAP
- put('ca.publish.enable', 'true')
- put('ca.publish.ldappublish.enable', 'false')
-
- # Create the file publisher, der only, not b64
- put('ca.publish.publisher.impl.FileBasedPublisher.class',
- 'com.netscape.cms.publish.publishers.FileBasedPublisher')
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.crlLinkExt',
- 'bin')
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.directory',
- self.prepare_crl_publish_dir())
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.latestCrlLink',
- 'true')
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.pluginName',
- 'FileBasedPublisher')
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.timeStamp',
- 'LocalTime')
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.zipCRLs',
- 'false')
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.zipLevel', '9')
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.Filename.b64',
- 'false')
- put('ca.publish.publisher.instance.FileBaseCRLPublisher.Filename.der',
- 'true')
-
- # The publishing rule
- put('ca.publish.rule.instance.FileCrlRule.enable', 'true')
- put('ca.publish.rule.instance.FileCrlRule.mapper', 'NoMap')
- put('ca.publish.rule.instance.FileCrlRule.pluginName', 'Rule')
- put('ca.publish.rule.instance.FileCrlRule.predicate', '')
- put('ca.publish.rule.instance.FileCrlRule.publisher',
- 'FileBaseCRLPublisher')
- put('ca.publish.rule.instance.FileCrlRule.type', 'crl')
-
- # Now disable LDAP publishing
- put('ca.publish.rule.instance.LdapCaCertRule.enable', 'false')
- put('ca.publish.rule.instance.LdapCrlRule.enable', 'false')
- put('ca.publish.rule.instance.LdapUserCertRule.enable', 'false')
- put('ca.publish.rule.instance.LdapXCertRule.enable', 'false')
-
- # If we are the initial master then we are the CRL generator, otherwise
- # we point to that master for CRLs.
- if not self.clone:
- # These next two are defaults, but I want to be explicit that the
- # initial master is the CRL generator.
- put('ca.crl.MasterCRL.enableCRLCache', 'true')
- put('ca.crl.MasterCRL.enableCRLUpdates', 'true')
- put('ca.listenToCloneModifications', 'true')
- else:
- put('ca.crl.MasterCRL.enableCRLCache', 'false')
- put('ca.crl.MasterCRL.enableCRLUpdates', 'false')
- put('ca.listenToCloneModifications', 'false')
+ # Enable file publishing, disable LDAP
+ ds.set('ca.publish.enable', 'true')
+ ds.set('ca.publish.ldappublish.enable', 'false')
+
+ # Create the file publisher, der only, not b64
+ ds.set(
+ 'ca.publish.publisher.impl.FileBasedPublisher.class',
+ 'com.netscape.cms.publish.publishers.FileBasedPublisher'
+ )
+ prefix = 'ca.publish.publisher.instance.FileBaseCRLPublisher.'
+ ds.set(prefix + 'crlLinkExt', 'bin')
+ ds.set(prefix + 'directory', self.prepare_crl_publish_dir())
+ ds.set(prefix + 'latestCrlLink', 'true')
+ ds.set(prefix + 'pluginName', 'FileBasedPublisher')
+ ds.set(prefix + 'timeStamp', 'LocalTime')
+ ds.set(prefix + 'zipCRLs', 'false')
+ ds.set(prefix + 'zipLevel', '9')
+ ds.set(prefix + 'Filename.b64', 'false')
+ ds.set(prefix + 'Filename.der', 'true')
+
+ # The publishing rule
+ ds.set('ca.publish.rule.instance.FileCrlRule.enable', 'true')
+ ds.set('ca.publish.rule.instance.FileCrlRule.mapper', 'NoMap')
+ ds.set('ca.publish.rule.instance.FileCrlRule.pluginName', 'Rule')
+ ds.set('ca.publish.rule.instance.FileCrlRule.predicate', '')
+ ds.set(
+ 'ca.publish.rule.instance.FileCrlRule.publisher',
+ 'FileBaseCRLPublisher'
+ )
+ ds.set('ca.publish.rule.instance.FileCrlRule.type', 'crl')
+
+ # Now disable LDAP publishing
+ ds.set('ca.publish.rule.instance.LdapCaCertRule.enable', 'false')
+ ds.set('ca.publish.rule.instance.LdapCrlRule.enable', 'false')
+ ds.set(
+ 'ca.publish.rule.instance.LdapUserCertRule.enable',
+ 'false'
+ )
+ ds.set('ca.publish.rule.instance.LdapXCertRule.enable', 'false')
+
+ # If we are the initial master then we are the CRL generator,
+ # otherwise we point to that master for CRLs.
+ if not self.clone:
+ # These next two are defaults, but I want to be explicit
+ # that the initial master is the CRL generator.
+ ds.set('ca.crl.MasterCRL.enableCRLCache', 'true')
+ ds.set('ca.crl.MasterCRL.enableCRLUpdates', 'true')
+ ds.set('ca.listenToCloneModifications', 'true')
+ else:
+ ds.set('ca.crl.MasterCRL.enableCRLCache', 'false')
+ ds.set('ca.crl.MasterCRL.enableCRLUpdates', 'false')
+ ds.set('ca.listenToCloneModifications', 'false')
def uninstall(self):
# just eat state
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 60d6c37b0d..4184a86bd6 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -25,6 +25,7 @@
import socket
import getpass
import gssapi
+import io
import ldif
import os
import re
@@ -32,6 +33,7 @@
import sys
import tempfile
import shutil
+import stat # pylint: disable=bad-python3-import
import traceback
import textwrap
from contextlib import contextmanager
@@ -435,6 +437,60 @@ def unquote_directive_value(value, quote_char):
return unescaped_value
+SENTINEL = object()
+
+class DirectiveSetter(object):
+ def __init__(self, filename, quotes=True, separator=' '):
+ self.filename = os.path.abspath(filename)
+ self.quotes = quotes
+ self.separator = separator
+ self.lines = None
+ self.stat = None
+
+ def __enter__(self):
+ with open(self.filename) as f:
+ self.stat = os.fstat(f.fileno())
+ self.lines = list(f)
+ return self
+
+ def __exit__(self, exc_type, exc_val, exc_tb):
+ if exc_type is not None:
+ return
+ directory, prefix = os.path.split(self.filename)
+ # use tempfile in same directory to have atomic rename
+ fd, name = tempfile.mkstemp(prefix=prefix, dir=directory, text=True)
+ with io.open(fd, mode='w', closefd=True) as f:
+ for line in self.lines:
+ if not isinstance(line, six.text_type):
+ line = line.decode('utf-8')
+ f.write(line)
+ self.lines = None
+ os.fchmod(f.fileno(), stat.S_IMODE(self.stat.st_mode))
+ os.fchown(f.fileno(), self.stat.st_uid, self.stat.st_gid)
+ self.stat = None
+ # flush and sync tempfile inode
+ f.flush()
+ os.fsync(f.fileno())
+
+ # rename file and sync directory inode
+ os.rename(name, self.filename)
+ dirfd = os.open(directory, os.O_RDONLY | os.O_DIRECTORY)
+ try:
+ os.fsync(dirfd)
+ finally:
+ os.close(dirfd)
+
+ def set(self, directive, value, quotes=SENTINEL, separator=SENTINEL):
+ if quotes is SENTINEL:
+ quotes = self.quotes
+ if separator is SENTINEL:
+ separator = self.separator
+ # materialize lines
+ # set_directive_lines() modify item, shrink or enlage line count
+ self.lines = list(set_directive_lines(
+ quotes, separator, directive, value, self.lines
+ ))
+
def set_directive(filename, directive, value, quotes=True, separator=' '):
"""Set a name/value pair directive in a configuration file.
@@ -455,8 +511,10 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
st = os.stat(filename)
with open(filename, 'r') as f:
lines = list(f) # read the whole file
- new_lines = set_directive_lines(
- quotes, separator, directive, value, lines)
+ # materialize new list
+ new_lines = list(set_directive_lines(
+ quotes, separator, directive, value, lines
+ ))
with open(filename, 'w') as f:
# don't construct the whole string; write line-wise
for line in new_lines:
@@ -480,8 +538,9 @@ def set_directive_lines(quotes, separator, k, v, lines):
new_line = ''.join([k, separator, v_quoted, '\n'])
found = False
+ matcher = re.compile(r'\s*{}'.format(re.escape(k + separator)))
for line in lines:
- if re.match(r'\s*{}'.format(re.escape(k + separator)), line):
+ if matcher.match(line):
found = True
if v is not None:
yield new_line
diff --git a/ipatests/test_ipaserver/test_install/test_installutils.py b/ipatests/test_ipaserver/test_install/test_installutils.py
index cc8fd3cf3f..8dbc7c4c2f 100644
--- a/ipatests/test_ipaserver/test_install/test_installutils.py
+++ b/ipatests/test_ipaserver/test_install/test_installutils.py
@@ -3,8 +3,11 @@
#
import os
+import shutil
import tempfile
+import pytest
+
from ipaserver.install import installutils
EXAMPLE_CONFIG = [
@@ -13,6 +16,18 @@
]
+@pytest.fixture
+def tempdir(request):
+ tempdir = tempfile.mkdtemp()
+
+ def fin():
+ shutil.rmtree(tempdir)
+
+ request.addfinalizer(fin)
+ return tempdir
+
+
+
class test_set_directive_lines(object):
def test_remove_directive(self):
lines = installutils.set_directive_lines(
@@ -55,3 +70,39 @@ def test_set_directive(self):
finally:
os.remove(filename)
+
+
+def test_directivesetter(tempdir):
+ filename = os.path.join(tempdir, 'example.conf')
+ with open(filename, 'w') as f:
+ for line in EXAMPLE_CONFIG:
+ f.write(line)
+
+ ds = installutils.DirectiveSetter(filename)
+ assert ds.lines is None
+ with ds:
+ assert ds.lines == EXAMPLE_CONFIG
+ ds.set('foo', '3') # quoted, space separated, doesn't change 'foo='
+ ds.set('foobar', None, separator='=') # remove
+ ds.set('baz', '4', False, '=') # add
+
+ with open(filename, 'r') as f:
+ lines = list(f)
+
+ assert lines == [
+ 'foo=1\n',
+ 'foo "3"\n',
+ 'baz=4\n'
+ ]
+
+ with installutils.DirectiveSetter(filename, True, '=') as ds:
+ ds.set('foo', '4') # doesn't change 'foo '
+
+ with open(filename, 'r') as f:
+ lines = list(f)
+
+ assert lines == [
+ 'foo="4"\n',
+ 'foo "3"\n',
+ 'baz=4\n'
+ ]
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
