MAVlink msg signing updated with recommended improvements

Signed-off-by: PritamP20 <pripritam7@gmail.com>

Author: PritamP20

INSTALL.md

@@ -116,6 +116,50 @@ You might need to also do:
 brew install uv python-tk@3.9
 ```
 
+## OS Keyring Setup (Optional - For MAVLink Signing)
+
+If you plan to use [MAVLink 2.0 message signing](https://mavlink.io/en/guide/message_signing.html),
+the application can securely store signing keys using your operating system's keyring.
+
+### Windows
+
+**No setup required** - Uses Windows Credential Manager (built-in).
+
+### macOS
+
+**No setup required** - Uses Keychain (built-in).
+
+### Linux
+
+Most desktop environments have keyring support pre-installed:
+
+- **GNOME**: Uses GNOME Keyring / Secret Service
+- **KDE**: Uses KWallet
+- **Headless/Server**: Falls back to encrypted file storage (no keyring)
+
+**If keyring is not installed**, install it for your distribution:
+
+```bash
+# Ubuntu/Debian (GNOME)
+sudo apt install gnome-keyring libsecret-1-0
+
+# Fedora (GNOME)
+sudo dnf install gnome-keyring libsecret
+
+# Arch Linux
+sudo pacman -S gnome-keyring libsecret
+```
+
+### Verifying Keyring Availability
+
+```python
+from ardupilot_methodic_configurator.backend_signing_keystore import SigningKeystore
+keystore = SigningKeystore()
+print(f"Keyring available: {keystore.keyring_available}")
+```
+
+If keyring is not available, the application automatically falls back to encrypted file storage.
+
 ## Install *Mission Planner* software on a PC or Mac
 
 1. Download and install [Mission Planner](https://firmware.ardupilot.org/Tools/MissionPlanner/).

ardupilot_methodic_configurator/backend_flightcontroller.py

@@ -486,7 +486,7 @@ def setup_signing(  # pylint: disable=too-many-arguments, too-many-positional-ar
         allow_unsigned_in: bool = True,
         initial_timestamp: int = 0,
         link_id: int = 0,
-    ) -> tuple[bool, str]:
+    ) -> bool:
         """
         Set up MAVLink 2.0 message signing for secure communication.
 
@@ -501,12 +501,13 @@ def setup_signing(  # pylint: disable=too-many-arguments, too-many-positional-ar
             link_id: Link ID for signing (0-255, default: 0)
 
         Returns:
-            tuple[bool, str]: (success, error_message)
-                - success is True if signing was set up successfully
-                - error_message is empty on success or contains error description
+            bool: True if signing was set up successfully
 
         Raises:
+            ConnectionError: If no flight controller connection is available
             ValueError: If key is not 32 bytes or link_id is out of range
+            NotImplementedError: If pymavlink version doesn't support signing
+            RuntimeError: If signing setup fails for other reasons
 
         Note:
             MAVLink signing provides authentication (not encryption).
@@ -515,9 +516,9 @@ def setup_signing(  # pylint: disable=too-many-arguments, too-many-positional-ar
 
         """
         if self.master is None:
-            error_msg = _("No flight controller connection available for signing setup")
-            logging_warning(error_msg)
-            return False, error_msg
+            msg = _("No flight controller connection available for signing setup")
+            logging_warning(msg)
+            raise ConnectionError(msg)
 
         if len(key) != 32:
             msg = f"Signing key must be 32 bytes, got {len(key)} bytes"
@@ -543,16 +544,16 @@ def setup_signing(  # pylint: disable=too-many-arguments, too-many-positional-ar
                 _("MAVLink signing configured: sign_outgoing=%(sign)s, allow_unsigned=%(unsigned)s"),
                 {"sign": sign_outgoing, "unsigned": allow_unsigned_in},
             )
-            return True, ""
+            return True
 
-        except AttributeError:
-            error_msg = _("MAVLink signing not supported by this pymavlink version")
-            logging_error(error_msg)
-            return False, error_msg
-        except Exception as exc:  # pylint: disable=broad-except
-            error_msg = _("Failed to set up MAVLink signing: %(error)s") % {"error": str(exc)}
-            logging_error(error_msg)
-            return False, error_msg
+        except AttributeError as exc:
+            msg = _("MAVLink signing not supported by this pymavlink version")
+            logging_error(msg)
+            raise NotImplementedError(msg) from exc
+        except Exception as exc:
+            msg = _("Failed to set up MAVLink signing: %(error)s") % {"error": str(exc)}
+            logging_error(msg)
+            raise RuntimeError(msg) from exc
 
     def _unsigned_callback(self, msg: object) -> bool:
         """
@@ -581,37 +582,40 @@ def _unsigned_callback(self, msg: object) -> bool:
         # This allows communication during signing setup and transition periods
         return True
 
-    def disable_signing(self) -> tuple[bool, str]:
+    def disable_signing(self) -> bool:
         """
         Disable MAVLink message signing.
 
         This removes signing configuration and returns to unsigned communication.
 
         Returns:
-            tuple[bool, str]: (success, error_message)
-                - success is True if signing was disabled successfully
-                - error_message is empty on success or contains error description
+            bool: True if signing was disabled successfully
+
+        Raises:
+            ConnectionError: If no flight controller connection is available
+            NotImplementedError: If pymavlink version doesn't support signing
+            RuntimeError: If disabling signing fails for other reasons
 
         """
         if self.master is None:
-            error_msg = _("No flight controller connection available")
-            logging_warning(error_msg)
-            return False, error_msg
+            msg = _("No flight controller connection available")
+            logging_warning(msg)
+            raise ConnectionError(msg)
 
         try:
             # Disable signing by passing None as key
             # Type ignore needed because MavlinkConnection is a Union including object fallback
             self.master.setup_signing(None, sign_outgoing=False, allow_unsigned_callback=None)  # type: ignore[union-attr]
             logging_info(_("MAVLink signing disabled"))
-            return True, ""
-        except AttributeError:
-            error_msg = _("MAVLink signing not supported by this pymavlink version")
-            logging_error(error_msg)
-            return False, error_msg
-        except Exception as exc:  # pylint: disable=broad-except
-            error_msg = _("Failed to disable MAVLink signing: %(error)s") % {"error": str(exc)}
-            logging_error(error_msg)
-            return False, error_msg
+            return True
+        except AttributeError as exc:
+            msg = _("MAVLink signing not supported by this pymavlink version")
+            logging_error(msg)
+            raise NotImplementedError(msg) from exc
+        except Exception as exc:
+            msg = _("Failed to disable MAVLink signing: %(error)s") % {"error": str(exc)}
+            logging_error(msg)
+            raise RuntimeError(msg) from exc
 
     def get_signing_status(self) -> dict[str, object]:
         """

ardupilot_methodic_configurator/backend_signing_config.py

@@ -0,0 +1,185 @@
+"""
+MAVLink 2.0 signing configuration persistence backend.
+
+This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator
+
+SPDX-FileCopyrightText: 2024-2025 Amilcar do Carmo Lucas <amilcar.lucas@iav.de>
+
+SPDX-License-Identifier: GPL-3.0-or-later
+"""
+
+import json
+import logging
+import os
+from pathlib import Path
+from typing import Any, Optional
+
+from platformdirs import user_data_dir
+
+from ardupilot_methodic_configurator.data_model_signing_config import (
+    CONFIG_VERSION,
+    VehicleSigningConfig,
+)
+
+
+class SigningConfigManager:
+    """
+    Manager for loading and saving signing configurations.
+
+    This class handles persistence of signing configurations to JSON files
+    with version control and concurrent access protection.
+
+    Config file format:
+    {
+        "version": 1,
+        "configs": {
+            "vehicle_id": {...}
+        }
+    }
+    """
+
+    def __init__(self, config_dir: Optional[Path] = None) -> None:
+        """
+        Initialize the configuration manager.
+
+        Args:
+            config_dir: Directory for storing configuration files.
+                       If None, uses platform-appropriate data directory.
+
+        """
+        self._logger = logging.getLogger(__name__)
+
+        if config_dir is None:
+            config_dir = Path(user_data_dir("ArduPilot Methodic Configurator"))
+
+        self._config_dir = config_dir
+        self._config_file = config_dir / "signing_configs.json"
+
+    def load_vehicle_config(self, vehicle_id: str) -> Optional[VehicleSigningConfig]:
+        """
+        Load signing configuration for a specific vehicle.
+
+        Args:
+            vehicle_id: Vehicle identifier
+
+        Returns:
+            VehicleSigningConfig if found, None otherwise
+
+        """
+        configs = self._load_all_configs()
+        if vehicle_id in configs:
+            try:
+                return VehicleSigningConfig.from_dict(configs[vehicle_id])
+            except (KeyError, ValueError) as exc:
+                self._logger.warning("Failed to load config for %s: %s", vehicle_id, exc)
+        return None
+
+    def save_vehicle_config(self, config: VehicleSigningConfig) -> bool:
+        """
+        Save signing configuration for a vehicle.
+
+        Args:
+            config: Vehicle signing configuration to save
+
+        Returns:
+            bool: True if saved successfully
+
+        """
+        try:
+            configs = self._load_all_configs()
+            configs[config.vehicle_id] = config.to_dict()
+            self._save_all_configs(configs)
+            return True
+        except Exception as exc:  # pylint: disable=broad-except
+            self._logger.exception("Failed to save config: %s", exc)
+            return False
+
+    def delete_vehicle_config(self, vehicle_id: str) -> bool:
+        """
+        Delete signing configuration for a vehicle.
+
+        Args:
+            vehicle_id: Vehicle identifier
+
+        Returns:
+            bool: True if deleted, False if not found
+
+        """
+        try:
+            configs = self._load_all_configs()
+            if vehicle_id in configs:
+                del configs[vehicle_id]
+                self._save_all_configs(configs)
+                return True
+            return False
+        except Exception as exc:  # pylint: disable=broad-except
+            self._logger.exception("Failed to delete config: %s", exc)
+            return False
+
+    def list_configured_vehicles(self) -> list[str]:
+        """
+        List all vehicles with saved configurations.
+
+        Returns:
+            list[str]: List of vehicle IDs
+
+        """
+        configs = self._load_all_configs()
+        return sorted(configs.keys())
+
+    def _load_all_configs(self) -> dict[str, Any]:
+        """Load all configurations from file with version handling."""
+        if not self._config_file.exists():
+            return {}
+
+        try:
+            with open(self._config_file, encoding="utf-8") as f:
+                data: dict[str, Any] = json.load(f)
+
+                if not isinstance(data, dict):
+                    self._logger.warning("Config file contains invalid data structure (not a dictionary)")
+                    return {}
+
+                version = data.get("version", 1)
+                if version > CONFIG_VERSION:
+                    self._logger.warning(
+                        "Config file version %d is newer than supported version %d. Some features may not work correctly.",
+                        version,
+                        CONFIG_VERSION,
+                    )
+
+                configs: dict[str, Any] = data.get("configs", data)
+                configs.pop("version", None)
+                return configs
+
+        except (json.JSONDecodeError, OSError) as exc:
+            self._logger.warning("Failed to load configs: %s", exc)
+            return {}
+
+    def _save_all_configs(self, configs: dict[str, Any]) -> None:
+        """Save all configurations to file with version and file locking."""
+        self._config_dir.mkdir(parents=True, exist_ok=True)
+
+        versioned_data = {
+            "version": CONFIG_VERSION,
+            "configs": configs,
+        }
+
+        temp_file = self._config_file.with_suffix(".tmp")
+        try:
+            with open(temp_file, "w", encoding="utf-8") as f:
+                if os.name != "nt":
+                    import fcntl  # noqa: PLC0415 # pylint: disable=import-outside-toplevel
+
+                    fcntl.flock(f.fileno(), fcntl.LOCK_EX)
+                json.dump(versioned_data, f, indent=2)
+
+            os.replace(temp_file, self._config_file)
+
+            if os.name != "nt":
+                os.chmod(self._config_file, 0o600)
+
+        except Exception:
+            if temp_file.exists():
+                temp_file.unlink()
+            raise