Adding a new connection property "useDefaultJaasConfig" to coexist with solutions that overwrite the system JAAS

[edanidzerda]

The existing feature of the JDBC driver to automatically authenticate using Kerberos without any external configuration is fantastic! The complexity of JAAS can be intimidating.

This feature I am proposing is the simplest way I saw to allow the JDBC driver to perform Kerberos authentication without any additional external configuration, when the JDBC driver coexists in the same application with libraries that configure JAAS at the system level.

This may not be a very common use case, but it is very useful in a corporate environment that provides, for instance, a Kafka Kerberos configuration, to let the JDBC Driver authenticate via Kerberos itself, without having to resort to configuring a common jaas.conf for all applications. In the case where the user has not specified a jaasConfigurationName, it seems reasonable to assume they are hoping to directly connect to a SQLServer using the authentication schema that they specified in their connection properties.

I did think that the existing jaasConfigurationName feature could be useful if it did allow a separate jaas.conf filename, instead of a configuration name. The documentation is a little confusing when it refers to this option as a filename and not a configuration name, so this patch also tweaks the Javadoc for those settings.

I recognize that adding a new connection property is not something the project will want to accept lightly, and I am open to alternative proposals that could solve the same problem! Thanks for taking the time to consider it.

[edanidzerda]

@microsoft-github-policy-service agree

[Jeffery-Wasty]

HI @edanidzerda,

Thank you for the PR, we'll take a look into it.

[edanidzerda]

Thanks for agreeing to look into it! I've been researching the issue further, and I feel very confident now that this would be an great addition for anyone combining SQL Server Kerberos authentication alongside Spring Kafka with Kerberos, as well as any other libraries that attempt to auto-configure the JAAS configuration.

For a little more background, the conflict we see in testing is that when Spring Kafka's beans are instantiated BEFORE a data source using mssql-jdbc, Spring Kafka creates a Kafka security configuration and overwrites the JVM's JAAS Configuration

			configurationEntries.put(KAFKA_CLIENT_CONTEXT_NAME,
					new AppConfigurationEntry[] { kafkaClientConfigurationEntry });
			Configuration.setConfiguration(new InternalConfiguration(configurationEntries));

When mssql-jdbc starts up after this, the clever JaasConfiguration uses Spring Kafka's configuration as the "delegate" and Kerberos authentication continues to work for both SQL Server and Kafka.

static {
        // Overrides the default JAAS configuration loader.
        // This one will forward to the default one in all cases but the default configuration is empty.
        Configuration.setConfiguration(new JaasConfiguration(Configuration.getConfiguration()));
    }

Unfortunately, if mssql-jdbc starts first, then Spring Kafka overwrites the Configuration object when it starts. While initial SQL connections succeed, connections shortly after startup fail because of No LoginModules configured for SQLJDBCDriver at javax.security.auth.login.LoginContext.init(LoginContext.java:264)

In the old days, we could configure a jaas.conf for both SQL and Kafka on our physical server or VM with Puppet and go grab a beer. In the current world of containers everywhere, having a configuration option to ignoreSystemJaas would be a great help for applications trying to use multiple libraries that may have conflicting approaches to setting the JAAS configuration.

Thanks again! 😃

[David-Engel]

Thanks for the clarification. This sounds like a useful addition. I was trying to understand the use case so we can appropriately document the setting. My initial thought is to reverse the setting logic (i.e. instead of ignoreSystemJaas, call it something like useDefaultJaasConfig) but with the added detail, I understand why you've suggested ignore. I'm not crazy about the term ignore but I'm not sure which option is better, ignoreSystemJaas or useDefaultJaasConfig (slightly favoring ignore at this point). Did you consider other options for the property name?

[edanidzerda]

Awesome!!!

I think naming things is one of the harder problems in computer science, and perhaps outside of it 😄 I tried not to overthink the name and just get it working!

I like useDefaultJaasConfig a little better, too. It more clearly defines what happens if you set it true I refactored the code and pushed it up.

I also added a warning message if jaasConfigurationName is non-default AND useDefaultJaasConfig is set. The jaasConfigurationName will be ignored, so I thought it would be helpful to the user to have a hint that it would not work.

I didn't see an obvious way to fail the connection, but I didn't spend a lot of time looking at the code for parsing the URL and I wasn't convinced that it was so serious as a configuration problem. I thought a warning level message that "hey! we're not using your config" was about right. However, I am open to any suggestion here.

                      authLogger.warning(toString() + String.format(
                              "Using default JAAS configuration, configured %s=%s will not be used.",
                              SQLServerDriverStringProperty.JAAS_CONFIG_NAME, configName));

I think it looks pretty good as useDefaultJaasConfig -- Let me know how else I can help get this added! Depending on when/if this gets approved, it would be great to back-port this to 10.2.x as Spring Boot 2.7 still specifies 10.2.3.jre8 as it's default dependency,

[tkyc]

@edanidzerda Everything looks good on our end. We'd like to include this in a preview release before a stable release so this change won't be slated in for our upcoming stable release, but rather the one after. Thanks for the contribution and your patience.

[edanidzerda]

That's great! I completely understand your need to move slowly with this project.

I see you added it to 12.5.0 milestone; are you open to backport to 10.2.x or 11 at some point?

Let me know if there's anything else I can do to help! Thanks again! 😄

[tkyc]

I'll check with the team again and let you know if it's possible to do a backport.

[edanidzerda]

Thanks for your consideration! I squashed my patch into a single commit to make backporting a more palatable task 😅

[tkyc]

@edanidzerda Sorry, after checking with the team, it looks like we've decided not to do a backport as this is a new feature eg. connection string property so it wouldn't make sense to do a backport.

[edanidzerda]

Thanks @tkyc for discussing and getting back to me! You all have been great to work with.

[lilgreenbird]

add tests that exercise the code added, currently this only has a test to add setting the property

[tkyc]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[dartoons]

Hello All, great job done! Just for my knowledge, when do you target the delivery of 12.5 version?
Thank you for your support:

[tkyc]

@dartoons Hello, 12.5 will likely release as a preview sometime in November and a stable release will release in the new year in January.