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

Imprv: リバースプロキシを経由したローカルファイルアップロード時、ホスト名を含まないようにする #282

Closed
rabitarochan opened this issue Feb 23, 2018 · 4 comments

Comments

@rabitarochan
Copy link
Contributor

Environment

Host

item version
OS Amazon Linux 2
crowi-plus 2.4.1-RC
node.js 6.13.0
npm 4.6.1
Using Docker no
Using crowi-plus-docker-compose no

(Accessing https://{CROWI_HOST}/admin helps you to fill in above versions)

Client

item version
OS Windows 10
browser Google Chrome, Internet Explorer

How to reproduce? (再現手順)

  1. crowi-plus を 3000 番ポートで起動する。
  2. httpd を 443 番ポート (HTTPS) で起動し、リクエストを localhost:3000 (crowi-plus) に転送するようにリバースプロキシを構築する。
  3. 任意のページでファイルをアップロードする。

What happens? (症状)

  • アップロードしたファイルのリンクに、apache のオリジン情報ではなく crowi-plus のオリジン情報が含まれるため、ファイルが参照できない。
![test.png](http://localhost:3000/uploads/xxxx/yyyy.png)

What is the expected result? (期待される動作)

  • ローカルを利用したファイルアップロード時には、サーバーのオリジン情報を付与しない。
![test.png](/uploads/xxxx/yyyy.png)
  • もしくは、リバースプロキシのオリジン情報を取得するようにする。
    • リクエストヘッダーの X-Forwarded-* を参照する。

Note

  • Amazon S3を利用したアップロード時には、 https://<バケット名>/... というURLになるため、同様の事象は発生しない。
  • リクエストヘッダーの X-Forwarded-* は偽装される可能性もあるため、サーバーのオリジン情報を付与しない方式のほうがいいと考える。
@yuki-takei
Copy link
Member

yuki-takei commented Feb 23, 2018

@rabitarochan ご報告ありがとうございます。そしてPRまで…助かります。
まず再現と、「サーバーのオリジン情報を付与しない方式」で解決するかどうかを確認します。

ただその方式で解決するかどうかとは別の観点でに、#282 をそのままマージするかどうかはちょっと検討が必要だと思っています。ちょっと考えてみます。

@yuki-takei
Copy link
Member

…と書きましたが…

fileUrl.match(/^https?/) は汚いので確かに消し去りたい。普通にマージしちゃってもいいような気がしてきました。

この attachment.js に若干手を入れたのが去年の5月でほとんど挙動を覚えていなくて不安…というだけですね。

@rabitarochan
Copy link
Contributor Author

@yuki-takei ファイルアップロードの方式は 3 種類 (aws, local, none) あって、それらの違いは local_modules 以下のコードで吸収されていました。

アクセス用 URL は generateUrl というメソッドで作成されています。AWS 方式の場合は http://<S3バケット名>/... という URL を作成していますので、PR したように削除しても問題ないと考えています。

上記の処理は Wiki のファイルアップロード以外に、アバター画像のアップロードでも使われていて、そちらも動作確認済みです。

...といいつつ、AWS 方式を試していないので検証は必要かと思います。

yuki-takei added a commit that referenced this issue Feb 23, 2018
@yuki-takei
Copy link
Member

yuki-takei commented Feb 23, 2018

@rabitarochan こちらですが、FILE_UPLOAD=local 時は base url からの相対パスが望ましい、と判断し、マージすることにしました。v2.4.2 のリリースをお待ちください。

因みにこちらでは以下のように実験してみました。

おまけ

crowi-plus の起動方法

実験結果

![test.png](http://localhost:3000/uploads/xxxx/yyyy.png)

ではなく、

![test.png](https://example.com/uploads/xxxx/yyyy.png)

が貼り付けられた。つまり期待される動作の

リクエストヘッダーの X-Forwarded-* を参照する。

に該当する挙動を、今回のPRのマージを行わなくても設定次第で実現可能だった。

考察

https-portal 利用の場合、

https://github.com/SteveLTN/https-portal/blob/118dd745992195a60a035335d47f9828c9f5700f/fs_overlay/var/lib/nginx-conf/default.ssl.conf.erb

が nginx のコンフィグとして設定されます。

crowi-plus 側では

https://github.com/weseek/crowi-plus/blob/6a0435ac5b58892ed48f2119e57de740ad23aa54/lib/crowi/express-init.js#L63

config.crowi['app:url'] を決定しますので、今回の nginx で言うとproxy_set_header 周りの記述が適切になされていれば、ご報告いただいた症状を回避できるのではないかと推察します。

proxy 側の設定が及ぼすその他の影響

今回はいただいた PR をマージすることにより、FILE_UPLOAD=local 設定下でのファイルアップロード処理に関してはその影響(リバプロの設定が十分かどうかに左右される)を免れることになります。

ただ、config.crowi['app:url'] を利用する箇所は他にもあって、特に Google OAuth 時に利用するコールバックURL、Slack OAuth 時に利用するリダイレクト用URLの生成ではシステムをリバプロの外側から見た際の正しいURLが必要になるので、いずれにせよリバプロ時に諸々の設定は必要になる、ということになるかと思います。

@yuki-takei yuki-takei changed the title Bug: リバースプロキシを経由したローカルファイルアップロード後のファイルが参照できない Imprv: リバースプロキシを経由したローカルファイルアップロード時、ホスト名を含まないようにする Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants