Pod ne vérifie pas les signatures des actions entrantes

[azmeuk]

Lorsqu'une instance envoie une action Announce, Update ou Delete, 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.

[azmeuk]

@LoanR
Je viens de pousser quelques commits (6ca8a3f cc5f45c f617adf et e6b9047) 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, Update et Delete sont bien envoyées par une instance à qui on a demandé la fédération
• Les Announce, Update et Delete sont bien envoyées par le propriétaire de la vidéo.

[LoanR]

Le commit c28f49b fait maintenant les checks avant de faire une quelconque modification sur la vidéo.

La CI devrait mieux tourner après le retrait des tâches pa11y... (cf EsupPortail#1170 (comment))
✌️