URL: https://github.com/freeipa/freeipa/pull/112
Author: martbab
 Title: #112: The first jab at fixing 
https://fedorahosted.org/freeipa/ticket/5809
Action: opened

PR body:
"""
There are two ways to fix the issue reported in the ticket:

1.) Make certificate handling code to generate nicknames that do not break
existing implementation of `installutils.set_directive`

2.) Extend the quoting abilities of the function so that it is less fragile
when encoding more funky values such as quoted RDNs

This PR opts for option 2.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/112/head:pr112
git checkout pr112
From 6db1f860dd13d90b039e71a08804bdd1f7f5a8fd Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 23 Sep 2016 15:53:41 +0200
Subject: [PATCH 1/2] Move character escaping function to ipautil

Functions `escape_seq` and `unescape_seq` have a generic use-case so it makes
sense to move them from `kerberos` to ipautil module so that other modules can
reuse them more readily.

https://fedorahosted.org/freeipa/ticket/5809
---
 ipapython/ipautil.py  | 27 +++++++++++++++++++++++++++
 ipapython/kerberos.py | 29 ++---------------------------
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 62d029d..fac76d1 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1484,3 +1484,30 @@ def is_fips_enabled():
         # Consider that the host is not fips-enabled if the file does not exist
         pass
     return False
+
+
+def unescape_seq(seq, *args):
+    """
+    unescape (remove '\\') all occurences of sequence in input strings.
+
+    :param seq: sequence to unescape
+    :param args: input string to process
+
+    :returns: tuple of strings with unescaped sequences
+    """
+    unescape_re = re.compile(r'\\{}'.format(seq))
+
+    return tuple(re.sub(unescape_re, seq, a) for a in args)
+
+
+def escape_seq(seq, *args):
+    """
+    escape (prepend '\\') all occurences of sequence in input strings
+
+    :param seq: sequence to escape
+    :param args: input string to process
+
+    :returns: tuple of strings with escaped sequences
+    """
+
+    return tuple(a.replace(seq, u'\\{}'.format(seq)) for a in args)
diff --git a/ipapython/kerberos.py b/ipapython/kerberos.py
index 298dbf1..a8ebc04 100644
--- a/ipapython/kerberos.py
+++ b/ipapython/kerberos.py
@@ -8,6 +8,8 @@
 import re
 import six
 
+from ipapython.ipautil import escape_seq, unescape_seq
+
 if six.PY3:
     unicode = str
 
@@ -58,33 +60,6 @@ def split_principal_name(principal_name):
     return tuple(COMPONENT_SPLIT_RE.split(principal_name))
 
 
-def unescape_seq(seq, *args):
-    """
-    unescape (remove '\\') all occurences of sequence in input strings.
-
-    :param seq: sequence to unescape
-    :param args: input string to process
-
-    :returns: tuple of strings with unescaped sequences
-    """
-    unescape_re = re.compile(r'\\{}'.format(seq))
-
-    return tuple(re.sub(unescape_re, seq, a) for a in args)
-
-
-def escape_seq(seq, *args):
-    """
-    escape (prepend '\\') all occurences of sequence in input strings
-
-    :param seq: sequence to escape
-    :param args: input string to process
-
-    :returns: tuple of strings with escaped sequences
-    """
-
-    return tuple(a.replace(seq, u'\\{}'.format(seq)) for a in args)
-
-
 @six.python_2_unicode_compatible
 class Principal(object):
     """

From 685e48ef2fca9e0bccacb789d8c15ce367f9b846 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 23 Sep 2016 15:56:46 +0200
Subject: [PATCH 2/2] mod_nss: use more robust quoting of NSSNickname directive

The code which handles configuration of mod_nss module must be more robust
when handling NSS nicknames generated from subject names containing quoted RDN
values.

https://fedorahosted.org/freeipa/ticket/5809
---
 ipaserver/install/httpinstance.py |  3 ++-
 ipaserver/install/installutils.py | 41 ++++++++++++++++++++++++++++-----------
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 00f8901..7914f4c 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -263,7 +263,8 @@ def __set_mod_nss_port(self):
             print("Updating port in %s failed." % paths.HTTPD_NSS_CONF)
 
     def __set_mod_nss_nickname(self, nickname):
-        installutils.set_directive(paths.HTTPD_NSS_CONF, 'NSSNickname', nickname)
+        installutils.set_directive(
+            paths.HTTPD_NSS_CONF, 'NSSNickname', nickname, quote_char="'")
 
     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/installutils.py b/ipaserver/install/installutils.py
index bf179a2..bbf4cc4 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -376,13 +376,34 @@ def update_file(filename, orig, subst):
         print("File %s doesn't exist." % filename)
         return 1
 
-def set_directive(filename, directive, value, quotes=True, separator=' '):
+def set_directive(filename, directive, value, quotes=True, separator=' ',
+                  quote_char='\"'):
     """Set a name/value pair directive in a configuration file.
 
-       A value of None means to drop the directive.
+    A value of None means to drop the directive.
 
-       This has only been tested with nss.conf
+    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`
     """
+
+    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))
+            )
+
+        return "{directive_sep}{value}\n".format(
+            directive_sep=directive_sep, value=transformed_value)
+
     valueset = False
     st = os.stat(filename)
     fd = open(filename)
@@ -391,19 +412,17 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
         if line.lstrip().startswith(directive):
             valueset = True
             if value is not None:
-                if quotes:
-                    newfile.append('%s%s"%s"\n' % (directive, separator, value))
-                else:
-                    newfile.append('%s%s%s\n' % (directive, separator, value))
+                newfile.append(
+                    format_directive(
+                        directive, value, separator, quotes, quote_char))
         else:
             newfile.append(line)
     fd.close()
     if not valueset:
         if value is not None:
-            if quotes:
-                newfile.append('%s%s"%s"\n' % (directive, separator, value))
-            else:
-                newfile.append('%s%s%s\n' % (directive, separator, value))
+            newfile.append(
+                format_directive(
+                    directive, value, separator, quotes, quote_char))
 
     fd = open(filename, "w")
     fd.write("".join(newfile))
-- 
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