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

Adding SPNEGO authentication support for Jetty Client #2868

Closed
CaoManhDat opened this issue Aug 29, 2018 · 25 comments
Closed

Adding SPNEGO authentication support for Jetty Client #2868

CaoManhDat opened this issue Aug 29, 2018 · 25 comments

Comments

@CaoManhDat
Copy link

Although this kind of authentication seems not follow the HTTP rules but in production Spnego/Kerberos is widely used, i.e: Hadoop, Ambari. This is also a requirement feature for us when trying to move from Apache HttpClient to Jetty Client.

@sbordet
Copy link
Contributor

sbordet commented Aug 29, 2018

See also #1708.

@sbordet
Copy link
Contributor

sbordet commented Sep 12, 2018

Work is in progress for this issue, and we also took the chance to rewrite the server side, which allows us to test the whole scenario. We have working code that allows us to derive few conclusions.

SPNEGO requires a 401+200 for every request.

This is a performance hit for GET requests, and a total bummer for POST requests, for obvious reasons.

Why people insist using SPNEGO with HTTP when it's so broken?

@sbordet
Copy link
Contributor

sbordet commented Sep 12, 2018

The only way around is to have the server-side emit a non-guessable cookie after the initial SPNEGO handshake.

Actually, SPNEGO emits a WWW-Authenticate header with the last token.
HttpClient could send back an Authorization header with the same token.
Or this can be done with Set-Cookie and Cookie.

The server would then need to check the incoming token and allow the request if the value matches.
This in turn requires the HttpSession in order to store the token value.

@gregw should we implement this workaround or just leave SPNEGO as is (i.e. 401+200 for every request - with problems for POST requests with non-reproducible content)?

@CaoManhDat
Copy link
Author

CaoManhDat commented Sep 14, 2018

IIUC in Hadoop, after the authorization process, the server will then return a cookie to the client, for subsequence requests, the cookie will be used instead. That seems their workaround for the performance problem.

sbordet added a commit that referenced this issue Sep 14, 2018
Implemented client-side SPNEGO authentication.
Reimplemented server-side SPNEGO authentication.
Added tests to verify behavior.

Signed-off-by: Simone Bordet <[email protected]>
@sbordet
Copy link
Contributor

sbordet commented Sep 14, 2018

@CaoManhDat I pushed working code to branch jetty-9.4.x-2868-spnego_client.
Can you please try it out?

@joshelser I have taken into account your observations in #1708 (use multiple Oids on server) in the new server-side implementation. Can you try it out?

To build:

$ git clone https://github.com/eclipse/jetty.project.git
$ cd jetty.project
$ git checkout jetty-9.4.x-2868-spnego_client
$ mvn clean install -DskipTests

Thanks!

sbordet added a commit that referenced this issue Sep 16, 2018
Added missing copyright header.

Signed-off-by: Simone Bordet <[email protected]>
@CaoManhDat
Copy link
Author

Hi @sbordet I'm meeting some trouble when trying to integrate your implementation to Solr, since the configuration is provided through JAAS, i.e

Server {
 com.sun.security.auth.module.Krb5LoginModule required
  useKeyTab=true
  keyTab="/keytabs/zkhost1.keytab"
  storeKey=true
  doNotPrompt=true
  useTicketCache=false
  debug=true
  principal="zookeeper/[email protected]";
};

In your implementation it seems that we must provide all the parameters manually.

@sbordet
Copy link
Contributor

sbordet commented Sep 18, 2018

@CaoManhDat you only have to provide the service principal and keyTab - all the other parameters are fixed and cannot be changed (otherwise it won't work).

And you have to provide the JAAS config name Server.

So with old code you need to A) set the system property with the JAAS config file path, and B) set the JAAS config name.

With the new code you need to A) set the service name, and B) set the keyTab path.

@CaoManhDat
Copy link
Author

CaoManhDat commented Sep 18, 2018

@sbordet Any reason for changing how the properties are set? Since in the old way it is quite simple and official, just by setting: -Djava.security.auth.login.config=jaas.conf. The configuration file will then be read automatically.

Here are stack-trace of the old code
screen shot 2018-09-18 at 5 55 06 pm

@sbordet
Copy link
Contributor

sbordet commented Sep 18, 2018

@CaoManhDat the old way was as official as the new way and it's not enough to only set the system property, unless you have hardcoded in the code the name of the JAAS configuration, Server (which by the way is not the default name; the default name is com.sun.security.jgss.accept).
And you need to ship an external file outside the web application (jaas.conf).
And you need to set a system property that precludes running 2 different servers/clients in the same JVM.
And you need to carefully write a number of properties in the JAAS configuration file that you must not change, so you don't know what can be changed and what must not be changed.

All in all, the old way is quite more complex than the new way.

Looking at the stack trace you posted, it is the client-side of the GSS negotiation. So the JAAS configuration should be called Client, not Server, and you want a principal that is a user name, not the service name (perhaps you showed the wrong jaas.conf? See how's confusing to use external files?).

My point being that on the client side you had to use system properties and external JAAS config files only because the implementation you were using was forcing you to do that.

Had that implementation done the opposite, you will be here asking me why you cannot set the principal via a setter 😉

I don't know... seems all more complicated to me.
I can split the client-side classes into an abstract SPNEGOAuthentication with 2 subclasses, one for configuring with setters and one with nothing that relies on magic system properties.

I'm a bit confused if you want this on the client-side or on the server-side.

@CaoManhDat
Copy link
Author

Sorry, the example conf file above is for the server-side. Here is an example for client

SolrClient {
 com.sun.security.auth.module.Krb5LoginModule required
 useKeyTab=true
 keyTab="/private/var/folders/9x/scy81vj16m12zn89zm3wdx6c0000gp/T/solr.cloud.TestSolrCloudWithKerberosAlt_FAC9AE7BA3C6EC54-001/tempDir-001/minikdc/keytabs"
 storeKey=true
 useTicketCache=false
 doNotPrompt=true
 debug=true
 principal="solr";
};

@CaoManhDat
Copy link
Author

@sbordet You're right, this seems reasonable to me.

Solr in that case must include some way to automatically convert the old magic properties to the new one. Since we want users to have a pleasure experience when update from the old version of Solr to a new one.

@CaoManhDat
Copy link
Author

@sbordet Thanks a lot, tests seem happy now, We will try to setup the authentication in a real cluster to check whether any errors can arise.

@sbordet
Copy link
Contributor

sbordet commented Sep 18, 2018

@CaoManhDat as I said we can have a subclass that works like your old way, but we need more information, in particular: how was the JAAS config name SolrClient set on the implementation?

@CaoManhDat
Copy link
Author

CaoManhDat commented Sep 18, 2018

@sbordet SolrClient is set by a system property named solr.kerberos.jaas.appname. So I think it should be passed as parameter of the subclass.

Currently Solr implement a class for doing this work

private static class SolrJaasConfiguration extends javax.security.auth.login.Configuration {

    private javax.security.auth.login.Configuration baseConfig;

    // the com.sun.security.jgss appNames
    private Set<String> initiateAppNames = new HashSet(
      Arrays.asList("com.sun.security.jgss.krb5.initiate", "com.sun.security.jgss.initiate"));

    public SolrJaasConfiguration() {
      try {
        this.baseConfig = javax.security.auth.login.Configuration.getConfiguration();
      } catch (SecurityException e) {
        this.baseConfig = null;
      }
    }

    @Override
    public AppConfigurationEntry[] getAppConfigurationEntry(String appName) {
      if (baseConfig == null) return null;

      String clientAppName = System.getProperty("solr.kerberos.jaas.appname", "Client");
      if (initiateAppNames.contains(appName)) {
        log.debug("Using AppConfigurationEntry for appName '"+clientAppName+"' instead of: " + appName);
        return baseConfig.getAppConfigurationEntry(clientAppName);
      }
      return baseConfig.getAppConfigurationEntry(appName);
    }
  }

@sbordet
Copy link
Contributor

sbordet commented Sep 18, 2018

Uhm, hardcoded system properties everywhere.

I'll think about this, but I prefer to not hardcode anything in Jetty.

If you can pass the JAAS config name to a subclass, then surely you can pass other configuration parameters so that there is no need to hardcode anything, right?

@CaoManhDat
Copy link
Author

CaoManhDat commented Sep 18, 2018

Right, I can pass other configuration parameters (with some casting and null checks). But are you sure that keyTab and principal are only parameters should be passed? What happen if useTicketCache=true.

This is my attempt so far, tests of Solr seems happy with this integration. We will now test the build against real kerberos setup. Will tell you about the result soon

    AppConfigurationEntry[] entries = jaasConfig.getAppConfigurationEntry(clientAppName);
    if (entries == null) {
      log.warn("Could not find login configuration entry for {}. " +
          "SPNego authentication may not be successful.", clientAppName);
      return authentication;
    }

    if (entries.length != 1) {
      log.warn("Multiple login modules are specified in the configuration file");
      return authentication;
    }

    Map<String, ?> options = entries[0].getOptions();
    String keyTab = (String) options.get("keyTab");
    if (keyTab != null) {
      authentication.setUserKeyTabPath(Paths.get(keyTab));
    }
    authentication.setServiceName("HTTP");
    authentication.setUserName((String) options.get("principal"));
    if ("true".equalsIgnoreCase((String) options.get("useTicketCache"))) {
      authentication.setUseTicketCache(true);
      String ticketCachePath = (String) options.get("ticketCache");
      if (ticketCachePath != null) {
        authentication.setTicketCachePath(Paths.get(ticketCachePath));
      }
      authentication.setRenewTGT("true".equalsIgnoreCase((String) options.get("renewTGT")));
    }

@sbordet
Copy link
Contributor

sbordet commented Sep 19, 2018

Ugh, no. I'll create a subclass so that you can do:

new JAASSPNEGOAUthentication(clientAppName);

so that you only set the JAAS system property and you're good.

@CaoManhDat
Copy link
Author

BTW: @gerlowskija helped on testing the integration in an AWS cluster with Kerberos setup. The result seems good. We are unable to find any problem. The current implementation seems solid!

@sbordet
Copy link
Contributor

sbordet commented Sep 19, 2018

@CaoManhDat let me recap:

  1. you have tested the client-side implementation only, with HttpClient.
  2. you are configuring the new Jetty SPNEGOAuthentication class using setters like the snippet you posted a couple of comments ago.
  3. with 1. and 2. everything works fine for you.

What do you use on the server? The old Jetty implementation? Something else?
Would you move to the new Jetty implementation?

@sbordet
Copy link
Contributor

sbordet commented Sep 19, 2018

@CaoManhDat there is another thing that needs to be configured on the client: the service name.
From your files above this is something like zookeeper/[email protected].
How do you configure the service name on the client? Another system property?

sbordet added a commit that referenced this issue Sep 19, 2018
Updates after review.

Signed-off-by: Simone Bordet <[email protected]>
@CaoManhDat
Copy link
Author

CaoManhDat commented Sep 20, 2018

@sbordet SolrCloud have 2 components

  • Nodes which act like servers, provide REST API. When needed nodes will act like clients to talk to other nodes.
  • Zookeeper quorum which is centralize configuration storage. Zookeeper nodes in this case will be server and Solr nodes will act like client.

All kind of servers/clients above are guarded by Kerberos. So the setup which contains zookeeper/[email protected] is used for config Zookeeper node rather than Solr nodes. That is another system properties (https://lucene.apache.org/solr/guide/6_6/kerberos-authentication-plugin.html#KerberosAuthenticationPlugin-SolrStartupParameters)
@gerlowskija : may be you want to add more information here?

Our current implementation of Kerberos on server side based on org.apache.hadoop.security.authentication.server.AuthenticationFilter
That seem works pretty well so far. So we don't want to change that in near future.

@CaoManhDat
Copy link
Author

BTW: What is the expected date for the next release of Jetty that include this implementation. Since Solr 8.0 tends to release in October. Therefore the switch from Apache Http Client to Jetty Client should be ready by then. If not I will probably have to do some workaround (like copy paste all the necessary source code of this implementation then remove it latter on the next release of Jetty).

sbordet added a commit that referenced this issue Sep 28, 2018
Renamed server-side classes and added javadocs.
Deprecated old server-side classes in favor of the new ones.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Oct 2, 2018
Added client-side classes javadocs.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Oct 2, 2018
Avoid hardcoded KDC port in tests.
Updated Krb5LoginModule options with refreshKrb5Config=true,
to make sure the KDC configuration is re-read for every test.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Oct 2, 2018
Updated server-side authentication logic after review.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Oct 2, 2018
Fixed server-side logic after review.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Oct 2, 2018
Restored behavior where authentication results are stored
no matter the response status.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Oct 2, 2018
Running tests only on JDK 11, as apparently other JDKs have problems
with AES encryption/decryption.
Another hypothesis is that Kerby does AES encryption differently from
what earlier JDKs expect.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Oct 2, 2018
Issue #2868 - Adding SPNEGO authentication support for Jetty Client.
sbordet added a commit that referenced this issue Oct 4, 2018
Removed old deprecated SPNEGO implementation on server-side.

Signed-off-by: Simone Bordet <[email protected]>
@sbordet
Copy link
Contributor

sbordet commented Oct 4, 2018

@CaoManhDat eventually we decided to not implement the version with hardcoded system properties.
You showed that it was possible for you to use the configurable version, so we stuck with that.

This is now merged in the main line of Jetty, and the next release should be in October - targeting a couple of weeks.

Thanks!

@joakime
Copy link
Contributor

joakime commented Apr 9, 2024

Jetty 11 is now at End of Community Support

Please try again with Jetty 12, and if you still have problems open a new issue against Jetty 12, don't revive this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants