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 57a75bd8ea2ccfe7ef61759cfdf38201ae6d9579 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 18af389f6a9dbb56ed6eeeb29ceed529b0d31619 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 5451ea971cc60f57fb64f8a3c068ac9191fa37f8 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 | 70 ++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index 4f93372..ab2596c 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -380,8 +380,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. @@ -391,25 +421,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) @@ -419,17 +443,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)) @@ -444,9 +464,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 """ @@ -459,8 +476,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 abc6186021bb01f5d5b5d25c237ef0bd842e330f 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
