EC-CUBE / ec-cube

EC-CUBE is the most popular e-commerce solution in Japan
https://www.ec-cube.net
Other
745 stars 648 forks source link

ログの出力で X-Forwarded-For が考慮されていない #4601

Closed mattintosh4 closed 4 years ago

mattintosh4 commented 4 years ago

概要(Overview)

下記の部分ですが REMOTE_ADDR を返しているためロードバランサー等を使用している環境ではログに出力される IP がすべてロードバランサーの IP になってしまいます。

https://github.com/EC-CUBE/ec-cube/blob/002ad401b57218c6f0458507ab02110d33b97458/src/Eccube/Log/Monolog/Processor/WebProcessor.php#L51-L54

期待する内容(Expect) or 要望 (Requirement)

X-Forwarded-For が存在する場合は X-Forwarded-For の値を返す方が良いのですが、以下の選択肢が出てくると思います。

, が増えても問題ないのであれば X-Forwarded-For を返す(, でログをパースしている場合に影響が出る可能性あり)

--- /dev/fd/63  2020-06-28 23:29:21.090798138 +0900
+++ src/Eccube/Log/Monolog/Processor/WebProcessor.php   2020-06-28 23:28:21.496937570 +0900
@@ -50,6 +50,9 @@

     public function getClientIp()
     {
+        if (!empty($this->serverData['HTTP_X_FORWARDED_FOR'])) {
+            return $this->serverData['HTTP_X_FORWARDED_FOR'];
+        }
         return isset($this->serverData['REMOTE_ADDR']) ? $this->serverData['REMOTE_ADDR'] : null;
     }

, が増えると問題になる場合は X-Forwarded-For の一番左を返す(一番左にクライアント IP が入っていることを信頼する場合)

--- /dev/fd/63  2020-06-28 23:29:21.090798138 +0900
+++ src/Eccube/Log/Monolog/Processor/WebProcessor.php   2020-06-28 23:28:21.496937570 +0900
@@ -50,6 +50,9 @@

     public function getClientIp()
     {
+        if (!empty($this->serverData['HTTP_X_FORWARDED_FOR'])) {
+           return array_map('trim', explode(',', $this->serverData['HTTP_X_FORWARDED_FOR']))[0];
+        }
         return isset($this->serverData['REMOTE_ADDR']) ? $this->serverData['REMOTE_ADDR'] : null;
     }

ロードバランサー下で使うことを前提としておらず、各々のカスタマイズの範疇ということであればご放念ください。

再現手順(Procedure)

ロードバランサーを使用している環境。

環境 (environment)

関連情報 (Ref)

okazy commented 4 years ago

私もロードバランサをかました時に同様の課題にぶち当たりました。。。

@mattintosh4 3系のリポジトリは4系のリポジトリから分かれ、以下に移行されています。 https://github.com/EC-CUBE/ec-cube3/issues

お手数ですが3系の以下のリポジトリでIssueを立て直していただけますでしょうか?

mattintosh4 commented 4 years ago

@okazy 自前の環境の場合はインフラ側で調整出来ないこともないのですが、他の部分で X-Forwarded-For を考慮している部分もあるのでこちらも考慮してみてはいかがかなと思いました。 3系は別になっていたのですね。失礼しました。後ほど立て直しさせていただきます。

mattintosh4 commented 4 years ago

3系の方に転機させていただきましたのでクローズします。 https://github.com/EC-CUBE/ec-cube3/issues/88

okazy commented 4 years ago

ありがとうございます!