Skip to content

Commit

Permalink
Fixed session/cookie and updated OpenEMR session/cookie strategy (ope…
Browse files Browse the repository at this point in the history
…nemr#2524)

Updated OpenEMR session/cookie strategy

1. If a session does not yet exist, then will start the OpenEMR session, which
    will create a cookie with OpenEMR name. If a session already exists, then
    this means portal is being used and will bypass setting of the OpenEMR session/cookie.
2. If using php version 7.3.0 or above, then will set the cookie_samesite in order
    to prevent csrf vulnerabilities. Setting it to Strict for now; if this is to
    strict on testing, then will instead set it to Lax.
3. Need to set cookie_httponly to false, since javascript needs to be able to
    access/modify the cookie to support separate logins into OpenEMR. This is important
    to support in OpenEMR since the application needs to robustly support access of
    separate patients via separate logins by same users. This is done via custom
    restore_session() javascript function; session IDs are effectively saved in the
    top level browser window.
4. Using use_strict_mode to optimize security.
5. Using sid_bits_per_character of 6 to optimize security. This does allow comma to
    be used in the session id, so need to ensure properly escape it when modify it in
    cookie.
6. Using sid_length of 48 to optimize security.
7. Setting gc_maxlifetime to 14400 since defaults for session.gc_maxlifetime is
    often too small.
8. Setting cookie_path to improve security when using different OpenEMR instances
    on same server to prevent session conflicts.
  • Loading branch information
bradymiller committed Jul 1, 2019
1 parent adbc515 commit aeb1b15
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 22 deletions.
3 changes: 2 additions & 1 deletion ccdaservice/ccda_gateway.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* @copyright Copyright (c) 2016-2017 Jerry Padgett <[email protected]>
* @license https://github.com/openemr/openemr/blob/master/LICENSE GNU General Public License 3
*/

//authencate for portal or main- never know where it gets used
session_start();
if (isset($_SESSION['pid']) && isset($_SESSION['patient_portal_onsite_two'])) {
Expand Down Expand Up @@ -82,7 +83,7 @@ function portalccdafetching($pid, $server_url, $parameterArray)
$parameters = http_build_query($parameterArray); // future use
try {
$ch = curl_init();
$url = $server_url . "/interface/modules/zend_modules/public/encounterccdadispatch/index?site=$site_id&me=" . session_id() . "&param=1&view=1&combination=$pid&recipient=patient";
$url = $server_url . "/interface/modules/zend_modules/public/encounterccdadispatch/index?site=$site_id&me=" . urlencode(session_id()) . "&param=1&view=1&combination=$pid&recipient=patient";
curl_setopt($ch, CURLOPT_URL, $url);
curl_setopt($ch, CURLOPT_HEADER, 0); // set true for look see
curl_setopt($ch, CURLOPT_FOLLOWLOCATION, 1);
Expand Down
64 changes: 48 additions & 16 deletions interface/globals.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,24 +89,56 @@
// only if you have some reason to.
$GLOBALS['OE_SITES_BASE'] = "$webserver_root/sites";

// The session name names a cookie stored in the browser.
// Now that restore_session() is implemented in javaScript, session IDs are
// effectively saved in the top level browser window and there is no longer
// any need to change the session name for different OpenEMR instances.
// On 4/8/17, added cookie_path to improve security when using different
// OpenEMR instances on same server to prevent session conflicts; also
// modified interface/login/login.php and library/restoreSession.php to be
// consistent with this.
// Defaults for session.gc_maxlifetime is often too small. You might choose to
// adjust it further.
// OpenEMR session/cookie strategy:
// 1. If a session does not yet exist, then will start the OpenEMR session, which
// will create a cookie with OpenEMR name. If a session already exists, then
// this means portal is being used and will bypass setting of the OpenEMR session/cookie.
// 2. If using php version 7.3.0 or above, then will set the cookie_samesite in order
// to prevent csrf vulnerabilities. Setting it to Strict for now; if this is to
// strict on testing, then will instead set it to Lax.
// 3. Need to set cookie_httponly to false, since javascript needs to be able to
// access/modify the cookie to support separate logins into OpenEMR. This is important
// to support in OpenEMR since the application needs to robustly support access of
// separate patients via separate logins by same users. This is done via custom
// restore_session() javascript function; session IDs are effectively saved in the
// top level browser window.
// 4. Using use_strict_mode to optimize security.
// 5. Using sid_bits_per_character of 6 to optimize security. This does allow comma to
// be used in the session id, so need to ensure properly escape it when modify it in
// cookie.
// 6. Using sid_length of 48 to optimize security.
// 7. Setting gc_maxlifetime to 14400 since defaults for session.gc_maxlifetime is
// often too small.
// 8. Setting cookie_path to improve security when using different OpenEMR instances
// on same server to prevent session conflicts.
if (session_status() === PHP_SESSION_NONE) {
// Only can run these when do not have an active session yet
// (for example, need to skip this in the portal where the session is already active)
ini_set('session.gc_maxlifetime', '14400');
ini_set('session.cookie_path', $web_root ? $web_root : '/');
session_name("OpenEMR");
// Below for main OpenEMR
if (version_compare(phpversion(), '7.3.0', '>=')) {
session_start([
'cookie_samesite' => "Strict",
'name'=> 'OpenEMR',
'use_strict_mode' => true,
'cookie_httponly' => false,
'sid_bits_per_character' => 6,
'sid_length' => 48,
'gc_maxlifetime' => 14400,
'cookie_path' => $web_root ? $web_root : '/'
]);
} else {
session_start([
'name' => 'OpenEMR',
'use_strict_mode' => true,
'cookie_httponly' => false,
'sid_bits_per_character' => 6,
'sid_length' => 48,
'gc_maxlifetime' => 14400,
'cookie_path' => $web_root ? $web_root : '/'
]);
}
} else {
// Below for OpenEMR patient portal
session_start();
}
session_start();

// Set the site ID if required. This must be done before any database
// access is attempted.
Expand Down
2 changes: 1 addition & 1 deletion interface/login/login.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ function imsubmitted() {
// This forces the server to create a new session ID.
var olddate = new Date();
olddate.setFullYear(olddate.getFullYear() - 1);
document.cookie = '<?php echo session_name() . '=' . session_id() ?>; path=<?php echo($web_root ? $web_root : '/');?>; expires=' + olddate.toGMTString();
document.cookie = '<?php echo urlencode(session_name()) . '=' . urlencode(session_id()) ?>; path=<?php echo($web_root ? $web_root : '/');?>; expires=' + olddate.toGMTString();
<?php } ?>
return false; //Currently the submit action is handled by the encrypt_form().
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@
'layout' => 'site/layout',
),
'session' => array(
'remember_me_seconds' => 2419200,
'use_cookies' => true,
'cookie_httponly' => true,

),
'moduleconfig' => array(

Expand Down
2 changes: 1 addition & 1 deletion library/restoreSession.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function restoreSession() {
<?php if ($GLOBALS['restore_sessions'] == 2) { ?>
alert('Changing session ID from\n"' + c[1] + '" to\n"' + oemr_session_id + '"');
<?php } ?>
document.cookie = oemr_session_name + '=' + oemr_session_id + '; path=<?php echo($web_root ? $web_root : '/');?>';
document.cookie = encodeURIComponent(oemr_session_name) + '=' + encodeURIComponent(oemr_session_id) + '; path=<?php echo($web_root ? $web_root : '/');?>';
}
}
<?php } ?>
Expand Down

0 comments on commit aeb1b15

Please sign in to comment.