-
Notifications
You must be signed in to change notification settings - Fork 428
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
Allow authenticating using Kerberos and a Principal/Password. #163
Allow authenticating using Kerberos and a Principal/Password. #163
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #163 +/- ##
============================================
+ Coverage 33.35% 33.43% +0.08%
- Complexity 1486 1487 +1
============================================
Files 97 98 +1
Lines 23569 23422 -147
Branches 3909 3845 -64
============================================
- Hits 7862 7832 -30
+ Misses 14141 14030 -111
+ Partials 1566 1560 -6
Continue to review full report at Codecov.
|
A few notes: Using a fully qualified username (for instance user@AD.EXAMPLE.COM) works as well. In the case where a keytab is used, the callback won't even be called. The global Kerberos mechanism may be enchanced as the Driver always retry even when the password is not valid (thus the return added). For Another Review: A possible improvement would be to stop trying when login() does fails (we don't need to wait for timeout in that case) with a proper error message. This commit should solve issue #66 |
b38f1c9
to
26dd540
Compare
The second commit also fixes issues with authentication taking a very long time even when Kerberos authentication is completely broken (eg: wrong keytab or wrong password) |
@pierresouchay Can you open this PR in dev branch? |
@v-suhame done |
This patch allow to authenticate using kerberos using the previous methods (eg: keytab) or specifying user/password either in properties or in connect method. This allows to use GUIs for instance to connect using Kerberos without having a sql user.
…a long time. This commit solves this by stopping directly authentication when login() fails. It means that if keytab is wrong OR provided principal/password are wrong, the driver immediatly return and does not retry in a endless loop.
26dd540
to
c4082f8
Compare
Updated the review to fix conflict due to #178 |
String message = String.format("%s due to %s (%s)", alwaysTriggered.getMessage(), le.getClass().getName(), le.getMessage()); | ||
if (callback.getUsernameRequested() != null) { | ||
message = String.format("Login failed for Kerberos principal '%s'. %s", callback.getUsernameRequested(), message); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be consistent with the existing code, can you add this new exception error string in SQLServerResource.java?
} | ||
|
||
private static String getAnyOf(Callback callback, Properties properties, String... names) | ||
throws UnsupportedCallbackException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the formatter on new code.
Added translated message for Kerberos login authentication errors.
@v-suhame DONE in new patch |
@pierresouchay Can you also push changes for IBM jre? |
@v-suhame As far as I know, there is no option on IBM JVM to disable callback according to https://www.ibm.com/support/knowledgecenter/en/SSYKE2_7.1.0/com.ibm.java.security.api.doc/jgss/com/ibm/security/auth/module/Krb5LoginModule.html So I suspect it works on IBM JVM without changes |
Tested PR with IBM JVM, Kerberos auth with username and password doesn't work. It looks like driver should switch between |
It means probably refactoring the creation of configurations according to connection parameters (and not in a static way) For another PR? |
Sounds good! I'll accept/merge this PR and leave the issue #66 open until it's fixed for IBM JVM. |
That's great ! IBM owners can still create their $HOME/.java.login.config file and specify the parameters of the config to make it work. I did actually plan to use several names for instead of hardcoded default "SQLJDBCDriver" (and so remove the static initialization) -> that would be the right time to allow switching to a default configuration that enables login with username/password for IBM JVM |
Hello, Is there a road map to develop driver for also IBM JVM to be able to use connection username and password properties for the kerberos authentication? We can provide kerberos authentication by using keytab and login config files; it' s OK. But it would be much better to authenticate by using connection username and password properties instead of keytab and login config files as it' s used for Oracle JVM. |
I did not test it, but it should work, did you try it? |
@bayramgns you might try by creating a .java.login.config in your $HOME directory and putting useDefaultCcache=false it seems. Which version of IBM's JVM are you using? |
@pierresouchay it works well for Oracle JVM. When you use Oracle JVM, you don' t need to use any keytab and login conf file; you only need to set up kerberos conf file and set driver connection properties. But for IBM JVM, it does not seem like that. I use IBMSDK 7.1 and mssql-jdbc-6.2.1.jre7.jar versions. As this change, I thought I won' t need to set any login conf property file; because you don' t need it for Oracle JVM.However, although I set login conf file as it seems below, it still does not work; SQLJDBCDriver { But when I set the login conf file like the below, it works well; SQLJDBCDriver { |
@pierresouchay could you check the case? |
@bayramgns What do you mean exactly? What are you testing?
Assuming you are in the third case, and according to this page:
I would try the following:
I don't have a Windows machine nor IBM JVM to test now, can you test this configuration and tell me whether it works? Current default configurations are the following: |
@pierresouchay I have good news :) It worked by using IBM Java Version 8. I used to work with IBM Java 7 version before and it was getting KerberosException. My purpose is connect to an MS SQL DB by authenticating with Kerberos protocol.I don' t want to use keytab file for username and password. I just want to set username and password connection properties instead of using keytab file. When using IBM Java 7, you have to create a keytab file and also provide "useKeytab" and "principal" properties in "SQLJDBCDriver .conf" file; otherwise it does not work. But when using IBM Java 8, it works by setting connection username/password properties(no need to create keytab file) and using a "SQLJDBCDriver .conf" file as you mentioned above (no need to provide useKeytab and principal properties). Anyway, I have no problem any more :) Thanks for help :) |
@bayramgns Ok, that's good news, can you try without the config file as well (I mean simply delete the file)? If it does not work, we might try setting those properties in the conf, so people you not even have to create the jaas config file. |
@pierresouchay , it does not work without the jaas config file. I' ve removed the auth login config jvm parameter and tried; but got "com.ibm.security.krb5.KrbException, status code: 31" exception. I guess the default value of "useDefaultCcache (true)" in JaasConfiguration class causes the error. Sounds great if you develop for setting these properties in the conf and no need to have jaas config file any more :) |
@pierresouchay @bayramgns 'But when using IBM Java 8, it works by setting connection username/password properties" - I tried the same. However, the driver picks up the logged in user instead of the userName and password provided in the connection string. Is your "run as" user and the username on the connection string one and the same? |
This closes #66
This patch allow to authenticate using kerberos using the previous
methods (eg: keytab) or specifying user/password either in properties
or in connect method.
This allows to use GUIs for instance to connect using Kerberos without
having a sql user.
If fixes issue #66