Skip to content

*[Sessions]: Now session is accessible if process is ran by root. #1360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 6, 2017

Conversation

EvgeniySpinov
Copy link
Contributor

Fixing bug: https://bugs.php.net/bug.php?id=69582

Use case: session is initiated by Apache with non-root permissions. Later on, session could be accessed by backend process, which runs under root, but needs to communicate with same session data as Apache process.

Didn't work before this commit: session file is owned by non-root, process owner != file owner, so this triggered file handle closure.

@laruence
Copy link
Member

please leave this one, and close others. we can merge it downwards if it is necessary.

@laruence laruence added the Bug label Jun 24, 2015
@EvgeniySpinov
Copy link
Contributor Author

Done. Weird, that CI build has failed for this Pull Request, while successfully passed for one of the closed ones.

accessed by backend with root permissions to execute some system tasks.

*/
if (fstat(data->fd, &sbuf) || (sbuf.st_uid != 0 && sbuf.st_uid != getuid() && sbuf.st_uid != geteuid() && getuid() != 0)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use portable zend_fstat in master.

Thanks.

@EvgeniySpinov
Copy link
Contributor Author

Done that.

php-pulls pushed a commit that referenced this pull request Jan 6, 2017
* pull-request/1360:
  Fixed bug #69582 session not readable by root in CLI
  news entry for PR #1360
@php-pulls php-pulls merged commit 650e073 into php:master Jan 6, 2017
@krakjoe
Copy link
Member

krakjoe commented Jan 6, 2017

Thanks :)

@EvgeniySpinov
Copy link
Contributor Author

@krakjoe Thanks for merging. Is there are any plans for merging it downstream, like at least 5.6?

@krakjoe
Copy link
Member

krakjoe commented Jan 6, 2017

It is present in all branches in supported release cycle. 5.6 is in security fix only cycle and so should not be having fixes like this merged, sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants