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

Thu Jul 26 08:47:46 EDT 2018

As promised, here's an update on this.

I managed to migrate 3 more keystores (2 of them for SAML), see the PR [1].
Most of the time I spent on figuring out which signatures need to match
with the JSON configuration and which don't (because they are testing
negative path for example). This is not very complicated task but very time
consuming (comparing generated PEM files with each other and occasionally
debugging remote WF app server). So at the moment we have there are 28
still to be migrated.

It becomes clear to me that this task is simply to large to be implemented
off the sprint. Let's prioritize it and schedule for some of the next
sprints. As for now, I'm closing the PR.

Thanks,
Sebastian

[1] https://github.com/keycloak/keycloak/pull/5410

On Thu, Jul 26, 2018 at 11:03 AM Sebastian Laskawiec <slaskawi at redhat.com>
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 at redhat.com> 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.
>
> [1] https://gist.github.com/slaskawi/b7e5f9973798345c1bef182ae8d8d55f
> [2]
> https://github.com/keycloak/keycloak/blob/54fcbf12b0740f092e2d9148fb4d06a634618839/testsuite/integration-arquillian/servers/app-server/jboss/pom.xml#L148
>
>
>> 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-03eb8cfddae682425845854ec51a2d53R246
>
>
>>
>> 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-207441f5cf5b6552db445afd0fe61492R28
>
>
>> 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 at redhat.com>
>> > 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-arquillian/tests/base/src/test/resources/adapter-test/demorealm.json#L231
>> >>
>> >> On Wed, Jul 25, 2018 at 2:07 PM Peter Skopek <pskopek at redhat.com>
>> 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/java/org/jboss/as/test/integration/security/common/VaultHandler.java
>> >>> On Wed, Jul 25, 2018 at 1:23 PM Sebastian Laskawiec <
>> slaskawi at 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-pkcs12
>> >>>> [1]
>> >>>>
>> >>
>> https://github.com/infinispan/infinispan/blob/master/server/integration/testsuite/pom.xml#L460
>> >>>> [2] https://github.com/keycloak/keycloak/pull/5410
>> >>>> _______________________________________________
>> >>>> keycloak-dev mailing list
>> >>>> keycloak-dev at lists.jboss.org
>> >>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> >> _______________________________________________
>> >> keycloak-dev mailing list
>> >> keycloak-dev at lists.jboss.org
>> >> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> >>
>> > _______________________________________________
>> > keycloak-dev mailing list
>> > keycloak-dev at lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>>
>>