Session: deprecating create_sid() method and add createSid()?

11 years ago by Andrey Andreev — view source — reply

unread

Hi Yasuo,

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

Cheers,
Andrey.

11 years ago by Yasuo Ohgaki — view source — reply

unread

Hi Andrey,

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

This change is not mine. It was added about 10 years ago, IIRC.

Having createSid() could be useful. For example, user may have certain
prefix for session
IDs. User ID prefix is especially useful to know how many active sessions
are there.
(Note: I advise to use MD5('user_id'.'random_secret') for user ID prefixing
if user_id
shouldn't be exposed.)

I've added session_create_id(). Therefore, if user start using it, it does
not hart much.
If users do not need modified session ID, they may call session_create_id()
simply.

The reason why this is added is Stefan Esser's strict session patch had
this, I guess.
However, his patch was not fully merged and session remained weak until
'use_strice_mode'
patch.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

11 years ago by Yasuo Ohgaki — view source — reply

unread

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

This change is not mine. It was added about 10 years ago, IIRC.

More details.
At that time, there wasn't object interface and object interface was added
few years later. Since there is procedural API for creating ID, create_sid()
method is added.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

11 years ago by Andrey Andreev — view source — reply

unread

Hi Andrey,

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

This change is not mine. It was added about 10 years ago, IIRC.

And for 10 years is has not been documented? Really, are you sure that
it is exposed to userland at all?

Having createSid() could be useful. For example, user may have certain
prefix for session
IDs. User ID prefix is especially useful to know how many active sessions
are there.
(Note: I advise to use MD5('user_id'.'random_secret') for user ID prefixing
if user_id
shouldn't be exposed.)

Surely that prefix can just be prepended to the $session_id that gets
passed to read(), write(), destroy() methods. This argument was given
to me when I proposed a session.match_ip feature. Why not add this as
an option to session_start()? Example:

session_start(['id_prefix' => 'foo_']);

(could be an ini setting, for optimization purposes)

I've added session_create_id(). Therefore, if user start using it, it does
not hart much.
If users do not need modified session ID, they may call session_create_id()
simply.

Again with the "I've added this", that I opposed previously for
session_serializer_name(), session_gc() ... What's session_create_id()
useful for? How would you utilize it?

The reason why this is added is Stefan Esser's strict session patch had
this, I guess.
However, his patch was not fully merged and session remained weak until
'use_strice_mode'
patch.

Now I'm confused ... is it added to the core, or just supported as a
part of the suhosin extension and/or hardening patch?

Cheers,
Andrey.

11 years ago by Yasuo Ohgaki — view source — reply

unread

Hi Andrey,

Hi Andrey,

On Mon, Mar 17, 2014 at 7:23 PM, Andrey Andreev narf@devilix.net
wrote:

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

This change is not mine. It was added about 10 years ago, IIRC.

And for 10 years is has not been documented? Really, are you sure that
it is exposed to userland at all?

It may be, but chances are rare. I suppose.
Nobody is using it actually. This would be the reason why it is not
documented
for a long time.

Having createSid() could be useful. For example, user may have certain

prefix for session
IDs. User ID prefix is especially useful to know how many active sessions
are there.
(Note: I advise to use MD5('user_id'.'random_secret') for user ID
prefixing
if user_id
shouldn't be exposed.)

Surely that prefix can just be prepended to the $session_id that gets
passed to read(), write(), destroy() methods. This argument was given
to me when I proposed a session.match_ip feature. Why not add this as
an option to session_start()? Example:

session_start(['id_prefix' => 'foo_']);

(could be an ini setting, for optimization purposes)

It could be easier with save handler. Users may register handler like

function session_create_sid( args here ) {
if (!empty($_SESSION['user_id'])) {
return create_session_id(md5($_SESSION['user_id']));
} else {
return create_session_id();
}
}

then they may call

session_regenerate_id()

to get prefixed session ID.

I've added session_create_id(). Therefore, if user start using it, it
does
not hart much.
If users do not need modified session ID, they may call
session_create_id()
simply.

Again with the "I've added this", that I opposed previously for
session_serializer_name(), session_gc() ... What's session_create_id()
useful for? How would you utilize it?

You are asking to write RFC for all additions for new release.

I think session_gc() is must have API. IMHO.
Periodic GC is better than probability based GC even with expire timestamp
(I mean
time stamp for session manager, not save handlers)

The reason why this is added is Stefan Esser's strict session patch had
this, I guess.
However, his patch was not fully merged and session remained weak until
'use_strice_mode'
patch.

Now I'm confused ... is it added to the core, or just supported as a
part of the suhosin extension and/or hardening patch?

Stefan proposed strict session patch and it wasn't merged fully. That's all.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

11 years ago by Pierre Joye — view source — reply

unread

hi,

For one, I appreciate the effort that both of you put on the session management.

It seems that you are somehow alone to discuss this issue and slightly
in circle right now.

I would suggest two steps:

sit down together for a chat and get your stuff together. It will by
far more efficient than mails
write one or more RFCs to fix what should be fixed, how and why (see
next point :)
provide more info about the actual critical security impact that
could be fixed by the changes
as of now, I failed to see any CVE related to what you are referring to

Cheers,

Pierre

@pierrejoye | http://www.libgd.org

11 years ago by Yasuo Ohgaki — view source — reply

unread

Hi Pierre,

For one, I appreciate the effort that both of you put on the session
management.

It seems that you are somehow alone to discuss this issue and slightly
in circle right now.

I would suggest two steps:

sit down together for a chat and get your stuff together. It will by
far more efficient than mails

Sounds good.
I'm not on IRC, which one should I use?

write one or more RFCs to fix what should be fixed, how and why (see
next point :)

Sure.

provide more info about the actual critical security impact that
could be fixed by the changes
as of now, I failed to see any CVE related to what you are referring to

There wouldn't be CVE as it may be implemented by user land.
I may try to ask MITRE to give me a CVE, though.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

11 years ago by Pierre Joye — view source — reply

unread

For one, I appreciate the effort that both of you put on the session
management.

It seems that you are somehow alone to discuss this issue and slightly
in circle right now.

I would suggest two steps:

sit down together for a chat and get your stuff together. It will by
far more efficient than mails

Sounds good.
I'm not on IRC, which one should I use?

write one or more RFCs to fix what should be fixed, how and why (see
next point :)

Sure.

provide more info about the actual critical security impact that
could be fixed by the changes
as of now, I failed to see any CVE related to what you are referring to

There wouldn't be CVE as it may be implemented by user land.
I may try to ask MITRE to give me a CVE, though.

I am more asking for actual exploits/flaws either in user land apps
relying on existing behaviors or in php itself. Asking a CVE for this
discussion does not sound like a good thing at this stage.

Cheers,

Pierre

@pierrejoye | http://www.libgd.org

11 years ago by Yasuo Ohgaki — view source — reply

unread

Hi Pierre,

I am more asking for actual exploits/flaws either in user land apps
relying on existing behaviors or in php itself. Asking a CVE for this
discussion does not sound like a good thing at this stage.

I agree. I will not bother with CVE.

Regards.

--
Yasuo Ohgaki
yohgaki@ohgaki.net

11 years ago by Andrey Andreev — view source — reply

unread

Hi,

hi,

For one, I appreciate the effort that both of you put on the session management.

It seems that you are somehow alone to discuss this issue and slightly
in circle right now.

I would suggest two steps:

sit down together for a chat and get your stuff together. It will by
far more efficient than mails

write one or more RFCs to fix what should be fixed, how and why (see
next point :)

provide more info about the actual critical security impact that
could be fixed by the changes
as of now, I failed to see any CVE related to what you are referring to

We'll surely do that.
In fact, I was just about to write Yasuo a private mail about some
security issues, because I didn't find an option to report a bug and
make it hidden. Is there such an option, or does the CVE assignment
process allow that? (I'm not familiar with it)

Cheers,
Andrey.

11 years ago by Yasuo Ohgaki — view source — reply

unread

Hi Andrey,

On Mon, Mar 17, 2014 at 11:15 PM, Pierre Joye pierre.php@gmail.com
wrote:

hi,

On Mon, Mar 17, 2014 at 10:09 PM, Yasuo Ohgaki yohgaki@ohgaki.net
wrote:

For one, I appreciate the effort that both of you put on the session
management.

It seems that you are somehow alone to discuss this issue and slightly
in circle right now.

I would suggest two steps:

sit down together for a chat and get your stuff together. It will by
far more efficient than mails

write one or more RFCs to fix what should be fixed, how and why (see
next point :)

provide more info about the actual critical security impact that
could be fixed by the changes
as of now, I failed to see any CVE related to what you are referring to

We'll surely do that.
In fact, I was just about to write Yasuo a private mail about some
security issues, because I didn't find an option to report a bug and
make it hidden. Is there such an option, or does the CVE assignment
process allow that? (I'm not familiar with it)

Getting CVE is easy. One just have to describe what the vulnerability is and
send request mail to MITRE. If personnel in MITRE agrees it as new
vulnerability,
then they give us new CVE, if not, they give us existing CVE.

I don't think this (session_regenerate_id() issue) is PHP's CVE issue as it
may
be avoided by user land like timing attack issue.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

11 years ago by Andrey Andreev — view source — reply

unread

Hi,

Getting CVE is easy. One just have to describe what the vulnerability is and
send request mail to MITRE. If personnel in MITRE agrees it as new
vulnerability,
then they give us new CVE, if not, they give us existing CVE.

I don't think this (session_regenerate_id() issue) is PHP's CVE issue as it
may
be avoided by user land like timing attack issue.

No, I'm not talking about session_regenerate_id() ... sorry that I
mentioned it in this thread. I'd rather not share that publicly until
it's resolved, and hence why my question was - can CVEs be hidden
until that happens?

Cheers,
Andrey.

11 years ago by Yasuo Ohgaki — view source — reply

unread

Hi Andrey,

No, I'm not talking about session_regenerate_id() ... sorry that I
mentioned it in this thread. I'd rather not share that publicly until
it's resolved, and hence why my question was - can CVEs be hidden
until that happens?

Details of vulnerability is hidden until reporter or vendor tells MITRE the
vulnerability is fixed.
(or they find out it was fixed. MITRE isn't the only organization that is
providing CVE)

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

11 years ago by Andrey Andreev — view source — reply

unread

Hi,

I've added session_create_id(). Therefore, if user start using it, it
does
not hart much.
If users do not need modified session ID, they may call
session_create_id()
simply.

Again with the "I've added this", that I opposed previously for
session_serializer_name(), session_gc() ... What's session_create_id()
useful for? How would you utilize it?

You are asking to write RFC for all additions for new release.

I think session_gc() is must have API. IMHO.
Periodic GC is better than probability based GC even with expire timestamp
(I mean
time stamp for session manager, not save handlers)

I'm not asking for RFC about everything new. But at least to my
knowledge, this wasn't even discussed - somebody might've suggested an
improvement to your idea. You just decide and add something without
others evaluating it, while at the same time, everybody else's feature
requests are put under cross-fire - this is unfair.

Anyway, you can disregard that, as I previously said - Leigh answered
all of my questions, including those about session_create_id() (might
be better to call it session_generate_id() though).

Cheers,
Andrey.

11 years ago by Leigh — view source — reply

unread

Hi Yasuo,

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

It was added in 5.5.1 - The PR was made 2 years ago by myself:
https://github.com/php/php-src/pull/109

It's not documented because I'm lazy, (it's one of those TODO items that
has become permanent), but I personally use it. The underlying method on
the session interface has been there for a long time (10 years might even
be accurate), but for some unknown reason, it was not exposed to userland
when all of the other methods were.

It is necessary for custom session handler classes because without it calls
to session_regenerate_id() do not inform the session handler about the
change of session id. The presence of this method allows you to capture
session id regeneration, and update files, databases, etc. as necessary,
and also allows you to fully customise the actual session id token that is
used. Custom prefixes, body content, encoding charset, etc.

Cheers,

Leigh.

11 years ago by Andrey Andreev — view source — reply

unread

HI,

Hi Yasuo,

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

It was added in 5.5.1 - The PR was made 2 years ago by myself:
https://github.com/php/php-src/pull/109

It's not documented because I'm lazy, (it's one of those TODO items that has
become permanent), but I personally use it. The underlying method on the
session interface has been there for a long time (10 years might even be
accurate), but for some unknown reason, it was not exposed to userland when
all of the other methods were.

It is necessary for custom session handler classes because without it calls
to session_regenerate_id() do not inform the session handler about the
change of session id. The presence of this method allows you to capture
session id regeneration, and update files, databases, etc. as necessary, and
also allows you to fully customise the actual session id token that is used.
Custom prefixes, body content, encoding charset, etc.

I see ... that (and the PR discussion) answers all of my questions,
thanks. Btw, I could take the docs out of your TODO list, as long as I
can figure out of all repos, which one do I need to fork so I can push
documentation additions ... the web interface sucks.

With that said, +1 for changing it to SessionHandler::createSid().

Cheers,
Andrey.

11 years ago by Chris Wright — view source — reply

unread

HI,

Hi Yasuo,

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

It was added in 5.5.1 - The PR was made 2 years ago by myself:
https://github.com/php/php-src/pull/109

It's not documented because I'm lazy, (it's one of those TODO items that has
become permanent), but I personally use it. The underlying method on the
session interface has been there for a long time (10 years might even be
accurate), but for some unknown reason, it was not exposed to userland when
all of the other methods were.

It is necessary for custom session handler classes because without it calls
to session_regenerate_id() do not inform the session handler about the
change of session id. The presence of this method allows you to capture
session id regeneration, and update files, databases, etc. as necessary, and
also allows you to fully customise the actual session id token that is used.
Custom prefixes, body content, encoding charset, etc.

I see ... that (and the PR discussion) answers all of my questions,
thanks. Btw, I could take the docs out of your TODO list, as long as I
can figure out of all repos, which one do I need to fork so I can push
documentation additions ... the web interface sucks.

Docs are still in SVN, by far the easiest way to do it is via the OE
(http://edit.php.net/).

If you want to check the English docs out via SVN, use
https://svn.php.net/repository/phpdoc/modules/doc-en

With that said, +1 for changing it to SessionHandler::createSid().

Cheers,
Andrey.

11 years ago by Andrey Andreev — view source — reply

unread

Docs are still in SVN, by far the easiest way to do it is via the OE
(http://edit.php.net/).

If you want to check the English docs out via SVN, use
https://svn.php.net/repository/phpdoc/modules/doc-en

Great, svn kind of sucks, but I still like it better than the online
editor. I'll look into it, thanks.

Cheers,
Andrey.

11 years ago by Yasuo Ohgaki — view source — reply

unread

Hi Leligh,

Hi Yasuo,

There's no SessionHandler::create_sid() or
SessionHandlerInterface::create_sid() documented (see your own link to
the docs) ... since when is this available?

I'm not sure if exposing it is a good idea anyway, why is it necessary?

It was added in 5.5.1 - The PR was made 2 years ago by myself:
https://github.com/php/php-src/pull/109

It's not documented because I'm lazy, (it's one of those TODO items that
has become permanent), but I personally use it. The underlying method on
the session interface has been there for a long time (10 years might even
be accurate), but for some unknown reason, it was not exposed to userland
when all of the other methods were.

It is necessary for custom session handler classes because without it calls
to session_regenerate_id() do not inform the session handler about the
change of session id. The presence of this method allows you to capture
session id regeneration, and update files, databases, etc. as necessary,
and also allows you to fully customise the actual session id token that is
used. Custom prefixes, body content, encoding charset, etc.

Thank you for clarification. I didn't realize when it was implemented.
Documentation at that time would be nice w/o session_create_id().
If it is added 5.5.1, it would be safer to rename.

I think procedural session_create_sid() interface was added long time ago,
but
it's not important. As it is deprecated by documentation and I wouldn't
change
the name.

User defined save handler documentation should be improved. I think
many users are returning invalid value for read() especially.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net