Add generic functions to read files and configurations from SFTP remote

-Move SFTP functionality sign_windows_installer -> read_remote_config
-Remove old functions for getting the remote .ini from an URL
-Add more generic functions to transfer files and read .ini configs
-Add command line options for new functionality
-Add unit tests for new functionality
-Avoid writing decrypted private key to file system
-Catch the new exception type PackagingError where relevant

Change-Id: I4b2c4341c2fc1795847fa0c193bcbc47869c61ad
Reviewed-by: Akseli Salovaara <akseli.salovaara@qt.io>

6 files changed, 398 insertions, 94 deletions

diff --git a/Pipfile b/Pipfile
index d5a4509de..6c257e973 100644
--- a/Pipfile
+++ b/Pipfile
@@ -43,7 +43,7 @@ pytest-xdist = "==2.5.0"
 types-requests = "==2.28.2"
 flake8-use-pathlib = "==0.2.1"
 types-aiofiles = "==0.8.0"
-
+types-paramiko = "==2.12.0.1"
 
 [requires]
 python_version = "3"  # Pipfile doesn't support specifying minimum version (>=3.6.2)
diff --git a/packaging-tools/notarize.py b/packaging-tools/notarize.py
index 31d389bac..3580c0388 100755
--- a/packaging-tools/notarize.py
+++ b/packaging-tools/notarize.py
@@ -40,6 +40,7 @@ from subprocess import CalledProcessError
 from typing import Optional
 
 from bld_utils import is_macos
+from installer_utils import PackagingError
 from logging_util import init_logger
 from read_remote_config import get_pkg_value
 from runner import run_cmd, run_cmd_silent
@@ -225,14 +226,11 @@ def key_from_remote_env(key: str) -> str:
     Returns:
         Returned value from get_pkg_value if no exception was handled or an empty string (str)
 
-    Raises:
-        Exception: Raised by get_pkg_value, handled by the function
-
     """
     try:
         return get_pkg_value(key)
-    except Exception:
-        return ""
+    except PackagingError:
+        return ""  # Do not raise here if remote environment is not in use
 
 
 def check_notarize_reqs() -> None:
diff --git a/packaging-tools/read_remote_config.py b/packaging-tools/read_remote_config.py
index d4c854280..c7c987865 100755
--- a/packaging-tools/read_remote_config.py
+++ b/packaging-tools/read_remote_config.py
@@ -34,26 +34,184 @@ import os
 import sys
 from configparser import ConfigParser
 from io import StringIO
-from typing import Any, Optional
-from urllib.request import urlopen
+from pathlib import Path
+from tempfile import TemporaryDirectory
+from typing import Dict, Optional, Union
 
+import pysftp  # type: ignore
+from cryptography.fernet import Fernet, InvalidToken
+from paramiko import (
+    AgentKey,
+    AuthenticationException,
+    PasswordRequiredException,
+    RSAKey,
+    SSHException,
+)
+
+from installer_utils import PackagingError
 from logging_util import init_logger
 
 log = init_logger(__name__, debug_mode=False)
 
 
-class RemotePkgConfigError(Exception):
+class RemoteConfigError(Exception):
     pass
 
 
-def read_packaging_keys_config_url(url: str) -> Any:
-    return urlopen(url).read().decode('utf-8').strip()
+def _get_private_key() -> bytes:
+    """
+    Read encrypted private key file from default path
+
+    Returns:
+        Key file content in a bytes array
+
+    Raises:
+        PackagingError: When path is invalid or file cannot be found
+    """
+    try:
+        k_path = Path.home() / "sshkeys" / os.environ["ID_RSA_FILE"]
+        k_path.resolve(strict=True)
+    except KeyError as err:
+        raise PackagingError("Could not determine private key path from env") from err
+    except FileNotFoundError as err:
+        raise PackagingError(f"Failed to locate private key from path: {k_path}") from err
+    log.info("Reading the private key: %s", k_path)
+    with k_path.open("rb") as private_key:
+        return private_key.read()
+
+
+def _get_decrypt_key() -> bytes:
+    """
+    Read Fernet decryption key from default path
+
+    Returns:
+        Key file content in a bytes array
+
+    Raises:
+        PackagingError: When path is invalid or file cannot be found
+    """
+    try:
+        k_path = Path(os.environ["PKG_NODE_ROOT"], os.environ["FILES_SHARE_PATH"])
+        k_path.resolve(strict=True)
+    except KeyError as err:
+        raise PackagingError("Could not determine decryption key path from env") from err
+    except FileNotFoundError as err:
+        raise PackagingError(f"Failed to locate decryption key from path: {k_path}") from err
+    log.info("Reading the pre-generated Fernet key: %s", k_path)
+    with open(k_path, "rb") as decrypt_key:
+        return decrypt_key.read()
+
+
+def _decrypt_private_key(key: bytes, decrypt_key: bytes) -> RSAKey:
+    """
+    Decrypt a Fernet encrypted key and return a RSA key object containing the decrypted key
+
+    Args:
+        key: Encrypted content to be decrypted (Fernet token)
+        decrypt_key: Key for decryption (Fernet base64-encoded 32-byte key)
+
+    Raises:
+        PackagingError: Raised on the decryption failures or if the resulting data is not valid
+    """
+    log.info("Decrypting private key using pre-generated Fernet key")
+    try:
+        fernet = Fernet(decrypt_key)
+        decrypted_key = fernet.decrypt(key)
+    except InvalidToken:
+        raise PackagingError("Failed to decrypt private key, got invalid Fernet token") from None
+    try:
+        return RSAKey(file_obj=StringIO(decrypted_key.decode(encoding="utf-8")))
+    except SSHException:
+        raise PackagingError("Failed to create RSA key object, invalid key format?") from None
+
+
+def download_remote_file_sftp(
+    remote_host: Optional[str] = os.getenv("SFTP_ADDRESS"),
+    remote_path: Optional[Path] = None,
+    local_path: Optional[Path] = None,
+    username: Optional[str] = os.getenv("SFTP_USER"),
+    private_key: Optional[Union[str, RSAKey, AgentKey]] = None
+) -> None:
+    """
+    Transfer the given remote file to a given folder via SFTP
+
+    Args:
+        remote_host: An address or a hostname for the remote server
+        remote_path: A file system path on the remote server to transfer from
+        local_path: A file system path where to save the file, by default set to current work dir
+        username: Name used for authenticating with the remote server
+        private_key: A private key object or string path to a key file when not using the default
 
+    Raises:
+        PackagingError: Raised on missing arguments
+        RemoteConfigError: Re-raised on SFTP errors from pysftp.Connection
+    """
+    if private_key is None:  # get default RSA private key
+        private_key = _decrypt_private_key(_get_private_key(), _get_decrypt_key())
+    if not remote_host or remote_path is None:
+        raise PackagingError("Remote host address and/or source path not specified")
+    if not all((username, private_key)):
+        raise PackagingError("SSH public key authentication options not specified")
+    log.debug("Transfer '%s:%s' -> '%s'", remote_host, remote_path, local_path)
+    cnopts = pysftp.CnOpts()
+    cnopts.hostkeys = None  # disable host key checking
+    try:
+        with pysftp.Connection(
+            host=remote_host,
+            username=username,
+            private_key=private_key,
+            cnopts=cnopts,
+        ) as sftp:
+            sftp.get(remotepath=remote_path.as_posix(), localpath=local_path)
+    except pysftp.ConnectionException:
+        raise RemoteConfigError("Connection to the remote server failed") from None
+    except pysftp.CredentialException:
+        raise RemoteConfigError("Problem with credentials") from None
+    except (IOError, OSError):
+        raise RemoteConfigError("File doesn't exist on the remote or unable to save it") from None
+    except PasswordRequiredException:
+        raise RemoteConfigError("Private key was not decrypted before use") from None
+    except AuthenticationException:
+        raise RemoteConfigError("Authenticating with credentials failed") from None
+    except SSHException:
+        raise RemoteConfigError("SSH2 protocol failure") from None
 
-def parse_packaging_keys_config(config: str) -> ConfigParser:
+
+def _read_remote_config_sftp(remote_ini_path: Path) -> str:
+    """
+    Transfer the given remote config file to a temporary dir via SFTP and return the file content
+
+    Args:
+        remote_ini_path: A file system path on the remote host to read from
+
+    Returns:
+        Remote config .ini contents
+
+    Raises:
+        RemoteConfigError: Re-raised on config download error from download_remote_file_sftp
+    """
+    with TemporaryDirectory() as temp_dir:
+        local_path = Path(temp_dir) / "config.ini"
+        try:
+            download_remote_file_sftp(remote_path=remote_ini_path, local_path=local_path)
+        except RemoteConfigError as err:
+            raise RemoteConfigError("Failed to receive remote config!") from err
+        with open(local_path, "rb") as config:
+            return config.read().decode('utf-8').strip()
+
+
+def _parse_remote_config(config: str) -> ConfigParser:
+    """
+    Parse config using ConfigParser
+
+    Args:
+        config: A string containing the .ini file content
+
+    Returns:
+        An instance of ConfigParser with the parsed config
+    """
     buf = StringIO(config)
     settings = ConfigParser()
-
     settings.read_file(buf)
     return settings
 
@@ -61,29 +219,104 @@ def parse_packaging_keys_config(config: str) -> ConfigParser:
 def get_pkg_value(
     key: str,
     section: str = "packaging",
-    url: Optional[str] = os.getenv("PACKAGING_KEYS_CONFIG_URL"),
+    remote_cfg_path: Optional[Path] = None
 ) -> str:
-    if getattr(get_pkg_value, 'pkg_remote_settings', None) is None:
-        if not url:
-            raise RemotePkgConfigError("Remote config URL not specified")
-        config = read_packaging_keys_config_url(url)
-        get_pkg_value.pkg_remote_settings = parse_packaging_keys_config(config)  # type: ignore
-    return get_pkg_value.pkg_remote_settings.get(section, key)  # type: ignore
+    """
+    Get value for section and key in remote packaging config ini (sftp)
+    Configs dict will be cached as a function attribute 'cfg_cache' for future calls
+
+    Args:
+        key: A key in the config section
+        section: A section in the config (if empty, first section is used)
+        remote_cfg_path: A file system location for the config file on the remote
+
+    Returns:
+        Value for key (and section) or empty string if it doesn't exist
+
+    Raises:
+        PackagingError: When the config path is not specified or found
+    """
+    # Use the default packaging config ini from env if not specified
+    if remote_cfg_path is None:
+        try:
+            default_config_path_env = os.environ["PACKAGING_KEYS_CONFIG_PATH"]
+        except KeyError as err:
+            raise PackagingError("Remote config path not found from env or not specified") from err
+        remote_cfg_path = Path(default_config_path_env)
+    # Cache config to a function attribute
+    if getattr(get_pkg_value, 'cfg_cache', None) is None:
+        get_pkg_value.cfg_cache: Dict[Path, ConfigParser] = {}  # type: ignore
+    if get_pkg_value.cfg_cache.get(remote_cfg_path, None) is None:  # type: ignore
+        try:
+            config = _read_remote_config_sftp(remote_cfg_path)
+        except RemoteConfigError as err:
+            raise RemoteConfigError("Error while receiving config from the server") from err
+        get_pkg_value.cfg_cache[remote_cfg_path] = _parse_remote_config(config)  # type: ignore
+    # Use the first section if an empty section was specified
+    section = section or get_pkg_value.cfg_cache[remote_cfg_path].sections()[0]  # type: ignore
+    # Return the value for the key, or an empty string
+    return get_pkg_value.cfg_cache[remote_cfg_path].get(section, key, fallback="")  # type: ignore
 
 
 def main() -> None:
     """Main"""
     parser = argparse.ArgumentParser(prog="Read values from remote config .ini file")
-    parser.add_argument("--url", dest="url", type=str, default=os.getenv("PACKAGING_KEYS_CONFIG_URL"),
-                        help="Url pointing to file to be read")
-    parser.add_argument("--section", type=str, default="packaging", help="The config section within the .ini")
-    parser.add_argument("--key", type=str, required=True, help="The config key within the section")
+    subparsers = parser.add_subparsers(dest="command")
+    # Subparser for read-remote-env
+    p_read_remote = subparsers.add_parser(
+        "read-remote-env", help="Read environment value from SFTP remote config"
+    )
+    p_read_remote.add_argument(
+        "--config", dest="config", type=str, default=os.getenv("PACKAGING_KEYS_CONFIG_PATH"),
+        help="A file system path on the remote pointing to file to be read"
+    )
+    p_read_remote.add_argument(
+        "--section", type=str, default="packaging", help="The config section within the .ini"
+    )
+    p_read_remote.add_argument(
+        "--key", type=str, required=True, help="The config key within the section"
+    )
+    # Subparser for fetch-remote-file
+    p_fetch_file = subparsers.add_parser(
+        "fetch-remote-file", help="Fetch a file from a remote SFTP server"
+    )
+    p_fetch_file.add_argument(
+        "--remote-path", type=str, required=True, help="Remote sftp path e.g. [user@][server:]path"
+    )
+    p_fetch_file.add_argument(
+        "--output-path", type=Path, default=None, help="Local save path for file (default=cwd)"
+    )
+    # Parse args
     args = parser.parse_args(sys.argv[1:])
-    if not args.url or not args.section:
+    if args.command == "read-remote-env":
+        if not all((args.config, args.section, args.key)):
+            p_read_remote.print_help(sys.stderr)
+            raise SystemExit("Invalid/missing arguments for read-remote-env")
+        log.info("%s: '%s'", args.key, get_pkg_value(args.key, args.section, args.config))
+    elif args.command == "fetch-remote-file":
+        if not args.remote_path:
+            p_fetch_file.print_help(sys.stderr)
+            raise SystemExit("Missing --remote-path for fetch-remote-file")
+        username = None
+        hostname = None
+        try:
+            if "@" in args.remote_path:  # user@server:path
+                username, args.remote_path = args.remote_path.split("@")
+                hostname, args.remote_path = args.remote_path.split(":")
+            elif ":" in args.remote_path:  # server:path
+                hostname, args.remote_path = args.remote_path.split(":")
+            remote_path = Path(args.remote_path)
+        except ValueError:
+            p_fetch_file.print_help(sys.stderr)
+            raise SystemExit("Invalid --remote-path: Expected [user@][server:]path") from None
+        download_remote_file_sftp(
+            remote_host=hostname or os.getenv("SFTP_ADDRESS"),
+            remote_path=remote_path,
+            local_path=args.output_path,
+            username=username or os.getenv("SFTP_USER"),
+        )
+    else:
         parser.print_help(sys.stderr)
-        raise SystemExit("--url or --section missing")
-
-    log.info("%s: '%s'", args.key, get_pkg_value(args.key, args.section, args.url))
 
 
 if __name__ == "__main__":
diff --git a/packaging-tools/sign_windows_installer.py b/packaging-tools/sign_windows_installer.py
index b7d10b1a6..f9e044bf3 100755
--- a/packaging-tools/sign_windows_installer.py
+++ b/packaging-tools/sign_windows_installer.py
@@ -33,7 +33,6 @@ import argparse
 import os
 import subprocess
 import sys
-from configparser import ConfigParser
 from datetime import datetime
 from pathlib import Path
 from subprocess import DEVNULL
@@ -41,46 +40,33 @@ from time import time
 from typing import List
 
 import pysftp  # type: ignore
-from cryptography.fernet import Fernet
 
 from installer_utils import PackagingError
 from logging_util import init_logger
+from read_remote_config import download_remote_file_sftp, get_pkg_value
 
 log = init_logger(__name__, debug_mode=False)
 timestamp = datetime.fromtimestamp(time()).strftime('%Y-%m-%d--%H:%M:%S')
 
 
-def _get_home_dir() -> str:
-    home_dir = os.getenv("HOME") or os.getenv("USERPROFILE")
-    if not home_dir:
-        raise PackagingError("Failed to determine home directory.")
-    return home_dir
-
-
-def _get_private_key() -> bytes:
-    log.info("Return the private key in the build agent")
-    k_path = Path(_get_home_dir(), "sshkeys", os.environ["ID_RSA_FILE"]).resolve(strict=True)
-    with open(k_path, "rb") as private_key:
-        return private_key.read()
-
-
-def _get_decrypt_key() -> bytes:
-    log.info("Return the pre-generated Fernet key")
-    k_path = Path(os.environ["PKG_NODE_ROOT"], os.environ["FILES_SHARE_PATH"]).resolve(strict=True)
-    with open(k_path, "rb") as decrypt_key:
-        return decrypt_key.read()
-
-
-def _handle_signing(file_path: str) -> None:
-    config = ConfigParser()
-    config.read(os.path.basename(os.environ["WINDOWS_SIGNKEYS_PATH"]))
-    section = config.sections()[0]
-    if section in config:
-        kvu = config[section]['kvu']
-        kvi = config[section]['kvi']
-        kvs = config[section]['kvs']
-        kvc = config[section]['kvc']
-        tr_sect = config[section]['tr']
+def _handle_signing(file_path: str, verify_signtool: str) -> None:
+    """
+    Sign executable from file_path using AzureSignTool with the configured options
+    Verify the signing with the verify_signtool specified
+
+    Args:
+        file_path: A string path to the file to be signed
+        verify_signtool: Name of the signtool executable used for verification
+
+    Raises:
+        PackagingError: When signing or verification is unsuccessful
+    """
+    remote_config = Path(os.environ["WINDOWS_SIGNKEYS_PATH"])
+    kvu = get_pkg_value("kvu", "", remote_config)
+    kvi = get_pkg_value("kvi", "", remote_config)
+    kvs = get_pkg_value("kvs", "", remote_config)
+    kvc = get_pkg_value("kvc", "", remote_config)
+    tr_sect = get_pkg_value("tr", "", remote_config)
     cmd_args_sign = ["AzureSignTool.exe", "sign", "-kvu", kvu, "-kvi", kvi, "-kvs", kvs, "-kvc", kvc, "-tr", tr_sect, "-v", file_path]
     log_entry = cmd_args_sign[:]
     log_entry[3] = "****"
@@ -91,57 +77,41 @@ def _handle_signing(file_path: str) -> None:
     log.info("Calling: %s", ' '.join(log_entry))
     sign_result = subprocess.run(cmd_args_sign, stdout=DEVNULL, stderr=DEVNULL, check=False)
     if sign_result.returncode != 0:
-        raise PackagingError(f"Package {file_path} signing  with error {sign_result.returncode}")
+        raise PackagingError(f"Package {file_path} signing with error {sign_result.returncode}")
     log.info("Successfully signed: %s", file_path)
-    signtool = Path(os.environ["WINDOWS_SIGNTOOL_X64_PATH"]).name
-    cmd_args_verify: List[str] = [signtool, "verify", "-pa", file_path]
+    cmd_args_verify: List[str] = [verify_signtool, "verify", "-pa", file_path]
     verify_result = subprocess.run(cmd_args_verify, stdout=DEVNULL, stderr=DEVNULL, check=False)
     if verify_result.returncode != 0:
         raise PackagingError(f"Failed to verify {file_path} with error {verify_result.returncode}")
     log.info("Successfully verified: %s", file_path)
 
 
-def decrypt_private_key() -> str:
-    log.info("decrypt private key using pre-generated Fernet key")
-    key = _get_decrypt_key()
-    fernet = Fernet(key)
-    decrypted_key = fernet.decrypt(_get_private_key())
-    temp_key_path = os.environ["PKG_NODE_ROOT"]
-    temp_file = os.path.join(temp_key_path, "temp_keyfile")
-    with open(temp_file, 'wb') as outfile:
-        outfile.write(decrypted_key)
-    return temp_file
-
-
-def download_signing_tools(path_to_key: str) -> None:
+def download_signing_tools(signtool: Path) -> None:
     try:
         cnopts = pysftp.CnOpts()
         cnopts.hostkeys = None
-        with pysftp.Connection(os.getenv("SFTP_ADDRESS"), username=os.getenv("SFTP_USER"), private_key=path_to_key, cnopts=cnopts) as sftp:
-            sftp.get(os.getenv("WINDOWS_SIGNKEYS_PATH"))
-            sftp.get(os.getenv("WINDOWS_SIGNTOOL_X64_PATH"))
-    except pysftp.SSHException:
-        raise PackagingError("FTP authentication failed!") from None
+        download_remote_file_sftp(remote_path=signtool)
+    except PackagingError:
+        raise PackagingError("Failed to download signing tools!") from None
 
 
 def sign_executable(file_path: str) -> None:
     log.info("Signing: %s", file_path)
     try:
-        key_path: str = decrypt_private_key()
-        download_signing_tools(key_path)
+        signtool = os.environ["WINDOWS_SIGNTOOL_X64_PATH"]
+    except KeyError as err:
+        raise PackagingError("Signtool path not found from env") from err
+    try:
+        download_signing_tools(Path(signtool))
         path = Path(file_path)
         if path.is_dir():
             for subpath in path.rglob('*'):
                 if subpath.is_file() and subpath.suffix in ['.exe', '.dll', '.pyd']:
-                    _handle_signing(str(subpath))
+                    _handle_signing(str(subpath), Path(signtool).name)
         else:
-            _handle_signing(file_path)
+            _handle_signing(file_path, Path(signtool).name)
     finally:
-        # cleanup temporary files
-        if "key_path" in locals():
-            os.remove(key_path)
-        os.remove(os.path.basename(os.environ["WINDOWS_SIGNKEYS_PATH"]))
-        os.remove(os.path.basename(os.environ["WINDOWS_SIGNTOOL_X64_PATH"]))
+        Path(Path.cwd() / Path(signtool).name).unlink()
 
 
 def main() -> None:
diff --git a/packaging-tools/tests/test_read_remote_config.py b/packaging-tools/tests/test_read_remote_config.py
new file mode 100644
index 000000000..3a3e72f5e
--- /dev/null
+++ b/packaging-tools/tests/test_read_remote_config.py
@@ -0,0 +1,103 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+
+#############################################################################
+#
+# Copyright (C) 2022 The Qt Company Ltd.
+# Contact: https://www.qt.io/licensing/
+#
+# This file is part of the release tools of the Qt Toolkit.
+#
+# $QT_BEGIN_LICENSE:GPL-EXCEPT$
+# Commercial License Usage
+# Licensees holding valid commercial Qt licenses may use this file in
+# accordance with the commercial license agreement provided with the
+# Software or, alternatively, in accordance with the terms contained in
+# a written agreement between you and The Qt Company. For licensing terms
+# and conditions see https://www.qt.io/terms-conditions. For further
+# information use the contact form at https://www.qt.io/contact-us.
+#
+# GNU General Public License Usage
+# Alternatively, this file may be used under the terms of the GNU
+# General Public License version 3 as published by the Free Software
+# Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+# included in the packaging of this file. Please review the following
+# information to ensure the GNU General Public License requirements will
+# be met: https://www.gnu.org/licenses/gpl-3.0.html.
+#
+# $QT_END_LICENSE$
+#
+#############################################################################
+
+import unittest
+from io import StringIO
+from pathlib import Path
+from typing import Any
+from unittest.mock import patch
+
+from cryptography.fernet import Fernet
+from ddt import data, ddt, unpack  # type: ignore
+from paramiko import RSAKey
+
+from installer_utils import PackagingError
+from read_remote_config import _decrypt_private_key, get_pkg_value
+
+
+def get_packaging_ini(_: Any) -> str:
+    return """
+    [foo_section]
+    foo=foo
+    [packaging]
+    foo=foobar
+    [bar_section]
+    bar=bar
+    """
+
+
+@ddt
+class TestRemoteConfig(unittest.TestCase):
+
+    def test_decrypt_private_key(self) -> None:
+        decrypt_key = Fernet.generate_key()
+        rsa_key = RSAKey.generate(bits=1024)
+        rsa_secret = StringIO()
+        rsa_key.write_private_key(rsa_secret)
+        rsa_secret.seek(0)
+        encrypted_secret = Fernet(decrypt_key).encrypt(bytes(rsa_secret.read(), encoding="utf-8"))
+        result = _decrypt_private_key(key=encrypted_secret, decrypt_key=decrypt_key)
+        self.assertEqual(result, rsa_key)
+
+    def test_decrypt_private_key_invalid_secret(self) -> None:
+        decrypt_key = Fernet.generate_key()
+        encrypted_secret = Fernet(decrypt_key).encrypt(b"")
+        with self.assertRaises(PackagingError):
+            _decrypt_private_key(key=encrypted_secret, decrypt_key=decrypt_key)
+
+    def test_decrypt_private_key_invalid_token(self) -> None:
+        decrypt_key = Fernet.generate_key()
+        with self.assertRaises(PackagingError):
+            _decrypt_private_key(key=b"", decrypt_key=decrypt_key)
+
+    def test_decrypt_private_key_invalid_fernet_key(self) -> None:
+        with self.assertRaises(ValueError):
+            _decrypt_private_key(key=b"", decrypt_key=b"")
+
+    @data(  # type: ignore
+        ("missing", "missing", ""),
+        ("packaging", "missing", ""),
+        ("", "foo", "foo"),
+        ("packaging", "foo", "foobar"),
+        ("foo_section", "foo", "foo"),
+        ("bar_section", "bar", "bar"),
+    )
+    @unpack  # type: ignore
+    @patch(
+        "read_remote_config._read_remote_config_sftp", side_effect=get_packaging_ini
+    )
+    def test_get_pkg_value(self, section: str, key: str, expected_result: str, _: Any) -> None:
+        result = get_pkg_value(key=key, section=section, remote_cfg_path=Path())
+        self.assertEqual(result, expected_result)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/packaging-tools/tests/testhelpers.py b/packaging-tools/tests/testhelpers.py
index aa6e15eef..73b4000ca 100644
--- a/packaging-tools/tests/testhelpers.py
+++ b/packaging-tools/tests/testhelpers.py
@@ -36,6 +36,7 @@ from subprocess import PIPE
 from typing import Any, Callable
 
 from bld_utils import is_windows
+from installer_utils import PackagingError
 from read_remote_config import get_pkg_value
 
 if sys.version_info < (3, 7):
@@ -73,6 +74,5 @@ def is_internal_file_server_reachable() -> bool:
         ping = sh.which("ping")
         ret = subprocess.run(args=[ping, "-c", "1", package_server], timeout=5, stdout=PIPE, stderr=PIPE, check=False)
         return ret.returncode == 0
-    except Exception:
-        pass
-    return False
+    except (sh.ErrorReturnCode, PackagingError):
+        return False