New security features in FreeMarker

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

New security features in FreeMarker

Attila Szegedi
This is going to be longish, but please bear with me.

I'm currently experimenting with a feature in my local copy of FreeMarker  
where templates can execute in their own Java protection domain. You might  
ask why is this desirable. The answer is that currently, FreeMarker  
templates execute with all the privileges that are also granted to  
freemarker.jar in the Java security policy. Clearly, there are situations  
where this is not acceptable and where there'd be a requirement to have  
privileges granted to templates explicitly. Now, instead of reinventing a  
complete security system, it is easiest and actually most rational to  
leverage the existing Java security framework where permissions are  
granted to code by assigning those permissions based on the location the  
code was loaded from - this is typically done in a security policy file,  
see <http://java.sun.com/j2se/1.4.2/docs/guide/security/PolicyFiles.html>

Therefore, if you have templates for some user in, say,  
/home/johndoe/htdocs/freemarker, you can add something like this to your  
Java policy file:

grant "file:///home/johndoe/htdocs/freemarker/" {
    permission ...
}

to regulate the permissions of templates coming from there.

(1) My first question is: do others also see this as a worthwhile feature  
to have?

This is easier to implement than it sounds, actually - first thing we need  
is add a java.security.CodeSource object to Template and Macro objects.  
CodeSource is basically a pair of (URL,Certificate[]), that is an URL  
specifying where the code came from and if it is also optionally digitally  
signed. A mechanism to assign a CodeSource to a template is  
straightforward if we create a new interface:

interface SecureTemplateLoader extends TemplateLoader {
     CodeSource getCodeSource(Object templateSource);
}

The interface gets implemented by all stock template loaders -  
URLTemplateLoader can even assign the Certificate[] correctly if the  
template is loaded from a signed JAR file. TemplateCache can then do an

if(config.isSecureTemplates() && loader instanceof SecureTemplateLoader) {
     codeSource = ((SecureTemplateLoader)loader).getCodeSource(source);
} else {
     codeSource = null;
}

and assign it to the loaded template. Note there's a  
Configuration.isSecureTemplates() property - it is set to false by  
default, so FreeMarker by default operates as earlier, running templates  
in its protection domain. Only by explicitly flipping this flag to true is  
the whole security stuff activated.

In Java, the effective set of permissions for a thread at a point in time  
is the intersection of the permissions of all Java classes on the stack  
(well, precisely speaking, from top to either the bottom of the stack or  
first AccessController.doPrivileged() block). For this to work with  
FreeMarker templates as well, we'll need a bit of a code generation -  
namely, we need to dynamically create one proxy class per every  
CodeSource, and make sure Template.process(), inclusion, and macro  
invocation pass through a method on this proxy class. This can clearly be  
optimized away for template inclusions and macro invocations coming from  
the same code source. This is all not a big deal, although we'll gather a  
runtime dependency on some code generator library.

(2) My second question is: is it okay if we gather a runtime dependency on  
ASM/CGLIB/whatever for this security feature?

My final question requires a bit more of an explanation. I've been  
analyzing the possible attack vectors on a secured system. Where I  
identified a potential vulnerability, I invented a permission and coded a  
check for it. I.e. to prevent someone from simply calling  
config.setSecureTemplates(false) after it was already set to true, the  
code now does this check:

public void setSecureTemplates(boolean secureTemplates) {
     if(secureTemplates == false && this.secureTemplates == true) {
         // Attempt to turn off secure templates is subject to permission
         // check
         SecurityManager sm = System.getSecurityManager();
         if(sm != null) {
             sm.checkPermission(new FreeMarkerPermission(
                     "setSecureTemplates"));
         }
     }
     ...

That is, any code wanting to turn it off after it was turned on must have a

grant path_to_jar_file {
     permission freemarker.core.FreeMarkerPermission "setSecureTemplates"
}

entry in its policy file. Similar to this is

     permission freemarker.core.FreeMarkerPermission "setTemplateLoader"

that is required in order for code to be able to set an arbitrary template  
loader using setTemplateLoader() -- we can't know if the caller wants to  
smuggle in a template loader that lies about the CodeSource of the loaded  
templates.

I must emphasize that these permissions are only checked when a)  
System.getSecurityManager() is not null and b)  
configuration.isSecureTemplates() is true. In an unsecured JVM, or in a  
FreeMarker configuration that wasn't told to be secure, none of this has  
any effect whatsoever.

Anyway, analyzing the attack vectors it became apparent that it's not much  
use if we have a system where templates can run with their own privileges  
if it is also possible for an attacker to change the code in a trusted  
template. And right now, it is trivial to invoke  
Template.getRootTreeNode() and to replace its contents using  
TemplateElement.setNestedBlock(). Thus, an attacker could try to inject  
his own code by manually crafting a parsed tree and calling  
template.getRootTreeNode().setNestedBlock() on it. The problem is, I'm not  
sure what can we do about it.

(3) Third question is: how to prevent a template from being modified after  
it was loaded?

Ideas:
1. We could have yet another permission named "getRootTreeNode" that is  
checked on Template.getRootTreeNode(), but then FreeMarker would have to  
be granted this permission itself in order to execute a template. This is  
clearly not going to fly.

2. We could change the TemplateElement API so that its nested content is  
immutable. This is easiest accomplished if it refuses setNestedContent()  
if it already has non-null content. Alternatively, it could be a final  
field set in the constructor. This other idea would also introduce massive  
changes to the parser. Also, it'd make the AST considerably harder to use  
for tools that want to modify the AST on the fly, and not just read-only  
analyze it - technically, whenever you'd want to modify a node, you'd have  
to create a new instance of it as well as all its ancestors.

As a mid-solution, we could have a method on TemplateElement named  
isMutable(), that is checked when setNestedContent() is invoked and  
existing content is not null. By default, it'd delegate to its parent  
element, or return true when it has no parent. We'd also have a special  
ImmutableTemplateElement class that'd return false. So, in order to make  
an AST (sub)tree immutable, all you'd have to do is place an  
ImmutableTemplateElement in its root. When a tool wants to modify the  
content of a TemplatElement, the TemplateElement would essentially walk  
its ancestor chain to determine whether it's mutable or not and if it is  
throw an IllegalStateException or such. So far, I like this idea best. As  
for immutability, there's a problem with recent change of fields in  
Expression subclasses to "public" but I'll discuss that in a separate  
e-mail thread.

Attila.

--
home: http://www.szegedi.org
weblog: http://constc.blogspot.com

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
FreeMarker-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-devel
Reply | Threaded
Open this post in threaded view
|

Re: New security features in FreeMarker

Chris Nokleberg
On 9/16/06, Attila Szegedi <[hidden email]> wrote:
> (2) My second question is: is it okay if we gather a runtime dependency on
> ASM/CGLIB/whatever for this security feature?

ASM is only ~30K and you could use jarjar.sf.net to repackage it under
the freemarker namespace to avoid any dependency (the classes would be
included in freemarker.jar). Let me know if you want any help writing
the bytecode/etc.

Chris

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
FreeMarker-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-devel
Reply | Threaded
Open this post in threaded view
|

Re: New security features in FreeMarker

Attila Szegedi
On Sat, 16 Sep 2006 19:40:49 +0200, Chris Nokleberg <[hidden email]>  
wrote:

>
> ASM is only ~30K and you could use jarjar.sf.net to repackage it under
> the freemarker namespace to avoid any dependency (the classes would be
> included in freemarker.jar). Let me know if you want any help writing
> the bytecode/etc.
>

Thanks, will do. Although right now, I think I'll manage it without any  
code generation whatsoever. All I really need is write the single  
implementation of the proxy class, and then rename the .class file to i.e.  
.clazz in the Ant build process so that it's loadable using  
Class.getResourceAsStream(). I.e.

abstract class SecureInvoker
{
     abstract void render(Environment e, TemplateElement t);
}

class SecureInvokerImpl extends SecureInvoker
{
     void render(Environment e, TemplateElement t)
     {
         e.render(t);
     }
}

Then all I need is rename SecureInvokerImpl.class to  
SecureInvokerImpl.clazz, so that whenever I need an instance of it bound  
to a concrete code source, I can load its bytes using  
getResourceAsStream() and pass it to SecureClassLoader.defineClass(...,  
CodeSource cs).

Of course, it does have a drawback of not working when not compiled from  
Ant (i.e. running unit tests from Eclipse), but I think I can live with  
that limitation :-)

Attila.

> Chris
>

--
home: http://www.szegedi.org
weblog: http://constc.blogspot.com

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
FreeMarker-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-devel
Reply | Threaded
Open this post in threaded view
|

Re: New security features in FreeMarker

Daniel Dekany
In reply to this post by Attila Szegedi
Saturday, September 16, 2006, 11:36:44 AM, Attila Szegedi wrote:

> This is going to be longish, but please bear with me.
>
> I'm currently experimenting with a feature in my local copy of FreeMarker
> where templates can execute in their own Java protection domain. You might
> ask why is this desirable. The answer is that currently, FreeMarker  
> templates execute with all the privileges that are also granted to  
> freemarker.jar in the Java security policy. Clearly, there are situations
> where this is not acceptable and where there'd be a requirement to have
> privileges granted to templates explicitly. Now, instead of reinventing a
> complete security system, it is easiest and actually most rational to
> leverage the existing Java security framework where permissions are  
> granted to code by assigning those permissions based on the location the
> code was loaded from - this is typically done in a security policy file,
> see
> <http://java.sun.com/j2se/1.4.2/docs/guide/security/PolicyFiles.html>
>
> Therefore, if you have templates for some user in, say,  
> /home/johndoe/htdocs/freemarker, you can add something like this to your
> Java policy file:
>
> grant "file:///home/johndoe/htdocs/freemarker/" {
>     permission ...
> }
>
> to regulate the permissions of templates coming from there.
>
> (1) My first question is: do others also see this as a worthwhile feature
> to have?
>
> This is easier to implement than it sounds, actually - first thing we need
> is add a java.security.CodeSource object to Template and Macro objects.
> CodeSource is basically a pair of (URL,Certificate[]), that is an URL
> specifying where the code came from and if it is also optionally digitally
> signed. A mechanism to assign a CodeSource to a template is  
> straightforward if we create a new interface:
>
> interface SecureTemplateLoader extends TemplateLoader {
>      CodeSource getCodeSource(Object templateSource);
> }
>
> The interface gets implemented by all stock template loaders -  
> URLTemplateLoader can even assign the Certificate[] correctly if the  
> template is loaded from a signed JAR file. TemplateCache can then do an
>
> if(config.isSecureTemplates() && loader instanceof SecureTemplateLoader) {
>      codeSource =
> ((SecureTemplateLoader)loader).getCodeSource(source);
> } else {
>      codeSource = null;
> }
>
> and assign it to the loaded template. Note there's a  
> Configuration.isSecureTemplates() property - it is set to false by  
> default, so FreeMarker by default operates as earlier, running templates
> in its protection domain. Only by explicitly flipping this flag to true is
> the whole security stuff activated.
>
> In Java, the effective set of permissions for a thread at a point in time
> is the intersection of the permissions of all Java classes on the stack
> (well, precisely speaking, from top to either the bottom of the stack or
> first AccessController.doPrivileged() block). For this to work with  
> FreeMarker templates as well, we'll need a bit of a code generation -
> namely, we need to dynamically create one proxy class per every  
> CodeSource, and make sure Template.process(), inclusion, and macro  
> invocation pass through a method on this proxy class. This can clearly be
> optimized away for template inclusions and macro invocations coming from
> the same code source. This is all not a big deal, although we'll gather a
> runtime dependency on some code generator library.
>
> (2) My second question is: is it okay if we gather a runtime dependency on
> ASM/CGLIB/whatever for this security feature?
>
> My final question requires a bit more of an explanation. I've been
> analyzing the possible attack vectors on a secured system. Where I  
> identified a potential vulnerability, I invented a permission and coded a
> check for it. I.e. to prevent someone from simply calling  
> config.setSecureTemplates(false) after it was already set to true, the
> code now does this check:
>
> public void setSecureTemplates(boolean secureTemplates) {
>      if(secureTemplates == false && this.secureTemplates == true) {
>          // Attempt to turn off secure templates is subject to permission
>          // check
>          SecurityManager sm = System.getSecurityManager();
>          if(sm != null) {
>              sm.checkPermission(new FreeMarkerPermission(
>                      "setSecureTemplates"));
>          }
>      }
>      ...
>
> That is, any code wanting to turn it off after it was turned on must have a
>
> grant path_to_jar_file {
>      permission freemarker.core.FreeMarkerPermission "setSecureTemplates"
> }
>
> entry in its policy file. Similar to this is
>
>      permission freemarker.core.FreeMarkerPermission "setTemplateLoader"
>
> that is required in order for code to be able to set an arbitrary template
> loader using setTemplateLoader() -- we can't know if the caller wants to
> smuggle in a template loader that lies about the CodeSource of the loaded
> templates.
>
> I must emphasize that these permissions are only checked when a)  
> System.getSecurityManager() is not null and b)  
> configuration.isSecureTemplates() is true. In an unsecured JVM, or in a
> FreeMarker configuration that wasn't told to be secure, none of this has
> any effect whatsoever.
>
> Anyway, analyzing the attack vectors it became apparent that it's not much
> use if we have a system where templates can run with their own privileges
> if it is also possible for an attacker to change the code in a trusted
> template. And right now, it is trivial to invoke  
> Template.getRootTreeNode() and to replace its contents using  
> TemplateElement.setNestedBlock(). Thus, an attacker could try to inject
> his own code by manually crafting a parsed tree and calling  
> template.getRootTreeNode().setNestedBlock() on it. The problem is, I'm not
> sure what can we do about it.
>
> (3) Third question is: how to prevent a template from being modified after
> it was loaded?
>
> Ideas:
> 1. We could have yet another permission named "getRootTreeNode" that is
> checked on Template.getRootTreeNode(), but then FreeMarker would have to
> be granted this permission itself in order to execute a template. This is
> clearly not going to fly.
>
> 2. We could change the TemplateElement API so that its nested content is
> immutable. This is easiest accomplished if it refuses setNestedContent()
> if it already has non-null content. Alternatively, it could be a final
> field set in the constructor. This other idea would also introduce massive
> changes to the parser. Also, it'd make the AST considerably harder to use
> for tools that want to modify the AST on the fly, and not just read-only
> analyze it - technically, whenever you'd want to modify a node, you'd have
> to create a new instance of it as well as all its ancestors.
>
> As a mid-solution, we could have a method on TemplateElement named  
> isMutable(), that is checked when setNestedContent() is invoked and  
> existing content is not null. By default, it'd delegate to its parent
> element, or return true when it has no parent. We'd also have a special
> ImmutableTemplateElement class that'd return false. So, in order to make
> an AST (sub)tree immutable, all you'd have to do is place an  
> ImmutableTemplateElement in its root. When a tool wants to modify the
> content of a TemplatElement, the TemplateElement would essentially walk
> its ancestor chain to determine whether it's mutable or not and if it is
> throw an IllegalStateException or such. So far, I like this idea best. As
> for immutability, there's a problem with recent change of fields in  
> Expression subclasses to "public" but I'll discuss that in a separate
> e-mail thread.

Ugh! Are you sure adding this feature worth it? You see, FM still has
space for improvements even with the basic templating functionality
(like parent child communication, etc), so I'm certain there are many
improvements that could be more rewarding than this. Or do you see an
important "market" for this feature?

--
Best regards,
 Daniel Dekany


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
FreeMarker-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-devel
Reply | Threaded
Open this post in threaded view
|

Re: New security features in FreeMarker

Attila Szegedi
On Sun, 17 Sep 2006 19:02:24 +0200, Daniel Dekany <[hidden email]>  
wrote:

>
> Ugh! Are you sure adding this feature worth it? You see, FM still has
> space for improvements even with the basic templating functionality
> (like parent child communication, etc), so I'm certain there are many
> improvements that could be more rewarding than this. Or do you see an
> important "market" for this feature?
>

Yes - security savvy enterprise systems in general. Any place where people  
deploy a Java server with a security policy in place. Since FreeMarker  
templates are code, it just makes sense for their privileges be  
regulatable just as any other code, especially since they can reach out to  
Java objects. JSP pages were already manageable through a policy, since  
they compile to .class files. Admins of websites where users can freely  
upload JSP pages (mostly corporate intranets, but there are also hosting  
companies offering JSP hosting) could sleep at night assured that even if  
a clever coworker/client uploads a JSP page with <% System.exit(1) %>, all  
he'll get is a SecurityException since the policy doesn't have a "grant  
java.lang.RuntimePermission "exit";" line for that users' directory :-).  
This is of course a rather corner-case example, but it shows the use case.  
With this feature, FreeMarker templates become security-manageable in  
identical fashion.

I have completed the implementation in the meantime, and tested it today.  
If there's no objection to this feature, I'll commit it to SVN. I must  
stress again that it doesn't change the default behavior of FM in any way.  
The user must explicitly invoke configuration.setSecure(true) to activate  
it.

Attila.

--
home: http://www.szegedi.org
weblog: http://constc.blogspot.com

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
FreeMarker-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-devel
Reply | Threaded
Open this post in threaded view
|

Re: New security features in FreeMarker

Daniel Dekany
Monday, September 18, 2006, 3:23:56 PM, Attila Szegedi wrote:


> On Sun, 17 Sep 2006 19:02:24 +0200, Daniel Dekany <[hidden email]>
> wrote:
>
>>
>> Ugh! Are you sure adding this feature worth it? You see, FM still has
>> space for improvements even with the basic templating functionality
>> (like parent child communication, etc), so I'm certain there are many
>> improvements that could be more rewarding than this. Or do you see an
>> important "market" for this feature?
>>
>
> Yes - security savvy enterprise systems in general. Any place where people
> deploy a Java server with a security policy in place.

But, apart from *theocratical* need, is there a need? I think for most
applications FreeMarker templates are just the internal parts of the
application, so they should just belong to the same security domain
where the application is, or in more generic case where the action
method is.

> Since FreeMarker templates are code, it just makes sense for their
> privileges be regulatable just as any other code, especially since
> they can reach out to Java objects. JSP pages were already
> manageable through a policy, since they compile to .class files.

JSP files are not MVC templates. This means that they are possibly
meant to be used alone, i.e. without closely depending on some "action
method" or something like that. Hence it clearly does make sense to
assign security policies to them. OTOH, MVC templates are not meant to
live alone. They are rather the second half of the "action method",
that was implemented not in Java Language but in FTL.

> Admins of websites where users can freely upload JSP pages (mostly
> corporate intranets, but there are also hosting companies offering
> JSP hosting) could sleep at night assured that even if a clever
> coworker/client uploads a JSP page with <% System.exit(1) %>, all
> he'll get is a SecurityException since the policy doesn't have a
> "grant java.lang.RuntimePermission "exit";" line for that users'
> directory :-). This is of course a rather corner-case example, but
> it shows the use case. With this feature, FreeMarker templates
> become security-manageable in identical fashion.
>
> I have completed the implementation in the meantime, and tested it today.
> If there's no objection to this feature, I'll commit it to SVN. I must
> stress again that it doesn't change the default behavior of FM in any way.
> The user must explicitly invoke configuration.setSecure(true) to activate
> it.

Well, since you already implemented it, it can remain, IMO... The only
negative effect is that each new feature of this kind (i.e. something
that is present in the codebase distributed, unlike for example a new
built-in) complicates the maintenance.

> Attila.

--
Best regards,
 Daniel Dekany


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
FreeMarker-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-devel