[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 Mark Thomas changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #13 from Mark Thomas --- Fixed in: - 10.1.x for 10.1.0-M14 onwards - 10.0.x for 10.0.21 onwards - 9.0.x for 9.0.63 onwards - 8.5.x for 8.5.79 onwards -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
Ping. On the topic of hardening, how far back do we want to do with this? Mark On 30/03/2022 12:41, bugzi...@apache.org wrote: https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #11 from Mark Thomas --- I've implemented this alternative approach for 10.1.x. It isn't as generic as forceString but it is sufficient to meet the original requirement. Two questions: 1. Should we back-port this? If so, how far? 2. Do we want to expand conversion so if the setter is for Type T that we can't convert and T has a constructor T(String) we use that constructor to create an instance of T and then pass that to the setter? - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #12 from quaff --- > 1. Should we back-port this? If so, how far? Yes, back to 8.x. > 2. Do we want to expand conversion so if the setter is for Type T that we > can't convert and T has a constructor T(String) we use that constructor to > create an instance of T and then pass that to the setter? I think we should keep it as simple as possible, BeanFactory is not widely used AFAIK. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #11 from Mark Thomas --- I've implemented this alternative approach for 10.1.x. It isn't as generic as forceString but it is sufficient to meet the original requirement. Two questions: 1. Should we back-port this? If so, how far? 2. Do we want to expand conversion so if the setter is for Type T that we can't convert and T has a constructor T(String) we use that constructor to create an instance of T and then pass that to the setter? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #10 from Mark Thomas --- (In reply to Remy Maucherat from comment #8) > No idea. But the BeanFactory doesn't use our IntrospectionUtils, as you just > said, and we're totally used to its very user friendly behavior. Doh! Of course. As a Bean factory it is following the Bean spec. I think we might be able to do something along the lines of if the setter method doesn't use String and we can't coerce it, is there a method identical to the setter part from it uses String? If so, use that. Is that worth implementing? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #9 from Christopher Schultz --- (In reply to Mark Thomas from comment #7) > 1. Has anyone got a suggestion to make enabling forceString support > configurable that doesn't involve a system property? JNDI environment variable? (lol just kidding). I think this is either a system property (preferable to me, even though system properties kinda suck) or an otherwise unnecessary global Listener. > 2. Is removing this feature entirely in 10.1.x reasonable? +1 to reasoning provided by remm and rjung (via email) -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #8 from Remy Maucherat --- (In reply to Mark Thomas from comment #7) > Looking at this in a bit more detail I have a couple of > observations/questions: > > 1. Has anyone got a suggestion to make enabling forceString support > configurable that doesn't involve a system property? Nope. > 2. Is removing this feature entirely in 10.1.x reasonable? I think it's fine. Overall JNDI should use real object factories. > 3. Why doesn't Introspector find the "setSSLIdentity(String)" method. It > looks like it should. Is improving the method matching in Introspector an > approach that could work long term? No idea. But the BeanFactory doesn't use our IntrospectionUtils, as you just said, and we're totally used to its very user friendly behavior. > The answer to the first part of 3 may need some research. That is on my TODO > list but I'm unlikely to get to it before January. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #7 from Mark Thomas --- Looking at this in a bit more detail I have a couple of observations/questions: 1. Has anyone got a suggestion to make enabling forceString support configurable that doesn't involve a system property? 2. Is removing this feature entirely in 10.1.x reasonable? 3. Why doesn't Introspector find the "setSSLIdentity(String)" method. It looks like it should. Is improving the method matching in Introspector an approach that could work long term? The answer to the first part of 3 may need some research. That is on my TODO list but I'm unlikely to get to it before January. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #6 from Rainer Jung --- The history of forceString (thanks Remy) can be seen in the log message of svn r1655312 or github d1cf73ab16da6fccde3c323e16b582be8d579008. I paste it here. I am totally open to drop it, if it now turns out to pose security risks. Enhance our naming BeanFactory. If a bean property exists which the Introspector presents us with a type that we don't have a string conversion for, but the bean actually has a method to set the property from a string, allow to provide this information to the BeanFactory. New attribute "forceString" taking a comma separated list of items as values. Each item is either a bean property name (e.g. "foo") meaning that there is a setter function "setFoo(String)" for that property. Or the item is of the form "foo=method" meaning that property "foo" can be set by calling "method(String)". This should make writing a custom bean factory obsolete in quite a few cases. Concrete use case was tibco TibjmsConnectionFactory which has an attribute SSLIdentity detected by Introspector as byte[] but which can be set by setSSLIdentity(String). Existing BeanFactory throws NamingException. Regards, Rainer -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #5 from quaff --- I agree that "forceString" should be disabled by default and removed in future version, It will increase safety, "you can configure the JNDI environment of Tomcat" is more harder since it need another gadget, let's remove "org.apache.naming.factory.BeanFactory" from existing gadgets. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #4 from Remy Maucherat --- The feature was added by Rainer in Jan 2015. The idea of the bean factory is to avoid having to use custom object factories (personally: I think using custom object factories is usually better), and this forceString increased flexibility further. Normally, once you get to the point where you can configure the JNDI environment of Tomcat, you can also configure everything else, so limiting forceString shouldn't add any extra safety. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #3 from Christopher Schultz --- Honestly, any "feature" that significantly reduces security should be difficult to enable. My initial reaction after reading that piece was "why is forceString enabled by default?" I don't know the history of that feature, so I'm not sure how popular it is or what the use-cases are. My guess is that, mostly, there are simple uses of JNDI in Tomcat. For more "exotic" use-cases, it shouldn't be too much trouble for an admin to enable this feature explicitly. It's also not clear to me how much *more* secure things are /without/ "forceString" available. JNDI lookups are, by definition, fairly sensitive things: if you allow users to control the lookups, they can kind of ... well, look-up ANYTHING. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 Mark Thomas changed: What|Removed |Added Severity|normal |enhancement --- Comment #2 from Mark Thomas --- To be crystal clear: There is no Apache Tomcat vulnerability here. To quote from the linked article: The actual problem here is not within the JDK or Apache Tomcat library, but rather in custom applications that pass user-controllable data to the "InitialContext.lookup()" function, as it still represents a security risk even in fully patched JDK installations. Moving this to an enhancement request. It is highly unlikely Tomcat will remove/disable existing functionality. Suggestions for mitigation / hardening that can improve security without impacting legitimate uses will be welcomed. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65736] Improve org.apache.naming.factory.BeanFactory to mitigate JNDI injection
https://bz.apache.org/bugzilla/show_bug.cgi?id=65736 --- Comment #1 from quaff --- Can we drop "forceString" supports? https://github.com/apache/tomcat/blob/f5a732e74e2a36442b2bf562c665917c4bb1167a/java/org/apache/naming/factory/BeanFactory.java#L150 -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
