Gitfed
bastien-mrq/gitfed / docs / security / AUDIT-2026-07-29.md
AUDIT-2026-07-29.md Code Preview

Audit de sécurité — nouvelles surfaces d'attaque (2026-07-29)

  • Périmètre : uniquement ce qui a été ajouté depuis le premier audit (AUDIT.md, commit 523f049) — clone HTTPS anonyme, notifications fédérées, dépôts épinglés, historique de commits, croissance du RPC admin, et le changement d'Ingress qui les accompagne.
  • Nature : revue de code adversariale + tests de reproduction pour chaque correctif.
  • État : les correctifs identifiés ci-dessous sont déjà appliqués dans ce commit, avec un test de régression pour chacun des deux qui s'y prêtaient (internal/store/notifications_test.go, internal/federation/notify_test.go).

1. Synthèse

Aucune faille critique (pas de fuite de dépôt privé, pas de contournement de l'authentification, pas d'injection). Les points trouvés sont des trous de limitation de débit / de plafonnement — la même catégorie que H1 dans l'audit précédent, désormais étendue aux deux nouvelles surfaces qui acceptent du trafic non authentifié : le clone HTTP anonyme et la réception de notifications fédérées.

ID Sévérité Titre Statut
M1 🟠 Medium Aucune limitation de débit sur le clone HTTP anonyme ✅ corrigé
M2 🟠 Medium Notifications non plafonnées par destinataire ✅ corrigé
M3 🟠 Medium Destinataire de notification jamais vérifié comme utilisateur réel ✅ corrigé
L1 🟡 Low stderr de git ignoré silencieusement sur les endpoints HTTP ✅ corrigé
L2 🟡 Low Le contenu d'une notification n'est pas vérifié, seule sa signature l'est ⚠️ atténué (texte), limite inhérente au modèle

2. Détail

🟠 M1 — Aucune limitation de débit sur le clone HTTP anonyme

Fichier : cmd/gitfed-web/handlers_git_http.go

/{owner}/{repo}.git/info/refs et /git-upload-pack n'avaient, avant ce correctif, aucune limite d'appels — contrairement au login (loginByIP) et à la réception de notifications (limiteur dédié). Chaque requête peut déclencher un sous-processus git upload-pack, et la fenêtre d'écriture y est délibérément étendue à 10 minutes (pour laisser le temps à un clone volumineux sur une connexion lente) — ce qui aggrave l'impact d'un flot de requêtes plutôt que de le limiter.

Correctif : ratelimit.go généralisé (loginLimiter → rateLimiter, réutilisable) ; nouveau gitHTTPByIP à 60 requêtes/minute par IP, appliqué avant toute autre logique dans les deux handlers. Vérifié manuellement : la 61ᵉ requête consécutive depuis la même IP reçoit un 429, un clone normal (1-2 requêtes) n'est jamais affecté.


🟠 M2 — Notifications non plafonnées par destinataire

Fichier : internal/store/notifications.go

Recevoir une notification ne suppose aucune relation préexistante — c'est volontaire (voir le commentaire de paquet dans internal/federation/notify.go). Mais sans plafond, n'importe quelle instance peut faire grossir indéfiniment la part d'un utilisateur réel dans la base en lui envoyant de nombreuses fausses réclamations distinctes (des Repo/Actor différents à chaque fois contournent la déduplication, qui ne joue que sur un doublon exact encore en attente).

Correctif : MaxNotificationsPerPrincipal = 200. Au-delà, la plus ancienne notification est évincée pour faire de la place à la nouvelle (plutôt que de rejeter la nouvelle, ce qui permettrait à du spam de masquer une notification légitime derrière lui). Testé dans TestNotificationCapEvictsOldest.


🟠 M3 — Destinataire de notification jamais vérifié comme utilisateur réel

Fichier : internal/federation/notify.go

La vérification d'origine ne contrôlait que le domaine du principal ciblé (bob@ailleurs.net → le suffixe doit être le domaine local), jamais que bob existe réellement. Une notification pour un nom d'utilisateur inventé était donc acceptée et stockée sans jamais pouvoir être vue par personne — un vecteur de gonflement de la base par du spam ciblant des comptes qui n'existent pas.

Correctif : st.GetUser(username) vérifié avant tout traitement supplémentaire (avant même la validation de fraîcheur ou l'appel réseau de vérification de signature — échoue tôt et sans consommer de ressources inutiles). Testé dans TestNotifyHandlerRejectsUnknownRecipient / TestNotifyHandlerAcceptsKnownRecipient.


🟡 L1 — stderr de git ignoré silencieusement

Fichier : cmd/gitfed-web/handlers_git_http.go

exec.Command sans Stderr défini envoie la sortie d'erreur vers /dev/null (comportement par défaut de os/exec en Go) — pas une fuite (rien n'était renvoyé au client), mais un angle mort opérationnel : un git upload-pack qui échoue sur un dépôt corrompu ne laissait aucune trace exploitable, contrairement au chemin SSH équivalent (internal/gitexec.Serve), qui capture correctement stderr.

Correctif : stderr capturé dans un buffer et inclus dans le log.Printf d'erreur côté serveur, jamais renvoyé au client — cohérent avec le reste du code base.


🟡 L2 — Le contenu d'une notification n'est pas vérifié

Fichier : internal/federation/notify.go

La signature prouve que le message vient bien de l'instance qui prétend l'envoyer — elle ne prouve rien sur la véracité de son contenu (Repo, Role, Actor sont des champs libres). N'importe quelle instance peut donc envoyer une notification prétendant "vous avez un accès admin sur tel dépôt" sans que ce soit vrai, à des fins d'ingénierie sociale (crédibiliser un message de phishing envoyé par un autre canal).

Analyse d'impact : c'est une limite inhérente à un système volontairement non-autoritaire (voir le commentaire de paquet) — accepter une notification ne fait qu'épingler un lien, jamais échanger le moindre identifiant ni ouvrir de session. Le pire abus reste donc « recevoir un message trompeur mais inerte », pas une élévation de privilège.

Traitement retenu : renforcement du texte affiché sur /notifications pour rendre cette limite explicite (« son contenu n'est pas vérifié indépendamment... votre accès réel est celui que le propriétaire de cette instance a réellement défini, peu importe ce qu'affirme une notification ») plutôt qu'un changement de mécanisme — ajouter une vérification de contenu supposerait que l'instance réceptrice puisse interroger l'ACL de l'instance émettrice, ce qui est exactement le couplage fort que ce système a été conçu pour éviter (voir DESIGN.md §3, non-objectifs).


3. Contrôles vérifiés et jugés sains

Surface Constat
repo.Path dans le clone HTTP Toujours résolu via store.GetRepo, jamais dérivé d'une entrée attaquant — même absence d'injection d'argument que sur le chemin SSH existant.
Redirections HTTP pendant la vérification de signature guardedDial s'applique à chaque connexion TCP que le Transport établit, y compris après une redirection — impossible de contourner le blocage d'IP privée par ce biais. Fetch rejette en plus tout document dont le domain déclaré ne correspond pas à celui demandé.
Exemption CSRF sur /git-upload-pack Le suffixe de chemin qui déclenche l'exemption n'est atteignable que par le handler handleGitUploadPack lui-même (aucune autre route ne se termine ainsi) — impossible de faire passer une requête vers un handler différent sous couvert de cette exemption.
Écriture via le clone HTTP Aucune route git-receive-pack n'existe côté HTTP, point de vérification trivial mais confirmé (grep sur le fichier de routes).
Portée des nouvelles méthodes admin.Ops (pins, notifications) Toujours appelées avec sess.Principal dérivé de la session HTTP, jamais d'un paramètre soumis par le client — cohérent avec le modèle de confiance déjà documenté du socket RPC (accès total une fois le socket atteint, la vraie barrière est côté HTTP).
XSS via un dépôt épinglé html/template échappe .Domain/.Repo dans le contexte HTML ; le préfixe https:// est en dur dans le template, donc une valeur du type javascript:... saisie comme "domaine" ne produit jamais une URI javascript: exploitable, juste un lien cassé.
Historique de commits Format git log fixe (non influencé par l'attaquant), chemin de dépôt toujours résolu via le store, taille bornée (--max-count=200).
Élargissement de l'Ingress (/.well-known/gitfed.json exact → /.well-known/ préfixe) Sans risque : la surface réelle reste bornée par les deux seules routes enregistrées côté gitfed-server (gitfed.json, gitfed-notify) ; tout le reste sous ce préfixe reçoit un 404 du mux Go, quelle que soit la largeur de la règle Ingress.

4. Note opérationnelle (hors code)

deploy/update.sh n'applique que deployment.yaml — un changement dans ingress.yaml, service.yaml, networkpolicy.yaml etc. doit être appliqué manuellement (kubectl apply -f deploy/k8s/<fichier>.yaml) après le déploiement, sans quoi il reste committé sans effet en production. C'est exactement ce qui s'est produit pour le changement de préfixe Ingress ci-dessus, repéré et corrigé pendant cette session.