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, commit523f049) — 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.