URL: https://github.com/freeipa/freeipa/pull/347
Author: martbab
 Title: #347: Improvements in {get|set}_directive functions
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/347/head:pr347
git checkout pr347
From d72842f18f381130aac8f422a0f161f4e1027645 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 16 Dec 2016 12:14:20 +0100
Subject: [PATCH 1/4] Fix the installutils.set_directive docstring

Add missing parameter descriptions and fix incorrect indentation

https://fedorahosted.org/freeipa/ticket/6354
---
 ipaserver/install/installutils.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 0d8a574..7f96eb2 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -388,11 +388,14 @@ def set_directive(filename, directive, value, quotes=True, separator=' ',
 
     This has only been tested with nss.conf
 
-   :param directive: directive name
-   :param value: value of the directive
-   :param quotes: whether to quote `value` in `quote_char`. If true, then
-        the `quote_char` are first escaped to avoid unparseable directives
-   :param quote_char: the character used for quoting `value`
+    :param filename: input filename
+    :param directive: directive name
+    :param value: value of the directive
+    :param quotes: whether to quote `value` in `quote_char`. If true, then
+        the `quote_char` are first escaped to avoid unparseable directives.
+    :param separator: character serving as separator between directive and
+        value
+    :param quote_char: the character used for quoting `value`
     """
 
     def format_directive(directive, value, separator, quotes, quote_char):

From 00f30522890c482844b1d878d3fa01da1506063a Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 16 Dec 2016 13:34:57 +0100
Subject: [PATCH 2/4] installutils: improve directive value parsing in
 `get_directive`

`get_directive` value parsing was improved in order to bring its logic
more in-line to changes in `set_directive`: a specified quoting
character is now unquoted and stripped from the retrieved value. The
function will now also error out when malformed directive is
encountered.

https://fedorahosted.org/freeipa/ticket/6460
---
 ipaserver/install/installutils.py | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 7f96eb2..4f93372 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -436,16 +436,31 @@ def format_directive(directive, value, separator, quotes, quote_char):
     fd.close()
     os.chown(filename, st.st_uid, st.st_gid) # reset perms
 
+
 def get_directive(filename, directive, separator=' '):
     """
     A rather inefficient way to get a configuration directive.
+
+    :param filename: input filename
+    :param directive: directive name
+    :param separator: separator between directive and value
+    :param quote_char: the characters that are used in this particular config
+        file to quote values. This character will be stripped and unescaped
+        from the raw value.
+
+    :returns: The (unquoted) value if the directive was found, None otherwise
     """
     fd = open(filename, "r")
     for line in fd:
         if line.lstrip().startswith(directive):
             line = line.strip()
-            result = line.split(separator, 1)[1]
-            result = result.strip('"')
+
+            (directive, sep, value) = line.partition(separator)
+            if not sep or not value:
+                raise ValueError("Malformed directive: {}".format(line))
+
+            result = value.strip().strip(quote_char)
+            result = ipautil.unescape_seq(quote_char, result)[0]
             result = result.strip(' ')
             fd.close()
             return result

From 6557e93d553d2d82e81f9548d0feed1d3331f57c Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Tue, 10 Jan 2017 17:15:33 +0100
Subject: [PATCH 3/4] Delegate directive value quoting/unquoting to separate
 functions

Separate functions were added to installutils module to quote/unquote a
string in arbitrary characters.

`installutils.get/set_directive` functions will use them to enclose
the directive values in double quotes/strip the double quotes from
retrieved values to maintain the original behavior.

These functions can be used also for custom quoting/unquoting of
retrieved values when desired.

https://fedorahosted.org/freeipa/ticket/6460
---
 ipaserver/install/installutils.py | 58 ++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 4f93372..3e9ae54 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -380,8 +380,34 @@ def update_file(filename, orig, subst):
         return 1
 
 
-def set_directive(filename, directive, value, quotes=True, separator=' ',
-                  quote_char='\"'):
+def quote_directive_value(value, quote_char):
+    """Quote a directive value
+    :param value: string to quote
+    :param quote_char: character which is used for quoting. All prior
+        occurences will be escaped before quoting to avoid unparseable value.
+    :returns: processed value
+    """
+    if value.startswith(quote_char) and value.endswith(quote_char):
+        return value
+
+    return "{quote}{value}{quote}".format(
+        quote=quote_char,
+        value="".join(ipautil.escape_seq(quote_char, value))
+    )
+
+
+def unquote_directive_value(value, quote_char):
+    """Unquote a directive value
+    :param value: string to unquote
+    :param quote_char: character to strip. All escaped occurences of
+        `quote_char` will be uncescaped during processing
+    :returns: processed value
+    """
+    unescaped_value = "".join(ipautil.unescape_seq(quote_char, value))
+    return unescaped_value.strip(quote_char)
+
+
+def set_directive(filename, directive, value, quotes=True, separator=' '):
     """Set a name/value pair directive in a configuration file.
 
     A value of None means to drop the directive.
@@ -395,21 +421,14 @@ def set_directive(filename, directive, value, quotes=True, separator=' ',
         the `quote_char` are first escaped to avoid unparseable directives.
     :param separator: character serving as separator between directive and
         value
-    :param quote_char: the character used for quoting `value`
     """
 
-    def format_directive(directive, value, separator, quotes, quote_char):
-        directive_sep = "{directive}{separator}".format(directive=directive,
-                                                        separator=separator)
-        transformed_value = value
-        if quotes:
-            transformed_value = "{quote}{value}{quote}".format(
-                quote=quote_char,
-                value="".join(ipautil.escape_seq(quote_char, value))
-            )
+    new_directive_value = ""
+    if value is not None:
+        value_to_set = quote_directive_value(value, '"') if quotes else value
 
-        return "{directive_sep}{value}\n".format(
-            directive_sep=directive_sep, value=transformed_value)
+        new_directive_value = "".join(
+            [directive, separator, value_to_set, '\n'])
 
     valueset = False
     st = os.stat(filename)
@@ -419,17 +438,13 @@ def format_directive(directive, value, separator, quotes, quote_char):
         if line.lstrip().startswith(directive):
             valueset = True
             if value is not None:
-                newfile.append(
-                    format_directive(
-                        directive, value, separator, quotes, quote_char))
+                newfile.append(new_directive_value)
         else:
             newfile.append(line)
     fd.close()
     if not valueset:
         if value is not None:
-            newfile.append(
-                format_directive(
-                    directive, value, separator, quotes, quote_char))
+            newfile.append(new_directive_value)
 
     fd = open(filename, "w")
     fd.write("".join(newfile))
@@ -459,8 +474,7 @@ def get_directive(filename, directive, separator=' '):
             if not sep or not value:
                 raise ValueError("Malformed directive: {}".format(line))
 
-            result = value.strip().strip(quote_char)
-            result = ipautil.unescape_seq(quote_char, result)[0]
+            result = unquote_directive_value(value.strip(), '"')
             result = result.strip(' ')
             fd.close()
             return result

From ed94035f59302a5ef2564a019c444932a3925346 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 16 Dec 2016 13:42:05 +0100
Subject: [PATCH 4/4] Explicitly handle quoting/unquoting of NSSNickname
 directive

Improve the single/double quote handling during parsing/unparsing of
nss.conf's NSSNickname directive. Single quotes are now added/stripped
explicitly when handling the certificate nickname.

https://fedorahosted.org/freeipa/ticket/6460
---
 ipaserver/install/httpinstance.py           |  4 +++-
 ipaserver/install/ipa_server_certinstall.py | 14 +++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index bacd5fc..7f5473d 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -252,8 +252,10 @@ def __set_mod_nss_port(self):
             print("Updating port in %s failed." % paths.HTTPD_NSS_CONF)
 
     def __set_mod_nss_nickname(self, nickname):
+        quoted_nickname = installutils.quote_directive_value(
+            nickname, quote_char="'")
         installutils.set_directive(
-            paths.HTTPD_NSS_CONF, 'NSSNickname', nickname, quote_char="'")
+            paths.HTTPD_NSS_CONF, 'NSSNickname', quoted_nickname, quotes=False)
 
     def set_mod_nss_protocol(self):
         installutils.set_directive(paths.HTTPD_NSS_CONF, 'NSSProtocol', 'TLSv1.0,TLSv1.1,TLSv1.2', False)
diff --git a/ipaserver/install/ipa_server_certinstall.py b/ipaserver/install/ipa_server_certinstall.py
index 8ef25ee..d07c7de 100644
--- a/ipaserver/install/ipa_server_certinstall.py
+++ b/ipaserver/install/ipa_server_certinstall.py
@@ -136,12 +136,20 @@ def install_http_cert(self):
         old_cert = installutils.get_directive(paths.HTTPD_NSS_CONF,
                                               'NSSNickname')
 
+        unquoted_cert = installutils.unquote_directive_value(
+            old_cert, quote_char="'")
+
         server_cert = self.import_cert(dirname, self.options.pin,
-                                       old_cert, 'HTTP/%s' % api.env.host,
+                                       unquoted_cert, 'HTTP/%s' % api.env.host,
                                        'restart_httpd')
 
-        installutils.set_directive(paths.HTTPD_NSS_CONF,
-                                   'NSSNickname', server_cert)
+        quoted_server_cert = installutils.quote_directive_value(
+            server_cert, quote_char="'")
+        installutils.set_directive(
+            paths.HTTPD_NSS_CONF,
+            'NSSNickname',
+            quoted_server_cert,
+            quotes=False)
 
         # Fix the database permissions
         os.chmod(os.path.join(dirname, 'cert8.db'), 0o640)
-- 
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

Reply via email to