code review request: 7089889: Krb5LoginModule.login() throws an exception if used without a keytab

[Weijun Wang]

Any more comment?

Thanks
Max

On 09/16/2011 01:57 PM, Weijun Wang wrote:
>
>
> On 09/16/2011 01:45 PM, Xuelei Fan wrote:
>> My first impression about the fix:
>> 1. Do you want to update getKeys() comments about isInitiator value?
>
> OK. I'll simply remove the "(in JAAS words, isInitiator=true and
> storeKey=true)" line.
>
>>
>> 2. It seems that when isInitiator is true, getKeys() should throw
>> exception if the state is not OK, right? If so, it looks like that the
>> update of getKeys() will not throw ISE if the state is INIT. Does INIT
>> state is enough in such case?
>
> The KrbAsReqBuilder class does not know about isInitiator so it cannot
> make any decision based on it. I'll leave that to its caller. When
> username/password is provided, getKeys() can be called as soon as the
> object is created, and it returns keys with default salt. If AS-REQ is
> performed and state goes OK and the method is called again, the salt
> might be updated in PA-DATA.
>
> Thanks
> Max
>
>
>>
>> Xuelei
>>
>> On Sep 16, 2011, at 12:00 PM, Weijun Wang<weijun.wang at oracle.com> wrote:
>>
>>> Hi All
>>>
>>> Code changes at
>>>
>>> http://cr.openjdk.java.net/~weijun/7089889/webrev.00/
>>>
>>> KrbAsReqBuilder maintains a state enforcement: at the beginning it's
>>> INIT, after doing an AS-REQ it's OK, and we used to only allow
>>> getKeys() to be called when the state is OK. Now if an acceptor has
>>> isInitiator=false, the OK state will never be reached. In most cases,
>>> an acceptor uses a keytab so getKeys() is not called. However, if it
>>> uses username/password (most likely in a peer-to-peer case and the
>>> acceptor chooses not to login), getKeys() is needed and an exception
>>> is thrown.
>>>
>>> This code change makes getKeys() callable at both INIT and OK states.
>>> In order to do this, the checkState() method now accepts varargs.
>>>
>>> I'll backport this to 7u2, and since 7u2 is now in phase 2, I think I
>>> need at least 2 reviewers.
>>>
>>> Thanks
>>> Max
>>>
>>> -------- Original Message --------
>>>
>>> *Change Request ID*: 7089889
>>>
>>> *Synopsis*: Krb5LoginModule.login() throws an exception if used
>>> without a keytab
>>>
>>>
>>> === *Description*
>>> ============================================================
>>> FULL PRODUCT VERSION :
>>> java version "1.7.0"
>>> Java(TM) SE Runtime Environment (build 1.7.0-b147)
>>> Java HotSpot(TM) Client VM (build 21.0-b17, mixed mode, sharing)
>>>
>>> A DESCRIPTION OF THE PROBLEM :
>>> The call to LoginContext.login() fails with an exception if the
>>> LoginContext is configured to use Krb5LoginModule with the following
>>> options:
>>>
>>> storeKey="true"
>>> useKeyTab="false"
>>> isInitiator="false"
>>>
>>> This appears to be a regression due to an attempt to solve bug
>>> '6941083'.
>>>
>>> The following source code is taken from
>>> Kb5LoginModule.attemptAuthentication(false):
>>>
>>> if (ktab == null) {
>>> promptForPass(getPasswdFromSharedState);
>>> builder = new KrbAsReqBuilder(principal, password);
>>> if (isInitiator) {
>>> // XXX Even if isInitiator=false, it might be
>>> // better to do an AS-REQ so that keys can be
>>> // updated with PA info
>>> cred = builder.action().getCreds();
>>> }
>>> if (storeKey) {
>>> encKeys = builder.getKeys();
>>> // When encKeys is empty, the login actually fails.
>>> // For compatibility, exception is thrown in commit().
>>> }
>>> }
>>>
>>> This code path results builder.getGeys() being called while the
>>> builder's state is 'INIT'.
>>> The builder asserts via checkState() that the state is REQ_OK, hence
>>> an exception.
>>>
>>> This is a regression, as JRE6 and prior versions called
>>> EncryptionKey.acquireSecretKeys()
>>> to obtain the keys in this case.
>>>
>>> REGRESSION. Last worked in version 6u26
>>>
>>> STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
>>> See the executable test case below. Program should be run on a
>>> Windows machine that is joined to a domain. Replace 'REALM', 'KDC',
>>> 'DOMAIN_USER' and 'DOMAIN_USER_PWD' as appropriate before running
>>>
>>> EXPECTED VERSUS ACTUAL BEHAVIOR :
>>> EXPECTED -
>>> "Login succeeded" printed to console
>>> ACTUAL -
>>> "Login failed" printed to console.
>>> Exception stacktrace printed to strerr
>>>
>>> ERROR MESSAGES/STACK TRACES THAT OCCUR :
>>> javax.security.auth.login.LoginException:
>>> java.lang.IllegalStateException: Cannot get keys at REQ_OK state
>>> at sun.security.krb5.KrbAsReqBuilder.checkState(Unknown Source)
>>> at sun.security.krb5.KrbAsReqBuilder.getKeys(Unknown Source)
>>> at
>>> com.sun.security.auth.module.Krb5LoginModule.attemptAuthentication(Unknown
>>> Source)
>>> at com.sun.security.auth.module.Krb5LoginModule.login(Unknown Source)
>>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>>> at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
>>> at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
>>> at java.lang.reflect.Method.invoke(Unknown Source)
>>> at javax.security.auth.login.LoginContext.invoke(Unknown Source)
>>> at javax.security.auth.login.LoginContext.access$000(Unknown Source)
>>> at javax.security.auth.login.LoginContext$4.run(Unknown Source)
>>> at javax.security.auth.login.LoginContext$4.run(Unknown Source)
>>> at java.security.AccessController.doPrivileged(Native Method)
>>> at javax.security.auth.login.LoginContext.invokePriv(Unknown Source)
>>> at javax.security.auth.login.LoginContext.login(Unknown Source)
>>> at LoginModuleExample.main(LoginModuleExample.java:43)
>>>
>>> at javax.security.auth.login.LoginContext.invoke(Unknown Source)
>>> at javax.security.auth.login.LoginContext.access$000(Unknown Source)
>>> at javax.security.auth.login.LoginContext$4.run(Unknown Source)
>>> at javax.security.auth.login.LoginContext$4.run(Unknown Source)
>>> at java.security.AccessController.doPrivileged(Native Method)
>>> at javax.security.auth.login.LoginContext.invokePriv(Unknown Source)
>>> at javax.security.auth.login.LoginContext.login(Unknown Source)
>>> at LoginModuleExample.main(LoginModuleExample.java:43)
>>>
>>> REPRODUCIBILITY :
>>> This bug can be reproduced always.
>>>
>>> ---------- BEGIN SOURCE ----------
>>> import java.util.HashMap;
>>> import java.util.Map;
>>>
>>> import javax.security.auth.callback.Callback;
>>> import javax.security.auth.callback.CallbackHandler;
>>> import javax.security.auth.callback.NameCallback;
>>> import javax.security.auth.callback.PasswordCallback;
>>> import javax.security.auth.login.AppConfigurationEntry;
>>> import
>>> javax.security.auth.login.AppConfigurationEntry.LoginModuleControlFlag;
>>> import javax.security.auth.login.Configuration;
>>> import javax.security.auth.login.LoginContext;
>>>
>>> import com.sun.security.auth.module.Krb5LoginModule;
>>>
>>> public class LoginModuleExample {
>>>
>>> // Replace with the realm and KDC of the Windows domain.
>>> private static final String REALM = "DEV-DEM.RECOMMIND.COM";
>>> private static final String KDC = "AU-DEV-DC01.dev-dem.recommind.com";
>>>
>>> // Replace with the username and password of any account on the
>>> // above domain. Account needs to be enabled and not locked out.
>>> private static final String DOMAIN_USER = "sgr";
>>> private static final String DOMAIN_USER_PWD = "0rodriguez)";
>>>
>>> private static final String EXAMPLE_SERVER_LOGIN = "example-server";
>>>
>>> /**
>>> * @param args
>>> */
>>> public static void main(String[] args) {
>>>
>>> System.setProperty("java.security.krb5.realm", REALM);
>>> System.setProperty("java.security.krb5.kdc", KDC);
>>>
>>> Configuration.setConfiguration(new CustomConfiguration());
>>>
>>> try {
>>> CallbackHandler handler = getUsernamePasswordHandler(DOMAIN_USER,
>>> DOMAIN_USER_PWD);
>>>
>>> LoginContext loginContext = new LoginContext(EXAMPLE_SERVER_LOGIN,
>>> handler);
>>>
>>> loginContext.login();
>>>
>>> System.out.println("Login succeeded");
>>> }
>>> catch (Exception e) {
>>>
>>> System.out.println("Login failed");
>>> e.printStackTrace();
>>> }
>>>
>>> }
>>>
>>> private static CallbackHandler getUsernamePasswordHandler(final
>>> String username, final String password) {
>>>
>>> final CallbackHandler handler = new CallbackHandler() {
>>>
>>> public void handle(final Callback[] callback) {
>>> for (int i = 0; i< callback.length; i++) {
>>> if (callback[i] instanceof NameCallback) {
>>> final NameCallback nameCallback = (NameCallback) callback[i];
>>> nameCallback.setName(username);
>>> }
>>> else if (callback[i] instanceof PasswordCallback) {
>>> final PasswordCallback passCallback = (PasswordCallback) callback[i];
>>> passCallback.setPassword(password.toCharArray());
>>> }
>>> }
>>> }
>>> };
>>>
>>> return handler;
>>> }
>>>
>>> final static class CustomConfiguration extends Configuration {
>>>
>>> @Override
>>> public AppConfigurationEntry[] getAppConfigurationEntry(String name) {
>>>
>>> AppConfigurationEntry[] entries = new AppConfigurationEntry[0];
>>>
>>> if (name.equals(EXAMPLE_SERVER_LOGIN)) {
>>> String krbModule = Krb5LoginModule.class.getName();
>>> LoginModuleControlFlag flag = LoginModuleControlFlag.REQUIRED;
>>> Map<String, String> options = new HashMap<String, String>();
>>>
>>> options.put("storeKey", "true");
>>> options.put("useKeyTab", "false");
>>> options.put("isInitiator", "false");
>>>
>>> AppConfigurationEntry entry = new AppConfigurationEntry(krbModule,
>>> flag, options);
>>>
>>> entries = new AppConfigurationEntry[] { entry };
>>> }
>>>
>>> return entries;
>>> }
>>> }
>>> }
>>>
>>> ---------- END SOURCE ----------
>>>
>>>