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.

Sign up for GitHub

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

Jump to bottom

SameSite cookie support #374

Merged
merged 7 commits into from
Feb 13, 2020
Merged

SameSite cookie support #374

merged 7 commits into from
Feb 13, 2020

Conversation

nanasess
Copy link
Contributor

@nanasess nanasess commented Feb 7, 2020 •
edited
Loading

  • SameSite=None を未サポートの UA 向けに, SameSite 属性を削除した互換用 cookie を発行する
  • UA の仕様により SameSite=None が SameSite=Strict と見なされて cookie が拒否された場合は, 互換用の cookie を読み込む
  • ローカル環境等、非SSL環境では SameSite 属性を空にし、 secure 属性を付与しない

refs EC-CUBE/ec-cube#4457

旧バージョンのサポート

2.4

こちらのパッチを適用する

2.11.x

こちらのパッチを適用する

2.12.x, 2.13.x

テスト

  • iOS12.0, iOS12.4, macOS 10.14 Safari 12.1.2, 過去バージョンのChromium(Chrome51 〜 66相当) で検証したが、 SameSite=None が Strict と扱われる ような挙動は再現しなかった
  • SameSite=None が Strict と扱われた際の動作をシミュレートするため、 SameSite=Strict に修正し、外部サイトからの POST で互換用 Cookie を読み込み、正常にセッションが継続することを確認

TODO

  • PHP7.3 or higher の対応
  • 互換用 cookie を読み込む際に secure 属性をチェックした方がいいかも
  • 互換用 cookie を読み込んだ後に削除した方がいいかも
  • ログ出力は削除予定

@nanasess
SameSite cookie support
c116980 
- TODO PHP7.3 or higher
- SameSite=None を未サポートの UA 向けに, SameSite 属性を削除した cookie を発行する
- SameSite=None が SameSite=Strict と見なされて cookie が拒否された場合は, 互換用の cookie を読み込む
@nanasess

This comment has been minimized.

Sign in to view
nanasess
nanasess commented Feb 7, 2020
View reviewed changes
data/class/helper/SC_Helper_Session.php Outdated
@@ -68,6 +68,11 @@ public function sfSessClose()
*/
public function sfSessRead($id)
{
if (empty($_COOKIE['ECSESSID']) && $id !== $_COOKIE['legacy-ECSESSID']) {
// session_id と $_COOKIE['legacy-ECSESSID'] が異なる場合は ECSESSID の cookie が拒否されたと見なす
GC_Utils_Ex::gfPrintLog('replace session id: '.$id.'=>'.$_COOKIE['legacy-ECSESSID']);

This comment was marked as resolved.

Sign in to view

@nanasess
ログ出力削除
00a6726
@nanasess
PHP7.3 or higher support
8ff48a4 
- secure オプションが無い場合は SameSite を空で送る
- 互換用 cookie は secure オプション必須にする
@nanasess
互換用 cookie を読み込んだあとは削除する
d3cd7dd
@nanasess
コメント修正
bf2bac1
@nanasess nanasess changed the title [WIP] SameSite cookie support SameSite cookie support Feb 10, 2020
@chihiro-adachi chihiro-adachi added this to the 2.17.1 milestone Feb 12, 2020
@kiy0taka kiy0taka merged commit 930e6e5 into EC-CUBE:master Feb 13, 2020
@asano-shinichi
Copy link

asano-shinichi commented Mar 3, 2020 •
edited
Loading

@nanasess
2.1X系に関しての確認ですが、こちらのパッチを当てることに加え、
config.php のHTTP_URLとHTTPS_URLのどちらにも https:// のURLを指定する必要がある
という認識でよろしいでしょうか。

@nanasess
Copy link
Contributor Author

nanasess commented Mar 3, 2020

@asano-shinichi はい、ご認識に問題ございません

@asano-shinichi
Copy link

@nanasess
ご確認頂きありがとうございます！
一部、HTTPの指定が残っているサイトがあったので、そのように対応致します。

@nanasess nanasess deleted the fix-samesite-cookie branch October 3, 2022 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
2.17.1
Development

Successfully merging this pull request may close these issues.
4 participants
@nanasess @asano-shinichi @kiy0taka @chihiro-adachi