En 3.1.8 avec les formulaires de formidable il y a la possibilité de stocker l'IP du visiteur, avec la config du serveur de Nfrance sur un site, l'IP stocké était systématiquement celle du serveur.
J'ai résolu ce souci en mettant la condition REMOTE_ADDR devant la condition de HTTP_X_FORWARDED_FOR
//
// On note le numero IP du client dans la variable $ip
//
if (isset($_SERVER['REMOTE_ADDR'])) {
$ip = $_SERVER['REMOTE_ADDR'];
}
voir aussi $_SERVER["REMOTE_ADDR"] gives server IP rather than visitor IP
Suite à https://core.spip.net/issues/3386#change-13809, je déplace ici.
En 3.1.8 avec les formulaires de formidable il y a la possibilité de stocker l'IP du visiteur, avec la config du serveur de Nfrance sur un site, l'IP stocké était systématiquement celle du serveur.
J'ai résolu ce souci en mettant la condition REMOTE_ADDR devant la condition de HTTP_X_FORWARDED_FOR
<pre>
//
// On note le numero IP du client dans la variable $ip
//
if (isset($_SERVER['REMOTE_ADDR'])) {
$ip = $_SERVER['REMOTE_ADDR'];
}
</pre>
https://core.spip.net/projects/spip/repository/revisions/23406/entry/spip/ecrire/inc_version.php#L253
voir aussi $_SERVER["REMOTE_ADDR"] gives server IP rather than visitor IP
Le problème c'est que si on met ça par défaut dans SPIP, ça permet à n'importe qui de faker son IP en envoyant un entete http REMOTE_ADDR.
Dans ton cas c'est sans risque car le REMOTE_ADDR vient de Varnish ou du reverse proxy qui est devant SPIP, et donc tu es sûr qu'il est fiable, mais difficile de généraliser sans risque…
Le problème c'est que si on met ça par défaut dans SPIP, ça permet à n'importe qui de faker son IP en envoyant un entete http REMOTE_ADDR.
Dans ton cas c'est sans risque car le REMOTE_ADDR vient de Varnish ou du reverse proxy qui est devant SPIP, et donc tu es sûr qu'il est fiable, mais difficile de généraliser sans risque…
Puisque ça semble difficile de généraliser le patch sans risque je pense qu'on peut fermer le ticket, et documenter ce cas et son contexte dans la doc de formidable par exemple. Ça vous va ? Statut changé à En cours
Puisque ça semble difficile de généraliser le patch sans risque je pense qu'on peut fermer le ticket, et documenter ce cas et son contexte dans la doc de formidable par exemple. Ça vous va ?
**Statut changé à En cours**
Le problème c'est que si on met ça par défaut dans SPIP, ça permet à n'importe qui de faker son IP en envoyant un entete http REMOTE_ADDR.
Dans ton cas c'est sans risque car le REMOTE_ADDR vient de Varnish ou du reverse proxy qui est devant SPIP, et donc tu es sûr qu'il est fiable, mais difficile de généraliser sans risque…
Dans le même genre, ce que je ne comprends pas dans ce bout de code, c'est qu'on écrase l'ip attrapée via HTTP_X_FORWARDED_FOR par REMOTE_ADDR si REMOTE_ADDR n'est pas 127.0.0.1 (hors dans mon cas, c'est l'ip d'un load balancer), donc elle n'est pas écrasée par HTTP_X_FORWARDED_FOR (qui est bonne elle)
Si l'ip est renseignée via HTTP_X_FORWARDED_FOR on ne devrait pas prendre celle-ci dans tous les cas, et ne faire confiance à REMOTE_ADDR qu'en dernier recours ?
cedric - a écrit :
> Le problème c'est que si on met ça par défaut dans SPIP, ça permet à n'importe qui de faker son IP en envoyant un entete http REMOTE_ADDR.
> Dans ton cas c'est sans risque car le REMOTE_ADDR vient de Varnish ou du reverse proxy qui est devant SPIP, et donc tu es sûr qu'il est fiable, mais difficile de généraliser sans risque…
Dans le même genre, ce que je ne comprends pas dans ce bout de code, c'est qu'on écrase l'ip attrapée via HTTP_X_FORWARDED_FOR par REMOTE_ADDR si REMOTE_ADDR n'est pas 127.0.0.1 (hors dans mon cas, c'est l'ip d'un load balancer), donc elle n'est pas écrasée par HTTP_X_FORWARDED_FOR (qui est bonne elle)
Si l'ip est renseignée via HTTP_X_FORWARDED_FOR on ne devrait pas prendre celle-ci dans tous les cas, et ne faire confiance à REMOTE_ADDR qu'en dernier recours ?
Ca ne devrait donc pas être un else if plutôt qu'un if ?
`
else if (isset($_SERVER['REMOTE_ADDR'])) {
$ip = $_SERVER['REMOTE_ADDR'];
}
`
https://core.spip.net/projects/spip/repository/revisions/23406/entry/spip/ecrire/inc_version.php#L253
Suite à https://core.spip.net/issues/3386#change-13809, je déplace ici.
En 3.1.8 avec les formulaires de formidable il y a la possibilité de stocker l'IP du visiteur, avec la config du serveur de Nfrance sur un site, l'IP stocké était systématiquement celle du serveur.
J'ai résolu ce souci en mettant la condition REMOTE_ADDR devant la condition de HTTP_X_FORWARDED_FOR
// // On note le numero IP du client dans la variable $ip // if (isset($_SERVER['REMOTE_ADDR'])) { $ip = $_SERVER['REMOTE_ADDR']; }https://core.spip.net/projects/spip/repository/revisions/23406/entry/spip/ecrire/inc_version.php#L253
voir aussi $_SERVER["REMOTE_ADDR"] gives server IP rather than visitor IP
Le problème c'est que si on met ça par défaut dans SPIP, ça permet à n'importe qui de faker son IP en envoyant un entete http REMOTE_ADDR.
Dans ton cas c'est sans risque car le REMOTE_ADDR vient de Varnish ou du reverse proxy qui est devant SPIP, et donc tu es sûr qu'il est fiable, mais difficile de généraliser sans risque…
Puisque ça semble difficile de généraliser le patch sans risque je pense qu'on peut fermer le ticket, et documenter ce cas et son contexte dans la doc de formidable par exemple. Ça vous va ?
Statut changé à En cours
cedric - a écrit :
Dans le même genre, ce que je ne comprends pas dans ce bout de code, c'est qu'on écrase l'ip attrapée via HTTP_X_FORWARDED_FOR par REMOTE_ADDR si REMOTE_ADDR n'est pas 127.0.0.1 (hors dans mon cas, c'est l'ip d'un load balancer), donc elle n'est pas écrasée par HTTP_X_FORWARDED_FOR (qui est bonne elle)
Si l'ip est renseignée via HTTP_X_FORWARDED_FOR on ne devrait pas prendre celle-ci dans tous les cas, et ne faire confiance à REMOTE_ADDR qu'en dernier recours ?
Ca ne devrait donc pas être un else if plutôt qu'un if ?
else if (isset($_SERVER['REMOTE_ADDR'])) { $ip = $_SERVER['REMOTE_ADDR']; }https://core.spip.net/projects/spip/repository/revisions/23406/entry/spip/ecrire/inc_version.php#L253
C'est vraiment une situation au cas par cas on ne change rien sur la config par defaut...
Statut changé à Fermé