Audit des paramètres du runtime Ğ1

Nodes Duniter-v2s
1

Étant donné que la migration est imminente, j’essaie au maximum d’auditer ce que je peux avant. Ce soir, j’ai passé en revue l’ensemble des paramètres définis dans le fichier runtime/g1/src/parameters.rs.

J’ai ouvert une merge request afin de réaliser quelques ajustements qui me semblent nécessaires pour des raisons de sécurité :

Modifications apportées

  • Réduction du BlockLength de 5 MiB à 3 MiB sur tous les runtimes (g1, gdev, gtest)
  • Augmentation de ReportLongevity à environ 28 jours sur tous les runtimes
  • Rétablissement de ChangeOwnerKeyPeriod (g1) à 6 mois au lieu de 1 mois
  • Ajustement de IdtyCreationPeriod (g1) à 5 jours au lieu de 1 jour

ChangeOwnerKeyPeriod

Le paramètre ChangeOwnerKeyPeriod avait été réduit de 6 mois à 1 mois.

Cette durée pourrait faciliter la prise de contrôle définitive d’un compte membre en cas de compromission.

En cas de piratage d’un compte peu actif, un attaquant peut modifier la clé propriétaire afin de s’approprier définitivement le compte.

Le rôle principal de ChangeOwnerKeyPeriod est précisément de laisser au membre victime un délai suffisant pour :
1. constater le piratage,
2. révoquer son compte à l’aide de l’ancienne clé,
3. se faire recertifier sur un nouveau compte.

Même un utilisateur actif mensuellement ne remarquera pas nécessairement immédiatement la compromission.
S’il n’émet pas de certification et ne surveille pas précisément l’évolution de son solde pour constater l’absence de nouveaux DUs, il peut ne rien détecter.

Une fois le problème perçu, la réaction immédiate n’est généralement pas la révocation. L’utilisateur va d’abord demander de l’aide. Il faudra ensuite :

  • qu’une personne compétente identifie clairement la situation,
  • qu’elle explique la nécessité de la révocation,
  • que l’utilisateur accepte psychologiquement de révoquer son compte.

Ce processus peut facilement prendre plusieurs mois.

Si la révocation n’intervient pas dans le délai autorisé, le membre peut se retrouver privé de compte membre pendant une durée pouvant aller jusqu’à 2 ans, le temps que ses certifications expirent, car ses certificateurs devront attendre cette expiration pour le certifier sur un nouveau compte conformément à la licence Ğ1.

Pour ces raisons, une durée de 6 mois constitue à mon sens un minimum prudent.

IdtyCreationPeriod

Le paramètre IdtyCreationPeriod est passé de 14 jours à 1 jour.

Ce paramètre devrait rester suffisamment long afin de limiter le spam de fausses demandes de création d’identité.

Dans la Ğ1 v1, il n’est possible de certifier une nouvelle personne que tous les 5 jours.
Il me semble cohérent d’aligner la fréquence maximale de création d’identité sur cette contrainte, soit une création possible tous les 5 jours.

Cela permet :

  • de limiter les tentatives automatisées,
  • de réduire la surface d’attaque liée aux identités éphémères,
  • de conserver une cohérence avec les contraintes sociales existantes.
8 Likes
2

Par contre @elois il aurait fallu te baser sur le branche 337-prepare-g1-launch actuellement en MR, je pense qu’on peut la merger en l’état.

3

C’est gênant de devoir se baser sur une autre MR qui reste ouverte jusqu’à la migration. Est-ce possible de merger !337 quand même avant la migration ? Je ne l’ai pas review, donc je ne sais pas si ça pose un problème ou non.

4

J’attendais des review mais elles ne se sont pas faites, en réalité j’ai préparé la semaine dernière 5 lancements de réseau live du runtime g1 basé sur cette branche, que j’ai corrigé au fur et à mesure à cette occasion, pour verrouiller le lancement et le comportement souhaité pour le lancement, il faut donc merger cette MR oui.
Je te réponds aussi sur ta MR Fix g1 parameters :slight_smile:

3 Likes
5

@poka Suite à ton commentaire sur GitLab, j’ai supprimé la réduction de BlockLength, qui n’est en effet pas une obligation de sécurité, mais une optimisation réseau.

En revanche, concernant le paramètre ChangeOwnerKeyPeriod, il était déjà à 6 mois. C’est @tuxmain qui l’a changé à 1 mois il y a quelques jours, sans autre justification que « coller à la gtest » : !346 (merged).

Il me semble vraiment important que ChangeOwnerKeyPeriod, en particulier, soit bien remis à 6 mois AVANT la migration. C’est moi qui ai créé ce paramètre, et 6 mois sont largement suffisants, car on ne change normalement de owner key que pour migrer de cryptographie.

2 Likes
6

Est-ce vraiment important d’augmenter ReportLongevity ? Si je me souviens bien c’est le temps pendant lequel on peut sanctionner un forgeron qui aurait mal agi ? Dans ce cas soit on a du monitoring et on le voit tout de suite, et le forgeron est dans la TdC donc il ne peut pas se cacher même si on ne réagit pas tout de suite, soit on ne voit rien et alors est-ce qu’une fraude à babe/grandpa est vraiment grave des mois plus tard ?

À cause de ce délai il faudra bien dire aux forgerons de ne surtout pas passer online avant d’avoir migré.

7

En fait, juste pour être précis : le runtime GTest devrait théoriquement, sur le papier, reprendre exactement les paramètres du runtime G1, pour reproduire son comportement sur le long terme.
Le fait que ces valeurs n’aient pas été modifiées sur le runtime G1 est plutôt de l’ordre de l’oubli d’alignement qu’autre chose. Les explications sur les changements de ces valeurs sont à chercher au niveau du runtime GTest. Tuxmain n’a fait que, à raison, aligner le runtime G1 avec celui de la GTest dont nous connaissons le comportement, qui a été testé.

Je sais que tu avais initialement choisi ces valeurs, mais elles ont été changées volontairement pour certaines raisons (que je n’ai plus en tête ni sous la main là tout de suite).

Je ne remets pas en cause ces choix hein, ni d’un côté ni de l’autre, je me permets simplement de rappeler le processus avant de continuer, et d’expliquer pourquoi cette MR de tuxmain n’est pas un choix brutal et arbitraire, mais plutôt le rattrapage d’un oubli d’alignement.

8

D’accord, mais je ne pouvais pas le savoir, car ce n’était pas le processus que nous utilisions jusque-là. Nous avions l’habitude d’avoir des durées plus courtes sur les réseaux de test afin de tester plus rapidement des scénarios qui prennent du temps à réaliser.

C’est moi-même qui avais fixé ChangeOwnerKeyPeriod à 1 mois dans le runtime gtest, justement (MR d’origine : feat(identity): add call change_owner_key (!86) · Merge requests · nodes / rust / Duniter v2S · GitLab), afin de pouvoir vérifier, au bout d’un mois sur gtest, qu’on ne peut effectivement plus révoquer et qu’on peut bien (re)changer d’owner key.

Du coup, il est faux de dire que :

Vous avez décidé, entre-temps, de changer de processus et d’avoir les mêmes durées entre gtest et g1, d’accord. Mais je ne pense pas que vous ayez audité toutes les valeurs de tous les paramètres à l’occasion de ce changement de processus. À ce moment, vous auriez dû copier les valeurs du runtime g1 pour les appliquer au runtime gtest (et non l’inverse).

Tu fais une erreur en supposant que toutes les valeurs en gtest ont été discutées comme des valeurs décidées pour la production. La plupart des valeurs en gtest ont été sélectionnées par moi-même il y a des années, selon une hypothèse de réseau avec des durées différentes (plus rapides) que la production.

Plusieurs de ces paramètres n’ont probablement jamais été réellement rediscutés depuis, et c’est manifestement le cas pour ChangeOwnerKeyPeriod.

1 Like
9

Ok je vois, petit soucis de coordination sur les process alors probablement. Attendons tout de même des retours de @HugoTrentesaux et @cgeek sur ces sujets :slight_smile:

@elois @cgeek je viens de merger la branche 337-prepare-g1-launch car ça devenait bloquant, merci de rebase vos branches sur master.

1 Like
10

Oui on doit tous les vérifier, si possible en trouvant la justification pour chacun d’entre eux. On n’est pas à l’abri d’une erreur ou d’un oubli. En tout cas ça fait partie de ma short-list pour le lancement.

7 Likes
11

C’est fait?

J’ai mis à jour les valeurs du runtime gtest pour aligner ChangeOwnerKeyPeriod et IdtyCreationPeriod avec celles du runtime g1.

J’ai réduit ReportLongevity à 1 h pour le lancement, avec un TODO pour le modifier après le lancement.

1 Like
12

J’avais supposé que les valeurs du runtime g1 datant de commits de 2022 étaient placeholder puisque certaines n’avaient aucun sens. Je me suis basé sur la gtest, les discussions ici et les recommendations d’élois. C’était encore ouvert à la discussion, c’est juste que ça a été mergé avant que vous ayez pu relire.

Merci de vos retours et corrections du coup !

2 Likes
13

Je n’ai rien trouvé de spécial, mais ce n’est pas un gage de sécurité puisque Eloïs vient de trouver un bug que ma relecture statique n’a pas détecté.

3 Likes