Disabling JNDI by default

[Stuart Douglas]

Hi Everyone,

I have opened a PR that will effectively disable JNDI by default:

https://github.com/quarkusio/quarkus/pull/22169

Basically as one of the first parts of startup a non-functional InitialContextFactoryBuilder is installed in the NamingManager, so any attempts to create an InitialContext will result in this disabled context being used.

I am pretty sure that the vast majority of Quarkus applications have no use for JNDI, so it seems like disabling it by default could be a nice safety measure, however I am curious to hear what others think.

Stuart

[George Gastaldi]

+1, I think that makes sense but I am not sure how other extensions integrating external servers (artemis/infinispan/narayana) would behave with this change. 

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAD%2BL2cwdvUreP_he53Bo_eTRQm%3D1b0DnQM%2BLkJMvPLV2RoLAiw%40mail.gmail.com.

[Stuart Douglas]

It should be ok, as I have changed it to now just throw NamingException rather than UnsupportedOperationException. If there are any issues the test suite should pick them up.

Stuart

[Sanne Grinovero]

Awesome 👌

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAD%2BL2cxmoE6Wp4Z_gPc0i2DjuCWj2hKLdr1Mpp83xB6TaKFMVA%40mail.gmail.com.

[clement escoffier]

Hello,

That’s great, and yes, disabling it by default makes sense.

On top of my mind, it might break Mongo. To resolve mongo+srv URLs, the mongo client uses JNDI. This feature is not supported in native (explicitly) but worked in JVM. So, we may have to update the mongo documentation to re-enable JNDI when you need this feature.

Clement

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAFm4XO0Fmo4yQBR-ym67KghbeJ-PFxvtX5oGTpq7Y91-3j3jWQ%40mail.gmail.com.

[Stuart Douglas]

I already found it will break elytron-ldap (which is no real surprise). The error message is very informatie and tells you exactly what you need to do to fix it, so I don't know if we even really need to update the docs if this is not something we are telling people to use.

Stuart

[clement escoffier]

On 14 Dec 2021 at 10:18:45, Stuart Douglas <sdou...@redhat.com> wrote:

I already found it will break elytron-ldap (which is no real surprise). The error message is very informatie and tells you exactly what you need to do to fix it, so I don't know if we even really need to update the docs if this is not something we are telling people to use.

Agreed! 

Clement

[Loïc MATHIEU]

Well, instead of breaking Mongo and LDAP we can have a way to enable it from the extensions build.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAKW6fic-LOpP4VBaHj%3DtK-mUKRWV6Q98-O5%2BeMxz%3DGW9EUe-eQ%40mail.gmail.com.

[Stuart Douglas]

I have done that for LDAP. Is this a commonly used thing in mongo? If it is an edge case I think it is ok to require a property to enable it.

Stuart

[Loïc MATHIEU]

When using a replicaset, it's very common to configure it this way.
Using SAAS solution like Mongo Atlas, this is the default.

We can however detect it by parsing the MongoDB connection String.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAD%2BL2cxC%3DQeUbmfwbUzv_UCw1eFFOsqh3MgmeLofO_nnMa2Jcw%40mail.gmail.com.

[Stuart Douglas]

On Tue, 14 Dec 2021 at 21:27, Loïc MATHIEU <loik...@gmail.com> wrote:

When using a replicaset, it's very common to configure it this way.
Using SAAS solution like Mongo Atlas, this is the default.

We can however detect it by parsing the MongoDB connection String.

That is a runtime property though, and this is a build time thing. I guess we should just enable this if mongo is present as well.

I guess my concern is the more scenarios we enable it in, the less useful it is, but the more likely it is to give a false sense of security.

It may also be possible to have some kind of 'known good' support for both this and ldap, however it is not as simple as just delegating to a default InitialContextFactory because InitialContext acts differently based on if the factory is registered or not.

If we have multiple use cases for JNDI though the 'known good' option may be worth doing. I guess we also need to decide if we actually want to guard against this. Hopefully there is not another log4shell level issues anything soon, and JNDI is not the only issue the JDK has, but it is a potentially easy to exploit vector that is mostly unused by moderns apps, so it seems like a good tradeoff.

Stuart

[Sanne Grinovero]

I wonder if Quarkus has now enough leverage so that by making such things inconvenient, people like the Mongodb maintainers can be persuaded to find a better alternative. Especially since it's already broken in native.

I'd say it's worth trying shutting down JNDI by default and try to stand our ground on such positions.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAD%2BL2cxmXrF_2J31g%3D3pnyb_P1pUCSks4OrHxzZqN1HFFkuy7A%40mail.gmail.com.

[Stuart Douglas]

The test suite passed for Mongo even without this change. Do you think it is worth adding tests for using mongo this way? It seems like testing the native support for this would be useful even without the JNDI issues.

Stuart

On Tue, 14 Dec 2021 at 21:27, Loïc MATHIEU <loik...@gmail.com> wrote:

[Guillaume Smet]

On Tue, Dec 14, 2021 at 10:53 AM Stuart Douglas <sdou...@redhat.com> wrote:

I have done that for LDAP. Is this a commonly used thing in mongo? If it is an edge case I think it is ok to require a property to enable it

I'm not sure I like that.

You give a false sense of security by disabling JNDI by default if, someday, by adding an extension, it gets automatically enabled.

I'd rather go with an explicit configuration property that you HAVE TO enable to benefit from the feature. I know it looks counter intuitive for users but I seriously think this feature is dangerous if we enable JNDI ourselves for certain extensions.

--

Guillaume

[Stuart Douglas]

I am not sure if I like it either :-)

The way I have justified it to myself is as follows:

- This is a 'best effort' to mitigate a vulnerability that may never happen

- Users of the quarkus-elytron-security-ldap extension will always need to add the property anyway, as the extension relies on JNDI, so the end result is the same

That said maybe it is worth the effort to create a special 'safe JNDI' implementation, so the factory will allow the creation of very specific things that we have pre-vetted.

Stuart

 

--

Guillaume

[Loïc MATHIEU]

Hi Stuart,

For MongoDB we don't use the mongo+srv protocol on our test suite as it'll require a specific installation that we cannot easily simulate in our test environment.
I can try to test your branch with a MongoDB Atlas cluster to see if it breaks, I can find some time to do this on Friday.

[Guillaume Smet]

On Wed, Dec 15, 2021 at 12:25 AM Stuart Douglas <sdou...@redhat.com> wrote:

I am not sure if I like it either :-)

The way I have justified it to myself is as follows:

- This is a 'best effort' to mitigate a vulnerability that may never happen

- Users of the quarkus-elytron-security-ldap extension will always need to add the property anyway, as the extension relies on JNDI, so the end result is the same

The scenario I have in mind is the following:

1/ I create my Quarkus app

2/ I assume JNDI is disabled given that's what is documented

3/ There is a JNDI exploit but I don't feel that much concerned because of 2/

4/ 2 months later, I add LDAP authentication via elytron-ldap

5/ I'm screwed

If at 4/, I get an error saying JNDI is disabled by default and I need to enable it with proper warnings, 5/ might not happen.

--

Guillaume

[Sanne Grinovero]

On Wed, 15 Dec 2021 at 08:23, Guillaume Smet <guillau...@gmail.com> wrote:

On Wed, Dec 15, 2021 at 12:25 AM Stuart Douglas <sdou...@redhat.com> wrote:

I am not sure if I like it either :-)

The way I have justified it to myself is as follows:

- This is a 'best effort' to mitigate a vulnerability that may never happen

- Users of the quarkus-elytron-security-ldap extension will always need to add the property anyway, as the extension relies on JNDI, so the end result is the same

The scenario I have in mind is the following:

1/ I create my Quarkus app

2/ I assume JNDI is disabled given that's what is documented

3/ There is a JNDI exploit but I don't feel that much concerned because of 2/

4/ 2 months later, I add LDAP authentication via elytron-ldap

5/ I'm screwed

+1

Consistency is important, and neither us nor end users will have the resources to test for various random combinations of extensions.

Not meaning to change the subject, but that's the same reason for which I decided to hardcode relaxing some Java modularity rules in quarkus core when we fixed issues in native-image on JDK17. (We've had a debate about if this warranted a BuildItem and automatically enable things based on specific extension opt-in).

Regarding JNDI, as mentioned in a previous email I think it's perfectly understandable by our users - especially after the recent news - to be strict by default and require an explicit opt-in. And this could be a good reason for the MongoDB developers to address the problem otherwise.

And it's simpler :)

Thanks

[Emmanuel Bernard]

A thing we can do is make sure the mongo extension config code does raise an even more contextual exception. 

"mongo+srv is using this crazy scary attack vector, are you fine with it? enable it by setting this property."

--

You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAFm4XO1cV%2B0jZNccDr0KL3bMpMf60D8rAXzhGUYwA0pYN08p2Q%40mail.gmail.com.

[Loïc MATHIEU]

Yeah, this can be done in a runtime init step.
I'll need to check first what it currently did, then I'll propose something.

[Scott Stark]

In between 4/5 we could also have the Quarkus JNDI implementation not resolve ObjectFactory bindings by default and require yet another flag to be enabled to do so. Without ObjectFactorys JNDI is much simpler and safer.

--

You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CALt0%2Bo_QyDxdKM4Joc1_Q-A3U3caA4EZKy9gRRuF%3DyZbqGCMjQ%40mail.gmail.com.

[Stuart Douglas]

On Thu, 16 Dec 2021 at 03:00, Scott Stark <sst...@redhat.com> wrote:

In between 4/5 we could also have the Quarkus JNDI implementation not resolve ObjectFactory bindings by default and require yet another flag to be enabled to do so. Without ObjectFactorys JNDI is much simpler and safer.

While that would fix the remote execution part it would still allow for information disclosure (assuming a similar vulnerability).

Definitely better than nothing though.

Stuart