misskey icon indicating copy to clipboard operation
misskey copied to clipboard

CSPを導入する

Open Ry0taK opened this issue 3 years ago • 1 comments

Summary

最低でもscript-srcbase-uriを全ページで指定し、XSSが存在した場合の軽減策として機能するようにしたい。

Description

CSPを適切に設定することでXSSが存在した場合にある程度影響を軽減できる。style-srcimg-src等は想定していない箇所が壊れる可能性があるので、一旦script-srcbase-uriを指定し、最低限無いよりマシな状況にしたい。 CSPについてはこちらを参照: https://developer.mozilla.org/ja/docs/Web/HTTP/CSP

(良さそうであれば実装してみるので教えてください。)

script-srcに関して

以下の理由により、JavaScriptを全部外部のファイルに分けてscript-src 'self'のみを使う形にするのが一番良さそう

nonceを用いたCSP

nonce形式を用いる場合、リクエストごとにランダムなnonceを生成してクライアントに返す必要がある。 また、そのnonceをそれぞれのscriptタグに付与する必要が出てくるが、@fastify/view/pugには動的にnonceを付与する機能が無いため、pugでテンプレートを描画した後に一度サーバー側でDOMをパースしてそれぞれのscriptタグにnonceを付与する必要が出てくると思われる。 (他にいい実装方法を思いつく方は教えてください。)

ハッシュを用いたCSP

scriptタグ内のコンテンツのハッシュを計算し、それを予めContent-Security-Policyヘッダで指定する必要がある。 この方法だとビルド時 or 起動時に一度だけ計算すればそれ以降はContent-Security-Policyヘッダに付与しておけば良くなる。 ただし、どこか(多分pug)でJavaScriptの内容がminifyされているため、minify後のコンテンツをどうにかして取ってハッシュを計算する必要がある。

script-src 'self'

script-src 'self'を用いる場合、ページ内に埋め込まれたスクリプト (= inline script)を全て外部のファイルに分離する必要がある。(別ファイルにすることによるパフォーマンスの低下は未計測です。) 確認できた限り以下を外部のファイルへ切り分ける必要がありそう。

https://github.com/misskey-dev/misskey/blob/6e61a36d05e886393a1ee204544420ad66fc6227/packages/backend/src/server/web/views/base.pug#L66-L71

(pugはJavaScriptの動的な生成をサポートしていなさそうなので、このインラインスクリプトをどうするか考える必要がある)

https://github.com/misskey-dev/misskey/blob/6e61a36d05e886393a1ee204544420ad66fc6227/packages/backend/src/server/web/views/bios.pug#L11-L12 https://github.com/misskey-dev/misskey/blob/19035c676cd1a8afc1d798ff85147a523df95985/packages/backend/src/server/web/views/cli.pug#L11-L12 https://github.com/misskey-dev/misskey/blob/19035c676cd1a8afc1d798ff85147a523df95985/packages/backend/src/server/web/views/flush.pug#L5-L7

https://github.com/misskey-dev/misskey/blob/19035c676cd1a8afc1d798ff85147a523df95985/packages/backend/assets/redoc.html#L22 最後のもののみintegrity属性を設定しているので、ここだけ例外的にハッシュを使用する形にしても良さそう

base-uriに関して

確認した限りbaseタグは使用していないため、base-uriは'self'決め打ちで良さそう。

Ry0taK avatar Feb 09 '23 14:02 Ry0taK

ありがとうございます。

とりあえずコンテキストの共有ですが、現在の server/web/views あたりの構造(どのようにバックエンドが HTML を生成してフロントエンドのアセットを配信するか)は aoi 時代からのものからあまり大胆に手を加えていないまま今日に至るので、大胆な変更もやぶさかでなく、見直しの自由度は高いです。

Related to #16, #5709, #8888

例えば

最後のもののみintegrity属性を設定しているので、ここだけ例外的にハッシュを使用する形にしても良さそう

は、ひとまず OpenAPI のドキュメントが見られる状態になることを目的としてシンプルに組み上げていたり(#4351)しているだけで、そもそも特段 CDN で切り出している理由はない認識です。

acid-chicken avatar Feb 09 '23 15:02 acid-chicken

とりあえずコンテキストの共有ですが、現在の server/web/views あたりの構造(どのようにバックエンドが HTML を生成してフロントエンドのアセットを配信するか)は aoi 時代からのものからあまり大胆に手を加えていないまま今日に至るので、大胆な変更もやぶさかでなく、見直しの自由度は高いです。

様々な方法を検討してみたのですが、以下のような理由で大幅な変更は行わずにJavaScriptファイルを外部に分ける形での対応が良さそうでした。

nonceを用いたCSP

@fastify/helmet を用いることによりdecoratorを使って動的なnonceの付与ができそうではあるが、この手法を使った場合毎回HTMLレスポンスが動的に変化してしまうため、キャッシュなどの観点から考えると望ましくない

ハッシュを用いたCSP

テンプレートエンジンを変更し、動的にインラインスクリプトのハッシュ値を計算できると嬉しいという点はありつつ、@fastify/viewがサポートしているテンプレートエンジンの中に当該の機能を持つものが無さそう (あるにはあるが、結局外部のファイルへ分割してそのファイルのハッシュ値を計算しているため、これを行う事によるメリットがあまり無い)

例えば

最後のもののみintegrity属性を設定しているので、ここだけ例外的にハッシュを使用する形にしても良さそう

は、ひとまず OpenAPI のドキュメントが見られる状態になることを目的としてシンプルに組み上げていたり(#4351)しているだけで、そもそも特段 CDN で切り出している理由はない認識です。

こちらのドキュメントに関してなのですが、どうやら 3a7182bfb5734599321fc03ea77c48b4dbc326d5 で参照していたエンドポイントが削除されているようで、どこからも参照されていない状態になっているようでした。 そのためこのファイルに関しては特別な対応を行わず、別途削除してしまって良さそうな雰囲気があります。

また、検討を進める中で懸念事項として、Misskeyをカスタマイズして外部のJavaScriptを挿入している場合にCSPの追加が破壊的変更となりうるという点に気が付きました。 例えばmisskey.ioにおいては、CloudflareのWeb Analyticsの利用やCloudflare AppsによるJavaScriptの挿入を行っているため、script-src 'self'を追加するとこれらのJavaScriptが動作しなくなります。 そのため、CSPの導入にあたっては設定ファイルで柔軟に変更できるようにしつつ、リリースノートにその旨を明記しておく必要がありそうです。

Ry0taK avatar Feb 10 '23 09:02 Ry0taK