URL: https://github.com/freeipa/freeipa/pull/834 Author: tomaskrizek Title: #834: [4.4] NSSNickname enclosed in single quotes causes ipa-server-certinstall failure Action: opened
PR body: """ Original PR: https://github.com/freeipa/freeipa/pull/347 https://pagure.io/freeipa/issue/6991 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/834/head:pr834 git checkout pr834
From 9a19c4c157631af6e2414b27776316ba45aef27d 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/6991 --- 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 07e438d6ca..51528a1d0a 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -384,11 +384,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 9cca3455dffcf81a63c9d8577652276cc24ad424 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/6991 --- 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 51528a1d0a..db207eb05c 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -432,16 +432,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 098a5244972717cc606e2c2b8e58dabc0826981b 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/6991 --- ipaserver/install/installutils.py | 70 ++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index db207eb05c..b1f52f28a9 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -376,8 +376,38 @@ 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)) + if (unescaped_value.startswith(quote_char) and + unescaped_value.endswith(quote_char)): + return unescaped_value[1:-1] + + return unescaped_value + + +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. @@ -387,25 +417,19 @@ def set_directive(filename, directive, value, quotes=True, separator=' ', :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 quotes: whether to quote `value` in double quotes. If true, then + any existing double quotes 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) @@ -415,17 +439,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)) @@ -440,9 +460,6 @@ def get_directive(filename, directive, separator=' '): :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 """ @@ -455,8 +472,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 ea49d40921ad6b3119fd2eccff9af8265188d9d1 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/6991 --- 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 60088b0842..613303ff48 100644 --- a/ipaserver/install/httpinstance.py +++ b/ipaserver/install/httpinstance.py @@ -262,8 +262,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 7bc39e356e..16fd6e4c1f 100644 --- a/ipaserver/install/ipa_server_certinstall.py +++ b/ipaserver/install/ipa_server_certinstall.py @@ -140,12 +140,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)
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
