diff --git a/NEWS.adoc b/NEWS.adoc index 0c8c44aaee..620c7381e9 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -520,10 +520,16 @@ https://github.com/networkupstools/nut/milestone/13 the overflows it addressed) in normal device interactions. [#3588] - NUT client libraries: + * `PyNUTClient`: fixed parsing of escaped descriptions and values in + server replies, including `GetUPSList()` and its consumers. Shared + parsing also handles `nutauth.conf` values, comments and included + filenames using NUT syntax. Authentication arguments are escaped + before transmission. Existing response types are preserved; single + quotes in configuration values are now literal. [issue #3620, PR #3629] * The Perl `UPS::Nut` module now decodes quoted values and backslash escapes in query/list replies and `nutauth.conf` entries, and quotes authentication arguments. Single quotes in configuration are literal. - Command descriptions now accept the server's `CMDDESC` response. + Command descriptions now accept the server's `CMDDESC` response. [PR #3634] * Complete support for actions documented in `docs/net-protocol.txt` was implemented in C++, Python and PERL bindings in-tree, and for Java in link:https://github.com/networkupstools/jNut[jNut] nearby. Among @@ -660,6 +666,9 @@ https://github.com/networkupstools/nut/milestone/13 * Added a `clean_exit()` handler similar to that in `upsmon`. [PR #3499] - `upsd` data server updates: + * Escape variable and command descriptions from `cmdvartab` when sending + `DESC` and `CMDDESC` responses, so embedded quotes, backslashes and + hashes are decoded correctly by clients. [PR #3629] * Validate complete numeric values and conversion ranges for `MAXAGE`, `TRACKINGDELAY`, `MAXCONN` and `CERTREQUEST` in `upsd.conf`, retaining the previous setting when conversion fails. Preserve public types diff --git a/UPGRADING.adoc b/UPGRADING.adoc index d51b3cbf68..3ae6ce7b4f 100644 --- a/UPGRADING.adoc +++ b/UPGRADING.adoc @@ -31,15 +31,28 @@ command line, in order to quickly pick up any other removed option flags. Changes from 2.8.5 to 2.8.6 --------------------------- +- PLANNED: Keep track of any further API clean-up? + +- `PyNUTClient` now decodes NUT escape sequences in server descriptions and + values exactly once. Applications which worked around the old behavior by + removing escape characters themselves should stop doing so. The existing + bytes/string return types are unchanged. [PR #3629] + +- Python's `nutauth.conf` reader now follows NUT quoting rules: single + quotes are literal characters, not string delimiters. Replace single + quotes used for grouping with double quotes, and quote or escape spaces + within values. Backslashes introduce literal characters rather than + being retained, and unquoted hashes begin comments. Pass unescaped + usernames and passwords to `PyNUTClient`; the client handles their wire + escaping. [PR #3629] + - The Perl `UPS::Nut` module now decodes NUT quoting and backslash escapes in query/list values and `nutauth.conf`, including include paths. Remove manual decoding of these returned values and pass literal credentials to `Authenticate()`. Single quotes in configuration are now literal, as in the C parser; replace single-quoted values with double-quoted values where grouping was intended. The legacy `Set()` argument contract is - unchanged. - -- PLANNED: Keep track of any further API clean-up? + unchanged. [PR #3634] - The common configuration and protocol parser now accepts literal `#` characters inside double quotes. An unescaped `#` outside double quotes diff --git a/docs/man/nutauth.conf.txt b/docs/man/nutauth.conf.txt index 04e03f117d..82305b5664 100644 --- a/docs/man/nutauth.conf.txt +++ b/docs/man/nutauth.conf.txt @@ -90,6 +90,13 @@ should be double-quoted and/or use escape sequences, like in other NUT files. Blank lines and characters after an un-quoted hash (`#`) are ignored. +Single quotes are literal characters; only double quotes group a value +containing spaces. A backslash escapes the following character once, +including a space, quote, backslash or hash. It does not introduce C or +Python escape sequences. These rules also apply to `INCLUDE` filenames. +For compatibility with older NUT parsers, escape a hash with a backslash +even inside double quotes. + Example: # Global defaults diff --git a/docs/nut.dict b/docs/nut.dict index ce562515f4..b28a4f8d4e 100644 --- a/docs/nut.dict +++ b/docs/nut.dict @@ -1,4 +1,4 @@ -personal_ws-1.1 en 3833 utf-8 +personal_ws-1.1 en 3842 utf-8 AAC AAS ABI @@ -480,11 +480,14 @@ Gathman Geerling Gembe Gert +GetEnumList GetRWVars +GetRangeList GetUPSCommands GetUPSList GetUPSNames GetUPSVars +GetVariableDescription Ghali Giese Gigabit @@ -2230,6 +2233,7 @@ filenames filesystem filesystems firewalling +firmware's firmwares fixNSS fmt @@ -2516,6 +2520,7 @@ kVA kadets kaminski kde +kernel's kex kext keychain diff --git a/scripts/python/module/Makefile.am b/scripts/python/module/Makefile.am index c820379a65..1db8eb67a7 100644 --- a/scripts/python/module/Makefile.am +++ b/scripts/python/module/Makefile.am @@ -13,7 +13,19 @@ # (having only shell scripting as a prerequisite suffices for that) all: PyNUTClient -check-local: +check-local: check-pynut-parser check-pynut-disconnect + +check-pynut-parser: + @if test -n "$(PYTHON_DEFAULT)" && test "$(PYTHON_DEFAULT)" != no ; then \ + for TEST in test_upslist.py test_protocol.py ; do \ + PYTHONPATH="$(builddir)" $(PYTHON_DEFAULT) "$(srcdir)/$$TEST" || exit $$? ; \ + done ; \ + else \ + echo "SKIP: PyNUT parser tests require a configured Python interpreter"; \ + fi + @echo "You may want to set up a NUT data server and run 'make tox' here: `pwd`" + +check-pynut-disconnect: @if test -n "$(PYTHON_DEFAULT)" && test "$(PYTHON_DEFAULT)" != no; then \ $(PYTHON_DEFAULT) "$(builddir)/test_nutclient.py" --disconnect ; \ else \ @@ -25,7 +37,7 @@ check-local: tox: dist .pypi-tools-tox tox -EXTRA_DIST = tox.ini MANIFEST.in +EXTRA_DIST = tox.ini MANIFEST.in test_upslist.py test_protocol.py NUT_SOURCE_GITREV_NUMERIC = @NUT_SOURCE_GITREV_NUMERIC@ PYTHON_DEFAULT = @PYTHON_DEFAULT@ diff --git a/scripts/python/module/PyNUT.py.in b/scripts/python/module/PyNUT.py.in index a96acf82f2..5444de3642 100644 --- a/scripts/python/module/PyNUT.py.in +++ b/scripts/python/module/PyNUT.py.in @@ -75,6 +75,82 @@ import re import os import sys + +def _nut_parse(lines): + """Yield logical lines of NUT tokens, preserving the input string type. + + Like parseconf, quotes only open at a token boundary, single quotes + are literal, and a backslash consumes the next character exactly once. + Keep protocol bytes intact; this is not a Unicode or shell decoder. + """ + fields = [] + word = [] + started = quoted = escaped = comment = False + for line in lines: + empty = line[:0] + for index in range(len(line)): + char = line[index:index + 1] + code = ord(char) + if comment: + if code == 10: + yield fields + fields = [] + comment = False + continue + if escaped: + if code != 10: + word.append(char) + escaped = False + continue + if quoted: + if code == 34: + fields.append(empty.join(word)) + word = [] + started = quoted = False + elif code == 92: + escaped = True + elif code != 10: + word.append(char) + continue + if code == 92: + started = escaped = True + elif code == 34 and not started: + started = quoted = True + elif code in (9, 10, 11, 12, 13, 32, 35, 61): + if started: + fields.append(empty.join(word)) + word = [] + started = False + if code == 35: + comment = True + elif code == 61: + fields.append(char) + elif code == 10: + yield fields + fields = [] + else: + word.append(char) + started = True + if quoted or escaped: + raise ValueError("Incomplete NUT quoted string or escape") + if started: + fields.append(empty.join(word)) + if fields: + yield fields + + +def _nut_tokens(line): + """Parse a single NUT response line or field.""" + return next(_nut_parse([line]), []) + + +def _nut_quote(value): + """Encode one ASCII protocol argument, including for older NUT peers.""" + if any(ord(char) < 32 or ord(char) == 127 for char in value): + raise ValueError("Control character in NUT argument") + return '"' + re.sub(r'([\\\\"#])', r'\\\1', value) + '"' + + ssl_available = False try: import ssl @@ -281,16 +357,12 @@ class AuthConf: try: AuthConf.printDebug( "readAuthConfFile(): Reading NUT AuthConf data from '%s'" % (filename) ) with open(filename, 'r') as f: - for line in f: - line = line.strip() - if not line or line.startswith('#'): + for fields in _nut_parse(f): + if not fields: continue + line = fields[0] if line.startswith('['): - # Chomp any trailing comments: - if '#' in line: - line = line[:line.index('#')].strip() - if not line.endswith(']'): raise PyNUTError("Invalid section header in '%s': '%s'" % (filename, line)) @@ -321,30 +393,19 @@ class AuthConf: continue # INCLUDE support - m = re.match(r'^(INCLUDE(?:_REQUIRED)?)\s+(.*)$', line, re.I) - if m: - inc_type = m.group(1).upper() - inc_file = m.group(2).strip() - if (inc_file.startswith('"') and inc_file.endswith('"')) or \ - (inc_file.startswith("'") and inc_file.endswith("'")): - inc_file = inc_file[1:-1] + inc_type = fields[0].upper() + if inc_type in ('INCLUDE', 'INCLUDE_REQUIRED') and len(fields) >= 2: + inc_file = fields[1] is_required = (inc_type == "INCLUDE_REQUIRED") AuthConf.printDebug( "readAuthConfFile(): INCLUDE '%s'" % (inc_file) ) AuthConf.readAuthConfFile(inc_file, is_required, (current_ac is None or current_ac == AuthConf.__global_defaults)) continue - if '=' in line: - key, value = line.split('=', 1) - key = key.strip() - keyUC = key.strip().upper() - value = value.strip() - - # FIXME: NUT parseconf for possibly escaped values is a bit more complicated than this: - # Remove quotes if present - if (value.startswith('"') and value.endswith('"')) or \ - (value.startswith("'") and value.endswith("'")): - value = value[1:-1] + if len(fields) >= 2 and fields[1] == '=': + key = fields[0] + keyUC = key.upper() + value = fields[2] if len(fields) >= 3 else '' if current_ac is None: if global_scope: @@ -970,24 +1031,16 @@ if something goes wrong. self.__starttls_retry_budget = self.__starttls_retry_budget_default if self.__login != None : - self.__send( ("USERNAME %s\n" % self.__login).encode('ascii') ) + self.__send( ("USERNAME %s\n" % _nut_quote(self.__login)).encode('ascii') ) result = self.__read_until( b"\n" ) if result[:2] != b"OK" : raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) if self.__password != None : - self.__send( ("PASSWORD %s\n" % self.__password).encode('ascii') ) + self.__send( ("PASSWORD %s\n" % _nut_quote(self.__password)).encode('ascii') ) result = self.__read_until( b"\n" ) if result[:2] != b"OK" : - if result == b"ERR INVALID-ARGUMENT\n" : - # Quote the password (if it has whitespace etc) - # TODO: Escape special chard like NUT does? - self.__send( ("PASSWORD \"%s\"\n" % self.__password).encode('ascii') ) - result = self.__read_until( b"\n" ) - if result[:2] != b"OK" : - raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) - else: - raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) + raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) # NOTE: no-op if "None" self.__tracking = self.SetTrackingMode(self.__tracking_wanted) @@ -1146,7 +1199,7 @@ if something goes wrong. if result[:4] == b"DESC" : # DESC "" off = len( ("DESC %s %s " % ( ups, var )).encode('ascii') ) - return result[off:-1].split(b'"')[1].decode('ascii') + return _nut_tokens(result[off:-1])[0].decode('ascii') else : raise PyNUTError( result.replace( b"\n", b"" ).decode('ascii') ) @@ -1167,7 +1220,7 @@ if something goes wrong. end_offset = 0 - ( len( ("END LIST ENUM %s %s\n" % ( ups, var )).encode('ascii') ) + 1 ) for current in result[:end_offset].split( b"\n" ) : - enum_list.append( current[offset:].split( b'"' )[1].decode('ascii') ) + enum_list.append( _nut_tokens(current[offset:])[0].decode('ascii') ) return enum_list @@ -1189,8 +1242,8 @@ if something goes wrong. for current in result[:end_offset].split( b"\n" ) : # RANGE "" "" - ranges = current[offset:].split( b'"' ) - range_list.append( { 'min' : ranges[1].decode('ascii'), 'max' : ranges[3].decode('ascii') } ) + ranges = _nut_tokens(current[offset:]) + range_list.append( { 'min' : ranges[0].decode('ascii'), 'max' : ranges[1].decode('ascii') } ) return range_list @@ -1230,7 +1283,11 @@ which is of little concern for Python2 but is important in Python3 for line in result.split( b"\n" ) : if line[:3] == b"UPS" : - ups, desc = line[4:-1].split( b'"' ) + fields = re.match( b'([^"]*)("(?:[^"\\\\]|\\\\.)*")$', line[4:] ) + if fields is None : + raise ValueError( "Invalid UPS list entry" ) + ups, desc = fields.groups() + desc = _nut_tokens(desc)[0] ups_list[ ups.replace( b" ", b"" ) ] = desc return( ups_list ) @@ -1271,8 +1328,7 @@ available vars. end_offset = 0 - ( len( ("END LIST VAR %s\n" % ups).encode('ascii') ) + 1 ) for current in result[:end_offset].split( b"\n" ) : - var = current[ offset: ].split( b'"' )[0].replace( b" ", b"" ) - data = current[ offset: ].split( b'"' )[1] + var, data = _nut_tokens(current[offset:]) ups_vars[ var ] = data return( ups_vars ) @@ -1314,7 +1370,7 @@ of the command as value end_offset = 0 - ( len( ("END LIST CMD %s\n" % ups).encode('ascii') ) + 1 ) for current in result[:end_offset].split( b"\n" ) : - var = current[ offset: ].split( b'"' )[0].replace( b" ", b"" ) + var = _nut_tokens(current[offset:])[0] # For each var we try to get the available description try : @@ -1325,7 +1381,10 @@ of the command as value raise PyNUTError else : off = len( ("CMDDESC %s %s " % ( ups, command )).encode('ascii') ) - desc = temp[off:-1].split(b'"')[1] + if temp[off:off + 1] != b'"': + raise PyNUTError + desc = _nut_tokens(temp[off:-1])[0] + # OLDER: # desc = temp[off:-1].split(b'"')[1] except : desc = var @@ -1353,8 +1412,7 @@ The result is presented as a dictionary containing 'key->val' pairs try : for current in result[:end_offset].split( b"\n" ) : - var = current[ offset: ].split( b'"' )[0].replace( b" ", b"" ) - data = current[ offset: ].split( b'"' )[1] + var, data = _nut_tokens(current[offset:]) rw_vars[ var ] = data except : diff --git a/scripts/python/module/README.adoc b/scripts/python/module/README.adoc index b98740a6d6..4497ca961e 100644 --- a/scripts/python/module/README.adoc +++ b/scripts/python/module/README.adoc @@ -33,9 +33,10 @@ versions 2.7, 3.4, 3.5, 3.7, 3.11 and 3.13. [NOTE] ====== -Text fields returned by methods are byte sequences (not locale-aware -strings), except NUT protocol error codes quoted into `PyNUTError` -exceptions which are decoded into strings as originally `ascii` text. +The dictionaries returned by `GetUPSList()`, `GetUPSVars()`, `GetRWVars()` +and `GetUPSCommands()` contain byte keys and values (not locale-aware +strings). NUT protocol error codes in `PyNUTError` exceptions are decoded +into strings as originally `ascii` text. This is of little concern for Python 2, but is important in Python 3. The `ups` argument to methods is handled as a string, so conversion may @@ -46,8 +47,8 @@ strUps = bUps.decode('ascii') bUps = strUps.encode('ascii') ---- -Only the names returned by `GetUPSNames()` method specifically are -converted into string type. +`GetUPSNames()`, `GetVariableDescription()`, `GetEnumList()` and the bounds +returned by `GetRangeList()` use ASCII-decoded strings. Since Python 3 support was added just recently, module code may later be converted to use string types like `str` or `unicode` in the returned @@ -58,6 +59,18 @@ Examples below *do not* specify the `b'some text'` markup that would be pedantically correct (for Python 3 at least). ====== +Description and value readers decode NUT escaping exactly once. Callers +should not remove backslashes from the returned data themselves: a literal +backslash in a value remains a backslash, and is not a Python escape. +Pass ordinary ASCII strings for the constructor's `login` and `password` +arguments, without adding NUT quotes or escapes. The client encodes these +arguments for transmission; control characters are rejected. + +The `nutauth.conf` reader uses NUT syntax for values and `INCLUDE` filenames: +double quotes group tokens, single quotes are literal, unquoted hashes +begin comments, and backslashes escape the next character. See +linkman:nutauth.conf[5] for configuration examples. + .List of methods in the class ------ class PyNUTClient : diff --git a/scripts/python/module/test_nutclient.py.in b/scripts/python/module/test_nutclient.py.in index 69ad5ca10d..b6c8dab0a2 100755 --- a/scripts/python/module/test_nutclient.py.in +++ b/scripts/python/module/test_nutclient.py.in @@ -124,7 +124,7 @@ if HAVE_UNITTEST: socket.create_connection = lambda *args: sock self.assertRaises(PyNUT.PyNUTError, client.__init__, login="test") client.__del__() - self.assertEqual(sock.events, [b"USERNAME test\n", b"LOGOUT\n", "close"]) + self.assertEqual(sock.events, [b"USERNAME \"test\"\n", b"LOGOUT\n", "close"]) self.assertEqual(client._PyNUTClient__srv_handler, None) finally: socket.create_connection = create_connection diff --git a/scripts/python/module/test_protocol.py b/scripts/python/module/test_protocol.py new file mode 100644 index 0000000000..ae0cc1e739 --- /dev/null +++ b/scripts/python/module/test_protocol.py @@ -0,0 +1,298 @@ +#!/usr/bin/env python +# Test NUT protocol and configuration quoting without external services. +# SPDX-License-Identifier: GPL-3.0-or-later + +import os +import shutil +import tempfile +import unittest + +import PyNUT +from test_upslist import RecordingSocket + + +class ReplySocket(RecordingSocket): + """ Only supply a reply after its exact request has been observed. """ + def __init__(self, exchanges): + RecordingSocket.__init__(self, []) + self.exchanges = list(exchanges) + + def sendall(self, data): + self.sent.append(data) + if not self.exchanges: + raise AssertionError('Unexpected request: %r' % data) + request, reply = self.exchanges.pop(0) + if request != data: + raise AssertionError('Expected %r, received %r' % (request, data)) + self.chunks.append(reply) + + def send(self, data): + self.sendall(data) + return len(data) + + +class ProtocolTest(unittest.TestCase): + def client(self, exchanges): + client = PyNUT.PyNUTClient(connect_now=False) + transport = ReplySocket(exchanges) + client._PyNUTClient__srv_handler = transport + return client, transport + + def check_value(self, encoded, expected): + cases = [ + ('GetVariableDescription', ('dummy', 'ups.id'), + b'GET DESC dummy ups.id\n', + b'DESC dummy ups.id "' + encoded + b'"\n', + expected.decode('ascii')), + ('GetEnumList', ('dummy', 'ups.id'), + b'LIST ENUM dummy ups.id\n', + b'BEGIN LIST ENUM dummy ups.id\nENUM dummy ups.id "' + encoded + + b'"\nEND LIST ENUM dummy ups.id\n', [expected.decode('ascii')]), + ('GetUPSVars', ('dummy',), b'LIST VAR dummy\n', + b'BEGIN LIST VAR dummy\nVAR dummy ups.id "' + encoded + + b'"\nEND LIST VAR dummy\n', {b'ups.id': expected}), + ('GetRWVars', ('dummy',), b'LIST RW dummy\n', + b'BEGIN LIST RW dummy\nRW dummy ups.id "' + encoded + + b'"\nEND LIST RW dummy\n', {b'ups.id': expected}), + ] + for method, args, request, reply, wanted in cases: + client, transport = self.client([(request, reply)]) + result = getattr(client, method)(*args) + self.assertEqual(result, wanted, method) + if isinstance(result, dict): + self.assertEqual(type(list(result.keys())[0]), type(b'')) + self.assertEqual(type(result[b'ups.id']), type(b'')) + elif isinstance(result, list): + self.assertEqual(type(result[0]), type(b''.decode('ascii'))) + else: + self.assertEqual(type(result), type(b''.decode('ascii'))) + self.assertEqual(transport.exchanges, []) + + def test_ordinary_readers(self): + for value in [b'Office UPS', b'', b' spaced ']: + self.check_value(value, value) + + def test_escaped_readers(self): + for encoded, expected in [ + (b'Office \\"Main\\" \\#1', b'Office "Main" #1'), + (b'\\\\n \\\\t \\\\x41', b'\\n \\t \\x41'), + (b'\\\\\\"\\\\', b'\\"\\'), + (b'\\q', b'q'), + (b"Owner's #1 = UPS", b"Owner's #1 = UPS"), + ]: + self.check_value(encoded, expected) + + def test_command_descriptions(self): + client, transport = self.client([ + (b'LIST CMD dummy\n', b'BEGIN LIST CMD dummy\n' + b'CMD dummy test.panel.start\nCMD dummy test.panel.stop\n' + b'END LIST CMD dummy\n'), + (b'GET CMDDESC dummy test.panel.start\n', + b'CMDDESC dummy test.panel.start "Start \\"panel\\" \\#1 \\\\n"\n'), + (b'GET CMDDESC dummy test.panel.stop\n', + b'CMDDESC dummy test.panel.stop "Stop panel"\n'), + ]) + result = client.GetUPSCommands('dummy') + self.assertEqual(result, {b'test.panel.start': b'Start "panel" #1 \\n', + b'test.panel.stop': b'Stop panel'}) + for key, value in result.items(): + self.assertEqual(type(key), type(b'')) + self.assertEqual(type(value), type(b'')) + self.assertEqual(transport.exchanges, []) + + def test_command_description_fallback(self): + client, transport = self.client([ + (b'LIST CMD dummy\n', b'BEGIN LIST CMD dummy\n' + b'CMD dummy test.panel.start\nEND LIST CMD dummy\n'), + (b'GET CMDDESC dummy test.panel.start\n', b'ERR UNKNOWN-COMMAND\n'), + ]) + self.assertEqual(client.GetUPSCommands('dummy'), + {b'test.panel.start': b'test.panel.start'}) + self.assertEqual(transport.exchanges, []) + + def test_range_text(self): + client, transport = self.client([ + (b'LIST RANGE dummy battery.charge.low\n', + b'BEGIN LIST RANGE dummy battery.charge.low\n' + b'RANGE dummy battery.charge.low "001" "010.50"\n' + b'RANGE dummy battery.charge.low "20" "100"\n' + b'END LIST RANGE dummy battery.charge.low\n'), + ]) + result = client.GetRangeList('dummy', 'battery.charge.low') + self.assertEqual(result, [{'min': '001', 'max': '010.50'}, + {'min': '20', 'max': '100'}]) + for limits in result: + for value in limits.values(): + self.assertEqual(type(value), type(b''.decode('ascii'))) + + def test_server_errors(self): + for method, args, request in [ + ('GetVariableDescription', ('dummy', 'ups.id'), b'GET DESC dummy ups.id\n'), + ('GetEnumList', ('dummy', 'ups.id'), b'LIST ENUM dummy ups.id\n'), + ('GetRangeList', ('dummy', 'ups.id'), b'LIST RANGE dummy ups.id\n'), + ('GetUPSVars', ('dummy',), b'LIST VAR dummy\n'), + ('GetRWVars', ('dummy',), b'LIST RW dummy\n'), + ('GetUPSCommands', ('dummy',), b'LIST CMD dummy\n'), + ]: + client, transport = self.client([(request, b'ERR UNKNOWN-UPS\n')]) + try: + getattr(client, method)(*args) + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), 'ERR UNKNOWN-UPS') + else: + self.fail('Expected PyNUTError from ' + method) + + +class AuthConfTest(unittest.TestCase): + def setUp(self): + PyNUT.AuthConf.freeAuthConfList() + PyNUT.AuthConf.setDebug(False) + self.directory = tempfile.mkdtemp(prefix='nut-authconf-') + + def tearDown(self): + PyNUT.AuthConf.freeAuthConfList() + shutil.rmtree(self.directory) + + def write_config(self, name, contents): + filename = os.path.join(self.directory, name) + with open(filename, 'w') as stream: + stream.write(contents) + return filename + + def read_config(self, contents): + filename = self.write_config('nutauth.conf', contents) + PyNUT.AuthConf.readAuthConfFile(filename, fatal_errors=True) + return PyNUT.AuthConf.getAuthConf(host='localhost', port=3493) + + def config_path(self, name): + return os.path.join(self.directory, name).replace('\\', '\\\\').replace( + '"', '\\"').replace('#', '\\#') + + def test_value_syntax(self): + for source, expected in [ + ('ordinary', 'ordinary'), ('""', ''), + ('"Office \\"Main\\" \\#1"', 'Office "Main" #1'), + ('"Owner\'s #1" # comment', "Owner's #1"), + ("'literal'", "'literal'"), + ('"\\\\n \\\\t \\\\x41"', '\\n \\t \\x41'), + ('unquoted\\ space', 'unquoted space'), + ('escaped\\#hash # comment', 'escaped#hash'), + ('"equals=value"', 'equals=value'), + ('left=right', 'left'), + ('"first"second', 'first'), + ('mid"quote', 'mid"quote'), + ('one\\\ntwo', 'onetwo'), + ('"one\\\ntwo"', 'onetwo'), + ]: + PyNUT.AuthConf.freeAuthConfList() + result = self.read_config('PASSWORD=' + source + '\n') + self.assertEqual(result.password, expected, source) + + def test_defaults_and_section_precedence(self): + result = self.read_config('USERNAME=global\nPASSWORD=global\n' + 'CERTPATH="/global path"\n[@localhost:3493] # host defaults\n' + 'PASSWORD=host\n[alice@localhost:3493]\nUSERNAME=ignored\n' + 'PASSWORD=user\n') + self.assertEqual(result.password, 'host') + result = PyNUT.AuthConf.getAuthConf(user='alice', host='localhost', port=3493) + self.assertEqual(result.user, 'alice') + self.assertEqual(result.password, 'user') + self.assertEqual(result.certpath, '/global path') + + def test_included_filenames(self): + self.write_config('include #1.conf', 'PASSWORD="from include"\n') + encoded = self.config_path('include #1.conf') + result = self.read_config('INCLUDE_REQUIRED "' + encoded + '" # comment\n') + self.assertEqual(result.password, 'from include') + PyNUT.AuthConf.freeAuthConfList() + self.write_config("owner's file.conf", 'CERTPATH="included path"\n') + encoded = self.config_path("owner's file.conf").replace(' ', '\\ ') + result = self.read_config('INCLUDE ' + encoded + '\nPASSWORD=local\n') + self.assertEqual(result.certpath, 'included path') + self.assertEqual(result.password, 'local') + + def test_included_filename_decoding(self): + # Record the include path without creating a Windows-invalid filename. + filename = self.write_config('nutauth.conf', + 'INCLUDE_REQUIRED "include \\"q\\"\\#1\\\\part.conf" # comment\n') + original = PyNUT.AuthConf.readAuthConfFile + included = [] + try: + PyNUT.AuthConf.readAuthConfFile = staticmethod( + lambda name, required, *args: included.append((name, required))) + original(filename, fatal_errors=True) + finally: + PyNUT.AuthConf.readAuthConfFile = staticmethod(original) + self.assertEqual(included, [('include "q"#1\\part.conf', True)]) + + def test_optional_and_required_missing_include(self): + missing = self.config_path('missing.conf') + result = self.read_config('INCLUDE "' + missing + '"\nPASSWORD=local\n') + self.assertEqual(result.password, 'local') + PyNUT.AuthConf.freeAuthConfList() + self.assertRaises(PyNUT.PyNUTError, self.read_config, + 'INCLUDE_REQUIRED "' + missing + '"\n') + + def connect(self, login, password, transport): + original = PyNUT.socket.create_connection + PyNUT.socket.create_connection = lambda *args, **kwargs: transport + try: + return PyNUT.PyNUTClient(login=login, password=password, + use_ssl=False, tracking=None) + finally: + PyNUT.socket.create_connection = original + + def test_authenticate_decoded_credentials(self): + result = self.read_config('USERNAME="Office \\"Main\\" \\#1"\n' + 'PASSWORD="path\\\\next \\"key\\" \\#2"\n') + transport = ReplySocket([ + (b'USERNAME "Office \\"Main\\" \\#1"\n', b'OK\n'), + (b'PASSWORD "path\\\\next \\"key\\" \\#2"\n', b'OK\n'), + ]) + client = self.connect(result.user, result.password, transport) + self.assertEqual(transport.exchanges, []) + client.disconnect() + + def test_authenticate_ordinary_and_errors(self): + transport = ReplySocket([(b'USERNAME "user"\n', b'ERR ACCESS-DENIED\n')]) + try: + self.connect('user', 'pass', transport) + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), 'ERR ACCESS-DENIED') + else: + self.fail('Expected username error') + self.assertEqual(transport.exchanges, []) + for reply in [b'OK\n', b'ERR ACCESS-DENIED\n', b'ERR INVALID-ARGUMENT\n']: + transport = ReplySocket([ + (b'USERNAME "user"\n', b'OK\n'), + (b'PASSWORD "pass"\n', reply), + ]) + if reply == b'OK\n': + client = self.connect('user', 'pass', transport) + client.disconnect() + else: + try: + self.connect('user', 'pass', transport) + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), reply[:-1].decode('ascii')) + else: + self.fail('Expected authentication error') + self.assertEqual(transport.exchanges, []) + + def test_credential_line_breaks_rejected(self): + # A line-oriented argument must not become more than one request. + for value in ['first\nsecond', 'first\rsecond']: + for login, password in [(value, 'pass'), ('user', value)]: + transport = RecordingSocket([b'OK\n', b'OK\n']) + self.assertRaises(ValueError, self.connect, login, password, transport) + try: + # Did we also handle socket disconnection correctly? + self.assertEqual(transport.sent, + [b'LOGOUT\n'] if login == value else [b'USERNAME "user"\n', b'LOGOUT\n']) + except AssertionError as ignored: + # If the check below fails, we get a log with the transport.sent[] list: + self.assertEqual(transport.sent, + [] if login == value else [b'USERNAME "user"\n']) + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/python/module/test_upslist.py b/scripts/python/module/test_upslist.py new file mode 100644 index 0000000000..615551a785 --- /dev/null +++ b/scripts/python/module/test_upslist.py @@ -0,0 +1,186 @@ +#!/usr/bin/env python +# Test the byte contract of LIST UPS without a running server. +# SPDX-License-Identifier: GPL-3.0-or-later + +import itertools +import unittest + +import PyNUT + + +class RecordingSocket(object): + def __init__(self, chunks): + self.chunks = list(chunks) + self.sent = [] + + def sendall(self, data): + self.sent.append(data) + + def recv(self, size): + if not self.chunks: + return b'' + data = self.chunks.pop(0) + if len(data) > size: + self.chunks.insert(0, data[size:]) + return data[:size] + + def close(self): + pass + + +def listing(rows): + return b'BEGIN LIST UPS\n' + rows + b'END LIST UPS\n' + + +class UPSListTest(unittest.TestCase): + def client(self, chunks): + client = PyNUT.PyNUTClient(connect_now=False) + transport = RecordingSocket(chunks) + client._PyNUTClient__srv_handler = transport + return client, transport + + def check_description(self, encoded, expected): + response = listing(b'UPS dummy "' + encoded + b'"\n') + client, transport = self.client([response]) + result = client.GetUPSList() + self.assertEqual(result, {b'dummy': expected}) + self.assertEqual(type(list(result.keys())[0]), type(b'')) + self.assertEqual(type(result[b'dummy']), type(b'')) + self.assertEqual(transport.sent, [b'LIST UPS\n']) + + def test_ordinary(self): + for desc in [b'Office UPS', b'', b'UPS', b' Office UPS ']: + self.check_description(desc, desc) + + def test_quotes(self): + for encoded, expected in [ + (b'Office \\"Main\\" UPS', b'Office "Main" UPS'), + (b'\\"Office', b'"Office'), + (b'Office\\"', b'Office"'), + (b'\\"', b'"'), + (b'\\"\\"', b'""'), + ]: + self.check_description(encoded, expected) + + def test_backslashes_and_literals(self): + for encoded, expected in [ + (b'\\\\', b'\\'), + (b'\\\\\\\\', b'\\\\'), + (b'\\\\\\"', b'\\"'), + (b'\\"\\\\', b'"\\'), + (b'\\\\n \\\\t \\\\x41', b'\\n \\t \\x41'), + (b"Owner's \\#1 = UPS; $HOME `a`", b"Owner's #1 = UPS; $HOME `a`"), + (b'\\n', b'n'), + ]: + self.check_description(encoded, expected) + + def test_escape_combinations(self): + # Literal encoded/decoded atoms, including adjacent escape boundaries. + atoms = [(b' ', b' '), (b'\\"', b'"'), + (b'\\\\', b'\\'), (b'\\#', b'#')] + for size in range(1, 5): + for parts in itertools.product(atoms, repeat=size): + self.check_description(b''.join(p[0] for p in parts), + b''.join(p[1] for p in parts)) + + def test_bytes(self): + for value in range(0x20, 0x80): + byte = chr(value).encode('latin-1') + encoded = b'\\' + byte if byte in (b'"', b'\\', b'#') else byte + self.check_description(encoded, byte) + # Synthetic wire bytes: upsd's config parser discards these, but the + # client must not introduce a Unicode decoding policy for descriptions. + self.check_description(b'\x80\xff\\"\xc3\xa9', b'\x80\xff"\xc3\xa9') + + def test_multiple_entries_and_names(self): + response = listing(b'UPS UPS_1-a.b "Office \\"Main\\""\n' + b'UPS second ""\n') + client, transport = self.client([response, response]) + self.assertEqual(client.GetUPSList(), + {b'UPS_1-a.b': b'Office "Main"', b'second': b''}) + names = client.GetUPSNames() + self.assertEqual(type(names), list) + self.assertEqual(sorted(names), ['UPS_1-a.b', 'second']) + for name in names: + self.assertEqual(type(name), type(b''.decode('ascii'))) + self.assertEqual(transport.sent, [b'LIST UPS\n', b'LIST UPS\n']) + + def test_empty_list(self): + client, transport = self.client([listing(b''), listing(b'')]) + self.assertEqual(client.GetUPSList(), {}) + self.assertEqual(client.GetUPSNames(), []) + + def test_fragmentation(self): + response = listing(b'UPS dummy "A\\\\\\"B\\#C"\n') + expected = {b'dummy': b'A\\"B#C'} + for split in range(1, len(response)): + client, transport = self.client([response[:split], response[split:]]) + self.assertEqual(client.GetUPSList(), expected) + client, transport = self.client([response[i:i+1] for i in range(len(response))]) + self.assertEqual(client.GetUPSList(), expected) + + def test_coalesced_responses(self): + # First recv includes part of the next response; __read_until must + # retain it through both the header and the end-of-list reads. + response = listing(b'UPS d "\\""\n') + client, transport = self.client([response + listing(b'') + b'OK\n']) + self.assertEqual(client.GetUPSList(), {b'd': b'"'}) + self.assertEqual(client.GetUPSList(), {}) + self.assertEqual(client._PyNUTClient__read_until(b'\n'), b'OK\n') + self.assertEqual(transport.chunks, []) + + def test_consumers(self): + response = listing(b'UPS dummy "Office \\"Main\\""\n') + client, transport = self.client([response + b'OK\n' + response + + b'BEGIN LIST CLIENT dummy\nCLIENT dummy 127.0.0.1\nEND LIST CLIENT dummy\n']) + self.assertEqual(client.DeviceLogin('dummy'), 'OK') + self.assertEqual(client.ListClients('dummy'), {b'dummy': [b'127.0.0.1']}) + self.assertEqual(transport.sent, [b'LIST UPS\n', b'LOGIN dummy\n', + b'LIST UPS\n', b'LIST CLIENT dummy\n']) + + def test_list_clients_fallback(self): + response = listing(b'UPS dummy "\\""\n') + client, transport = self.client([response + b'ERR INVALID-ARGUMENT\n' + response + + b'BEGIN LIST CLIENT dummy\nEND LIST CLIENT dummy\n']) + self.assertEqual(client.ListClients(), {}) + + def test_unknown_ups(self): + response = listing(b'UPS dummy "\\""\n') + for method in ['DeviceLogin', 'ListClients']: + client, transport = self.client([response]) + try: + getattr(client, method)('missing') + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), 'ERR UNKNOWN-UPS') + else: + self.fail('Expected PyNUTError') + self.assertEqual(transport.sent, [b'LIST UPS\n']) + + def test_server_error(self): + client, transport = self.client([b'ERR ACCESS-DENIED\n']) + try: + client.GetUPSList() + except PyNUT.PyNUTError as exc: + self.assertEqual(str(exc), 'ERR ACCESS-DENIED') + else: + self.fail('Expected PyNUTError') + + def test_eof(self): + for response in [b'', b'BEGIN LIST UPS\nUPS dummy "unfinished']: + client, transport = self.client([response]) + self.assertRaises(EOFError, client.GetUPSList) + + def test_malformed_rows(self): + for row in [b'UPS dummy unquoted\n', b'UPS dummy "raw "quote""\n', + b'UPS dummy "desc" "extra"\n', b'UPS dummy "missing close\n', + b'UPS dummy "dangling\\"\n']: + client, transport = self.client([listing(row)]) + self.assertRaises(ValueError, client.GetUPSList) + + def test_ignored_lines(self): + client, transport = self.client([listing(b'OTHER line\n\nUPS dummy "Office"\n')]) + self.assertEqual(client.GetUPSList(), {b'dummy': b'Office'}) + + +if __name__ == '__main__': + unittest.main() diff --git a/server/netget.c b/server/netget.c index aebe1249da..cb57aaffe3 100644 --- a/server/netget.c +++ b/server/netget.c @@ -71,6 +71,7 @@ static void get_desc(nut_ctype_t *client, const char *upsname, const char *var) const upstype_t *ups; const char *varptr; const char *desc; + char esc[PCONF_DEFAULT_WORDLEN_LIMIT * 2 + 1]; ups = get_ups_ptr(upsname); @@ -93,7 +94,8 @@ static void get_desc(nut_ctype_t *client, const char *upsname, const char *var) desc = desc_get_var(varptr); if (desc) - sendback(client, "DESC %s %s \"%s\"\n", upsname, var, desc); + sendback(client, "DESC %s %s \"%s\"\n", upsname, var, + pconf_encode(desc, esc, sizeof(esc))); else sendback(client, "DESC %s %s \"Description unavailable\"\n", upsname, var); } @@ -103,6 +105,7 @@ static void get_cmddesc(nut_ctype_t *client, const char *upsname, const char *cm const upstype_t *ups; const char *cmdptr; const char *desc; + char esc[PCONF_DEFAULT_WORDLEN_LIMIT * 2 + 1]; ups = get_ups_ptr(upsname); @@ -125,7 +128,8 @@ static void get_cmddesc(nut_ctype_t *client, const char *upsname, const char *cm desc = desc_get_cmd(cmdptr); if (desc) - sendback(client, "CMDDESC %s %s \"%s\"\n", upsname, cmd, desc); + sendback(client, "CMDDESC %s %s \"%s\"\n", upsname, cmd, + pconf_encode(desc, esc, sizeof(esc))); else sendback(client, "CMDDESC %s %s \"Description unavailable\"\n", upsname, cmd); diff --git a/tests/NIT/nit.sh b/tests/NIT/nit.sh index 738586886a..a064c8a3ad 100755 --- a/tests/NIT/nit.sh +++ b/tests/NIT/nit.sh @@ -4126,6 +4126,74 @@ testcase_sandbox_python_with_upsmon_credentials() { fi } +testcase_sandbox_python_escaping() { + log_separator + log_info "[testcase_sandbox_python_escaping] Check escaped data and credentials with PyNUT" + if $PYTHON - "${TOP_BUILDDIR}/scripts/python/module" << 'PY' +import os +import sys + +sys.path.insert(0, sys.argv[1]) +import PyNUT + +port = int(os.environ['NUT_PORT']) +value = b'Rack\'s "A" #1 \\n' +text = value.decode('ascii') +client = PyNUT.PyNUTClient(host='127.0.0.1', port=port, timeout=3) +try: + assert client.GetUPSList() == {b'dummy': value, b'ordinary': b'Ordinary UPS'} + assert sorted(client.GetUPSNames()) == ['dummy', 'ordinary'] + for result in (client.GetUPSVars('dummy'), client.GetRWVars('dummy')): + assert result[b'ups.model'] == value, result + assert result[b'ups.mfr'] == b'Ordinary Manufacturer', result + assert all(isinstance(k, bytes) and isinstance(v, bytes) for k, v in result.items()) + description = client.GetVariableDescription('dummy', 'ups.model') + assert description == text and isinstance(description, type(text)), description + assert client.GetVariableDescription('dummy', 'ups.mfr') == 'Ordinary variable' + commands = client.GetUPSCommands('dummy') + assert commands[b'load.off'] == value, commands + assert all(isinstance(k, bytes) and isinstance(v, bytes) for k, v in commands.items()) +finally: + client.disconnect() + +PyNUT.AuthConf.freeAuthConfList() +PyNUT.AuthConf.readAuthConfFile(os.path.join(os.environ['NUT_CONFPATH'], 'escape-auth.conf'), fatal_errors=True) +auth = PyNUT.AuthConf.getAuthConf(host='127.0.0.1', port=port) +assert auth.user == "nit'escape" and auth.password == text +client = PyNUT.PyNUTClient.from_authconf(auth, timeout=3) +try: + # USERNAME/PASSWORD alone acknowledge receipt; LOGIN verifies the credentials. + assert client.DeviceLogin('dummy') == 'OK' +finally: + client.disconnect() + +client = PyNUT.PyNUTClient(host='127.0.0.1', port=port, login='nit-plain', password='ordinary', timeout=3) +try: + assert client.DeviceLogin('ordinary') == 'OK' +finally: + client.disconnect() + +client = PyNUT.PyNUTClient(host='127.0.0.1', port=port, login=auth.user, password='wrong', timeout=3) +try: + try: + client.DeviceLogin('dummy') + except PyNUT.PyNUTError as error: + assert str(error) == 'ERR ACCESS-DENIED', str(error) + else: + raise AssertionError('Incorrect password was accepted') +finally: + client.disconnect() +print('Escaped values, descriptions, INCLUDE credentials and authentication passed') +PY + then + PASSED="`expr $PASSED + 1`" + else + log_error "[testcase_sandbox_python_escaping] Error: PyNUT regression check failed" + FAILED="`expr $FAILED + 1`" + FAILED_FUNCS="$FAILED_FUNCS testcase_sandbox_python_escaping" + fi +} + testcases_sandbox_python() { isTestablePython && [ -n "${PYTHON}" ] || { SKIPPED_FUNCS="${SKIPPED_FUNCS} testcase_sandbox_python_without_credentials testcase_sandbox_python_with_credentials testcase_sandbox_python_with_upsmon_credentials" @@ -4749,6 +4817,7 @@ testgroup_sandbox() { log_separator sandbox_forget_configs + testgroup_sandbox_python_escaping } testgroup_sandbox_python() { @@ -4758,6 +4827,102 @@ testgroup_sandbox_python() { log_separator sandbox_forget_configs + testgroup_sandbox_python_escaping +} + +testgroup_sandbox_python_escaping() { + isTestablePython && [ -n "${PYTHON}" ] || { + SKIPPED_FUNCS="$SKIPPED_FUNCS testcase_sandbox_python_escaping" + SKIPPED="`expr $SKIPPED + 1`" + return 0 + } + + # This fixture replaces the ordinary sandbox only after its other tests finish. + stop_daemons + generatecfg_upsd_trivial + generatecfg_ups_trivial + # MSYS converts process arguments, but not paths written into config files. + ESCAPING_CONFPATH="$NUT_CONFPATH" + if command -v cygpath >/dev/null 2>&1 ; then + ESCAPING_CONFPATH="$(cygpath -m "$NUT_CONFPATH")" \ + || die "Failed to convert the escaping fixture path" + fi + ESCAPING_CONFPATH="$(printf '%s' "$ESCAPING_CONFPATH" | \ + sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' -e 's/#/\\#/g')" \ + || die "Failed to escape the fixture path for NUT configuration" + printf 'DATAPATH "%s"\n' "$ESCAPING_CONFPATH" >> "$NUT_CONFPATH/upsd.conf" \ + || die "Failed to configure the temporary cmdvartab path" + cat >> "$NUT_CONFPATH/ups.conf" << 'EOF' +[dummy] + driver = dummy-ups + desc = "Rack's \"A\" \#1 \\n" + port = escaping.dev + mode = dummy-once +[ordinary] + driver = dummy-ups + desc = "Ordinary UPS" + port = escaping.dev + mode = dummy-once +EOF + [ $? = 0 ] || die "Failed to populate escaping ups.conf" + cat > "$NUT_CONFPATH/escaping.dev" << 'EOF' +ups.status: OL +ups.mfr: Ordinary Manufacturer +ups.model: "Rack's \"A\" \#1 \\n" +EOF + [ $? = 0 ] || die "Failed to populate escaping dummy data" + cat > "$NUT_CONFPATH/cmdvartab" << 'EOF' +VARDESC ups.model "Rack's \"A\" \#1 \\n" +VARDESC ups.mfr "Ordinary variable" +CMDDESC load.off "Rack's \"A\" \#1 \\n" +EOF + [ $? = 0 ] || die "Failed to populate escaping cmdvartab" + cat > "$NUT_CONFPATH/upsd.users" << 'EOF' +[nit'escape] + password = "Rack's \"A\" \#1 \\n" + upsmon secondary +[nit-plain] + password = ordinary + upsmon secondary +EOF + [ $? = 0 ] || die "Failed to populate escaping upsd.users" + printf 'INCLUDE_REQUIRED "%s/escape credentials.conf"\n' "$ESCAPING_CONFPATH" \ + > "$NUT_CONFPATH/escape-auth.conf" || die "Failed to populate escaping INCLUDE" + printf '[@127.0.0.1:%s]\n' "$NUT_PORT" > "$NUT_CONFPATH/escape credentials.conf" \ + || die "Failed to populate escaping authconf section" + cat >> "$NUT_CONFPATH/escape credentials.conf" << 'EOF' +USERNAME = nit'escape +PASSWORD = "Rack's \"A\" \#1 \\n" +EOF + [ $? = 0 ] || die "Failed to populate escaping credentials" + if $I_AM_ROOT ; then + chmod 644 "$NUT_CONFPATH/upsd.users" + else + chmod 640 "$NUT_CONFPATH/upsd.users" + fi + chmod 644 "$NUT_CONFPATH/escaping.dev" "$NUT_CONFPATH/cmdvartab" + chmod 600 "$NUT_CONFPATH/escape-auth.conf" "$NUT_CONFPATH/escape credentials.conf" + + SANDBOX_CONFIG_GENERATED=true + sandbox_start_upsd || die "Failed to start escaping sandbox upsd" + execcmd dummy-ups -a dummy ${ARG_USER} ${ARG_FG} & + PID_DUMMYUPS="$!" + execcmd dummy-ups -a ordinary ${ARG_USER} ${ARG_FG} & + PID_DUMMYUPS1="$!" + COUNTDOWN=20 + while [ "$COUNTDOWN" -gt 0 ]; do + if runcmd upsc -A none -W 2 dummy@localhost:$NUT_PORT ups.mfr \ + && [ x"$CMDOUT" = x"Ordinary Manufacturer" ]; then + break + fi + sleep 1 + COUNTDOWN="`expr $COUNTDOWN - 1`" + done + [ "$COUNTDOWN" -gt 0 ] || die "Escaping dummy driver did not become ready" + + testcase_sandbox_python_escaping + log_separator + sandbox_forget_configs } testcase_sandbox_parseconf() {