From d212e6ab44d6ada75b96e7ad4041f75ca5f40937 Mon Sep 17 00:00:00 2001 From: Florian Date: Sat, 5 Sep 2026 01:43:12 +0200 Subject: [PATCH] feat: proteger les cles privees via ACL NTFS (icacls) sous Windows chmod(0o600) n'a aucun effet sur les permissions NTFS reelles : sous Windows, une cle privee generee restait accessible a tout compte ayant acces au dossier (permissions heritees usuelles : Users/Authenticated Users). app/security/key_protection.py : protect_private_key() verrouille l'ACL du fichier via icacls (/inheritance:r puis /grant:r :F), plutot que d'ajouter une dependance pywin32 - plus lourde a empaqueter avec PyInstaller pour un simple changement de permissions. Repli chmod(0o600) sous Linux/macOS. Echec silencieux (icacls absent, systeme de fichiers sans ACL...), comme le faisait deja l'appel chmod qu'elle remplace. Branchee dans KeyManager.generate_key() (cf commit suivant). Non appliquee a import_key() : une cle importee pointe potentiellement vers un chemin hors du controle de l'application (~/.ssh partage avec d'autres outils), et la consigne est de ne jamais rendre une cle moins accessible qu'elle ne l'etait - ni, par prudence, de modifier sans le demander des permissions definies par l'utilisateur lui-meme. Verifie manuellement avec icacls apres generation reelle : seul le compte Windows courant apparait dans l'ACL du fichier. Co-Authored-By: Claude Sonnet 5 --- app/security/__init__.py | 0 app/security/key_protection.py | 57 ++++++++++++++++++++++ tests/test_key_protection.py | 89 ++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 app/security/__init__.py create mode 100644 app/security/key_protection.py create mode 100644 tests/test_key_protection.py diff --git a/app/security/__init__.py b/app/security/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/app/security/key_protection.py b/app/security/key_protection.py new file mode 100644 index 0000000..4a6002d --- /dev/null +++ b/app/security/key_protection.py @@ -0,0 +1,57 @@ +"""Restreint l'acces a une cle privee au seul utilisateur courant. + +Sous Windows, `chmod(0o600)` n'a aucun effet sur les permissions NTFS reelles : +on verrouille donc l'ACL du fichier via `icacls` (outil Windows standard), plutot +que d'ajouter une dependance pywin32 - plus lourde a empaqueter avec PyInstaller +pour un simple changement de permissions. Sous Linux/macOS, chmod suffit. + +`icacls fichier /inheritance:r` retire les entrees heritees du dossier parent +(qui donnent en general acces a "Users"/"Authenticated Users"), puis +`/grant:r :F` accorde le controle total au seul utilisateur courant. +Le proprietaire d'un fichier NTFS garde toujours le droit de modifier son ACL +meme quand celle-ci est vide entre les deux commandes : il n'y a pas de risque de +se retrouver bloque hors de son propre fichier. +""" +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +_NO_WINDOW = getattr(subprocess, "CREATE_NO_WINDOW", 0) + + +def protect_private_key(path: Path) -> None: + """Ne rend jamais une cle plus accessible qu'elle ne l'etait ; peut echouer + silencieusement (systeme de fichiers sans ACL/permissions, icacls absent...), + comme le faisait deja l'appel chmod qu'elle remplace.""" + if os.name == "nt": + _protect_windows(Path(path)) + else: + try: + Path(path).chmod(0o600) + except OSError: + pass + + +def _current_user_account() -> str: + domain = os.environ.get("USERDOMAIN", "") + name = os.environ.get("USERNAME", "") + return f"{domain}\\{name}" if domain else name + + +def _protect_windows(path: Path) -> None: + account = _current_user_account() + if not account.strip("\\"): + return + try: + subprocess.run( + ["icacls", str(path), "/inheritance:r"], + capture_output=True, check=True, creationflags=_NO_WINDOW, + ) + subprocess.run( + ["icacls", str(path), "/grant:r", f"{account}:F"], + capture_output=True, check=True, creationflags=_NO_WINDOW, + ) + except (subprocess.CalledProcessError, OSError, FileNotFoundError): + pass diff --git a/tests/test_key_protection.py b/tests/test_key_protection.py new file mode 100644 index 0000000..f827035 --- /dev/null +++ b/tests/test_key_protection.py @@ -0,0 +1,89 @@ +"""Protection des cles privees : verifie la sequence d'appels icacls sous Windows +(sans dependre des ACL reelles du systeme de fichiers, pour rester fiable quelle +que soit la machine qui execute le test) et le bon aiguillage vers le repli chmod +ailleurs que sous Windows.""" +import sys, pathlib +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) + +import os +import subprocess +import tempfile +from pathlib import Path +from unittest.mock import patch + +from app.security.key_protection import protect_private_key, _protect_windows + +tmp = Path(tempfile.mkdtemp()) +key_file = tmp / "id_test" +key_file.write_text("contenu-cle-privee", encoding="utf-8") + +print("=== 1. Sous Windows : /inheritance:r puis /grant:r pour l'utilisateur courant ===") +calls = [] + + +def fake_run(cmd, **kwargs): + calls.append(cmd) + return subprocess.CompletedProcess(cmd, 0) + + +with patch.dict(os.environ, {"USERDOMAIN": "MACHINE", "USERNAME": "florian"}): + with patch("subprocess.run", side_effect=fake_run): + _protect_windows(key_file) + +print(" commandes executees:") +for c in calls: + print(" ", c) +assert len(calls) == 2, calls +assert calls[0] == ["icacls", str(key_file), "/inheritance:r"], calls[0] +assert calls[1] == ["icacls", str(key_file), "/grant:r", "MACHINE\\florian:F"], calls[1] +print(" OK") + +print() +print("=== 2. Sans domaine (poste hors domaine) : compte = USERNAME seul ===") +calls.clear() +with patch.dict(os.environ, {"USERDOMAIN": "", "USERNAME": "florian"}): + with patch("subprocess.run", side_effect=fake_run): + _protect_windows(key_file) +assert calls[1][-1] == "florian:F", calls[1] +print(" OK") + +print() +print("=== 3. Un echec icacls ne remonte jamais (best effort, comme l'ancien chmod) ===") +with patch("subprocess.run", side_effect=OSError("icacls introuvable")): + _protect_windows(key_file) # ne doit lever aucune exception +print(" aucune exception propagee : OK") + +with patch("subprocess.run", side_effect=subprocess.CalledProcessError(1, "icacls")): + _protect_windows(key_file) +print(" code de sortie non nul : aucune exception propagee non plus : OK") + +print() +print("=== 4. protect_private_key() aiguille vers icacls sous Windows ===") +with patch("os.name", "nt"), patch("app.security.key_protection._protect_windows") as windows_impl: + protect_private_key(key_file) + assert windows_impl.called +print(" os.name == 'nt' -> _protect_windows() appele : OK") + +print() +print("=== 5. protect_private_key() retombe sur chmod(0o600) hors Windows ===") +with patch("os.name", "posix"), patch("pathlib.Path.chmod") as chmod_mock: + protect_private_key(key_file) + chmod_mock.assert_called_once_with(0o600) +print(" os.name != 'nt' -> chmod(0o600) appele : OK") + +print() +print("=== 6. Ne rend jamais la cle MOINS protegee : jamais d'octroi a un groupe large ===") +all_granted_users = set() +with patch.dict(os.environ, {"USERDOMAIN": "MACHINE", "USERNAME": "florian"}): + with patch("subprocess.run", side_effect=fake_run): + calls.clear() + _protect_windows(key_file) +for cmd in calls: + if "/grant:r" in cmd: + all_granted_users.add(cmd[-1].split(":")[0]) +print(" comptes ayant reçu un acces:", all_granted_users) +assert all_granted_users == {"MACHINE\\florian"}, all_granted_users +print(" OK") + +print() +print("KEY_PROTECTION_WINDOWS_ACL : TOUS LES TESTS OK")