Re: [keycloak-dev] Proposal - use Ant Plugin for generating JKS files

My preference is programmatic approach too. See inline. On Thu, Jul 26, 2018 at 11:13 AM Sebastian Laskawiec <slaskawi(a)redhat.com&gt; wrote: > Answering Stian's and Marek's email at the same time. > > Just FYI - I've created a ticket to track this work - > https://issues.jboss.org/browse/KEYCLOAK-7940 > > On Wed, Jul 25, 2018 at 8:16 PM Marek Posolda <mposolda(a)redhat.com&gt; > wrote: > > > IMO Creating keystore files programatically as Peter suggested, looks > > better than creating them through maven-antrun-plugin. It seems that > > creating with maven will mean lots of the stuff in pom.xml files, > > however generating programatically will probably allow some better > > reusability (common utility class in the testsuite to easily generate > > keystore in various locations). Also programatic generation will work > > when running the test from Intellij Idea without previous maven build. > > > > Perhaps I didn't make it clear enough - the JKS autogeneration script will > mostly be executed in our test suite. I've just checked if you can run any > of the tests with a clean repo (without invoking `mvn package` or `mvn > install`) and it failed with [1]. So it seems you always need to execute > Maven prior to invoking any tests from the IDE (at least from the > Arquillian testsuite). > The programmable approach Peter suggested looks very nice. However, I'm not > sure if this is a good fit for us since we modify app servers during the > build [2]. Those servers expect a keystore (and sometimes a truststore > too) > to be in certain places upon the server bootstrap. When the Arquillian > testsuite if fired, it's simply to late to generate keystores on the fly. > It is not too late. We modify the app servers during the build, and will be only controlling their lifecycle in the testsuite, which is current work in progress to be merged soon. [1] So one can always prepare the config for the keystore and generate the keystore (if it does not exist) in Java to appropriate place (ideally location would be set via system property).

Adapter tests can then make use of the generated keystores since they could be available to them via Arquillian-injected resources. That would make tests that cope with encryption and/or signing simpler and I see quite a benefit for at least SAML tests.

I believe this would be a neat follow-up on [1]. [1] https://issues.jboss.org/browse/KEYCLOAK-4407 [1] https://gist.github.com/slaskawi/b7e5f9973798345c1bef182ae8d8d55f > [2] > > https://github.com/keycloak/keycloak/blob/54fcbf12b0740f092e2d9148fb4d06a... > > > > Sebastian, yesterday I didn't mention on the call that we probably don't > > test X509 authentication with IBM JDK. I recalled it just now :) The > > thing is, that X509 tests are not executed during default build as they > > require Wildfly (they don't work on embedded undertow) and also > > phantomJS (no other browser is supported). So I am pretty sure they are > > not executed during the various matrix jobs for various java versions > > (Vasek Muzikar can correct me if I am wrong). Which means they probably > > don't work on IBM JDK too. > > > > In Infinispan we had exactly the same problem. Keystores generated by > OpenJDK (or Oracle JDK) didn't work for IBM JDK. Therefore we added this > [3] little trick. > > [3] > > https://github.com/keycloak/keycloak/pull/5410/files#diff-03eb8cfddae6824... > > > > > > Marek > > > > > > On 25/07/18 15:50, Stian Thorgersen wrote: > > > Sounds like this could be a time-consuming task and the tests are > working > > > at the moment. > > > I think I've solved the worst case - in some of the scenarios (like > `DemoServletsAdapterTest#testOIDCUiLocalesParamForwarding`) we expect the > certificate in keystore to be the same as the one from the realm JSON > configuration. I might be wrong here (I'm still the "new guy") but the > reason for this is that keystore.jks is used by the adapter whereas the > auth server uses the certificate from json file. Those need to match. > Therefore, I had to add a small piece of the code that does automatic > replacement of PEM certificates in the configuration [4]. I could probably > use some latest syntactic sugar (Optional#flatmap and Streams) but I think > the old way is more readable in this case (but of course, comments are > more > than welcome, especially on the PR). > > [4] > > https://github.com/keycloak/keycloak/pull/5410/files#diff-207441f5cf5b655... > > > > So not sure I see a need to start a bulk refactoring here. > > > > Even though it might seem like a redundant thing to do, I would still > prefer to replace all existing JKS entries. The main reason for this is > that I'm a bit afraid that we'll end with a mixture of pushed JKS files > and > automatically generated ones. > > Let's make a deal. I'll look more into this today and will see how many of > those I'll manage to replace. Maybe it won't be that bad after all... > > > > > The json file you're mentioning has a cert in pem format in the json > file > > > that is then stored in the DB. This doesn't have anything to do with > > > keystores/truststores. We expect Keycloak to be able to handle a cert > > > provided in the json file (or through admin endpoints) and store it in > > the > > > DB. > > > > > > Now for SSL tests with the server itself I can see the value in what > you > > > are proposing. Keystore/truststore for Keycloak https connections and > > > truststores for mutual SSL could probably be improved. However, it > seems > > to > > > be working at the moment, so not sure this is required. > > > > > > > > On Wed, 25 Jul 2018 at 15:30, Sebastian Laskawiec < > slaskawi(a)redhat.com&gt; > > > wrote: > > > > > >> Thanks Darran and Peter for the insight! > > >> > > >> I've just noticed that generating JKS files is one thing but we will > > also > > >> need to pre-process RealmRepresentation and replace all > private/public > > keys > > >> (like this one for example [1]) with values from > keystores/truststores. > > >> > > >> At the moment I'm thinking about replacing [1] with something like > this: > > >> "jwt.credential.certificate" : > > >> > > >> > > > "certificate:keystore=classpath:keystore.jks,password=password,alias=secure-portal,type=pkcs12" > > >> similarly to private keys (if needed): > > >> "privateKey" : > > >> > > >> > > > "private-key:keystore=classpath:keystore.jks,password=password,alias=secure-portal,type=pkcs12" > > >> > > >> Later on, while converting JSON to RealmRepresentation I plan to > > traverse > > >> all nodes and replace replace those which start with private-key or > > >> certificate. > > >> > > >> Let me know if you have a better idea. > > >> > > >> [1] > > >> > > >> > > > https://github.com/keycloak/keycloak/blob/master/testsuite/integration-ar... > > >> > > >> On Wed, Jul 25, 2018 at 2:07 PM Peter Skopek <pskopek(a)redhat.com&gt; > > wrote: > > >> > > >>> Hi Sebastian, > > >>> > > >>> I think that generation of Keystore on the fly is the only way to > go. > > >>> You can use Java Keystore API generate it entirely from Java code > (no > > >>> need to run maven). > > >>> > > >>> Here [1] is an example of utility class used in wildfly testsuite to > > >>> generate Vault (basically just keystore). > > >>> It will be easy to strip vault related stuff and use it in our > > testsuite. > > >>> > > >>> Note that there is one more drawback of storing generated JKS files, > > >>> which is that in some cases they are not binary compatible between > > >>> different JDK (particularly Oracle and IBM). > > >>> > > >>> HTH, > > >>> Peter > > >>> > > >>> [1] > > >>> > > >> > > > https://github.com/wildfly/wildfly/blob/master/testsuite/shared/src/main/... > > >>> On Wed, Jul 25, 2018 at 1:23 PM Sebastian Laskawiec < > > slaskawi(a)redhat.com > > >>> > > >>> wrote: > > >>>> Hey, > > >>>> > > >>>> Together with Sebi we are working on Certificate-based > authentication > > >> for > > >>>> the clients. Our work will require adding at least 2-3 keystores to > > the > > >>>> codebase with different DNs and I think this might be a good > > >> opportunity > > >>> to > > >>>> revisit the way we handle JKS files in the tests. > > >>>> > > >>>> Currently we push JKS files directly into our repo, which has a > couple > > >> of > > >>>> drawbacks: > > >>>> - it is hard to figure out what's inside the JKS, it requires > looking > > >> up > > >>>> for password (usually in some JSON configuration file or hardcoded > in > > >> the > > >>>> test) and using keytool (or some similar one) to explore its > content. > > >>>> - It is not git-friendly. Every time we update JKS content we > > >> effectively > > >>>> store another binary file (git doesn't understand binary file > changes > > >> and > > >>>> can not diff it). > > >>>> - we use many different naming and password schemes in our tests. > > >>>> - it is hard to migrate all keystores to pkcs12 at the same time > (JKS > > >>>> format is deprecated) [0] > > >>>> > > >>>> I believe most of the issues could be addressed by generating JKS > > files > > >>> on > > >>>> the fly - during the build. In Infinispan we did it with Maven Ant > > >> Plugin > > >>>> [1]. I already created a very limited POC for Keycloak and you can > > >> check > > >>> it > > >>>> out here [2]. Unfortunately, the process of reverse engineering all > > >> those > > >>>> files is quite time-consuming, so I would like to know your opinion > > >>> before > > >>>> moving on. > > >>>> > > >>>> Of course, generating JKS files on the fly has some drawbacks: > > >>>> - It increases build time (~1s per keytool invocation, and we > probably > > >>> will > > >>>> have more than 30 of them). > > >>>> - it makes testing from IDE a bit harder, you need to run Maven and > > >>> process > > >>>> test resources before doing anything. A common workaround is to use > > >> `mvn > > >>>> clean install -DskipTests` and then opening your IntelliJ. > > >>>> > > >>>> Please let me know what you think. > > >>>> > > >>>> Thanks, > > >>>> Sebastian > > >>>> > > >>>> [0] > > >>>> > > >> > > > https://blogs.oracle.com/jtc/jdk9-keytool-transitions-default-keystore-to... > > >>>> [1] > > >>>> > > >> > > > https://github.com/infinispan/infinispan/blob/master/server/integration/t... > > >>>> [2] https://github.com/keycloak/keycloak/pull/5410 > > >>>> _______________________________________________ > > >>>> keycloak-dev mailing list > > >>>> keycloak-dev(a)lists.jboss.org > > >>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev > > >> _______________________________________________ > > >> keycloak-dev mailing list > > >> keycloak-dev(a)lists.jboss.org > > >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > > >> > > > _______________________________________________ > > > keycloak-dev mailing list > > > keycloak-dev(a)lists.jboss.org > > > https://lists.jboss.org/mailman/listinfo/keycloak-dev > > > > > > > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev >