View previous topic :: View next topic |
Author |
Message |
Aristophan Smarty Regular
Joined: 10 Jan 2011 Posts: 96
|
Posted: Mon Oct 10, 2011 11:45 am Post subject: Smarty_Security roadmap |
|
|
Hi
The smarty_security.php says:
Code: | /*
* FIXME: Smarty_Security API
* - getter and setter instead of public properties would allow cultivating an internal cache properly
* - current implementation of isTrustedResourceDir() assumes that Smarty::$template_dir and Smarty::$config_dir are immutable
* the cache is killed every time either of the variables change. That means that two distinct Smarty objects with differing
* $template_dir or $config_dir should NOT share the same Smarty_Security instance,
* as this would lead to (severe) performance penalty! how should this be handled?
*/ |
Is there a roadmap when this will be done or started? (If this is to much internal, please feel free to erase this thread) |
|
Back to top |
|
rodneyrehm Administrator
Joined: 30 Mar 2007 Posts: 674 Location: Germany, border to Switzerland
|
Posted: Mon Oct 10, 2011 11:54 am Post subject: |
|
|
API changes should not be done in bug-releases. That leaves the next minor release (3.2) as a possible target.
We haven't discussed this topic internally yet. You found a note I made during my last code review… If you have ideas / opinions, please share.
How is this important to you? _________________ Twitter |
|
Back to top |
|
Aristophan Smarty Regular
Joined: 10 Jan 2011 Posts: 96
|
Posted: Mon Oct 10, 2011 1:21 pm Post subject: |
|
|
Thank you, globe.
I am sorry, I dont really know yet how this could get important for me... I just found that note digging through security to understand its main diffs to Smarty2.
API changes, like using new getters and setters, normally mean changing the calling code again, until it stabilizes, thats why I asked.
About the performing Smarty_Security instance issue ... I have no clue on how or why $template_dir and $config_dir could change dynamically... |
|
Back to top |
|
rodneyrehm Administrator
Joined: 30 Mar 2007 Posts: 674 Location: Germany, border to Switzerland
|
Posted: Mon Oct 10, 2011 1:43 pm Post subject: |
|
|
They wouldn't change automagically! The described problem arises with the following setup
Code: | $smarty = new Smarty();
$smarty->setTemplateDir("/foo");
$smarty->enableSecuirty();
$smarty2 = new Smarty(); // or clone $smarty;
// note: $smarty2->security_policy === $smarty->security_policy
$smarty2->setTemplateDir("/foo/bar");
// Smarty_Security builds cache based on /foo of $smarty
$smarty->fetch("test.tpl");
// Smarty_Security builds cache based on /foo/bar of $smarty2
$smarty2->fetch("test.tpl");
// Smarty_Security builds cache based on /foo of $smarty
// this call would ideally use the cache built in the first call instead of expensively rebuilding it.
$smarty->fetch("test.tpl"); |
If you're using a single instance (or multiple instances with the same $template_dir and $config_dir) there's nothing to worry about. Otherwise it would currently make sense to set things up like
Code: | $smarty = new Smarty();
$smarty->setTemplateDir("/foo");
$smarty->enableSecurity();
$smarty2 = new Smarty(); // or clone $smarty;
$smarty2->enableSecurity();
// now $smarty2->security_policy !== $smarty->security_policy
$smarty2->setTemplateDir("/foo/bar"); |
that's pretty much it. _________________ Twitter |
|
Back to top |
|
Aristophan Smarty Regular
Joined: 10 Jan 2011 Posts: 96
|
Posted: Mon Oct 10, 2011 2:23 pm Post subject: |
|
|
I see... sounds reasonable!
Please (just for the record...): why should I want this, if I can have it in a single instance like
Code: | $smarty->setTemplateDir("/foo");
$smarty->addTemplateDir(array("/foo/bar", "/foo/foo"));
$smarty->setConfigDir("/foo/bar");
$smarty->enableSecurity(); | ??
And ... is there already a clone objects property? |
|
Back to top |
|
rodneyrehm Administrator
Joined: 30 Mar 2007 Posts: 674 Location: Germany, border to Switzerland
|
Posted: Mon Oct 10, 2011 2:29 pm Post subject: |
|
|
As I said, If you're using one Smarty instance (and clones thereof, that share the same $template_dir and $config_dir) you're totally safe.
The problem only arises when multiple Smarty instances with varying $template_dir and $config_dir _share_ the same Smarty_Security instance.
For most people this won't be an issue, because they're not using security anyways. For the people using security, this only applies for the default security implementation, with the above mentioned circumstances in effect. So the number of people experiencing this performance killer is probably not that high. (Which is why we haven't thoroughly discussed the matter, yet.) _________________ Twitter |
|
Back to top |
|
Aristophan Smarty Regular
Joined: 10 Jan 2011 Posts: 96
|
Posted: Tue Oct 11, 2011 11:19 am Post subject: |
|
|
May I just add another question here...
Could we additionally have some sort of testInstall() output regarding security settings (secure_dir, trusted_dir etc) ?
Thank you |
|
Back to top |
|
rodneyrehm Administrator
Joined: 30 Mar 2007 Posts: 674 Location: Germany, border to Switzerland
|
Posted: Tue Oct 11, 2011 11:23 am Post subject: |
|
|
What exactly do you expect to see here? _________________ Twitter |
|
Back to top |
|
Aristophan Smarty Regular
Joined: 10 Jan 2011 Posts: 96
|
Posted: Tue Oct 11, 2011 11:39 am Post subject: |
|
|
Well, at least all set paths like secure_dir, trusted_dir and other path/security related config links.
I think this would help investigating, if the smarty configs are set as originally requested.
Edit
Yet another short test about living in a secured enviroment... with an OK, would be nice to have too. |
|
Back to top |
|
Aristophan Smarty Regular
Joined: 10 Jan 2011 Posts: 96
|
Posted: Sat Oct 15, 2011 6:55 pm Post subject: |
|
|
As waiting for some enlightment and puzzling with the security class and testInstall() method in 3.1.3, I discovered set cache dir (tested with testInstall) is not working as expected.
Code: | $this->setCacheDir('/var/www/tests/cache'); |
is not set and empty, while
Code: | $this->setCacheDir('cache/'); |
works without problems.
This is a different behaviour compared to compile dir.
May I kindly just ask again to have some output in testInstall() regarding enabled security settings.
I wish I had some user friendly checks, if everything set with Smarty_Security, by default or modified by the user, is deeply secured. An output seeing all defined paths, allowed or denied functions, modifiers, etc. in testInstall would be great to have too.
Thank you! |
|
Back to top |
|
rodneyrehm Administrator
Joined: 30 Mar 2007 Posts: 674 Location: Germany, border to Switzerland
|
Posted: Sun Oct 16, 2011 9:57 am Post subject: |
|
|
The checks for compile_dir and cache_dir are now fixed in SVN trunk.
As for displaying Smarty_Security and stuff in testInstall(), I figure we should introduce a new method.
Use testInstall() to check if the configured directories exist. Use info() to get an output similar to phpinfo(). Visualize the complete configuration. I guess we could create something like that for 3.2. _________________ Twitter |
|
Back to top |
|
Aristophan Smarty Regular
Joined: 10 Jan 2011 Posts: 96
|
Posted: Sun Oct 16, 2011 12:58 pm Post subject: |
|
|
Hi Rodney
Well, these changes have no effekt, since
Code: | // test if registered compile_dir is accessible
$__compile_dir = $smarty->getCompileDir();
$_compile_dir = realpath($__compile_dir);
if (!$__compile_dir) {
$status = false;
$message = "FAILED: {$__compile_dir} does not exist";
.... |
is wellformed and not buggy at all, since Code: | $this->setCompileDir('/some/path/to/my/templates_c'); | sets the correct dir and gets accessed truly with getCompileDir().
The same codepart for $__cache_dir = $smarty->getCacheDir(); is troubling, while the setCacheDir is not set with
Code: | $this->setCacheDir('/some/path/to/my/cache'); |
and cannot get accessed via getCacheDir(). The wrong set path must be somewhere else.
Yeah, introducing a new method "info()" for security settings and security checks is fine for me, thank you. |
|
Back to top |
|
rodneyrehm Administrator
Joined: 30 Mar 2007 Posts: 674 Location: Germany, border to Switzerland
|
Posted: Sun Oct 16, 2011 1:05 pm Post subject: |
|
|
I have no clue what you're talking about. Please provide (more complete) code examples to enlighten me… _________________ Twitter |
|
Back to top |
|
Aristophan Smarty Regular
Joined: 10 Jan 2011 Posts: 96
|
Posted: Sun Oct 16, 2011 1:20 pm Post subject: |
|
|
Oh shit, I am deeply sorry for troubling the horses
I just recognized my absolute path did have a tiny little bug...
Please revert your changes. |
|
Back to top |
|
rodneyrehm Administrator
Joined: 30 Mar 2007 Posts: 674 Location: Germany, border to Switzerland
|
Posted: Sun Oct 16, 2011 1:28 pm Post subject: |
|
|
My changes were indeed necessary. But glad you figured it out _________________ Twitter |
|
Back to top |
|
|