You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/08/21 20:10:05 UTC

[GitHub] [logging-log4j2] ppkarwasz opened a new pull request, #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

ppkarwasz opened a new pull request, #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008

   When running with a security manager, `ServiceLoaderUtil` will call `ServiceLoader` with the privileges of the caller.
   
   This probably requires further testing to be sure it works with both JPMS and a SecurityManager.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] rgoers commented on pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
rgoers commented on PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008#issuecomment-1223164496

   The only thing I don't like about this is its heavy reliance on SecurityManager stuff. This is fine in release-2.x but in master I'd want to skip all this if there is no SecurityManager enabled.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] jamezp commented on pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
jamezp commented on PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008#issuecomment-1224721906

   > @jamezp, can you provide more details? My understanding is that if `foo.jar` calls `ServiceLoaderUtil.loadServices` the call will be made as if `foo.jar` called `AccessController.doPrivileged(() -> ServiceLoader.load(...)` directly. My tests seem to confirm it: the `AccessControlContext` cached by the `ServiceLoader` contains just the domain of the caller, not `log4j-api.jar`.
   
   @ppkarwasz Yes, that is correct that the `AccessControlContext` will be that of `foo.jar`. I could just be overly paranoid really :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] jvz commented on pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
jvz commented on PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008#issuecomment-1235981554

   > Nevertheless, should I cherry-pick this to `master` or we do not support the `SecurityManager` in 3.x?
   
   Considering Java 11 still has that API, go for it. We can consider removal of SecurityManager stuff when and if it becomes a compatibility issue going forward.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz merged pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
ppkarwasz merged PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] boris-unckel commented on pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
boris-unckel commented on PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008#issuecomment-1223668787

   @jamezp My reproducer shows this works. Could you have a look at it, please?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz commented on pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008#issuecomment-1222888125

   @boris-unckel, does this change solve your problem?
   
   The `AccessControl.doPrivileged` is called from the caller's domain using a `PrivilegedAction` also in the caller's domain.
   
   As a result all calls to `ServiceLoaderUtil.loadServices` from inside `log4j-api` and `log4j-core` succeed, but a call from inside the Wildfly test code does not (as expected).
   
   This prevents from using this tool from an unprivileged domain.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] jamezp commented on pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
jamezp commented on PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008#issuecomment-1224332275

   My only concern with this would be is it exposes the privileged action. Essentially anyone that uses this can get around the security manager using the protection domain from log4j-api.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz commented on pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008#issuecomment-1224367694

   @jamezp, can you provide more details? My understanding is that if `foo.jar` calls `ServiceLoaderUtil.loadServices` the call will be made as if `foo.jar` called `AccessController.doPrivileged(() -> ServiceLoader.load(...)` directly. My tests seem to confirm it: the `AccessControlContext` cached by the `ServiceLoader` contains just the domain of the caller, not `log4j-api.jar`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz commented on pull request #1008: LOG4J2-3579 Calls ServiceLoader with caller's security context

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on PR #1008:
URL: https://github.com/apache/logging-log4j2/pull/1008#issuecomment-1235943647

   @rgoers, I don't see the heavy reliance on `SecurityManager`: I do use a `PrivilegedAction`, but just as synonym for `Supplier`. The part the really relies on the `SecurityManager` is 3 lines long.
   
   Nevertheless, should I cherry-pick this to `master` or we do not support the `SecurityManager` in 3.x?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org