Skip to content

Commit

Permalink
Move and increase gpg command timeout
Browse files Browse the repository at this point in the history
Prior to this commit, settings.SUBPROCESS_TIMEOUT=3 (seconds) was
used as default and also to configure timeouts for any subprocess
spawned via the securesystemslib process interface, including
running the gpg command to sign, export keys and check version.

The small timeout regularly lead to test failures on slow test
runners. Furthermore, configuring the timeout via global is bad
practice and error-prone (see secure-systems-lab#219, secure-systems-lab#345).

This patch defines a custom constant, to be used only for gpg
subprocesses and increases its value to 10 seconds.

To override this default, an optional timeout argument will be
added to relevant gpg interfaces in a subsequent commit.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
  • Loading branch information
lukpueh committed Feb 9, 2023
1 parent e809bf9 commit c8a3f73
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 4 deletions.
9 changes: 8 additions & 1 deletion securesystemslib/gpg/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,20 @@

log = logging.getLogger(__name__)

GPG_TIMEOUT = 10


@functools.lru_cache(maxsize=3)
def is_available_gnupg(gnupg: str) -> bool:
"""Returns whether gnupg points to a gpg binary."""
gpg_version_cmd = gnupg + " --version"
try:
process.run(gpg_version_cmd, stdout=process.PIPE, stderr=process.PIPE)
process.run(
gpg_version_cmd,
stdout=process.PIPE,
stderr=process.PIPE,
timeout=GPG_TIMEOUT,
)
return True
except (OSError, subprocess.TimeoutExpired):
return False
Expand Down
6 changes: 5 additions & 1 deletion securesystemslib/gpg/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
)
from securesystemslib.gpg.constants import (
FULLY_SUPPORTED_MIN_VERSION,
GPG_TIMEOUT,
NO_GPG_MSG,
SHA256,
gpg_export_pubkey_command,
Expand Down Expand Up @@ -127,6 +128,7 @@ def create_signature(content, keyid=None, homedir=None):
check=False,
stdout=process.PIPE,
stderr=process.PIPE,
timeout=GPG_TIMEOUT,
)

# TODO: It's suggested to take a look at `--status-fd` for proper error
Expand Down Expand Up @@ -307,7 +309,9 @@ def export_pubkey(keyid, homedir=None):
# TODO: Consider adopting command error handling from `create_signature`
# above, e.g. in a common 'run gpg command' utility function
command = gpg_export_pubkey_command(keyid=keyid, homearg=homearg)
gpg_process = process.run(command, stdout=process.PIPE, stderr=process.PIPE)
gpg_process = process.run(
command, stdout=process.PIPE, stderr=process.PIPE, timeout=GPG_TIMEOUT
)

key_packet = gpg_process.stdout
key_bundle = get_pubkey_bundle(key_packet, keyid)
Expand Down
1 change: 1 addition & 0 deletions securesystemslib/gpg/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ def get_version() -> Version:
stdout=process.PIPE,
stderr=process.PIPE,
universal_newlines=True,
timeout=constants.GPG_TIMEOUT,
)

full_version_info = gpg_process.stdout
Expand Down
9 changes: 7 additions & 2 deletions tests/test_gpg.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
parse_signature_packet,
)
from securesystemslib.gpg.constants import (
GPG_TIMEOUT,
PACKET_TYPE_PRIMARY_KEY,
PACKET_TYPE_SUB_KEY,
PACKET_TYPE_USER_ATTR,
Expand Down Expand Up @@ -234,14 +235,18 @@ def setUpClass(self): # pylint: disable=bad-classmethod-argument
# erroneous gpg data in tests below.
keyid = "F557D0FF451DEF45372591429EA70BD13D883381"
cmd = gpg_export_pubkey_command(keyid=keyid, homearg=homearg)
proc = process.run(cmd, stdout=process.PIPE, stderr=process.PIPE)
proc = process.run(
cmd, stdout=process.PIPE, stderr=process.PIPE, timeout=GPG_TIMEOUT
)
self.raw_key_data = proc.stdout
self.raw_key_bundle = parse_pubkey_bundle(self.raw_key_data)

# Export pubkey bundle with expired key for key expiration tests
keyid = "E8AC80C924116DABB51D4B987CB07D6D2C199C7C"
cmd = gpg_export_pubkey_command(keyid=keyid, homearg=homearg)
proc = process.run(cmd, stdout=process.PIPE, stderr=process.PIPE)
proc = process.run(
cmd, stdout=process.PIPE, stderr=process.PIPE, timeout=GPG_TIMEOUT
)
self.raw_expired_key_bundle = parse_pubkey_bundle(proc.stdout)

def test_parse_pubkey_payload_errors(self):
Expand Down

0 comments on commit c8a3f73

Please sign in to comment.