Problèmes de sécurité impactant la protection de la vie privée

[piotrcki]

Bonjour,

@X-Cli et moi-même venons de publier un article mettant en évidence des problèmes de sécurité qui peuvent impacter le respect de la vie privée des utilisateurs.

Nous sommes disponibles pour échanger à ce sujet, publiquement ou non.

[maximebochon]

Passionnant tout ça, merci pour ce partage. Quelques remarques :

L'inutilité du chiffrement du profil stocké dans le localStorage a été majoritairement reconnue par la communauté dans la pull request #58. Je cite notamment @theblackhole :

L'intérêt n'est donc pas d'avoir un truc ultra-sécurisé [...], juste de compliquer la vie d'éventuelles personnes malveillantes... Je vois ça un peu comme un grillage : ça n'empêche pas un intrus de rentrer mais ça lui complique un peu la vie car il faudra qu'il passe par dessus.

Mais de là à dire que le chiffrement "ne sert à rien d’autre qu’à induire un faux sentiment de sécurité", ça me paraît gratuitement accusateur car rien n'est communiqué à ce sujet à l'utilisateur final, même pas dans une note de bas de page. Personnellement je vois plus ça comme simplement une dépendance supplémentaire sans valeur ajoutée.

Pour le reste en effet, il est malheureux de constater que l'inclusion du site officiel de l'attestation dans une iframe n'est pas du tout verrouillée au niveau HTTP, alors que ce devrait être une précaution de base. Je l'illustre ici avec Google comme élément de comparaison :

N'étant pas expert en sécurité, j'ai essayé dans la mesure du possible de comprendre les risques que vous décrivez. Est-ce qu'on peut dire que dans tous les cas, la récupération des informations utilisateur repose soit sur la compromission d'une ressource du site officiel soit sur une faille critique du navigateur ? (je n'ai pas eu le temps de jouer suffisamment avec postMessage mais de ce que je comprends, sans un code malicieux présent dans le site de l'iframe, on ne peut rien en tirer, d'où l'évocation dans votre article d'une attaque du type MITM comme moyen de compromission d'une ressource)

[X-Cli]

Bonjour,

Nous vous remercions pour l'intérêt que vous portez à ces remarques sur la sécurité de l'application de génération des attestations.

Concernant la couche de chiffrement avec secure-ls, nous pouvons comprendre que le ton semble accusateur. C'est probablement un manque de pédagogie de notre part ; entre spécialistes en sécurité informatique, ce genre de bibliothèques et d'usages méritent ce genre de mots, mais ils peuvent paraitre trop violent pour les non-spécialistes. Nous allons essayer d'expliquer notre chemin de pensées pour affirmer qu'elle est délétère dans le cas présent (et en général).

La cryptographie n'est pas une fin en soi. La cryptographie est une contremesure à un risque : celui d'interception ou de vol d'un message, dans le cas d'espèce. Les moyens pour réaliser ce risque sont multiples : 

• une injection de code (type XSS) dans la page (cela inclut les Universal XSS où l'injection de code se fait par le biais d'une extension navigateur) ;

• une application web tierce hébergée sur le même domaine que l'application d'émission d'attestation. Quand j'étais responsable de la formation en sécurité Web de l'ANSSI, je concluais ma journée de formation sur la sécurité des applications dans le navigateur en rappelant que "une origine = une application" car il est impossible d'isoler efficacement plusieurs applications servies par une même origine ;

• les attaques Spectre qui permettent d'extraire les informations lors de leur manipulation, par l'observation de conséquences du traitement de la donnée à extraire sur l'architecture matérielle de l'ordinateur la manipulant (spécifiquement le CPU).

Dans les deux premiers cas, quelque soit la méthode employée, la présence de code tiers (injecté ou exécuté dans le même contexte/la même origine) entraine l'invalidation de la contremesure, soit par piégage du code, soit par interception du résultat du déchiffrement. L'apport en sécurité est effectivement nul, et comparable à une porte fermée mais dont le mécanisme est librement exposé.

Mais dans le cas d'espèce, la situation est encore plus désespérée : la clé de chiffrement est hardcodée. Si le secret était un mot de passe tapé par l'utilisateur, le risque d'injection de code serait toujours présent. Mais ici, cela signifie que la donnée est chiffrée uniquement pour les personnes qui ne se donnent pas la peine de lancer la procédure de déchiffrement avec le clé connue et publique, directement disponible dans le code. C'est l'équivalent d'une porte fermée, avec la clé laissée dans la serrure. La porte n'est fermée que pour les personnes qui ne se donnent pas la peine de tourner la clé.

Cependant, cela ne s'arrête pas là : on pourrait se dire "tant mieux si ça arrête les attaquants qui ne se donnent pas cette peine". Cependant, le simple fait d'ajouter du code dans une application comporte un risque : celui d'augmenter la probabilité que le code de l'application contienne un bug, et pire, un bug de sécurité. Cela s'appelle, dans le jargon des spécialistes en sécurité de l'information : accroitre la surface d'attaque. Ce peut être un bug codé par accident ou u bug ajouté volontairement pour piéger l'application. Ce bug peut même être absent du dépot VCS publiquement visible, et n'exister que dans la version publiée par l'outil de packaging. Cela s'appelle des attaques sur la chaine d'approvisionnement, et cela s'est déjà vu de nombreuses fois dans l'Histoire récente.

Donc, comme toujours, c'est une question de compromis : ajouter une bibliothèque pour réduire un risque et augmenter la surface d'attaque, ou ne pas l'ajouter et ignorer le risque.

Dans le cas d'espèce, la réduction du risque apportée par secure-ls avec un mot de passe hardcodée est proche de zéro, et nous pensons qu'il n'est pas raisonnable d'accroitre la surface d'attaque de la sorte.

Notez que nous n'avons pas encore parlé du contenu de la bibliothèque. Mais, en l'occurrence, vous avez certainement déjà entendu l'adage : "on ne développe pas sa propre cryptographie". Cet adage inclut le fait d'implémenter des algorithmes cryptographiques existants, mais le faire mal. C'est précisément le cas de cette bibliothèque, qui utilise crypto-js. D'une part, secure-ls utilise une version vendorée de crypto-js qui est vieille de deux ans et qui contient des vulnérabilités critiques (comme le fait de ne pas utiliser un générateur de nombres aléatoires cryptographiquement sûr...). D'autre part, crypto-js effectue une implémentation naive des algorithmes, alors qu'il existe dans Webcrypto des implémentations performantes et sécurisées. Bref, cette bibliothèque est un musée des horreurs.

---

Concernant la question suivante :

 Est-ce qu'on peut dire que dans tous les cas, la récupération des informations utilisateur repose soit sur la compromission d'une ressource du site officiel soit sur une faille critique du navigateur ? 

La question n'est pas vraiment : "Est-ce que le site media.interieur.gouv.fr contient, a contenu ou contiendra quelque part, même de façon évanescente ou uniquement pour certains utilisateurs ou certaines adresses IP du code qui servirait spécifiquement à pister les citoyens grâce aux informations mémorisées par le formulaire de génération d'attestations ?"

Personne n'a la réponse à cette question (pas même le Ministère de l'Intérieur si l'une des ressources de https://media.interieur.gouv.fr est un jour compromise), et il est impossible de le prouver, présentemment et encore moins dans le temps.

Pour en revenir à la question du traitement du risque, il se trouve que le risque qu'un jour l'origine https://media.interieur.gouv.fr/ contienne du code permettant ce pistage/cette exfiltration des état civils des citoyens est non-nul. Pire encore, des services de renseignement pourraient tout à fait être intéressés par ce genre d'informations, fut-il légal de les obtenir ainsi. En conséquence, il nous semble impératif de traiter ce risque, tant le risque encourru est grand pour les citoyens et les libertés civiles.

[maximebochon]

Merci pour cette réponse particulièrement détaillée.

[theblackhole]

Merci pour cet article et ces explications vraiment intéressantes ! ça m'intéresse car je suis à l'origine de cette fonctionnalité dans son format actuel (mis à part quelques petites adaptations): à la base je l'avais proposée dans attestation-couvre-feu-covid-19#4, puis elle a été adaptée pour l'attestation courante par @tar-gezed dans #58 puis copié-collé par le Ministère de l'intérieur... parce que le merge des PR -que ce soit celle-ci ou les autres similaires proposées- et les discussions c'est pour les nuls ! ^^).

Le choix que j'ai fait de cette lib partait d'une bonne intention (je pensais pouvoir limiter les risques en cas de fuite de données) mais je comprends maintenant clairement les risques introduits par cette lib qui font qu'elle empire les choses plutôt que d'être "mieux que rien"
Après, il n'y a pas d'intention de donner de sentiment de fausse impression de sécurité (mais je peux comprendre le doute si le Ministère était à l'origine du choix de cette lib), simplement un manque de connaissances de ma part et le choix du Ministère d'ignorer la discussion autour de secure-ls dans #58
Par contre je pense qu'ils étaient clairement conscient des risques puisque cette PR a été catégorisée dans "Stockage des données (trop sensible)" sur leur tableau : https://github.com/LAB-MI/attestation-deplacement-derogatoire-q4-2020/projects/1#column-11475547

Quant à l'origin, je suis tout à fait d'accord avec vous, ça m'a toujours interpellé que l'attestation soit servie sur un hostname aussi général que media.interieur.gouv.fr . Probablement parce que c'est facile, ça existe déjà, ce ne sont que des fichiers statiques (sauf que derrière ça, les données des citoyens sont en jeu)

Et le manque de CSP et X-Frame-Options me fait maintenant réfléchir à réutiliser ma propre instance pour générer l'attestation...

[tar-gezed]

Merci pour cet article très intéressant. En effet lorsque j'ai adapté le code de @theblackhole je n'avais absolument aucune ambition de mettre en place un système entièrement sécurisé : l'idée principale était de simplifier l'utilisation de l'application pour éviter une saisie des informations, le tout sans changer le flow utilisateur proposé (j'avais pensé a un "chiffrement" des données avec un mot de passe saisi par l'utilisateur). L'approche adoptée est loin d'être sans faille, et je n'ai pas du tout penser à vérifier les aspects sécurité présent (ou pas) mis en place par le gouvernement au moment de ma PR.
Le manque de CSP, X-Frame-Options, CORS etc... est juste scandaleux.
Merci d'avoir mis en lumière certains aspects qui nécessiteraient d'être corrigés pour rendre l'application plus sûre, l'utilisation de la fonctionnalité de sauvegarde des données de l'utilisateur dans sa forme actuelle est en effet à éviter.

[X-Cli]

Merci pour vos retours positifs. 👍

Est-ce que quelqu'un a un contact au Minint pour faire avancer les choses de leur côté ?

Est-ce que quelqu'un de plus compétent que moi en Javascript pourrait proposer une PR qui :

• revert la fonctionnalité de sauvegarde ;
• ajoute la suppression automatique des données enregistrées au préalable ;
• ajoute une politique CSP via une balise http-equiv.

Je peux relire, mais mes compétences en JS au-delà des PoC pour illustrer une problématique de sécurité sont vite atteintes.

Merci à vous.

[theblackhole]

Est-ce que quelqu'un a un contact au Minint pour faire avancer les choses de leur côté ?

Non mais ils ont une adresse mail et un Twitter : https://beta.interieur.gouv.fr/#contact

Est-ce que quelqu'un de plus compétent que moi en Javascript pourrait proposer une PR

Désolé, je peux le faire mais ça sera sans moi : après ce qu'ils ont fait, j'ai décidé de ne plus accorder de temps à ce projet

Est-ce que l'utilisation des URI Fragments (proposé par @a2br dans #50) serait une alternative envisageable à la suppression pure et dure de la mémorisation des informations ? (car c'est quand même bien pratique au final ^^)

[a2br]

Est-ce que quelqu'un de plus compétent que moi en Javascript pourrait proposer une PR

Je pourrais, mais comme @theblackhole, leur vision de l'open source m'a quelque peu dégouté. Ce dernier mois, ils n'ont review aucune PR, et j'ai assez peu d'espoir pour #50, qui stagne. L'intérêt du projet lui-même a baissé depuis la fonctionnalité 'Attestation' de TousAntiCovid, qui est très bien faite, ou les apps mobile alternatives. Je ne pense pas que ce service ait beaucoup d'avenir, mais il faudrait en effet au moins proposer de régler ces ambiguïtés au niveau sécurité.

[X-Cli]

Je comprends.

Concernant l'utilisation des URI fragments, nous sommes partagés, avec @piotrcki.

Les URI fragments peuvent être bruteforcés par des attaques de type "browser history sniffing".
Dans le cas d'un état civil, il serait imaginable de faire une hypothèse raisonnable de qui se connecte à partir de quelle IP, et de confirmer l'hypothèse à l'aide d'une telle attaque, grâce au formulaire et à la persistence de l'historique de navigation.

C'est un modèle d'attaquant beaucoup moins fort que le présent problème, mais il est existant. D'un autre côté, le simple fait d'accéder au formulaire par un navigateur en navigation privée élimine le risque. En outre, il n'existe pas d'attaque publique de type "browser history sniffing" sur les navigateurs à jour. Donc, en ce qui me concerne, c'est une solution satisfaisante.

Néanmoins, si on veut aller plus loin, il serait possible de combiner l'usage actuel de secure-ls (malgré ses nombreux défauts) avec les URI fragments pour réhausser la sécurité : stocker le secret non dans le code source mais dans un URI fragment. Si ce secret est propre à chaque utilsateur et généré à partir de WebCrypto, les attaques par bruteforce sur l'historique seraient irréalistes, et la persistence des données en localStorage ne serait plus un problème puisque la clé ne serait disponible que si l'utilisateur navigue depuis son bookmark.

[theblackhole]

Néanmoins, si on veut aller plus loin, il serait possible de combiner l'usage actuel de secure-ls (malgré ses nombreux défauts) avec les URI fragments pour réhausser la sécurité : stocker le secret non dans le code source mais dans un URI fragment. Si ce secret est propre à chaque utilsateur et généré à partir de WebCrypto, les attaques par bruteforce sur l'historique seraient irréalistes, et la persistence des données en localStorage ne serait plus un problème puisque la clé ne serait disponible que si l'utilisateur navigue depuis son bookmark.

Ohhh j'aime beaucoup l'idée ! Beaucoup plus pratique que de demander un mot de passe à chaque fois. Je pense que je vais retenir cette solution pour un futur projet perso/pro/ou pourquoi pas pour la création d'une lib alternative à secure-ls plus moderne et utilisant cette mécanique... Enfin si elle n'existe pas déjà, et si elle peut apporter quelque chose de plus que l'utilisation directe du LS + WebCrypto + URI fragments :)