From 184d4e7e4079651c79af9f2e90d2544b50d91d4e Mon Sep 17 00:00:00 2001 From: Daniel Elero Date: Wed, 13 Mar 2019 12:52:28 +0100 Subject: [PATCH] Add latest changes from in-toto (ab1e904caa) --- ci-requirements.txt | 2 +- dev-requirements.txt | 2 +- securesystemslib/gpg/common.py | 60 +++++++++++------ securesystemslib/gpg/process.py | 110 ++++++++++++++++++++++++++++++-- securesystemslib/gpg/util.py | 95 +++++++++++++++++++++------ tests/test_process.py | 74 ++++++++++++++++++++- 6 files changed, 294 insertions(+), 49 deletions(-) diff --git a/ci-requirements.txt b/ci-requirements.txt index d9f0b25e..2c943ff0 100644 --- a/ci-requirements.txt +++ b/ci-requirements.txt @@ -5,5 +5,5 @@ coverage coveralls six colorama -mock; python_version < '3.3' +mock; subprocess32; python_version < '3' diff --git a/dev-requirements.txt b/dev-requirements.txt index 19918fc7..bc04122e 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -5,7 +5,7 @@ tox==3.2.1 coveralls==1.3.0 coverage==4.5.1 colorama==0.3.9 -mock==2.0.0; python_version < '3.3' +mock==2.0.0; subprocess32; python_version < '3' --editable . diff --git a/securesystemslib/gpg/common.py b/securesystemslib/gpg/common.py index 43781738..5932ce21 100644 --- a/securesystemslib/gpg/common.py +++ b/securesystemslib/gpg/common.py @@ -35,7 +35,7 @@ SignatureAlgorithmNotSupportedError) from securesystemslib.gpg.formats import GPG_HASH_ALGORITHM_STRING -log = logging.getLogger('securesystemslib.gpg.common') +log = logging.getLogger('securesystemslib_gpg_common') def parse_pubkey_payload(data): @@ -196,7 +196,7 @@ def parse_pubkey_bundle(data, keyid): for idx, public_key in enumerate( [master_public_key] + list(sub_public_keys.values())): if public_key and public_key["keyid"].endswith(keyid.lower()): - if idx > 1: + if idx > 1: # pragma: no cover log.warning("Exporting master key '{}' including subkeys '{}'. For" " passed keyid '{}'.".format(master_public_key["keyid"], ", ".join(list(sub_public_keys.keys())), keyid)) @@ -237,7 +237,7 @@ def parse_signature_packet(data): A signature dictionary matching - securesystemslib.gpg.formats.SIGNATURE_SCHEMA + securesystemslib.gpg.formats.SIGNATURE_SCHEMA """ data, junk_length, junk_type = securesystemslib.gpg.util.parse_packet_header( @@ -308,25 +308,46 @@ def parse_signature_packet(data): ptr += unhashed_octet_count keyid = "" - # Newer signature types contain the full keyid in subpacket 33 - for subpacket_tuple in hashed_subpacket_info: - if subpacket_tuple[0] == FULL_KEYID_SUBPACKET: # pragma: no cover - # NOTE: The first byte of the payload is a version number - # https://archive.cert.uni-stuttgart.de/openpgp/2016/06/msg00004.html - keyid = binascii.hexlify(subpacket_tuple[1][1:]).decode("ascii") - break - short_keyid = "" - # We also return the short keyid, because the full might not be available - for subpacket_tuple in unhashed_subpacket_info: - if subpacket_tuple[0] == PARTIAL_KEYID_SUBPACKET: # pragma: no branch - short_keyid = binascii.hexlify(subpacket_tuple[1]).decode("ascii") - break + + # Parse Issuer (short keyid) and Issuer Fingerprint (full keyid) from hashed + # and unhashed signature subpackets. Full keyids are only available in newer + # signatures. (see RFC4880 and rfc4880bis-06 5.2.3.1.) + # NOTE: A subpacket may be found either in the hashed or unhashed subpacket + # sections of a signature. If a subpacket is not hashed, then the information + # in it cannot be considered definitive because it is not part of the + # signature proper. + # (see RFC4880 5.2.3.2.) + # NOTE: Signatures may contain conflicting information in subpackets. In most + # cases, an implementation SHOULD use the last subpacket, but MAY use any + # conflict resolution scheme that makes more sense. + # (see RFC4880 5.2.4.1.) + # Below we only consider the last and favor hashed over unhashed subpackets + for subpacket_type, subpacket_data in \ + unhashed_subpacket_info + hashed_subpacket_info: + if subpacket_type == FULL_KEYID_SUBPACKET: + # NOTE: The first byte of the subpacket payload is a version number + # (see rfc4880bis-06 5.2.3.28.) + keyid = binascii.hexlify(subpacket_data[1:]).decode("ascii") + + # We also return the short keyid, because the full might not be available + if subpacket_type == PARTIAL_KEYID_SUBPACKET: + short_keyid = binascii.hexlify(subpacket_data).decode("ascii") + # Fail if there is no keyid at all (this should not happen) if not (keyid or short_keyid): # pragma: no cover - raise ValueError("The signature packet seems to be corrupted. It is" - " missing the issuer subpacket.") + raise ValueError("This signature packet seems to be corrupted. It does " + "not have an 'Issuer' or 'Issuer Fingerprint' subpacket (see RFC4880 " + "and rfc4880bis-06 5.2.3.1. Signature Subpacket Specification).") + + # Fail keyid and short keyid are specified but don't match + if keyid and not keyid.endswith(short_keyid): # pragma: no cover + raise ValueError("This signature packet seems to be corrupted. The key ID " + "'{}' of the 'Issuer' subpacket must match the lower 64 bits of the " + "fingerprint '{}' of the 'Issuer Fingerprint' subpacket (see RFC4880 " + "and rfc4880bis-06 5.2.3.28. Issuer Fingerprint).".format( + short_keyid, keyid)) # Uncomment this variable to obtain the left-hash-bits information (used for # early rejection) @@ -338,7 +359,6 @@ def parse_signature_packet(data): return { 'keyid': "{}".format(keyid), 'short_keyid': "{}".format(short_keyid), - 'other_headers': \ - binascii.hexlify(data[:other_headers_ptr]).decode('ascii'), + 'other_headers': binascii.hexlify(data[:other_headers_ptr]).decode('ascii'), 'sig': binascii.hexlify(signature).decode('ascii') } diff --git a/securesystemslib/gpg/process.py b/securesystemslib/gpg/process.py index 3382a318..d86adf9f 100644 --- a/securesystemslib/gpg/process.py +++ b/securesystemslib/gpg/process.py @@ -19,14 +19,19 @@ - require the Py3 subprocess backport `subprocess32` on Python2, - securesystemslib.gpg namespace subprocess constants (DEVNULL, PIPE) and - provide a custom `subprocess.run` wrapper + - provide a special `run_duplicate_streams` function """ +import io import logging +import os import shlex - -import six +import sys +import tempfile +import time import securesystemslib.gpg.formats as formats +import six if six.PY2: import subprocess32 as subprocess # pragma: no cover pylint: disable=import-error @@ -38,16 +43,13 @@ # If the process does not terminate after timeout seconds, a # subprocess.TimeoutExpired exception will be raised. SUBPROCESS_TIMEOUT = 3 - - DEVNULL = subprocess.DEVNULL PIPE = subprocess.PIPE -log = logging.getLogger('securesystemslib.gpg.process') +log = logging.getLogger('securesystemslib_gpg_process') -# TODO: Properly duplicate standard streams (issue #11) def run(cmd, check=True, timeout=SUBPROCESS_TIMEOUT, **kwargs): """ @@ -121,3 +123,99 @@ def run(cmd, check=True, timeout=SUBPROCESS_TIMEOUT, **kwargs): del kwargs["stdin"] return subprocess.run(cmd, check=check, timeout=timeout, **kwargs) + + + + +def run_duplicate_streams(cmd, timeout=SUBPROCESS_TIMEOUT): + """ + + Provide a function that executes a command in a subprocess and returns its + exit code and the contents of what it printed to its standard streams upon + termination. + + NOTE: The function might behave unexpectedly with interactive commands. + + + + cmd: + The command and its arguments. (list of str, or str) + Splits a string specifying a command and its argument into a list + of substrings, if necessary. + + timeout: (default see settings.SUBPROCESS_TIMEOUT) + If the timeout expires, the child process will be killed and waited + for and then subprocess.TimeoutExpired will be raised. + + + securesystemslib.exceptions.FormatError: + If the `cmd` is a list and does not match + securesystemslib.gpg.formats.LIST_OF_ANY_STRING_SCHEMA. + + OSError: + If the given command is not present or non-executable. + + subprocess.TimeoutExpired: + If the process does not terminate after timeout seconds. Default + is `settings.SUBPROCESS_TIMEOUT` + + + The side effects of executing the given command in this environment. + + + A tuple of command's exit code, standard output and standard error + contents. + + """ + if isinstance(cmd, six.string_types): + cmd = shlex.split(cmd) + else: + formats.LIST_OF_ANY_STRING_SCHEMA.check_match(cmd) + + # Use temporary files as targets for child process standard stream redirects + # They seem to work better (i.e. do not hang) than pipes, when using + # interactive commands like `vi`. + stdout_fd, stdout_name = tempfile.mkstemp() + stderr_fd, stderr_name = tempfile.mkstemp() + try: + with io.open(stdout_name, "r") as stdout_reader, \ + os.fdopen(stdout_fd, "w") as stdout_writer, \ + io.open(stderr_name, "r") as stderr_reader, \ + os.fdopen(stderr_fd, "w") as stderr_writer: + + # Start child , writing standard streams to temporary files + proc = subprocess.Popen(cmd, stdout=stdout_writer, + stderr=stderr_writer, universal_newlines=True) + proc_start_time = time.time() + + stdout_str = stderr_str = "" + stdout_part = stderr_part = "" + + # Read as long as the process runs or there is data on one of the streams + while proc.poll() is None or stdout_part or stderr_part: + + # Raise timeout error in they same manner as `subprocess` would do it + if (timeout is not None and + time.time() > proc_start_time + timeout): + proc.kill() + proc.wait() + raise subprocess.TimeoutExpired(cmd, timeout) + + # Read from child process's redirected streams, write to parent + # process's standard streams and construct retuirn values + stdout_part = stdout_reader.read() + stderr_part = stderr_reader.read() + sys.stdout.write(stdout_part) + sys.stderr.write(stderr_part) + sys.stdout.flush() + sys.stderr.flush() + stdout_str += stdout_part + stderr_str += stderr_part + + finally: + # The work is done or was interrupted, the temp files can be removed + os.remove(stdout_name) + os.remove(stderr_name) + + # Return process exit code and captured stream + return proc.poll(), stdout_str, stderr_str diff --git a/securesystemslib/gpg/util.py b/securesystemslib/gpg/util.py index 56712e39..568173a6 100644 --- a/securesystemslib/gpg/util.py +++ b/securesystemslib/gpg/util.py @@ -27,7 +27,7 @@ import securesystemslib.gpg.exceptions import securesystemslib.gpg.process -log = logging.getLogger('securesystemslib.gpg.common') +log = logging.getLogger('securesystemslib_gpg_common') def get_mpi_length(data): @@ -105,6 +105,9 @@ def parse_packet_header(data, expected_type=None): securesystemslib.gpg.exceptions.PacketParsingError + If the new format packet length encodes a partial body length + If the old format packet length encodes an indeterminate length + If header or body length could not be determined If the expected_type was passed and does not match the packet type @@ -112,24 +115,79 @@ def parse_packet_header(data, expected_type=None): The RFC4880-compliant packet payload, its length and its type. + (see RFC 4880 4.3. for the list of available packet types) """ data = bytearray(data) - packet_type = (data[0] & 0x3c ) >> 2 - packet_length_bytes = data[0] & 0x03 + header_len = None + body_len = None + + # If Bit 6 of 1st octet is set we parse a New Format Packet Length, and + # an Old Format Packet Lengths otherwise + if data[0] & 0x40: # pragma: no cover + # In new format packet lengths the packet type is encoded in Bits 5-0 of + # the 1st octet of the packet + packet_type = data[0] & 0x3f + + # The rest of the packet header is the body length header, which may + # consist of one, two or five octets. To disambiguate the RFC, the first + # octet of the body length header is the second octet of the packet. + if data[1] < 192: + header_len = 2 + body_len = data[1] + + elif data[1] >= 192 and data[1] <= 223: + header_len = 3 + body_len = (data[1] - 192 << 8) + data[2] + 192 + + elif data[1] >= 224 and data[1] < 255: + raise securesystemslib.gpg.exceptions.PacketParsingError("New length " + "format packets of partial body lengths are not supported") + + elif data[1] == 255: + header_len = 6 + body_len = data[2] << 24 | data[3] << 16 | data[4] << 8 | data[5] + + else: + # raise PacketParsingError below if lengths cannot be determined + pass - ptr = 3 - if packet_length_bytes == 1: - packet_length = struct.unpack(">H", data[1:ptr])[0] else: - packet_length = data[1] - ptr = 2 + # In old format packet lengths the packet type is encoded in Bits 5-2 of + # the 1st octet and the length type in Bits 1-0 + packet_type = (data[0] & 0x3c ) >> 2 + length_type = data[0] & 0x03 + + # The body length is encoded using one, two, or four octets, starting + # with the second octet of the packet + if length_type == 0: + body_len = data[1] + header_len = 2 + + elif length_type == 1: # pragma: no branch + header_len = 3 + body_len = struct.unpack(">H", data[1:header_len])[0] + + elif length_type == 2: # pragma: no cover + header_len = 5 + body_len = struct.unpack(">I", data[1:header_len])[0] + + elif length_type == 3: # pragma: no cover + raise securesystemslib.gpg.exceptions.PacketParsingError("Old length " + "format packets of indeterminate length are not supported") + + else: # pragma: no cover + # raise PacketParsingError below if lengths cannot be determined + pass + + if header_len == None or body_len == None: # pragma: no cover + raise securesystemslib.gpg.exceptions.PacketParsingError("Could not " + "determine packet length") if expected_type != None and packet_type != expected_type: # pragma: no cover - raise securesystemslib.gpg.exceptions.PacketParsingError( - "Expected packet {}, but got {} instead!".format(expected_type, - packet_type)) + raise securesystemslib.gpg.exceptions.PacketParsingError("Expected packet " + "{}, but got {} instead!".format(expected_type, packet_type)) - return data[ptr:ptr+packet_length], ptr+packet_length, packet_type + return data[header_len:header_len+body_len], header_len+body_len, packet_type def compute_keyid(pubkey_packet_data): @@ -190,12 +248,12 @@ def parse_subpackets(subpacket_octets): length = subpacket_octets[ptr] ptr += 1 - if length > 192 and length < 255 : # pragma: no cover + if length >= 192 and length < 255 : # pragma: no cover length = ((length - 192 << 8) + (subpacket_octets[ptr] + 192)) ptr += 1 if length == 255: # pragma: no cover - length = struct.unpack(">I", subpacket_octets[ptr:ptr + 4]) + length = struct.unpack(">I", subpacket_octets[ptr:ptr + 4])[0] ptr += 4 packet_type = subpacket_octets[ptr] @@ -221,10 +279,11 @@ def get_version(): """ command = securesystemslib.gpg.constants.GPG_VERSION_COMMAND - process = securesystemslib.gpg.process.run(command, - stdout=securesystemslib.gpg.process.PIPE, - stderr=securesystemslib.gpg.process.PIPE, - universal_newlines=True) + process = securesystemslib.gpg.process.run( + command, + stdout=securesystemslib.gpg.process.PIPE, + stderr=securesystemslib.gpg.process.PIPE, + universal_newlines=True) full_version_info = process.stdout version_string = re.search(r'(\d\.\d\.\d+)', full_version_info).group(1) diff --git a/tests/test_process.py b/tests/test_process.py index 9283a419..688f7b70 100644 --- a/tests/test_process.py +++ b/tests/test_process.py @@ -16,18 +16,21 @@ Test subprocess interface. """ +import io import os +import shlex +import sys import tempfile import unittest -import securesystemslib.exceptions import securesystemslib.gpg.process +import mock class Test_Process(unittest.TestCase): """Test subprocess interface. """ - def test_input_vs_stdin(self): + def test_run_input_vs_stdin(self): """Test that stdin kwarg is only used if input kwarg is not supplied. """ # Create a temporary file, passed as `stdin` argument @@ -45,18 +48,83 @@ def test_input_vs_stdin(self): # stdin is only used if input is not supplied securesystemslib.gpg.process.run(cmd.format("use stdin kwarg"), - stdin=stdin_file) + stdin=stdin_file) # Clean up stdin_file.close() os.remove(path) + def test_run_duplicate_streams(self): + """Test output as streams and as returned. """ + # Command that prints 'foo' to stdout and 'bar' to stderr. + cmd = ("python -c \"" + "import sys;" + "sys.stdout.write('foo');" + "sys.stderr.write('bar');\"") + + # Create and open fake targets for standard streams + stdout_fd, stdout_fn = tempfile.mkstemp() + stderr_fd, stderr_fn = tempfile.mkstemp() + with io.open(stdout_fn, "r") as fake_stdout_reader, \ + os.fdopen(stdout_fd, "w") as fake_stdout_writer, \ + io.open(stderr_fn, "r") as fake_stderr_reader, \ + os.fdopen(stderr_fd, "w") as fake_stderr_writer: + + # Backup original standard streams and redirect to fake targets + real_stdout = sys.stdout + real_stderr = sys.stderr + sys.stdout = fake_stdout_writer + sys.stderr = fake_stderr_writer + + # Run command + ret_code, ret_stdout, ret_stderr = \ + securesystemslib.gpg.process.run_duplicate_streams(cmd) + + # Rewind fake standard streams + fake_stdout_reader.seek(0) + fake_stderr_reader.seek(0) + + # Assert that what was printed and what was returned is correct + self.assertTrue(ret_stdout == fake_stdout_reader.read() == "foo") + self.assertTrue(ret_stderr == fake_stderr_reader.read() == "bar") + # Also assert the default return value + self.assertEqual(ret_code, 0) + + # Reset original streams + sys.stdout = real_stdout + sys.stderr = real_stderr + + # Remove fake standard streams + os.remove(stdout_fn) + os.remove(stderr_fn) + + + def test_run_duplicate_streams_arg_return_code(self): + """Test command arg as string and list and return code. """ + cmd_str = ("python -c \"" + "import sys;" + "sys.exit(100)\"") + cmd_list = shlex.split(cmd_str) + + for cmd in [cmd_str, cmd_list]: + return_code, _, _ = \ + securesystemslib.gpg.process.run_duplicate_streams(cmd) + self.assertEqual(return_code, 100) + + + def test_run_duplicate_streams_timeout(self): + """Test raise TimeoutExpired. """ + with self.assertRaises(securesystemslib.gpg.process.subprocess.TimeoutExpired): + securesystemslib.gpg.process.run_duplicate_streams("python --version", + timeout=-1) + def test_incorrect_cmd_argument(self): """Test that exception is raised when cmd argument is not a string. """ with self.assertRaises(securesystemslib.exceptions.FormatError): securesystemslib.gpg.process.run(1) + if __name__ == "__main__": unittest.main()