Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

java.vendor name changed in openj9 #2139

Closed
krismarc opened this issue Jun 5, 2023 · 16 comments · Fixed by #2150 or #2209
Closed

java.vendor name changed in openj9 #2139

krismarc opened this issue Jun 5, 2023 · 16 comments · Fixed by #2150 or #2209

Comments

@krismarc
Copy link

krismarc commented Jun 5, 2023

Shouldn't this be considered?
ibmruntimes/Semeru-Runtimes#30

$ ./java -XshowSettings:properties -version 2>&1 | grep java.vendor
ava.vendor = International Business Machines Corporation
java.vendor.url = https://www.ibm.com/semeru-runtimes
java.vendor.url.bug = https://github.com/ibmruntimes/Semeru-Runtimes/issues
java.vendor.version = 11.0.14.1

$ ./java -XshowSettings:properties -version 2>&1 | grep java.vendor
java.vendor = IBM Corporation
java.vendor.url = https://www.ibm.com/semeru-runtimes
java.vendor.url.bug = https://github.com/ibmruntimes/Semeru-Runtimes/issues
java.vendor.version = 17.0.6.0

return SYSTEM_JRE.startsWith("IBM");

final String ibmLoginModule = "com.ibm.security.auth.module.Krb5LoginModule";

if my understanding is correct, starting from version 17 your condition in isIBM method is met so driver stopped using com.sun.* libs and started looking for com.ibm.*

which leads to
javax.security.auth.login.LoginException (No LoginModule found for com.ibm.security.auth.module.Krb5LoginModule)

@krismarc
Copy link
Author

krismarc commented Jun 5, 2023

@pshipton
Copy link

pshipton commented Jun 5, 2023

The ibm login module should only be used on IBM Java 8, which contains the following settings:
java.vendor = IBM Corporation
java.vm.name = IBM J9 VM

This is different from Semeru builds, which sets the following:
java.vendor = IBM Corporation
java.vm.name = Eclipse OpenJ9 VM

and in some older Semeru builds of jdk8, jdk11
java.vendor = International Business Machines Corporation

i.e. a test for the ibm login module could be if the system property java.vm.name starts with "IBM".

@Jeffery-Wasty
Copy link
Contributor

Hi @krismarc,

Thank you for bringing this to our attention. We'll look more into this, but you appear to be correct that there have been discrepancies with IBM within the driver.

@mstoodle
Copy link

mstoodle commented Jun 6, 2023

Just wanted to suggest that it would be more robust to probe for the thing you're actually concerned about (e.g. some specific com.ibm.security class which indicates the presence of the IBM Java 8 security implementation) rather than checking the vendor/vm property values and assuming that means something about the security implementation. Otherwise you make it hard for a vendor to change anything about their distribution...

@krismarc
Copy link
Author

krismarc commented Jun 6, 2023

That's what I meant in the other issue. Having a substring of property checked doesn't look good to me and sounds like kind of antipattern.

@Jeffery-Wasty
Copy link
Contributor

Jeffery-Wasty commented Jun 21, 2023

The fix is still in development and won't be looked at again until after the 12.4 GA release.

@kadirchisty
Copy link

Hi @Jeffery-Wasty - Please let's know when new version-12.4.0 of mssql-jdbc would be available for use?

@Jeffery-Wasty
Copy link
Contributor

Hi @kadirchisty,

The PR still needs changes, and development was halted until 12.4 release was finished. 12.4 will be out today, but the next release to include this fix will be 12.5 (release window to be discussed with team). The next stable release to include this should be early 2024.

@kadirchisty
Copy link

kadirchisty commented Aug 1, 2023 via email

@Jeffery-Wasty
Copy link
Contributor

We'll look into what we can do about the release schedule. If there is a hotfix for this release, it would include this fix.

@kadirchisty
Copy link

Hello @Jeffery-Wasty, Please let's know if the fix would be included in current release?

Thanks!

@Jeffery-Wasty
Copy link
Contributor

Jeffery-Wasty commented Aug 11, 2023

We're reviewing the PR now. The plan is for this to be included in a stable hotfix release, planned for August 24.

@Jeffery-Wasty
Copy link
Contributor

Jeffery-Wasty commented Aug 17, 2023

This is now included in the 12.4.1 stable hotfix release, set to release on August 24.

EDIT: The release is delayed to August 30.

@kadirchisty
Copy link

@Jeffery-Wasty - Wanted to check if 12.4.1 is released?

@Jeffery-Wasty
Copy link
Contributor

Our apologies, the release had to be delayed a week. We're now planning on releasing on August 30.

@lilgreenbird
Copy link
Contributor

re-opening since fix is being reverted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed Issues
6 participants