-
Notifications
You must be signed in to change notification settings - Fork 39
Set gss token #290
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
base: main
Are you sure you want to change the base?
Set gss token #290
Conversation
Signed-off-by: Irfan Sharif <[email protected]>
Signed-off-by: Irfan Sharif <[email protected]>
Signed-off-by: Irfan Sharif <[email protected]>
Signed-off-by: Irfan Sharif <[email protected]>
Signed-off-by: Irfan Sharif <[email protected]>
nadiramra
left a comment
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.
I am not sure I like what is being done here - the mixing of password and GSS token is not good.
Signed-off-by: Julia Yan <[email protected]>
ThePrez
left a comment
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.
Two main changes requested, see individual comments
| @@ -1016,9 +1023,13 @@ public void generateProfileToken(ProfileTokenCredential profileToken, String use | |||
| case AS400.AUTHENTICATION_SCHEME_GSS_TOKEN: | |||
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.
What we really should do is define a new constant and associated switch case here, AS400.AUTHENTICATION_SCHEME_KRB_TICKET
This is more pure as it keeps the direct-kerb-ticket path and the GSS path separate
| authenticationBytes = (gssCredential_ == null) | ||
| ? TokenManager.getGSSToken(systemName_, gssName) | ||
| : TokenManager2.getGSSToken(systemName_, gssCredential_); | ||
| if (!this.kerbTicket_.isEmpty()){ |
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.
this change (and other similar changes) would slide into the new switch statement(s) for AUTHENTICATION_SCHEME_KRB_TICKET mentioned in other review comment
| private transient CredentialVault kerbTicket_; | ||
|
|
||
| // Prefix used to indicate that the password contains a base64-encoded Kerberos token. | ||
| public static final String KERBEROS_PREFIX = "_KERBEROSAUTH_"; |
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.
The whole logic around KERBEROS_PREFIX is actually a pretty cool feature, but it still seems non-optimal to conflate kerberos tickets with the password.
As implemented, it allows non-programmatic connections like JDBC to use a kerberos ticket, very useful.
More importantly, the Mapepire client uses this new _KERBEROSAUTH_ scheme. See https://github.com/Mapepire-IBMi/mapepire-python/pull/84/files
So if we redesign this, we need to also redesign the Mapepire client, or at least the Mapepire server (middleware). In the Mapepire case, the ultimate constraining factor is that we're passing this through an HTTP basic auth mechanism which only supports userid and password. Hence the reason the prefix was invented. It's hacky and potentially problematic in the unlikely case someone has a password starting with this prefix, but it's honestly the best we can do.
My intuition says we should do the following:
- Remove the
KERBEROS_PREFIXlogic from the toolbox entirely - Move associated code (namely
isKerbTicket()) into the Mapepire server code itself. Changes may be able to be fully contained within this method: https://github.com/Mapepire-IBMi/mapepire-server/blob/9c9a3e2c866d80dd8551dbba6e11842c4a240494/src/main/java/com/github/ibm/mapepire/SystemConnection.java#L114
@julesyan + @nadiramra , thoughts?
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.
I started looking at pulling the necessary changes into the mapepire-server project, but it's more complex than initially thought. Mapepire-server gets its connection through DriverManager.getConnection() which has no support for sending a kerberos token.
I see why it was implemented in this way, to support Kerberos through the standard DriverManager classes. Which I still assert is a useful feature.
So I think this boils down to three high-level options:
Option 1: existing implementation
Pros:
- We know it works, and it covers all our bases
Cons:
- conflates kerberos ticket and password
Option 2: change mapepire-server to not use DriverManager interfaces
Pros:
- Pretty easy to implement
- keeps password and kerberos ticket separate
Cons:
- Creates a dual maintenance path for future versions of the server if we exoect to leverage other JDBC drivers
- Makes use of native JDBC driver more complicated
- If we've made it this far, we might as well publish a new feature of JDBC support for kerberos!
Option 3: create a new JDBC property for kerberos ticket (I believe this is best)
Pros:
- keeps password and kerberos ticket separate
- unlikely to have backward/forward compatibility issues
Cons:
- Adding a new connection property is more involved
@julesyan / @nadiramra / @jeber-ibm , thoughts?
| private transient CredentialVault kerbTicket_; | ||
|
|
||
| // Prefix used to indicate that the password contains a base64-encoded Kerberos token. | ||
| public static final String KERBEROS_PREFIX = "_KERBEROSAUTH_"; |
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.
I started looking at pulling the necessary changes into the mapepire-server project, but it's more complex than initially thought. Mapepire-server gets its connection through DriverManager.getConnection() which has no support for sending a kerberos token.
I see why it was implemented in this way, to support Kerberos through the standard DriverManager classes. Which I still assert is a useful feature.
So I think this boils down to three high-level options:
Option 1: existing implementation
Pros:
- We know it works, and it covers all our bases
Cons:
- conflates kerberos ticket and password
Option 2: change mapepire-server to not use DriverManager interfaces
Pros:
- Pretty easy to implement
- keeps password and kerberos ticket separate
Cons:
- Creates a dual maintenance path for future versions of the server if we exoect to leverage other JDBC drivers
- Makes use of native JDBC driver more complicated
- If we've made it this far, we might as well publish a new feature of JDBC support for kerberos!
Option 3: create a new JDBC property for kerberos ticket (I believe this is best)
Pros:
- keeps password and kerberos ticket separate
- unlikely to have backward/forward compatibility issues
Cons:
- Adding a new connection property is more involved
@julesyan / @nadiramra / @jeber-ibm , thoughts?
|
My vote is for option 3.
-----------------------------------------------
Nadir Amra
e-mail: ***@***.***
From: Jesse Gorzinski ***@***.***>
Date: Monday, December 29, 2025 at 6:21 AM
To: IBM/JTOpen ***@***.***>
Cc: Nadir K Amra ***@***.***>, Mention ***@***.***>
Subject: [EXTERNAL] Re: [IBM/JTOpen] Set gss token (PR #290)
This Message Is From an External Sender
This message came from outside your organization.
Report Suspicious<https://us-phishalarm-ewt.proofpoint.com/EWT/v1/AdhS1Rd-!97FUXBmRvUP0Hf_STQyIOge4uEgAWR3dNLGMZm44VjD-8QAX5VIneUrgnce8HgBZJYPargFEs_rJs7aydFVk-Tfn4H8yYS8$>
@ThePrez requested changes on this pull request.
________________________________
In src/main/java/com/ibm/as400/access/AS400.java<#290 (comment) >:
@@ -388,6 +389,48 @@ public class AS400 implements Serializable, AutoCloseable
private boolean forcePrompt_ = false;
private int validateSignonTimeOut_ = 0;
+ private transient CredentialVault kerbTicket_;
+
+ // Prefix used to indicate that the password contains a base64-encoded Kerberos token.
+ public static final String KERBEROS_PREFIX = "_KERBEROSAUTH_";
I started looking at pulling the necessary changes into the mapepire-server project, but it's more complex than initially thought. Mapepire-server gets its connection through DriverManager.getConnection() which has no support for sending a kerberos token.
I see why it was implemented in this way, to support Kerberos through the standard DriverManager classes. Which I still assert is a useful feature.
So I think this boils down to three high-level options:
Option 1: existing implementation
Pros:
* We know it works, and it covers all our bases
Cons:
* conflates kerberos ticket and password
Option 2: change mapepire-server to not use DriverManager interfaces
Pros:
* Pretty easy to implement
* keeps password and kerberos ticket separate
Cons:
* Creates a dual maintenance path for future versions of the server if we exoect to leverage other JDBC drivers
* Makes use of native JDBC driver more complicated
* If we've made it this far, we might as well publish a new feature of JDBC support for kerberos!
Option 3: create a new JDBC property for kerberos ticket (I believe this is best)
Pros:
* keeps password and kerberos ticket separate
* unlikely to have backward/forward compatibility issues
Cons:
* Adding a new connection property is more involved
@julesyan<https://github.com/julesyan > / @nadiramra<https://github.com/nadiramra > / @jeber-ibm<https://github.com/jeber-ibm > , thoughts?
—
Reply to this email directly, view it on GitHub<#290 (review) >, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AJBIC5W3KB57C5KDB7IOEBT4EEMGBAVCNFSM6AAAAACNXKUIQCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMMJVG42TKMBQG4 >.
You are receiving this because you were mentioned.
|
|
After some thought, I prefer using the password char[] parameter to pass the kerberos ticket. The main problem that I am worried about is passing kerberos tickets as strings. In Java, the storage associated with a string cannot be forcefully cleared. If a kerberos ticket is stored as a string, then it may reside in memory for a long time and can be found by an attacker scanning the memory. Also, if the driver is storing a kerberos ticket, it should be encrypted or obfuscated so it cannot be easily found. I thought about passing an encrypted ticket as a property and the encryption key as a property. However, if the two properties are passed using the same string, we will still have an attack vector that can easily be exploited. If we pass the kerberbos ticket using the existing char[] password support, then the ticket can be cleared from memory and the ticket can be obfuscated like the password currently is. |
|
Again, not a fan of dual use parameters for authentication. Always can create a new method that extends password for gss tokens or add a new method that and infrastructure to mirror password support for kerberos tickets. |
Updated PR of #279