Hi all,
This RFC changes default session settings and introduces a new
setting that disables possible timing attack against session ID. All
of them help to improve general session ID security except
hash_bits_per_character change.
NIST discourages use of SHA-1 years ago. It proposes to use
SHA-256 as the default hash function for session. To reduce
size of session ID string, hash_bits_per_character=6 is proposed.
https://wiki.php.net/rfc/secure-session-options-by-default
Thank you for voting!
P.S. Although, the change is trivial, if anyone would like to see
patch for this, I'll prepare one ASAP.
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi there,
I remember that the use_strict_mode=on added in PHP 5.5 was broken and
would prevent the application from setting a session ID by itself. This is
actually fairly common, about 800k Drupal 7 websites do that.
Unless this is fixed, this setting just cannot be enabled by default.
Damien Tournoud
Hi all,
This RFC changes default session settings and introduces a new
setting that disables possible timing attack against session ID. All
of them help to improve general session ID security except
hash_bits_per_character change.NIST discourages use of SHA-1 years ago. It proposes to use
SHA-256 as the default hash function for session. To reduce
size of session ID string, hash_bits_per_character=6 is proposed.https://wiki.php.net/rfc/secure-session-options-by-default
Thank you for voting!
P.S. Although, the change is trivial, if anyone would like to see
patch for this, I'll prepare one ASAP.--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Damien,
I remember that the
use_strict_mode=onadded in PHP 5.5 was broken and
would prevent the application from setting a session ID by itself. This is
actually fairly common, about 800k Drupal 7 websites do that.Unless this is fixed, this setting just cannot be enabled by default.
It was simple mistake during merge. It is fixed next release already. Do
not worry about it.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Damien,
I remember that the
use_strict_mode=onadded in PHP 5.5 was broken and
would prevent the application from setting a session ID by itself. This
is
actually fairly common, about 800k Drupal 7 websites do that.Unless this is fixed, this setting just cannot be enabled by default.
It was simple mistake during merge. It is fixed next release already. Do
not worry about it.
It seemed to be part of the patch of the respective rfc:
https://gist.github.com/yohgaki/1379668#file-php-trunk-strict-session-patch-L602
How was this a merge issue?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
--
Tjerk
Hi Tjerk,
On Tue, Feb 18, 2014 at 7:56 PM, Tjerk Meesters tjerk.meesters@gmail.comwrote:
It seemed to be part of the patch of the respective rfc:
https://gist.github.com/yohgaki/1379668#file-php-trunk-strict-session-patch-L602
How was this a merge issue?
Could you point out what's the exact issue here?
It's gist and it's not the patch merged.
tests/session_id_basic.phpt
var_dump(session_id());
var_dump(session_id("test"));
var_dump(session_id());
var_dump(session_id("1234567890"));
var_dump(session_id());
// Turn off strice mode, since it does not allow uninitialized session ID
ini_set('session.use_strict_mode',false);
var_dump(session_start());
var_dump(session_id());
var_dump(session_destroy());
var_dump(session_id());
echo "Done";
ob_end_flush();
?>
--EXPECTF--
*** Testing session_id() : basic functionality ***
string(0) ""
string(0) ""
string(4) "test"
string(4) "test"
string(10) "1234567890"
bool(true)
string(10) "1234567890"
bool(true)
string(0) ""
Done
It works like this. Is there issue with this behavior?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Tjerk,
On Tue, Feb 18, 2014 at 7:56 PM, Tjerk Meesters tjerk.meesters@gmail.comwrote:
It seemed to be part of the patch of the respective rfc:
https://gist.github.com/yohgaki/1379668#file-php-trunk-strict-session-patch-L602
How was this a merge issue?
Could you point out what's the exact issue here?
It's gist and it's not the patch merged.
In that case, the RFC should be updated with the corresponding commit logs
(part of the process) when it was merged with the project; this makes it
easier to discuss historical commits based on it.
tests/session_id_basic.phpt
var_dump(session_id());
var_dump(session_id("test"));
var_dump(session_id());
var_dump(session_id("1234567890"));
var_dump(session_id());
// Turn off strice mode, since it does not allow uninitialized session ID
ini_set('session.use_strict_mode',false);
var_dump(session_start());
var_dump(session_id());
var_dump(session_destroy());
var_dump(session_id());echo "Done";
ob_end_flush();
?>
--EXPECTF--
*** Testingsession_id(): basic functionality ***
string(0) ""
string(0) ""
string(4) "test"
string(4) "test"
string(10) "1234567890"
bool(true)
string(10) "1234567890"
bool(true)
string(0) ""
DoneIt works like this. Is there issue with this behavior?
That looks okay, thanks for getting back to me :)
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
--
Tjerk
Hi Tjerk,
On Tue, Feb 18, 2014 at 8:58 PM, Tjerk Meesters tjerk.meesters@gmail.comwrote:
In that case, the RFC should be updated with the corresponding commit logs
(part of the process) when it was merged with the project; this makes it
easier to discuss historical commits based on it.
Thank you for point it out.
I'll update the RFC. If document is not updated, I'll update it, too.
Regarding session_id() behavior, it could be made to accept user specified
session ID independent of "use_strict_mode". To accept user specified
session ID when "use_strict_mode=on", there should be an API that generates
secure session ID. Otherwise, user may create insecure ID and set it.
Any call for RFC, anyone? I'm willing to propose this.
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Yasuo,
This was previously discussed but I have to agree with Andrey (and maybe
even go beyond what he said) - the hash_bits change doesn't belong in this
RFC. First, it has no security implications and it's not entirely clear
from the RFC. Second, I don't feel that the implications of that change are
clear, beyond some mention that "this could not be an issue for almost all
apps", which personally I don't think is accurate - but either way, we need
some better analysis here.
From my point of view I don't think a few extra characters per session going
over the wire are worth the potential obscure BC break changing the default
here will cause with certain session backends and/or apps, and we shouldn't
include this change in this RFC. If we want to compact the session id's,
proposing a change for this default can be done in a separate RFC that
discusses the pros and cons of doing it, independent of security.
Another thing that I think this RFC is missing is some clearer explanation
on what kind of apps don't work with the proposed changes (most notably
use_strict_mode=on and cookie_httponly). Even though my gut is that these
two proposed changes are good - the RFC should include explanations of the
code patterns and/or types of apps and/or modules that will be affected by
this; "Most apps should not be affected" isn't enough in an RFC IMHO.
Last - the voting period should be at least a week, right now it's 5 (or
maybe 6, depending on your POV) days.
Thanks!
Zeev
-----Original Message-----
From: yohgaki@gmail.com [mailto:yohgaki@gmail.com] On Behalf Of Yasuo
Ohgaki
Sent: Monday, February 17, 2014 6:27 AM
To: internals@lists.php.net
Subject: [PHP-DEV] [VOTE] Secure Session Module Options/Internal by
DefaultHi all,
This RFC changes default session settings and introduces a new setting
that
disables possible timing attack against session ID. All of them help to
improve
general session ID security except hash_bits_per_character change.NIST discourages use of SHA-1 years ago. It proposes to use
SHA-256 as the default hash function for session. To reduce size of
session ID
string, hash_bits_per_character=6 is proposed.https://wiki.php.net/rfc/secure-session-options-by-default
Thank you for voting!
P.S. Although, the change is trivial, if anyone would like to see patch
for this,
I'll prepare one ASAP.--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Zeev,
This was previously discussed but I have to agree with Andrey (and maybe
even go beyond what he said) - the hash_bits change doesn't belong in this
RFC. First, it has no security implications and it's not entirely clear
from the RFC. Second, I don't feel that the implications of that change
are
clear, beyond some mention that "this could not be an issue for almost all
apps", which personally I don't think is accurate - but either way, we need
some better analysis here.From my point of view I don't think a few extra characters per session
going
over the wire are worth the potential obscure BC break changing the default
here will cause with certain session backends and/or apps, and we shouldn't
include this change in this RFC. If we want to compact the session id's,
proposing a change for this default can be done in a separate RFC that
discusses the pros and cons of doing it, independent of security.
I agree. This would be removed. This means vote is closed and reopened
again.
I'm not sure why compiled and php.ini-* default differs now, but it's
irrelevant issue here anyway.
Another thing that I think this RFC is missing is some clearer explanation
on what kind of apps don't work with the proposed changes (most notably
use_strict_mode=on and cookie_httponly). Even though my gut is that these
two proposed changes are good - the RFC should include explanations of the
code patterns and/or types of apps and/or modules that will be affected by
this; "Most apps should not be affected" isn't enough in an RFC IMHO.
I'll add explanations and possible side effects. I think of few.
Last - the voting period should be at least a week, right now it's 5 (or
maybe 6, depending on your POV) days.
This is mistake. My apologies.
Thank you for your comment.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net