Running under a SecurityManager
Hello,
As noticed by Boris Unckel in LOG4J2-3579[1], there are some issues
when running Log4j2 under a SecurityManager, both from the
`ServiceLoader` and `PropertiesUtil` perspective.
>From the `PropertiesUtil` perspective:
* version 2.17.2 (cr. [2]) ignored all property sources that caused
an `AccessControllerException` (actually it probably ignored all
property sources from the classloader that caused the exception). This
behavior was introduced as a fix to LOG4J2-2266 [3].
* the `ServiceLoaderUtil` I backported from `master` to 2.18.0
catches `ServiceLoaderException`s on a per-property source basis, but
it does not catch `AccessControllerException`s which can cause global
failures, as the one described by Boris.
I am wondering what is the best course of action in this case:
1. We can keep silently ignoring all exceptions thrown by the
`ServiceLoader` and the iterators it produces,
2. We can introduce some `AccessController#doPrivileged` calls in the
right places (cf. the PRs [4] and [5] by Boris).
I would prefer the second solution, but it requires adding security
checks to all the available property sources. The environment and
system properties sources are protected by internal Java security
checks, but the `log4j2.component.properties` and Spring property
sources are available for everyone to read. Some malicious code could
for example call
`PropertiesUtil.getProperty("log4j2.keyStorePassword")`.
What do you think?
Piotr
[1] https://issues.apache.org/jira/browse/LOG4J2-3579
[2]
https://github.com/apache/logging-log4j2/blob/c33646f61850619c756797122f4fc4c53f7013f1/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L437
[3] https://issues.apache.org/jira/browse/LOG4J2-2266
[4] https://github.com/apache/logging-log4j2/pull/1006
[5] https://github.com/apache/logging-log4j2/pull/1007
Re: Running under a SecurityManager
Side note: The SecurityManager will eventually go away, which might not
matter for us for a long time!
See https://openjdk.org/jeps/411
Gary
On Sun, Aug 21, 2022 at 8:12 AM Piotr P. Karwasz
wrote:
> Hello,
>
> As noticed by Boris Unckel in LOG4J2-3579[1], there are some issues
> when running Log4j2 under a SecurityManager, both from the
> `ServiceLoader` and `PropertiesUtil` perspective.
>
> From the `PropertiesUtil` perspective:
>
> * version 2.17.2 (cr. [2]) ignored all property sources that caused
> an `AccessControllerException` (actually it probably ignored all
> property sources from the classloader that caused the exception). This
> behavior was introduced as a fix to LOG4J2-2266 [3].
> * the `ServiceLoaderUtil` I backported from `master` to 2.18.0
> catches `ServiceLoaderException`s on a per-property source basis, but
> it does not catch `AccessControllerException`s which can cause global
> failures, as the one described by Boris.
>
> I am wondering what is the best course of action in this case:
>
> 1. We can keep silently ignoring all exceptions thrown by the
> `ServiceLoader` and the iterators it produces,
> 2. We can introduce some `AccessController#doPrivileged` calls in the
> right places (cf. the PRs [4] and [5] by Boris).
>
> I would prefer the second solution, but it requires adding security
> checks to all the available property sources. The environment and
> system properties sources are protected by internal Java security
> checks, but the `log4j2.component.properties` and Spring property
> sources are available for everyone to read. Some malicious code could
> for example call
> `PropertiesUtil.getProperty("log4j2.keyStorePassword")`.
>
> What do you think?
>
> Piotr
>
> [1] https://issues.apache.org/jira/browse/LOG4J2-3579
> [2]
> https://github.com/apache/logging-log4j2/blob/c33646f61850619c756797122f4fc4c53f7013f1/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L437
> [3] https://issues.apache.org/jira/browse/LOG4J2-2266
> [4] https://github.com/apache/logging-log4j2/pull/1006
> [5] https://github.com/apache/logging-log4j2/pull/1007
>
Re: Running under a SecurityManager
Hello all, thanks Piotr to take care for the topic. One thing to consider: > The environment and system properties sources are protected by internal Java > security checks,... Unfortunately not after applying the fix: PropertiesUtil[1] loads all the services which provide a PropertySource inside the doPrivileged including the default log4j2 implementations[2] which include the system properties [3]. Both fix approaches are not good at the moment. In practice nearly all frameworks require Property* permissions, due to caching / loading all etc. But that is not a good reason to introduce a leak. Maybe a alternative with more refactoring: Only the really needed properties are loaded, without a util method, without a service in between. The SecurityExceptions are thrown and not silently ignored. Any service implementation has to care itself. I don't know enough about service loading: Would any service lookup inside a doPrivileged block cause a constructor to be called inside the same block? :-( One thing in general: Could someone explain the usecase behind catching SecurityExceptions and silently dropping them? [4][5][6][7][8] Please explain it for the case that an authorized administrator with the knowledge and right to grant permissions wants to set the permissions correct. Please explain it for an monitoring system (ELK or something) which is configured to alert for SecurityExceptions. The hope of a near end of the SecurityManager must be delayed till December 2030[9] as long as you want to support Java 8 with log4j2 ;-) Due to very positive experience in application testing and production I'm not happy about the deprecation, but that is offtopic. Regards Boris [1] https://github.com/apache/logging-log4j2/blob/b734a4f66af868f03dafafe5de92999058096eca/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L477 [2] https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/resources/META-INF/services/org.apache.logging.log4j.util.PropertySource [3] https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/SystemPropertiesPropertySource.java#L44 [4] https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/SystemPropertiesPropertySource.java#L45 [5] https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/SystemPropertiesPropertySource.java#L76 [6] https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/SystemPropertiesPropertySource.java#L85 [7] https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L405 [8] https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L456 [9] https://www.oracle.com/java/technologies/java-se-support-roadmap.html
Re: Running under a SecurityManager
Fixing semantics of running with a SecurityManager in the 2.x branch makes perfect sense. In 3.x, we might be able to relax that type of code as it’s mostly module related security boundaries we need to continue supporting. — Matt Sicker > On Aug 21, 2022, at 11:32, . . wrote: > > Hello all, > > thanks Piotr to take care for the topic. One thing to consider: > >> The environment and system properties sources are protected by internal Java >> security checks,... > > Unfortunately not after applying the fix: PropertiesUtil[1] loads all the > services which provide a PropertySource inside the doPrivileged including the > default log4j2 implementations[2] which include the system properties [3]. > Both fix approaches are not good at the moment. In practice nearly all > frameworks require Property* permissions, due to caching / loading all etc. > But that is not a good reason to introduce a leak. Maybe a alternative with > more refactoring: Only the really needed properties are loaded, without a > util method, without a service in between. The SecurityExceptions are thrown > and not silently ignored. Any service implementation has to care itself. > > I don't know enough about service loading: Would any service lookup inside a > doPrivileged block cause a constructor to be called inside the same block? :-( > > One thing in general: Could someone explain the usecase behind catching > SecurityExceptions and silently dropping them? [4][5][6][7][8] Please explain > it for the case that an authorized administrator with the knowledge and right > to grant permissions wants to set the permissions correct. Please explain it > for an monitoring system (ELK or something) which is configured to alert for > SecurityExceptions. > > The hope of a near end of the SecurityManager must be delayed till December > 2030[9] as long as you want to support Java 8 with log4j2 ;-) Due to very > positive experience in application testing and production I'm not happy about > the deprecation, but that is offtopic. > > Regards > Boris > > [1] > https://github.com/apache/logging-log4j2/blob/b734a4f66af868f03dafafe5de92999058096eca/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L477 > [2] > https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/resources/META-INF/services/org.apache.logging.log4j.util.PropertySource > [3] > https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/SystemPropertiesPropertySource.java#L44 > [4] > https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/SystemPropertiesPropertySource.java#L45 > [5] > https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/SystemPropertiesPropertySource.java#L76 > [6] > https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/SystemPropertiesPropertySource.java#L85 > [7] > https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L405 > [8] > https://github.com/apache/logging-log4j2/blob/release-2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java#L456 > [9] https://www.oracle.com/java/technologies/java-se-support-roadmap.html
