Smarty Forum Index Smarty
WARNING: All discussion is moving to https://reddit.com/r/smarty, please go there! This forum will be closing soon.

Smarty_Security roadmap

 
This forum is locked: you cannot post, reply to, or edit topics.   This topic is locked: you cannot edit posts or make replies.    Smarty Forum Index -> Smarty 3
View previous topic :: View next topic  
Author Message
Aristophan
Smarty Regular


Joined: 10 Jan 2011
Posts: 96

PostPosted: Mon Oct 10, 2011 11:45 am    Post subject: Smarty_Security roadmap Reply with quote

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
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Mon Oct 10, 2011 11:54 am    Post subject: Reply with quote

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… Wink If you have ideas / opinions, please share.

How is this important to you?
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
Aristophan
Smarty Regular


Joined: 10 Jan 2011
Posts: 96

PostPosted: Mon Oct 10, 2011 1:21 pm    Post subject: Reply with quote

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
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Mon Oct 10, 2011 1:43 pm    Post subject: Reply with quote

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
View user's profile Send private message Visit poster's website
Aristophan
Smarty Regular


Joined: 10 Jan 2011
Posts: 96

PostPosted: Mon Oct 10, 2011 2:23 pm    Post subject: Reply with quote

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
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Mon Oct 10, 2011 2:29 pm    Post subject: Reply with quote

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
View user's profile Send private message Visit poster's website
Aristophan
Smarty Regular


Joined: 10 Jan 2011
Posts: 96

PostPosted: Tue Oct 11, 2011 11:19 am    Post subject: Reply with quote

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
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Tue Oct 11, 2011 11:23 am    Post subject: Reply with quote

What exactly do you expect to see here?
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
Aristophan
Smarty Regular


Joined: 10 Jan 2011
Posts: 96

PostPosted: Tue Oct 11, 2011 11:39 am    Post subject: Reply with quote

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. Wink
Back to top
View user's profile Send private message
Aristophan
Smarty Regular


Joined: 10 Jan 2011
Posts: 96

PostPosted: Sat Oct 15, 2011 6:55 pm    Post subject: Reply with quote

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! Smile
Back to top
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Sun Oct 16, 2011 9:57 am    Post subject: Reply with quote

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
View user's profile Send private message Visit poster's website
Aristophan
Smarty Regular


Joined: 10 Jan 2011
Posts: 96

PostPosted: Sun Oct 16, 2011 12:58 pm    Post subject: Reply with quote

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
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Sun Oct 16, 2011 1:05 pm    Post subject: Reply with quote

I have no clue what you're talking about. Please provide (more complete) code examples to enlighten me…
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
Aristophan
Smarty Regular


Joined: 10 Jan 2011
Posts: 96

PostPosted: Sun Oct 16, 2011 1:20 pm    Post subject: Reply with quote

Oh shit, I am deeply sorry for troubling the horses Wink
I just recognized my absolute path did have a tiny little bug... Embarassed
Please revert your changes.
Back to top
View user's profile Send private message
rodneyrehm
Administrator


Joined: 30 Mar 2007
Posts: 674
Location: Germany, border to Switzerland

PostPosted: Sun Oct 16, 2011 1:28 pm    Post subject: Reply with quote

My changes were indeed necessary. But glad you figured it out
_________________
Twitter
Back to top
View user's profile Send private message Visit poster's website
Display posts from previous:   
This forum is locked: you cannot post, reply to, or edit topics.   This topic is locked: you cannot edit posts or make replies.    Smarty Forum Index -> Smarty 3 All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group
Protected by Anti-Spam ACP