7 Replies Latest reply on May 31, 2019 10:10 AM by Andrew Dinn

    Byteman rule to intercept a call to equals and make it compare the principals in KeycloakUndertowAccount ?

    Issa Gueye Newbie

      Hi Byteman Experts,

       

      I'm working on test case for isolating an issue in KEYCLOAK-10329 bug [1]. We would need to implement a byteman rule to intercept a call to equals() and make it compare the principals in KeycloakUndertowAccount [2].

       

      The reasoning behind this is that we found an issue as when Keycloak (or RH-SSO) is in use in EAP as adapter, the PicketBox in EAP gets a KeycloakUndertowAccount as credential and this class is not a Comparable, a char[] or a byte[], so PicketBox uses equals() to determine if the credential being presented matches that which is found in the cache. But as this particular class doesn't implement Comparable nor equals(), a different instance of the same class results in a cache miss and we ran into a whole new JAAS login taking place, which gives us a different Subject instance.

       

      We would like to add/implement an equals() in that class [2] and make it compare with the principals in the same.

       

      Is this something possible using Byteman ?

       

      Any help much appreciated.

       

      Thanks!

       

      [1] https://issues.jboss.org/browse/KEYCLOAK-10329

      [2] keycloak/KeycloakUndertowAccount.java at master · keycloak/keycloak · GitHub

        • 1. Re: Byteman rule to intercept a call to equals and make it compare the principals in KeycloakUndertowAccount ?
          Andrew Dinn Master

          Hi Issa,

           

          Thanks for reporting you problem here on the forum.

           

          I'm not clear exactly what you are trying to do nor why you want to do it. So, my initial reaction is that Byteman may be able to help but not necessarily in the way you are asking or for the purpose you have in mind.

           

          Firstly, let's deal with the immediate request. Byteman cannot add a new method to any class. Byteman never attempts to change class structure. All it ever does is redefine method bytecode. So, what you are proposing -- adding an implementation of equals to class KeycloakUndertowAccount -- is not something Byteman can help you with. However, that does not mean all hope is lost.

           

          A rather drastic solution to tweak the current behaviour would be to have Byteman redefine Object.equals. You could inject a rule that detects when this is an instance of KeycloakUndertowAccount and, if so, calls out to a special check method. That is possible although it is not straightforward to achieve because of classloader issues (Object is in the bootstrap path and KeycloakUndertowAccount will either be in the system path or, more likely, be loaded by a JBoss Modules loader, so you will need to use reflection to perform both the instanceof check and the call to the equals method).

           

          If you were using a rule liek that to test whether a fix to use a specific comparison for a KeycloakUndertowAccount is the correct solution then I'd suggest maybe you go ahead and try it (and can advise on details if you want -- but see below for alternatives). If you want to use Byteman to hot patch the Keycloak code so that it uses this correction in EAP then I think that would be too risky. You could probably make it work reliably but it could very easily compromise performance.

           

          Before considering other alternatives, let's just address that last point. Are you simply trying to find what is broken here? Or are you looking for some way to hot patch Keycloak? If it is the latter then while it is true that Byteman can be used to hot patch broken code that should only ever be a temporary measure and/or a last resort. We could achieve the same outcome by releasing a Keycloak upgrade and that would be preferable by far. Hot patching is all very clever and pretty safe but it is not 100% risk free (performance degradation being the main concern, but there are other things to worry about e.g. interactions with other agents). Patching certainly doesn't beat simply fixing bugs and pushing out updates. Hot patching with Byteman has only ever been used once as a long term solution in our customer deployments to repair a library that was out of our control (it fixed a wrong value return in a SQL driver that we were not expecting to be updated for a matter of years).

           

          So, what other options are there? Well, rather than modify Object.equals() it may also be possible to inject a rule into the PicketBox code that corrects the invalid test. Whether that is possible depends on the specifics of how the call is made and how the return value is used. So, if you could point me at the actual PicketBox client code (indeed, all points where it performs the comparison) I might be able to provide a rule you can try. Depending on what sort of rule is needed this might make it easier to test a fix (it probably won't run into classloader issues) and might pose less of a risk than modifying Object.equals(). If this is needed as a temporary fix for a customer deployment while a new KeyCloak release is prepared then it might also be worth proposing it as a hot patch.

           

          So, please provide more details ad I'll try to advise.

           

          regards,

           

           

          Andrew Dinn

          • 2. Re: Byteman rule to intercept a call to equals and make it compare the principals in KeycloakUndertowAccount ?
            Issa Gueye Newbie

            Hi Andrew,

             

            Thanks a lot for reviewing this post and looking into it.

             

            > Before considering other alternatives, let's just address that last point. Are you simply trying to find what is broken here? Or are you looking for some way to hot patch Keycloak?

             

            Well, it is more for testing purposes to find what is broken, though it can be somehow interpreted as kind of "test/hot patch" as per the investigation made with Keycloak Engineering in KEYCLOAK-10329  it is likely what will fix the issue (or one possible way of fixing).

             

            But so, for now we just wanted to confirm the issue first with further testing using byteman rule, before a real fix is going to be implemented in KEYCLOAK-10329.

             

            We are also thinking of a possible hot patch testing fix, but the issue is that the environment is very complex one with multiple components integrated together and it runs also on OCP (they use this RH-PAM image [1], which bundled EAP + Keycloak binaries together). We first implemented some byteman rule to isolate the issue from the JCA layer at first place to confirm something is wrong more on the PicketBox/Keycloak layer and this is where we are looking for implementing a further byteman rule for equals() in that KeycloakUndertowAccount class. This would help us to at least confirm the PicketBox/Keycloak Engineering findings on analyzing the issue. Issue is that seems multiple database connection pools are created for requests made to the RH-PAM/KIE server when SSO is added in the picture. More on the context of the issue itself can be read here http://post-office.corp.redhat.com/archives/jboss-support-eap6/2019-April/msg00122.html  and here http://post-office.corp.redhat.com/archives/jboss-support-eap6/2019-May/msg00144.html  for why we need a byteman rule.

             

            Thanks & Regards,

            Issa

             

            [1] Container Catalog - Red Hat Customer Portal

            • 3. Re: Byteman rule to intercept a call to equals and make it compare the principals in KeycloakUndertowAccount ?
              Andrew Dinn Master

              rhn-support-igueye  wrote:

               

              We first implemented some byteman rule to isolate the issue from the JCA layer at first place to confirm something is wrong more on the PicketBox/Keycloak layer and this is where we are looking for implementing a further byteman rule for equals() in that KeycloakUndertowAccount class. This would help us to at least confirm the PicketBox/Keycloak Engineering findings on analyzing the issue.

              Could you point me at the existing Byteman rules and the source code for the class they are being injected into. That would be a good place to start from.

               

              regards,

               

               

              Andrew Dinn

              • 4. Re: Byteman rule to intercept a call to equals and make it compare the principals in KeycloakUndertowAccount ?
                Issa Gueye Newbie

                Well, this is just one rule (in below) and it was more to help isolate the issue from the JCA layer as I said. It helped to check the call from JCA out to the subject factory in the security layer to see the subject used to create a pool. The source code of class is at [1]

                 

                ~~~

                RULE org.jboss.security.SubjectFactory.createSubject

                INTERFACE org.jboss.security.SubjectFactory

                METHOD createSubject(java.lang.String)

                AT EXIT

                IF true

                DO traceStack("[BMAN] createSubject(domain=" + $1 + ") => " + $! + "\n", 10);

                ENDRULE

                ~~~

                 

                [1] https://github.com/picketbox/picketbox/blob/master/security-spi/spi/src/main/java/org/jboss/security/SubjectFactory.java

                • 5. Re: Byteman rule to intercept a call to equals and make it compare the principals in KeycloakUndertowAccount ?
                  Andrew Dinn Master

                  Hi Issa,

                   

                  Apologies for the delay in replying.

                   

                  The example doesn't really make it any easier to identify how the credentials are being handled by PicketBox. Can you point me at the PicketBox method that is checking the credentials -- the one that ends up calling the equals method. Is it method JBossCachedAuthenticationManager.validateCache() that is failing to handle the problematic credentials? If so then you could maybe use a rule like this:

                   

                  RULE handle KeyCloak credentials

                  CLASS JBossCachedAuthenticationManager

                  METHOD validateCache

                  AFTER WRITE $subjectCredential

                  HELPER MyHelper

                  IF ($subjectCredential.getClass().getSimpleName.equals("KeycloakUndertowAccount"))

                  DO return KeycloakCompare($subjectCredential, $credential)

                  ENDRULE

                   

                  You would have to implement a class called MyHelper which is able to compare two instances of KeycloakUndertowAccount and ensure it is in scope when executing method validateCache.

                   

                  regards,

                   

                   

                  Andrew Dinn

                   

                  • 6. Re: Byteman rule to intercept a call to equals and make it compare the principals in KeycloakUndertowAccount ?
                    Stefan Guilhen Apprentice

                    Hi Andrew, how have you been?

                     

                    The idea of checking if byteman could be used to intercept a call to equals on a class that doesn't implement it was mine (since byteman is already being used to collect some information at the customer's environment) and Issa has been trying to make this work. I suspected it would not be possible but wasn't really sure.

                     

                    You are correct about the place where the credentials are compared. PicketBox's JBossCachedAuthenticationManager checks if the cache already contains an entry corresponding to the principal and then proceeds to compare the stored credential with the one being provided to isValid(). I believe the rule you posted is exactly what we are looking for: if the stored credential is an instance of KeycloakUndertowAccount we defer the credentials comparison to the helper class. In this case, MyHelper would need to have a public boolean keycloakCompare(Object subjectCredential, Object credential) method, right? I suppose the params need to be of type object unless we cast them prior to invoking the helper.

                     

                    Thanks for all the help with this!

                    Stefan

                     

                    PS: been a while since your last trip to Brazil - time to submit another talk just to pay us another visit

                    • 7. Re: Byteman rule to intercept a call to equals and make it compare the principals in KeycloakUndertowAccount ?
                      Andrew Dinn Master

                      Hi Stefan,

                       

                      Nice to hear from you! I'm doing very well. I hope all is well with you.

                       

                      If I could find an excuse to visit Sao Paolo again I'd be there like a shot. However, much as I would love it that might be a bad idea. The rest of the Newcastle office would probably lynch me if I was offered a second trip before any of them had even been once!

                       

                      I'm glad I managed to work out the right place to inject the rule. I am sure this will work except for one possible wrinkle to do with class loaders. Your helper is going to have to be able to access Keycloak instances. That's ok if the Keycloak classes are in scope at the injection point. However, I have a strong suspicion that the Picketbox and Keycloak classes will belong to separate JBoss Modules. That's why I tested the class name in the rule. I could have written the condition like this:

                       

                        IF ($subjectCredential.getClass() == org.org.keycloak.adapters.undertow.KeycloakUndertowAccount.class)

                       

                      Byteman understands class literals ok. The problem is that class KeycloakUndertowAccount might not be visible to the loader of JBossCachedAuthenticationManager, i.e. the class identified by the CLASS clause.

                       

                      If that is a problem then you need to do two things:

                       

                      1) will pass the credential arguments to the helper method as Object instances

                      2) compare the Object instances using reflection

                       

                      You might wonder why 2 is needed? Well, your helper also needs to be visible to the classloader of JBossCachedAuthenticationManager. When Byteman tries to resolve the name in the HELPER clause it does so by calling classForName on the taregt clas's loader. If you want to a helper to be visible then you normally put it in the bootstrap/system path. You could also bundle it in the module for JBossCachedAuthenticationManager or i a parent moduel that does not hide it from its child modules. However, neither of those will solve the problem that KeycloakUndertowAccount is not visible because it is in a different, unrrelated JBoss Module.

                       

                      So, if the helper wants to inspect the two credentials and see if they are 'equal' in some way that relates to the data contained in their fields then it will have to do so using reflection to access that field data. Depending on what the data is and the accessibility of the fields this may be easy or it may be tricky. If you can identify a suitable way of establishing equality I might be able to help work out how to implement a Helper equals method. Let me know if you want any advice.

                       

                      regards,

                       

                       

                      Andrew Dinn