Bug 844673

Summary: JON CP test connection not working with valid credentials
Product: [Other] RHQ Project Reporter: Simeon Pinder <spinder>
Component: ContentAssignee: Simeon Pinder <spinder>
Status: CLOSED CURRENTRELEASE QA Contact: Mike Foley <mfoley>
Severity: high Docs Contact:
Priority: urgent    
Version: unspecifiedCC: hrupp, jsanda, myarboro, skondkar
Target Milestone: CR02   
Target Release: JON 3.1.1   
Hardware: Unspecified   
OS: Unspecified   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 840512 Environment:
Last Closed: 2013-09-03 11:01:42 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 840512    
Bug Blocks: 846021    
Description Flags
Stack_trace none

Description Simeon Pinder 2012-07-31 07:13:49 EDT
+++ This bug was initially created as a clone of Bug #840512 +++

Description of problem:
Install JON  3.1.0.GA and insert valid CSP credentials, the try to 'Test Connection' or 'Synchronize'. In both cases the UI displays an error message as the valid credentials are not being accepted.

Test failed - failed to connect to the remote repository for [JBoss CP Patch Feed] - check the configuration and make sure the remote repository is up and reachable. Details: java.lang.reflect.InvocationTargetException:null -> org.rhq.enterprise.server.plugin.pc.content.SyncException:Invalid login credentials specified for user [spinder]. Make sure this user has an active account at the CSP and that the password is correct.  

Version-Release number of selected component (if applicable):

How reproducible:
Every time.

Steps to Reproduce:
1. Install fresh JON 3.1.0.GA instance and log into the server.
2. Navigate to Administration/Content/Content Sources and update the default connection with valid CSP credentials.
3. Select the Content Source and attemt to 'Test Connection' or 'Synchronize'.
Actual results:
Failed connection due to invalid credentials.

Expected results:
Valid test results or synchronization.

Additional info:

--- Additional comment from spinder@redhat.com on 2012-07-16 10:14:28 EDT ---

I've narrowed this down to an issue with obfuscation. The problem is visible with the JON 3.1.0.GA release, but does not appear to be in master or in an RHQ instance built off of the JON 3.1.0.GA tag.

The password shows up correctly obfuscated in the database, but is not being correctly unobfuscated for some reason. The relevant JPA annotations applied to ObfuscatedPropertySimple seem to be correct and valid for the non-GA builds so it is still uncertain why this is happening. 

The JON 3.1.0.GA release was not signed so it does not appear that signing has anything to do with this issue.

--- Additional comment from spinder@redhat.com on 2012-07-16 10:51:28 EDT ---

Created attachment 598471 [details]
patch for obfuscation issue with content source.

--- Additional comment from spinder@redhat.com on 2012-07-16 15:55:14 EDT ---

Created attachment 598520 [details]
Updated patch.

--- Additional comment from spinder@redhat.com on 2012-07-16 17:57:34 EDT ---

Pushed to master 39e9490178. This will need to be cherrypicked to the release branch for JON 3.1.1 and for hotfixes pending GSS/Project management input.

--- Additional comment from spinder@redhat.com on 2012-07-17 09:25:43 EDT ---

Note to QA. This has only manifested in JON builds so we'll need another JON build to QA/verify this.

--- Additional comment from lkrejci@redhat.com on 2012-07-19 10:57:49 EDT ---

Created attachment 599182 [details]
Patch for ObfuscatedPropertySimple

I'm attaching a patch for ObfuscatedPropertySimple that uses a different approach to transparent obfuscation by introducing a transient "clear text" field that acts as the value in memory while letting Hibernate persist the obfuscated "stringValue".

The reason why the previous approach didn't work, I think, is that by changing the value of the persisted field in @Post* method, we effectively made Hibernate mark the property as dirty on each load.

Now, during a simple fetch, Hibernate optionally flushes all the dirty fields, which caused the former @PreUpdate to trigger. But because the fetch (I believe) was not enclosed in a transaction, the @PostUpdate never ran (I believe it runs only during commit to actually update), which left our obfuscated properties with the obfuscated value in memory (which was unexpected by the code).

--- Additional comment from spinder@redhat.com on 2012-07-21 19:01:49 EDT ---

As mentioned by Lukas, we finally figured out what was behind the obfuscation problems. The issue was that the initial implementation for property obfuscation  was based on symmetric annotations (Pre/PostPersist,Pre/PostUpdate) and PostLoad to obfuscate and deobfuscate passwords(always write to db obfuscated, but in memory unobfuscated).  

With a normal load of a Content Source there'd be PostLoad -> PreUpdate -> PostUpdate lifecycle calls as the property value would be cycled from obfuscated to clear text and back and forth. 

However depending upon this persistence lifecycle mechanism, which itself caused additional persistence calls, caused a couple issues:
i)When updating an existing content source you'd go through the PostLoad->PreUpdate->PostUpdate three times. It was consistent but redundant.
ii)On reboot, when there was no commit occurring(but the Content Sources were loaded as part of the Content Provider initialization), the PostUpdate call never ran(See above for Lukas's explanation). 

The solution was to re-implement the Obfuscation logic to:
- do a lot less translation from clear-text <-> obfuscated 
- do away with the usage of symmetric hibernate lifecyle events 
- to always persist the obfuscated string to db but keep transient clear-text version  
- modify the construction/copy logic to handle obfuscation and clear-text content creation

--- Additional comment from spinder@redhat.com on 2012-07-21 19:04:01 EDT ---

Added a fix to the latest ObfuscatedPropertySimple patch attached and added better protection against the situation where unobfuscated passwords may have been persisted to the database because of this bug.  

This is now fixed in master with commit: 0170a5ac909fd412

The fix when applied to an installed JON 3.1.0.GA instance will require the users to manually reset the credentials on their content source instances. This is necessary because there is no way for us to reliably detect whether the existing property was obfuscated or not before this patch was applied.

The fix was reviewed by Lukas and I and additionally I tested the change in master and on a JON 3.1.0.GA instance.  The worst case scenario where a password was persisted in unobfuscated form was also tested. In such a situation we log a message about the offending property but this only a WARN as the credential reset(last step of patch) with properly correct the state.

Leaving this in ON_DEV pending a little more review by Charles.

--- Additional comment from spinder@redhat.com on 2012-07-21 19:20:36 EDT ---

Created attachment 599552 [details]
Obfuscation logging captures.
Comment 1 Simeon Pinder 2012-08-06 10:45:58 EDT
See original comment(https://bugzilla.redhat.com/show_bug.cgi?id=840512#c12) for original test description:

We need to test the CSP content source we ship with works in all fresh installs and upgrades: e.g.
-fresh install of jon311: setup works ok
-content source not setup in jon310, upgraded to jon311: setup works ok
-content source setup and working in jon310, upgraded to jon311: continues to work
-content source setup and working in jon301, upgraded to jon311: continues to work
Comment 2 Simeon Pinder 2012-08-06 11:45:40 EDT
This has been applied to 3.1.1 branch with commit: ebdb6f0e7f9d

Moving this to MODIFIED for testing with next JON binary.
Comment 3 John Sanda 2012-08-13 22:16:29 EDT
Moving to ON_QA since JON 3.1.1 ER2 build is availble - https://brewweb.devel.redhat.com/buildinfo?buildID=228250
Comment 4 Sunil Kondkar 2012-08-29 08:37:13 EDT
Tested on ON 3.1.1.ER3 build.

I tried below steps on fresh install of JON 3.1.1.ER3 build and getting an error in server log.

1. Fresh Install jon311 ( JON 3.1.1.ER3 build )
2. Navigate to Administration->Content-> Content Sources
3. Click on the link 'JBoss CP Patch Feed'
4. Click on button 'Edit'.
5. Lazy Load : True
6. Download Mode : database
7. Default Feed URL:
8. Enter CSP username and password
9. Active : Yes
10. Click on button 'Save'
11. Observe the server log.

 - Attaching the error I got in server log.

 - Environment Details:

OS: Fedora release 13 (Goddard)
Build: Version: 3.1.1.ER3
Build Number: 37108ca:67c6de8
Database: PostgreSQL 8.4.2
Browser: Firefox 10.0.2
Java Version: jre1.7.0_05

 - Clicking on 'Test Connection' button displays below in UI:

Test failed - failed to connect to the remote repository for [JBoss CP Patch Feed] - check the configuration and make sure the remote repository is up and reachable. Details: java.lang.reflect.InvocationTargetException:null -> org.rhq.enterprise.server.plugin.pc.content.SyncException:Invalid login credentials specified for user [skondkar1@redhat.com]. Make sure this user has an active account at the CSP and that the password is correct. 

 - Tried testing by changing the Feed URL as below and observed the same error in server log.


 - I tried to test whether my credentials are working. I entered below URL in browser and entered the credentials, observed that it works in browser.

Also tested on Oracle 11g and observed the same behaviour.
Comment 5 Sunil Kondkar 2012-08-29 08:40:51 EDT
Created attachment 607896 [details]
Comment 6 Sunil Kondkar 2012-08-29 10:07:26 EDT
Worked with simeon on this. Below are some observations:

The table 'rhq_config_property' has 'dtype' values - 'obfuscated' for names 'Password' and 'proxypassword'.

The string_value is:  26e461e889bd1b1d207a6df87216de44

Deleted the content source 'JBoss CP Patch Feed' and then created a new one with the same name using 'JBoss Patch Content Source' type.
After clicking 'Save', the server log displayed stack trace. Attaching the stack trace.

After that I clicked 'TEST CONNECTION' button displayed 'Test passed - the remote repository for [JBoss CP Patch Feed] is available.' in UI.
Comment 7 Sunil Kondkar 2012-08-29 10:08:20 EDT
Created attachment 607917 [details]
Comment 8 Simeon Pinder 2012-08-30 09:34:34 EDT
After quite a bit of investigation, this most recent failure to authenticate is caused by an odd path to the construction of the ObfuscatedPropertySimple happening at startup/installation.  The initial property is instantiated by reflection which bypasses the constructors that correctly handle obfuscation+clear text creation.

If you reboot the JON server before attempting to test the Content Sources then the Obfuscated property is correctly loaded and testing/authentication happens correctly.

The fix is to add one extra check for clearText initialization.  I'm still doing some more testing on this to see if I can see if there are any more edge cases to be aware of. Will probably ping Lukas as well to have an extra pair of eyes on this.
Comment 9 Simeon Pinder 2012-08-31 08:42:29 EDT
With the earlier fix, incorrect usage of the pre/postUpdate and Load annotations were combined with odd behavior from Hibernate to cause unreliability within the obfuscation implementation.  This time around, the same Hibernate unreliability, causes a slightly different issue that only shows up until a server reboot.

The fix is to add a lazy check to initClearTextValue, when the  obfuscated property is read.  This costs nothing, defends against any situation where the @PostLoad annotation is not respected at initialization or any other time and is a very precise fix for this issue.

To reiterate, the earlier changes fixed the obfuscation from being affected by inconsistent behavior and only uses the PostLoad to load clearText passwords in memory only. This fix ensures that clearTextPasswords are always loaded when they're needed.  

This has been fixed and pushed to master with commit: 1293a88ef64918b6401bd

I've tested this fix on a patched JON 3.1.1.ER2 and it works well and reliably.

This needs to be also checked into the release branch to complete the fix for this issue.  Leaving this in the ON_DEV state to get signoff from charles and mfoley.
Comment 10 Charles Crouch 2012-08-31 10:45:40 EDT
I'm +1 on this for jon311
Comment 11 Simeon Pinder 2012-08-31 11:29:59 EDT
This is cherry-picked to release branch with commit: 0d0f22bd7f7e224945ac5da

Moving this to MODIFIED for inclusion and retesting with next binary build. Likely 3.1.1.CR2
Comment 12 John Sanda 2012-09-06 01:27:57 EDT
CR2 build is available at https://brewweb.devel.redhat.com/buildinfo?buildID=232185. Moving to ON_QA.
Comment 13 Sunil Kondkar 2012-09-13 09:40:01 EDT
Verified on JON 3.1.1.CR2 fresh install. Verified with 'Test Connection' and 'Synchronize'. Also verified below upgrade scenarios:

-content source not setup in JON310, upgraded to JON311CR2: setup works ok.
-content source setup which was not working in JON310, upgraded to JON311CR2: 'Test Connection' and 'Synchronize' works ok.
-content source setup and working in JON301, upgraded to JON311CR2: continues to work.
Comment 14 Heiko W. Rupp 2013-09-03 11:01:42 EDT
Bulk closing of old issues in VERIFIED state.