Commons Logging
  1. Commons Logging
  2. LOGGING-26

Security policy configuration, SimpleLog uses System.getProperties()

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Nightly Builds
    • Fix Version/s: 1.0.3
    • Labels:
      None
    • Environment:

      Operating System: Solaris
      Platform: PC

      Description

      SimpleLog uses System.getProperties to get a list of existing
      org.apache.commons.logging.* properties.

      If commons-logging is running within an application which uses
      the Java SecurityManager such as Tomcat this requires granting
      java.util.PropertyPermission "*", "read" to not only
      commongs-logging.jar, but all other jar files with classes
      on the stack.

      This makes it impossible to restrict access to reading properties
      for any API's on the stack.

      SimpleLog should get each individual property it needs separately.

      This would apply to any other code which uses System.getProperties() also.

        Activity

        Hide
        Glenn Nielsen added a comment -

        Correction, it requires the following permission to be granted:

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

        What is really bad about this is that you have to grant permission
        to write all properties.

        Show
        Glenn Nielsen added a comment - Correction, it requires the following permission to be granted: java.util.PropertyPermission "*", "read,write"; What is really bad about this is that you have to grant permission to write all properties.
        Hide
        Craig McClanahan added a comment -

        This was fixed in version 1.3 of SimpleLog to swallow any security exceptions
        caused by this, so the fix is in current nightly builds and will be included in
        the next released version.

        Show
        Craig McClanahan added a comment - This was fixed in version 1.3 of SimpleLog to swallow any security exceptions caused by this, so the fix is in current nightly builds and will be included in the next released version.
        Hide
        Glenn Nielsen added a comment -

        Just swallowing the security exceptions doesn't solve the problem.
        What if you need SimpleLog to use properties? It can't use any
        properties unless it is granted:

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

        This is too broad of a permission grant to give SimpleLog and all
        other API's which may be on the stack.

        Show
        Glenn Nielsen added a comment - Just swallowing the security exceptions doesn't solve the problem. What if you need SimpleLog to use properties? It can't use any properties unless it is granted: java.util.PropertyPermission "*", "read,write"; This is too broad of a permission grant to give SimpleLog and all other API's which may be on the stack.
        Hide
        Richard A. Sitze added a comment -

        This cannot be easily corrected:

        On one hand, SimpleLog has a requirement to pickup unspecified properties
        (those properties beginning with a prefix) describing logging levels for
        different categories.

        On the other, SimpleLog has turned into a real (although simple log
        implementation.

        This leaves us with THREE choices for satisfactorily resolving this defect:

        1. Simplify SimpleLog & remove offending code, or

        2. Make SimpleLog a NotSoSimpleLog with it's own property file, and
        remove offending calls to System.getProperties() - use property file
        instead.

        3. Acknowledge that in a proper J2EE environment SimpleLog may not be
        the best answer, so it doesn't matter anyway...

        Show
        Richard A. Sitze added a comment - This cannot be easily corrected: On one hand, SimpleLog has a requirement to pickup unspecified properties (those properties beginning with a prefix) describing logging levels for different categories. On the other, SimpleLog has turned into a real (although simple log implementation. This leaves us with THREE choices for satisfactorily resolving this defect: 1. Simplify SimpleLog & remove offending code, or 2. Make SimpleLog a NotSoSimpleLog with it's own property file, and remove offending calls to System.getProperties() - use property file instead. 3. Acknowledge that in a proper J2EE environment SimpleLog may not be the best answer, so it doesn't matter anyway...
        Hide
        Glenn Nielsen added a comment -

        I would guess that jakarta project API's get used within a
        servlet or other server side container > 90 % of the time.
        If it exists it will get used. Those developing applications
        which use SimpleLog may not be as security aware as system
        administrators who have to deploy the application.
        IMHO this rules out option 3.

        Option 1 requires removing features from SimpleLog which
        may not be best for those already using SimpleLog.

        Option 2 seems like the best solution. It doesn't remove
        any features and allows SimpleLog to be used in a J2EE
        container which implements a SecurityManager.

        Show
        Glenn Nielsen added a comment - I would guess that jakarta project API's get used within a servlet or other server side container > 90 % of the time. If it exists it will get used. Those developing applications which use SimpleLog may not be as security aware as system administrators who have to deploy the application. IMHO this rules out option 3. Option 1 requires removing features from SimpleLog which may not be best for those already using SimpleLog. Option 2 seems like the best solution. It doesn't remove any features and allows SimpleLog to be used in a J2EE container which implements a SecurityManager.
        Hide
        Adrian Sutton added a comment -

        Perhaps a slight improvement over version 2 would be to use the properties file
        if available and then fall back on to the system properties if that fails?

        We use SimpleLog in a signed applet and make heavy use of the system properties
        configurability as it allows us to vary the logging level very easily (a must
        if you want end users to do it). Having a separate configuration file would be
        much less convienient for this use.

        Show
        Adrian Sutton added a comment - Perhaps a slight improvement over version 2 would be to use the properties file if available and then fall back on to the system properties if that fails? We use SimpleLog in a signed applet and make heavy use of the system properties configurability as it allows us to vary the logging level very easily (a must if you want end users to do it). Having a separate configuration file would be much less convienient for this use.
        Hide
        Richard A. Sitze added a comment -

        Eliminated System.getProperties().
        Instead, on each query for a property, we call System.getProperty.
        (private) helper methods are getStringProperty() and getBooleanProperty()

        Show
        Richard A. Sitze added a comment - Eliminated System.getProperties(). Instead, on each query for a property, we call System.getProperty. (private) helper methods are getStringProperty() and getBooleanProperty()

          People

          • Assignee:
            Unassigned
            Reporter:
            Glenn Nielsen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development