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

- Thread: https://externals.io/message/73209
- Original archive: https://news-web.php.net/php.internals/73209
- Started by Andrey Andreev on 2014-03-17T10:23:58+00:00
- Messages: 19
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #73209 — Andrey Andreev — 2014-03-17T10:23:58+00:00

URL: https://externals.io/message/73209#73209

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.

---

## #73212 — Yasuo Ohgaki — 2014-03-17T10:51:07+00:00

URL: https://externals.io/message/73209#73212 · In reply to: #73209

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

---

## #73214 — Yasuo Ohgaki — 2014-03-17T11:01:28+00:00

URL: https://externals.io/message/73209#73214 · In reply to: #73212

> 
>
> > […]
>
> 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

---

## #73215 — Andrey Andreev — 2014-03-17T11:10:05+00:00

URL: https://externals.io/message/73209#73215 · In reply to: #73212

> Hi Andrey,
>
> 
> > […]
>
>
> 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.

---

## #73218 — Leigh — 2014-03-17T13:22:42+00:00

URL: https://externals.io/message/73209#73218 · In reply to: #73209

> 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.

---

## #73219 — Andrey Andreev — 2014-03-17T13:57:17+00:00

URL: https://externals.io/message/73209#73219 · In reply to: #73218

HI,


>
> 
> > […]
>
>
> 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.

---

## #73220 — Chris Wright — 2014-03-17T14:11:45+00:00

URL: https://externals.io/message/73209#73220 · In reply to: #73219

> HI,
>
> 
> > […]
>
> 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.

---

## #73221 — Andrey Andreev — 2014-03-17T15:07:49+00:00

URL: https://externals.io/message/73209#73221 · In reply to: #73220

> 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.

---

## #73232 — Yasuo Ohgaki — 2014-03-17T21:09:10+00:00

URL: https://externals.io/message/73209#73232 · In reply to: #73215

Hi Andrey,



> 
> > […]
> wrote:
> > […]
>
> 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
> > […]
> prefixing
> > […]
>
> 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.


>
> > […]
> does
> > […]
> `session_create_id()`
> > […]
>
> 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)


>
> > […]
>
> 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

---

## #73234 — Pierre Joye — 2014-03-17T21:15:01+00:00

URL: https://externals.io/message/73209#73234 · In reply to: #73232

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

---

## #73235 — Yasuo Ohgaki — 2014-03-17T21:16:29+00:00

URL: https://externals.io/message/73209#73235 · In reply to: #73218

Hi Leligh,



> 
>
> > […]
>
> 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

---

## #73237 — Yasuo Ohgaki — 2014-03-17T21:24:12+00:00

URL: https://externals.io/message/73209#73237 · In reply to: #73234

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

---

## #73238 — Pierre Joye — 2014-03-17T21:26:34+00:00

URL: https://externals.io/message/73209#73238 · In reply to: #73237

> 
> > […]
>
>
> Sounds good.
> I'm not on IRC, which one should I use?
>
> > […]
>
>
> Sure.
>
> > […]
>
>
> 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

---

## #73240 — Yasuo Ohgaki — 2014-03-17T21:30:42+00:00

URL: https://externals.io/message/73209#73240 · In reply to: #73238

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

---

## #73246 — Andrey Andreev — 2014-03-17T21:56:13+00:00

URL: https://externals.io/message/73209#73246 · In reply to: #73232

Hi,


> > […]
>
>
> 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.

---

## #73247 — Andrey Andreev — 2014-03-17T21:59:51+00:00

URL: https://externals.io/message/73209#73247 · In reply to: #73234

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.

---

## #73248 — Yasuo Ohgaki — 2014-03-17T22:07:08+00:00

URL: https://externals.io/message/73209#73248 · In reply to: #73247

Hi Andrey,



> On Mon, Mar 17, 2014 at 11:15 PM, Pierre Joye <pierre.php@gmail.com>
> wrote:
> > […]
> wrote:
> > […]
> management.
> > […]
>
> 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

---

## #73249 — Andrey Andreev — 2014-03-17T22:20:48+00:00

URL: https://externals.io/message/73209#73249 · In reply to: #73248

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.

---

## #73250 — Yasuo Ohgaki — 2014-03-17T22:40:00+00:00

URL: https://externals.io/message/73209#73250 · In reply to: #73249

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