From 30fd0c428ca60ee5d5297404be6ddd362ffb0360 Mon Sep 17 00:00:00 2001 From: Min-Soo Kim Date: Sun, 29 Jul 2018 12:15:24 +0900 Subject: [PATCH] Improve cookie security; Secure flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SSL 항상 사용 옵션인 경우 쿠키도 이에 맞추어 SSL 인 경우에만 사용되도록 secure flag 를 추가합니다. 선택적 SSL 인 경우 SSL 이 적용되지 않은 구간에서도 쿠키를 읽을 수 있어야 하므로, 적용하지 않습니다. 이 PR 로 변경되는 내용 - Context 클래스에 checkSslEnforce 메소드 추가 - SSL 항상 사용 옵션일 경우, 가능한 secure 플래그를 달아서 쿠기 굽기 - SSO 쿠키의 경우 javascript 접근이 필요 없을 것으로 예상 ( https://github.com/rhymix/rhymix/pull/1034 ) 되어서 `httpOnly` 플래그도 추가. 안드로이드 웹뷰의 경우 StackOverFlow 의 Reading secure cookies in android WebView 라는 글을 참고하면, 읽어오는 것이 가능하다고 합니다. 주소에 프로토콜을 적지 않을 경우 secure flag 가 달린 쿠키는 정상적으로 읽어오지 않는 듯 하니 안드로이드 웹뷰를 사용하시는 분들은 대응이 필요할 것으로 보입니다. https: //github.com/rhymix/rhymix/pull/1034 를 실수로 merge 하여서 다시 올립니다. Co-Authored-By: Kijin Sung --- classes/context/Context.class.php | 23 ++++++++++++++++--- classes/mobile/Mobile.class.php | 2 +- common/framework/session.php | 2 +- common/js/common.js | 3 ++- .../editor/skins/ckeditor/file_upload.html | 2 +- modules/member/member.view.php | 2 +- .../skins/ncenter_login/js/ncenter.js | 2 +- 7 files changed, 27 insertions(+), 9 deletions(-) diff --git a/classes/context/Context.class.php b/classes/context/Context.class.php index e05eb5e42..e44f60a42 100644 --- a/classes/context/Context.class.php +++ b/classes/context/Context.class.php @@ -306,7 +306,7 @@ class Context { if($_COOKIE['lang_type'] !== $lang_type) { - setcookie('lang_type', $lang_type, $_SERVER['REQUEST_TIME'] + 3600 * 24 * 1000, '/'); + setcookie('lang_type', $lang_type, time() + 86400 * 365, '/', null, self::isAlwaysSSL()); } } elseif($_COOKIE['lang_type']) @@ -322,7 +322,7 @@ class Context if(!strncasecmp($lang_code, $_SERVER['HTTP_ACCEPT_LANGUAGE'], strlen($lang_code))) { $lang_type = $lang_code; - setcookie('lang_type', $lang_type, $_SERVER['REQUEST_TIME'] + 3600 * 24 * 1000, '/'); + setcookie('lang_type', $lang_type, time() + 86400 * 365, '/', null, self::isAlwaysSSL()); } } } @@ -623,6 +623,23 @@ class Context return self::get('_use_ssl'); } + /** + * Return ssl status + * + * @param boolen $purge_cache Set true to get uncached SSL_enforce value. + * @return boolean (true|false) + */ + public static function isAlwaysSSL($purge_cache = false) + { + static $ssl_only = null; + if(is_null($ssl_only) || $purge_cache === true) + { + $ssl_only = (self::get('site_module_info')->security === 'always' ? true : false); + } + return $ssl_only; + } + + /** * Return default URL * @@ -1775,7 +1792,7 @@ class Context return; } - if(self::get('_use_ssl') == 'always') + if(self::isAlwaysSSL()) { $ssl_mode = ENFORCE_SSL; } diff --git a/classes/mobile/Mobile.class.php b/classes/mobile/Mobile.class.php index 4bb976cd4..ef69cef99 100644 --- a/classes/mobile/Mobile.class.php +++ b/classes/mobile/Mobile.class.php @@ -73,7 +73,7 @@ class Mobile $uatype = $uahash . ':' . (self::$_ismobile ? '1' : '0'); if ($cookie !== $uatype) { - setcookie('rx_uatype', $uatype, 0); + setcookie('rx_uatype', $uatype, 0, null, null, Context::isAlwaysSSL()); $_COOKIE['rx_uatype'] = $uatype; } diff --git a/common/framework/session.php b/common/framework/session.php index d7c659b4e..512b917f2 100644 --- a/common/framework/session.php +++ b/common/framework/session.php @@ -295,7 +295,7 @@ class Session if(!$is_default_domain && !\Context::get('sso_response') && $_COOKIE['sso'] !== md5($current_domain)) { // Set sso cookie to prevent multiple simultaneous SSO validation requests. - setcookie('sso', md5($current_domain), 0, '/'); + setcookie('sso', md5($current_domain), 0, '/', null, \Context::isAlwaysSSL(), true); // Redirect to the default site. $sso_request = Security::encrypt($current_url); diff --git a/common/js/common.js b/common/js/common.js index 14243dce4..7280af53b 100644 --- a/common/js/common.js +++ b/common/js/common.js @@ -1055,7 +1055,8 @@ function getOuterHTML(obj) { function setCookie(name, value, expire, path) { var s_cookie = name + "=" + escape(value) + ((!expire) ? "" : ("; expires=" + expire.toGMTString())) + - "; path=" + ((!path) ? "/" : path); + "; path=" + ((!path) ? "/" : path) + + ((enforce_ssl) ? ";secure" : ""); document.cookie = s_cookie; } diff --git a/modules/editor/skins/ckeditor/file_upload.html b/modules/editor/skins/ckeditor/file_upload.html index 4e80dc0e8..f62561f56 100644 --- a/modules/editor/skins/ckeditor/file_upload.html +++ b/modules/editor/skins/ckeditor/file_upload.html @@ -7,7 +7,7 @@
{$lang->edit->upload_file} - +

{$lang->ckeditor_about_file_drop_area}

diff --git a/modules/member/member.view.php b/modules/member/member.view.php index d6057bd33..65974fa00 100644 --- a/modules/member/member.view.php +++ b/modules/member/member.view.php @@ -192,7 +192,7 @@ class memberView extends member function dispMemberSignUpForm() { //setcookie for redirect url in case of going to member sign up - setcookie("XE_REDIRECT_URL", $_SERVER['HTTP_REFERER']); + setcookie("XE_REDIRECT_URL", $_SERVER['HTTP_REFERER'], 0, '/', null, Context::isAlwaysSSL()); $member_config = $this->member_config; diff --git a/widgets/login_info/skins/ncenter_login/js/ncenter.js b/widgets/login_info/skins/ncenter_login/js/ncenter.js index b7f3fddf0..92b073686 100644 --- a/widgets/login_info/skins/ncenter_login/js/ncenter.js +++ b/widgets/login_info/skins/ncenter_login/js/ncenter.js @@ -8,7 +8,7 @@ dt.setTime(dt.getTime() + (d * 24 * 60 * 60000)); e = "; expires=" + dt.toGMTString(); } - document.cookie = n + "=" + v + e + "; path=/"; + document.cookie = n + "=" + v + e + "; path=/" + ((enforce_ssl) ? ";secure" : ""); } var n = $('#nc_container');