Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Sécurité] Màj : données d'identification en clair -> Refonte système de màj #411

Closed
Draky50110 opened this issue Feb 4, 2014 · 36 comments

Comments

@Draky50110
Copy link

Hello.

J'ai décelé un bug, ou une faille, ou un truc qui m'interpelle en tout cas.

On peut faire une mise à jour sans être identifié !
Démarche :

  • on upload le contenu du ZIP (dev pour ma part)
  • on va sur l'URL

On voit les infos de connex (enfin identifiant) et même les infos de connex à la base MySQL, ce qui me semble... dangereux ?

Faudrait pas s'authentifier AVANT le lancement de la mise à jour ?

@Alkarex
Copy link
Member

Alkarex commented Feb 4, 2014

Lorsque le fichier install.php est présent, le système est en mode installation.
Pour plus de sécurité, il est par exemple possible de restreindre les connexions à une IP précise pendant cette phase d'installation / mise à jour.
http://httpd.apache.org/docs/trunk/howto/access.html

@Draky50110
Copy link
Author

Bah en cas de mise à jour, faudrait occulter les infos vu que c'est une mise à jour...

@marienfressinaud
Copy link
Member

Je pense aussi que la protection devrait être intégrée au niveau de FreshRSS. Après peut-être pas besoin de s'authentifier mais simplement exécuter la mise à jour en ne demandant que les infos obligatoires qui apparaissent d'une version à l'autre ? (exemple, le nom d'utilisateur)

@Alkarex
Copy link
Member

Alkarex commented Feb 16, 2014

Oui, demander uniquement les nouvelles informations éviterait aussi les problèmes d'utilisateurs qui changent les données de BD alors qu'ils souhaitent une mise à jour sur place #417

@marienfressinaud
Copy link
Member

J'aimerais revoir la procédure de mise à jour. J'aimerais notamment automatiser tout ça plutôt que de devoir aller chercher le code complet à chaque fois :

  1. D'abord, aller chercher régulièrement s'il existe des màj sur freshrss.org
  2. Télécharger le script qui va bien selon la version
  3. Exécuter ce script qui téléchargerait le code source sur GitHub, demanderait des infos supplémentaires si nécessaire, mettrait à jour la BDD, etc.

Ainsi on pourrait simplifier le script d'install qui commence à devenir dur à relire ;) Et comme ce script ne sera accessible qu'à l'admin, plus de soucis !

@marienfressinaud
Copy link
Member

Au passage, j'enlève tout le code en rapport avec des mises à jour dans install.php

@Draky50110
Copy link
Author

Je plussoie la proposition des mises à jour :)
Avec un backup automatisé avant ;)

@marienfressinaud
Copy link
Member

Oui, évidemment :p

@Draky50110
Copy link
Author

Tu peux faire un backup de la base aussi ?
Ou juste les fichiers ?

@marienfressinaud
Copy link
Member

@Draky50110 > au moins les fichiers, normalement il ne devrait pas y avoir de soucis pour les màj de BDD désormais (tout au plus un ajout ou suppression de colonne). Je verrai s'il est possible de faire des dumps. Au pire tu as la fonction pour exporter les articles ;)

Sinon j'ai créé une nouvelle branche pour la refonte de la fonctionnalité d'update : 411-update-system

Pour le moment j'ai viré les références au système précédent d'update, j'ai fait quelques améliorations pour l'étape 2 et modifié le coding style…

@Draky50110
Copy link
Author

N'étant pas suicidaire, je préfère attendre la branche "dev" :p
Et rajouter une case à cocher "Afficher toutes les catégories même si seulements les articles non lus sont affichés" ? :D

@marienfressinaud
Copy link
Member

Oui faut pas encore utiliser ma branche, elle n'est pas stable du tout ^^ C'est juste histoire de suivre mon travail. De toutes façons aujourd'hui je me lance dans plein de trucs à la fois :p

Pour la case à cocher je pense qu'Alkarex va s'en charger prochainement… sinon je peux le faire dans la journée ;)

@marienfressinaud
Copy link
Member

Je viens de réagir que si l'on veut mettre les fichiers à jour automatiquement, il fautque FreshRSS ait les droits d'écriture sur app/ p/ et lib/ ! >< Du coup je ne sais pas trop si on doit essayer d'automatiser ça où laisser l'utilisateur le faire à la main (sachant que le script s'occupera déjà de faire un backup, mettre à jour la BDD, faire mumuse avec les fichiers quand nécessaire, etc.)

@Draky50110
Copy link
Author

Bah tu peux faire un test sur les droits jute au début non ?
S'il y a les droits ça se met à jour de A à Z sinon, ça dit de les mettre et de cliquer sur "Vérifier encore" ou "Lire la doc pour mettre à jour soi-même" un truc du genre...

marienfressinaud added a commit that referenced this issue Aug 9, 2014
- Check on update.freshrss.org for new updates
- Download script
- Apply script
- Need translations and verifications

NOTE: current script on server indicates version 0.7.3 is an update
of 0.8-dev ==> IT'S ONLY FOR MY TESTS!

Script just does a backup of ./data actually...

See #411
@marienfressinaud
Copy link
Member

Et hop, la base est posée, faudra améliorer un peu tout ça et surtout écrire le(s) scripts de mise à jour côté serveur.

marienfressinaud referenced this issue Aug 10, 2014
Update script must implement 4 functions:
- apply_update() to perform the update (most important). Return true if
  all is ok, else false.
- need_info_update() returns true if we need more info for update, else
  false. If this function always returns false, you don't need to
  implement following functions (but it's better to not forget)
- ask_info_update() should be a HTML form to ask infos. Method must be
  post and action must point to _url('update', 'apply') (or leave it
  blank)
- save_info_update() is called for POST requests (to save form from
  ask_info_update())
@marienfressinaud marienfressinaud self-assigned this Aug 10, 2014
@marienfressinaud marienfressinaud changed the title [Sécurité] Faille lors de la mise à jour : données d'identification en clair ? [Sécurité] Màj : données d'identification en clair -> Refonte système de màj Aug 10, 2014
@marienfressinaud
Copy link
Member

J'ai changé le nom du ticket vu que c'est parti sur la refonte du système de mise à jour. Au passage j'ai amélioré l'API de màj :)

@Draky50110
Copy link
Author

Par contre, maintenant, quand je remplace les fichiers, il me redemande l'ident/user lors de la mise à jour et aussi les infos MySQL.
Il ne peut pas les prendre dans ce qui existe déjà ?

@marienfressinaud
Copy link
Member

Il ne faut pas utiliser le système d'installation, ce sera totalement séparé !

@marienfressinaud
Copy link
Member

Là j'ai un premier système fonctionnel (je viens de réussir à passer de la 0.8-dev vers la 0.7.3, c'est pas dans le bon sens mais c'est pour tester ;))

@marienfressinaud
Copy link
Member

Le dépôt pour le système de màj côté serveur est dispo : https://github.com/marienfressinaud/update.freshrss.org

@marienfressinaud
Copy link
Member

Voilà, le système de mise à jour est dispo et fonctionnel ! :) Par contre, quelques trucs à savoir :

  • Pour le moment, niveau sécurité c'est pas encore top : un man-in-the-middle est possible et pourrait être catastrophique. J'envisage au moins de passer par HTTPS. Le problème c'est niveau certificat qui sera auto-signé :(
  • Il faudrait aussi checker l'intégrité du script téléchargé (signature md5 ou sha-1 ?).
  • Je ne teste pas encore les droits sur /app /lib /p /constants.php, si ça plante… ça plante.
  • LA 0.8-dev SE MET À JOUR VERS LA 0.7.3 POUR LES TESTS se met à jour sur la branche dev !

Y a besoin de retours maintenant :)

@Draky50110
Copy link
Author

Ha ok, parce voici ce que je fais pour mettre à jour ma dev en SSH :

  • rm -rf ancienbackup
  • wget github/dev/zip
  • unzip dev.zip
  • cp -rv frss-dev/data/ Freshdecompressed/data
  • mv frss-dev ancienbackup
  • mv Freshdecompressed frss-dev

C'est clair ? lol

Du coup, quand je retourne sur l'instance avec Firefox, bah ça mets à jour.. en demandant les ident

@marienfressinaud
Copy link
Member

Bah là il faudra attendre au moins la 0.7.4 pour voir le système arriver. Faudra voir pour l'inclure dans la 0.8-dev bientôt.

Le système sera :

  1. Installer FRSS une première fois
  2. Aller dans l'onglet "Mise à jour", cliquer sur "Vérifier les mises à jour", puis valider
  3. Attendre tout au plus 2 secondes.
  4. C'est tout bon :)

@marienfressinaud
Copy link
Member

Désormais la 0.8-dev se met à jour sur la branche de dev ;)

@Alkarex
Copy link
Member

Alkarex commented Aug 11, 2014

@marienfressinaud Pour éviter les certificats auto-signés, voir https://www.startssl.com gratuit que j'utilise.
L'auto-signé n'est pas un problème (et parfois même mieux) si on peut installer ses propres certificats sur les clients, mais ça ne marche pas pour des connexions qui doivent être accessibles par le public.
P.S. : Gandi (français) a des prix raisonnables https://www.gandi.net/ssl/

@marienfressinaud
Copy link
Member

@Alkarex > oui je sais, j'en ai déjà un pour mon domaine + un sous-domaine (sinon je ne pouvais pas utiliser mon FirefoxOS pour les mails et agenda). Mon problème c'est que freshrss.org se situe sur le même serveur et je ne suis pas sûr que ça soit possible de faire du "multi-certificats" avec mon multi-domaines :(

@Alkarex
Copy link
Member

Alkarex commented Aug 11, 2014

Il faut faire un second certificat, et ensuite si tu gères l'ensemble des domaines du serveur, il y a plusieurs techniques possibles, avec ou sans SNI, par exemple avec un simple RewriteCond %{HTTP_HOST} ...

@marienfressinaud
Copy link
Member

Bonne idée ! J'ai commencé la procédure, mais fallait que je configure le serveur mail sur le ndd freshrss.org, autant dire que ça m'a prit du temps. Je pense avoir à peu près fini mais il faut attendre que la config DNS se propage !

@marienfressinaud
Copy link
Member

Serveur configuré ! https://update.freshrss.org/ Ça n'a pas été de la tarte mais ça semble fonctionner :)

Edit : il manquant encore le paramètre SSLCertificateChainFile, c'est réglé !

marienfressinaud referenced this issue Aug 12, 2014
- Add some curl checks
- Refactor code
@Alkarex
Copy link
Member

Alkarex commented Aug 12, 2014

👍

@Draky50110
Copy link
Author

Du coup, c'est implémenté dans dev ?
Si je mets à jour via SSH comme d'hab., je pourrais faire les màj DEV suivante avec cet "outils" ?

@marienfressinaud
Copy link
Member

Non pas encore, c'est encore dans une branche à part parce que je n'ai pas tout à fait fini !

@Draky50110
Copy link
Author

👍

marienfressinaud added a commit that referenced this issue Sep 8, 2014
Add new update system.

See #411
marienfressinaud added a commit that referenced this issue Sep 8, 2014
@marienfressinaud
Copy link
Member

Ajouté dans /dev :) Par contre, pour les développeurs, je continue de préconiser les mises à jour par Git.

Je vous conseille de tester aussi pour le moment avec des données non critiques (même si un backup est fait).

Signalez-moi tout problème :)

@marienfressinaud
Copy link
Member

Ah bah en voilà un premier sur mon serveur, ça me remonte " Arf ! Le serveur de mise à jour n’a pas été trouvé. [https://update.freshrss.org?v=0.8-dev] "

Edit : problème de certificat ! Je regarderai plus tard…

@marienfressinaud
Copy link
Member

Avec l'ajout du système de mises à jour, il n'y a plus besoin de l'accès à la page d'installation : problème initial résolu.

Pour les discussions et problèmes à propos du nouveau système de màj, je propose d'ouvrir des tickets spécifiques (ça a déjà commencé ^^).

Le dernier problème que j'ai remonté vient d'une mauvaise config de mon serveur, pas de FRSS.

Je ferme donc ce ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants