groovy
  1. groovy
  2. GROOVY-5116

Groovy enforces the use of the the dangerous permission java.util.PropertyPermission "*" "read,write" when using a SecurityManager

    Details

    • Type: Improvement Improvement
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.8.3
    • Fix Version/s: None
    • Component/s: groovy-runtime
    • Labels:
    • Number of attachments :
      0

      Description

      In several occurrences in the code, the system properties are accessed in this manner:

      groovy.grape.Grape.java

      private static boolean enableGrapes = Boolean.valueOf(System.getProperties().getProperty("groovy.grape.enable", "true"));
      

      The use of System.getProperties() forces the use of this permission in the SecurityManager:

       java.util.PropertyPermission "*" "read,write"

      This is not really desired in security sensitive environments. It is not possible to use more fine-grained permission declaration like e.g.:

       java.util.PropertyPermission "groovy.*" "read,write"

      This problem could be easily avoided by accessing the properties in this manner:

      private static boolean enableGrapes = Boolean.valueOf(System.getProperty("groovy.grape.enable", "true"));
      

      Without the use of System.getProperties() it is not mandatory to set the dangerous write permission on all system properties and more fine-grained security permissions like in the example could be used.

        Activity

        Hide
        Guillaume Laforge added a comment -

        There are also some calls to System.properties or System.properties['someKey'] as well, which are in the end call System.getProperties()

        Show
        Guillaume Laforge added a comment - There are also some calls to System.properties or System.properties ['someKey'] as well, which are in the end call System.getProperties()
        Hide
        Guillaume Laforge added a comment -

        To be precise, related to the GroovyConsole in GTKDefaults.groovy, WindowsDefaults.groovy.
        And in relation to groovysh, in CommandSupport.groovy and Groovysh.groovy.

        Show
        Guillaume Laforge added a comment - To be precise, related to the GroovyConsole in GTKDefaults.groovy, WindowsDefaults.groovy. And in relation to groovysh, in CommandSupport.groovy and Groovysh.groovy.
        Hide
        blackdrag blackdrag added a comment -

        Benjamin, could you give us the trace of such an SecurityException? So that I can see where the offending call is actually done

        Show
        blackdrag blackdrag added a comment - Benjamin, could you give us the trace of such an SecurityException? So that I can see where the offending call is actually done
        Hide
        Benjamin Wolff added a comment -

        I did some testing and currently, I could not reproduce the described behaviour any longer.

        However, the issue with the getProperties() usage still remains.

        Just for the archives: I'm using Grails 2.0.4, which in turn bundles Groovy 1.8.6. The environment is an Apache Tomcat 7 with activated security manager. The catalina.policy file is the default one with the following additional permissions, which I identified to be required as a baseline for this environment:

        // Additional permissions to get Grails applications running
        grant {
        	// Necessary permissions already present in CERN default cfg
        	permission java.net.SocketPermission "*", "connect, resolve";
        	permission java.lang.RuntimePermission "createClassLoader";
        	permission java.lang.RuntimePermission "getClassLoader";
        	permission java.lang.RuntimePermission "accessDeclaredMembers";
        	permission java.lang.RuntimePermission "getProtectionDomain";
        	permission java.lang.RuntimePermission "accessClassInPackage.*";
        	permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
        	permission java.util.PropertyPermission "*", "read";
        	permission java.io.FilePermission "${catalina.home}${file.separator}logs${file.separator}-", "read, write, delete";
        	permission java.io.FilePermission "${catalina.home}${file.separator}temp${file.separator}-", "read, write, delete";
        	permission java.io.FilePermission "${catalina.home}${file.separator}temp", "read";
        
        	// Additional file permission for listing the content of the logs folder 
        	permission java.io.FilePermission "${catalina.home}${file.separator}logs", "read";
        	
        	// Necessary additional permissions for Grails/Spring/Hibernate
        	permission java.util.PropertyPermission "net.sf.ehcache.*", "read,write";
        	permission java.util.PropertyPermission "grails.*", "read,write";
        	permission java.util.PropertyPermission "user.language", "read,write";
        	permission java.lang.RuntimePermission "setContextClassLoader";
        	permission java.lang.RuntimePermission "modifyThread";
        	
        	// Necessary Groovy permissions
        	permission groovy.security.GroovyCodeSourcePermission "/groovy/shell";
        	permission groovy.security.GroovyCodeSourcePermission "/groovy/script";
        };
        
        Show
        Benjamin Wolff added a comment - I did some testing and currently, I could not reproduce the described behaviour any longer. However, the issue with the getProperties() usage still remains. Just for the archives: I'm using Grails 2.0.4, which in turn bundles Groovy 1.8.6. The environment is an Apache Tomcat 7 with activated security manager. The catalina.policy file is the default one with the following additional permissions, which I identified to be required as a baseline for this environment: // Additional permissions to get Grails applications running grant { // Necessary permissions already present in CERN default cfg permission java.net.SocketPermission "*" , "connect, resolve" ; permission java.lang.RuntimePermission "createClassLoader" ; permission java.lang.RuntimePermission "getClassLoader" ; permission java.lang.RuntimePermission "accessDeclaredMembers" ; permission java.lang.RuntimePermission "getProtectionDomain" ; permission java.lang.RuntimePermission "accessClassInPackage.*" ; permission java.lang.reflect.ReflectPermission "suppressAccessChecks" ; permission java.util.PropertyPermission "*" , "read" ; permission java.io.FilePermission "${catalina.home}${file.separator}logs${file.separator}-" , "read, write, delete" ; permission java.io.FilePermission "${catalina.home}${file.separator}temp${file.separator}-" , "read, write, delete" ; permission java.io.FilePermission "${catalina.home}${file.separator}temp" , "read" ; // Additional file permission for listing the content of the logs folder permission java.io.FilePermission "${catalina.home}${file.separator}logs" , "read" ; // Necessary additional permissions for Grails/Spring/Hibernate permission java.util.PropertyPermission "net.sf.ehcache.*" , "read,write" ; permission java.util.PropertyPermission "grails.*" , "read,write" ; permission java.util.PropertyPermission "user.language" , "read,write" ; permission java.lang.RuntimePermission "setContextClassLoader" ; permission java.lang.RuntimePermission "modifyThread" ; // Necessary Groovy permissions permission groovy.security.GroovyCodeSourcePermission "/groovy/shell" ; permission groovy.security.GroovyCodeSourcePermission "/groovy/script" ; };
        Hide
        Paul King added a comment -

        Thanks for your investigations Benjamin. Some very useful information. By way of summary for this issue as far as I see it, given that the Grape usage has been fixed and that this now seems fixed in a grails context, it just remains for us to decide what if anything we need to do for the GroovyMain case.

        Show
        Paul King added a comment - Thanks for your investigations Benjamin. Some very useful information. By way of summary for this issue as far as I see it, given that the Grape usage has been fixed and that this now seems fixed in a grails context, it just remains for us to decide what if anything we need to do for the GroovyMain case.

          People

          • Assignee:
            Unassigned
            Reporter:
            Benjamin Wolff
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated: