azmeuk
commented
1 month ago @LoanR
Je viens de pousser quelques commits (6ca8a3f07ef0d3dbc380562f30526167ff602dfa cc5f45cdcc1584533520f1df297364bd34e17ef8 f617adf69caab04a694149eb8892e678a3a877c5 et e6b9047fea2e32418940bbb8ff262e0e9b26ff4f) qui implémentent la vérification des signatures pour les actions Announce, Update et Delete.
Quelques infos :
- Dès qu'on reçoit une requête dans la inbox, on vérifie les signatures avant d'envoyer les tâches dans celery. J'ai fait comme ça pour copier le comportement de peertube et renvoyer une 403 lorsque la signature est mauvaise. Pour autant, vérifier la signature implique de télécharger la clé publique depuis l'instance du client, et donc faire une requête HTTP hors celery, ce qui est pas ouf. On peut aussi envisager de faire la vérification des signatures dans les tâches celery et non pas dans la vue, et renvoyer une 204 dans tous les cas au client. Je te laisse choisir.
- J'ai désactivé la vérification pour les tests unitaires existants, parce que ça faisait un truc bien compliqué à mettre en place. Justement à cause de cette histoire de requête qui doit récupérer la clé publique du client.
- J'ai pu tester que ça fonctionnait bien, et donc que par exemple une mise à jour sur une vidéo peertube est bien prise en compte dans pod (et que donc pod valide correctement les signatures générées par peertube). À l'inverse y'a des tests unitaires qui vérifient que des signatures non valides sont rejetées, mais j'ai pas testé dans l'environnement de dev de générer des requêtes avec des signatures invalides.
- Je n'ai pas envoyé ces commits dans la branche de la PR principale. Je te laisse faire ça.
Par ailleurs, je ne sais pas si ces vérifications sont suffisantes : ce que l'on fait désormais c'est vérifier que les payloads sont correctement signées avec la clé publique de l'acteur. Autrement dit on vérifie que l'acteur est bien celui qu'il prétend être. Par contre on ne vérifie pas si l'acteur a le droit de modifier les vidéos ni s'il provient d'une instance fédérée.
Je pense qu'il faudrait rajouter quelque part un test qui vérifie que:
- Les
Announce,UpdateetDeletesont bien envoyées par une instance à qui on a demandé la fédération - Les
Announce,UpdateetDeletesont bien envoyées par le propriétaire de la vidéo.
LoanR
Lorsqu'une instance envoie une action
Announce,UpdateouDelete, on ne vérifie pas les signatures des messages. Ça signifie que n'importe qui peut envoyer un message pour créer, mettre à jour ou supprimer une vidéo.