karatelabs / karate

Test Automation Made Simple
https://karatelabs.github.io/karate
MIT License
8.09k stars 1.94k forks source link

Support custom client certificates for Mutual Auth #281

Closed maribowman closed 6 years ago

maribowman commented 6 years ago

I want to use my own client certs to test a service which uses Mutual Auth. So instead of only passing in the protocol, it would be neat to also specify a truststore + password. Something like this:

* configure ssl = { trustStore: 'classpath:security/trustStore.jks', password: 'secret', algorithm: 'TLSv1.2' }

or with a default jasypt encryption:

* configure ssl = { trustStore: 'classpath:security/trustStore.jks', password: 'ENC(ZqRBcxftoCD33dUPHX0liHvNH5xdfrUCmGw=)', algorithm: 'TLSv1.2' }

Currently in com.intuit.karate.http.HttpUtils:

    public static SSLContext getSslContext(String algorithm) {
        TrustManager[] certs = new TrustManager[]{new LenientTrustManager()};
        SSLContext ctx = null;
        if (algorithm == null) {
            algorithm = "TLS";
        }
        try {
            ctx = SSLContext.getInstance(algorithm);
            ctx.init(null, certs, new SecureRandom());
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
        return ctx;
    }

Retrieve SslContext with custom client cert(s):

    public static SSLContext getSslContext(URL trustStoreURL, char[] password, String algorithm) {
        SSLContextBuilder sslContextBuilder = new SSLContextBuilder();
        try {
            sslContextBuilder.loadTrustMaterial(trustStoreURL, password);
            sslContextBuilder.useProtocol(algorithm);
            return sslContextBuilder.build();
        } catch (GeneralSecurityException | IOException e) {
            throw new IllegalStateException("Error while creating SslContext", e);
        }
    }
ptrthomas commented 6 years ago

my understanding is 'standard' system properties work, and hence there is no need to add a configure option for this, does that make sense ? Can re-open if needed.

refer: https://github.com/intuit/karate/issues/76

mattjm commented 6 years ago

I actually did just this in a fork:

https://github.com/mattjm/karate

Except it takes PEM text instead of a keystore (because java keystores are ridiculous). I can submit a pull request for it if developers think it would be useful--it wasn't at all clear to me how to do cert auth with java arguments (plus then you have to spend four hours remembering how to put together a keystore).

ptrthomas commented 6 years ago

@mattjm I'm open to this. appreciate the commits and the doc. to be honest, haven't had to do this much so not sure about the usefulness. looks like an additional dependency e.g. bouncy-castle ? since netty is in the picture now, I'd like an integration test which I'd be happy to add. one more question is does this work for jersey. maybe this is useful when karate works as a mock / server - probably a nice use-case for terminating SSL but being able to log plain-text

interested in this, will think through and understand more.

ptrthomas commented 6 years ago

@MariAkaMachine @mattjm can you review this commit and let me know what you think: https://github.com/intuit/karate/commit/9718c98f1223bc5ce6f140416ffe645f83f7f1ff

maribowman commented 6 years ago

Awesome, thank you! I just took a look at the two commits and the code looks alright to me.

ptrthomas commented 6 years ago

@MariAkaMachine great ! I have actually made a preview release version 0.7.0.RC4 so it would be great if you can try it out.

I also have a question - since by default * configure ssl = true should work for any server in theory, what is the advantage or need for you to actually use a certificate for the client ?

maribowman commented 6 years ago

Thank you! Ive gotta say your responsiveness is really impressive - keep it up! :ok_hand::bowtie:

To give you some background information: Im working on a business app which expects rest calls from mobile hardware in the field. Unfortunately, OAuth is not supported on the client side yet. This basically forces us to implement and support Mutual Auth to verify the callers identity. Does this make any sense to you? Normally Id fork your repository and add (minor) adjustments as needed. But since this is not my private project Ive got to make sure that our 3rd-party dependencies are maintained by a community or something comparable.

ptrthomas commented 6 years ago

@MariAkaMachine that makes total sense, I'd never considered this scenario. I can imagine you would have a test that given a certificate - expect a response (identity), and maybe a negative scenario or two. thanks !

maribowman commented 6 years ago

Exactly! :wink: I love your framework so far and I wouldnt want to include something else just for this scenario.

mattjm commented 6 years ago

@prthomas Thanks! This looks cleaner than my solution.

@MariAkaMachine Have you tested RC4 yet? I'm not able to get it to work in my environment, but I was with a few tweaks.

On first glance it appears as written we're just setting up a trust store and not a key store--we need the private key out of the key store to get client cert authentication to work. The trust store is optional depending on if we want to verify the cert the server presents against a known root certificate--but that isn't required for authentication (whether or not it's a good idea to run without it is another question). I'm testing with the Apache HTTP client.

I need to look at it a little more tomorrow and I should be able to say something intelligent then.

ptrthomas commented 6 years ago

@mattjm I think you are right, and you would have noticed I left the keyStore part commented out suspecting that that was more appropriate for clients. yes, would be good to get feedback from @MariAkaMachine

mattjm commented 6 years ago

I noticed that. On further testing:

-we definitely need LoadKeyMaterial(). LoadTrustMaterial() does not import any private keys.
-In my environment cert auth fails unless I remove LoadTrustMaterial() (or if I bump the httpclient version to 4.5.4 and set "TrustAllStrategy". From what I can see in my debugger, the trust store is grabbing the client's public cert/key from the keystore and ignoring the CA root cert (my store has the private and public keys for the client, and the CA root cert). I'm not able to dig into the functions far enough to see why this is.

In any case--we definitely need to add LoadKeyMaterial(). It seems odd to me that server cert checking doesn't happen at all if I eliminate LoadTrustMaterial()--I would expect the default Java store to be used (although apparently that's a bit messy too: https://stackoverflow.com/questions/5206010/using-apache-httpclient-for-https ). The documentation from Java/Apache is not great here.

I would like to solve this, but I'm setting this up to do some stuff at work and I can't justify the time to sort out server cert verification right now since I don't need it.

I'll submit a one line PR for my working version...you can decide to merge as is or work off of it.

ptrthomas commented 6 years ago

@mattjm thanks ! I'm going to let this bake for a couple of days and if you get a chance to dig more, that would be great. One question is - should the name of the config key be changed from trustStore to keyStore.

I'm totally ok with upgrading http client, let me attempt that now in develop so that you can use that as a base.

@MariAkaMachine would like your inputs as well.

maribowman commented 6 years ago

sry, stuck at work with higher prioritized tasks! :unamused:

unfortunately, I didnt get it to work. but I also didnt spend much time on it! I will look into it now...

heres a resttemplate builder I wrote at work. maybe theres something you could use?! https://gist.github.com/MariAkaMachine/62cac478d905695aa503bd9842731bbb

maribowman commented 6 years ago

@mattjm on first sight, I thought youre right to use LoadKeyMaterial(). it is pretty confusing as the LoadTrustMaterial() is the exact same method except for one tiny exception. the XManager instance. read this for further info.

@ptrthomas concerning the config key: personally, I think semantically keyStore would be a better match because it is providing and not validating any creds. however, according to the post above Id go with trustStore. :wink:

am I the only one or is anyone else getting this on develop: com.intuit.karate.exception.KarateException: javascript function call failed: ReferenceError: "karate" is not defined -> also karate-core tests fail during the build. -> the same config works fine for v0.6.2

ptrthomas commented 6 years ago

@MariAkaMachine just upgrade your JRE version and this problem will go away. do refer to the upgrade guide also: https://github.com/intuit/karate/wiki/Upgrading-To-0.7.0

mattjm commented 6 years ago

There is something weird about how openssl assembles PEM text into PKCS12 files. Openssl adds the ca files in a way that the Java trust store methods don't find them.

rt.jar!\sun\security\validator\KeyStores.class

if (var0.isCertificateEntry(var3)) {
                    Certificate var4 = var0.getCertificate(var3);
                    if (var4 instanceof X509Certificate) {
                        var1.add((X509Certificate)var4);
                    }
                } else if (var0.isKeyEntry(var3)) {
                    Certificate[] var6 = var0.getCertificateChain(var3);
                    if (var6 != null && var6.length > 0 && var6[0] instanceof X509Certificate) {
                        var1.add((X509Certificate)var6[0]);
                    }
                }

No matter how I load the CA file, this method always sees only a private key and it just grabs the first cert in the chain, which is the public cert associated with the private key. Note I've tried the '-certfile' and '-CAfile' arguments for openssl to specify the CA cert. The former puts the root cert in the cert chain for the private key and it never gets loaded since it's the second cert in the chain. The latter puts root cert into some state where the method above doesn't even see it.

I don't know if this is an OpenSSL bug, or a limitation of using PKCS12 files instead of keystores.

I really, really dislike keystores so I'm inclined to just set TrustAllStrategy and ignore the problem. Generally when you're testing you're not using real data so if someone impersonates your server and intercepts some test data...no big deal.

I would say the configuration parameter should be keyStore instead of trustStore--the post @MariAkaMachine mentions seems to agree. I might have missed some sarcasm here.

I still had to add in the loadKeyMaterial method to get client cert auth to work. See: https://github.com/mattjm/karate/commit/be442ef847b5cf1d1c0d7d2daa9af6c6f1cc8f57

karate-jersey crashes and burns currently--I'll take a look later today.

mattjm commented 6 years ago

It looks to me like with jersey the HTTP client is initializing before karate-config.js is parsed.

ptrthomas commented 6 years ago

@mattjm FWIW this commit implements server-side SSL custom cert / key for Netty: https://github.com/intuit/karate/commit/fdf2cb0ae0dace36a839c4bdc10e6cf7ccb3be8e

And that supports PEM files. Haven't explored yet, but there's a chance Netty would have similar helpers for the client-side.

Also I thought we PKCS12 is not mandatory, and we can switch the "type".

ptrthomas commented 6 years ago

I did some reading and man this is so confusing. anyway, I figured the best thing is to support both being able to specify a keyStore AND trustStore. so here is an example:

https://github.com/intuit/karate/blob/c259b1a1056fb6ce5da4b021bf8416636bb8271a/karate-demo/src/test/java/ssl/ssl-keystore.feature

references: http://maultech.com/chrislott/blog/20120107_httpscerts.html http://drumcoder.co.uk/blog/2011/oct/18/httpclient-client-side-certificates/

mattjm commented 6 years ago

OK--correct, we don't have to use PKCS12 (I kind of had some tunnel vision there). But it's good folks have the option to use something else if they need to. It looks like the other options are various flavors of Java keystores.

Certs are definitely confusing, especially in Java--I had to explain to a vendor last week how to fix their keystore in a piece of software they wrote. :-/

I think adding a truststore configuration parameter is good--that way users can decide for themselves if they want to set it up or use a java keystore or PKCS12 or whatever. I can add a section to the documentation about all this once the parameters are finalized.

It's already getting a bit messy, but could you add another configuration parameter like "trustAllCAs" that toggles trustAllStrategy? That way folks can decide for themselves how loose they want to be with certificate trust. The trustStore parameter could be optional if this new parameter is specified.

The way it is now, with trustAllStrategy hard coded, there's no point in specifying a trustStore because any cert will be trusted anyway.

ptrthomas commented 6 years ago

The way it is now, with trustAllStrategy hard coded, there's no point in specifying a trustStore because any cert will be trusted anyway.

Totally makes sense, will do. No, wait. Actually a question, is this really relevant for an HTTP test-suite if the primary need ( e.g. use-case of @MariAkaMachine ) is to incorporate the client key in the request ? Plus one thing that bugs me is that I couldn't figure out the equivalent for Jersey.

Hey @MariAkaMachine if this works for you, I'm tempted to stop work on this and cut release 0.7.0 as soon as possible, do comment !

About PKCS12 I eventually couldn't figure how to use a different type actually :|

ptrthomas commented 6 years ago

sorry @mattjm re-reading your posts more carefully:

It looks like the other options are various flavors of Java keystores. Yes !

Generally when you're testing you're not using real data so if someone impersonates your server and intercepts some test data...no big deal. Yes !

So maybe we'll just go with trustAll: true and if needed in the future we can have a custom class that has this strategy for Jersey. I suspect that Jersey is already doing a 'trust all' by default. Ugh, had enough of certificates for now.

Do go ahead with a PR @mattjm if you have time - you have been of great help.

maribowman commented 6 years ago

sorry guys for not being very responsive here! :see_no_evil: unfortunately, I couldnt make it work so far. I just pulled the lase changes and will spend some more time with it now.

PS: this is the third or fourth time I underestimate what a massive pain in the * this is!

mattjm commented 6 years ago

I got the new pieces in and updated the documentation:

https://github.com/mattjm/karate/tree/karate-develop-3

Also updated a few tests here and there related to the various keystores, although I'm not entirely sure what your paradigm is so it may need some adjustment still.

There is a slightly glaring problem in that the loadKeyMaterial method doesn't allow me to just load a keystore and use the system default trust strategy. So right now we trust all certs no matter what. :-/ I think I may need to look at a bit of a refactor for this part. I still have an imperfect understanding of how the trust engine loads.

With this build cert auth works in my environment using Apache. It all compiles, but the Jersey tests fail. It looks like Jersey needs something similar to a TrustAllStrategy. It doesn't look like that's hard to implement (I saw some stuff about creating a custom TrustManager), but I can't justify spending any more time on it right now. Maybe in a week or so. I think the work I need to do to get cert trust working in Apache is very similar to the work I need to do to get cert trust not working (i.e. trust all certs) in Jersey.

I'm not going to do a PR right now since it's technically broken, but you're welcome to pull out any useful pieces here.

ptrthomas commented 6 years ago

thanks, will take a look.

ptrthomas commented 6 years ago

adding a source for reference reading on mutual-TLS: https://tyk.io/docs/security/tls-and-ssl/mutual-tls/

ptrthomas commented 6 years ago

@mattjm thanks a lot for the doc, which I incorporated. wish you had the PR though, to show up as a committer

I tried to put what I figured (untested) would be a way to use full CA chain for Apache, read through the commit if you have time. anyway I think we are done for this version with this: https://github.com/intuit/karate/commit/eb8decf00bb30ce576e3b404ae64a1aca1897f83

jbadeau commented 6 years ago

@mattjm I looked at your fork but didn't see any pem specific code changes. Can you point me to where you made the changes,

mattjm commented 6 years ago

@jbadeau I will get rid of that fork--it worked but the current master upstream branch is far more polished. @ptrthomas has it taking a PKCS12 cert, which is easy enough to get from PEM using openssl (https://www.sslshopper.com/article-most-common-openssl-commands.html).

If you were interested in using the PEM code for something else, send email (email in my profile)--I think it's a library I have somewhere.

ptrthomas commented 6 years ago

adding a reference link since it may come in useful in the future:

https://tersesystems.com/blog/2018/07/27/debug-java-tls-ssl-provider/

ptrthomas commented 5 years ago

adding one more reference link: https://www.baeldung.com/java-ssl-handshake-failures

mattjm commented 5 years ago

I'll add a couple comments about SSL debugging from my years of experience supporting TLS mutual auth:

  1. Don't use Java keystores, use PKCS12. Java keystores require everything be done just perfectly in exactly the right order...and even then they won't work half the time.

  2. Trust path problems can also be caused by missing intermediate certificates (e.g. you trust the root cert, but the server isn't presenting the intermediate certs in the trust path). You might also need to include intermediates with your client certificate (this is much easier with PKCS12 vs a java keystore).

  3. (client debugging only) If you're really stumped you can set up an Apache server, enable certificate authentication, set the logLevel very high, then point your HTTP client at the server. Obviously the Apache server won't serve up what your client expects, but it will still try to handshake with the server and you can get really good information out of the debug logs.

  4. (shouldn't happen with Karate since it doesn't use the Windows certificate keystore but I'll mention it anyway) 9 times out 10 on Windows, handshake failures are because you're missing a certificate's private key or don't have permissions on it.

ptrthomas commented 5 years ago

@mattjm wow, thanks matt ! by any chance are you interested in taking up this challenge - https://github.com/intuit/karate/projects/3#card-22529464 or at least if I get started with a Netty sample, hope you can help debug SSL handshakes, this is where I got stuck the last time I tried

mattjm commented 5 years ago

Let me ruminate on that one a bit...could be really interesting. I can help debug SSL in any case...that is one thing I am definitely good at. Can you add me to the Slack channel? We can discuss there without cluttering up this thread.

ptrthomas commented 5 years ago

@mattjm done, I used the mail address on your github profile

bharatgarg211 commented 3 years ago

Hi,

I majorly works on Test Automation using c# , please let me know if I can proceed with the same or not in writing custom code if needed while writing my feature as my org can't change the tech . Also please suggest if I can add karate api as a dependency in my Visual Studio 2019 Professional.

Thanks

ptrthomas commented 3 years ago

@bharatgarg211 no you can't. for further questions please use stack overflow, thanks.