Catch exception when creating passwords

[mkanoor]

https://bugzilla.redhat.com/show_bug.cgi?id=1422903

If the password was built using a different key set the password
to nil when we get password errors.

Links

• https://bugzilla.redhat.com/show_bug.cgi?id=1422903

Steps for Testing/QA

Documented in BZ

[gmcculloug]

@mkanoor Was this a change in behavior? I would think having invalid encryption strings should not silently fail.

[Fryguy]

CodeClimate reports: "private (on line 430) does not make singleton methods private. Use private_class_method or private inside a class << self block instead."

Is this intended to be a private method?

[kbrock]

yes, this should be a private method

[nmaludy]

@gmcculloug Yes, in previous versions (CloudForms 4.0) the instantiate call would return a valid object however the password property would be nil.

# instantiate an instance
test = $evm.instantiate('/Configuration/Test/test')
# test is a valid object
password = test.decrypt('password')
# password is nil

In 4.1 this behavior changed. Now, if there is an instance that contains a password field created with a different key, then the object returned by instantiate is nil.

# instantiate an instance
test = $evm.instantiate('/Configuration/Test/test')
# test is nil
password = test.decrypt('password')
# exception is raised because test is nil

[miq-bot]

Checked commits mkanoor/manageiq@676bc7c~...1a22bd5 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍰

[kbrock]

This is good that we are no longer blowing up

But the user should not be storing data in our database encrypted with a different v2_key.
It is a bug with their process.

I'm surprised that this ever worked in the past.
I think the current behavior is reasonable.

Think we need to give them a way to restore from backup.

(also feel we shouldn't be storing the encrypted passwords in the data they export... but that is a different conversation)

[kbrock]

yes, this should be a private method

[gmcculloug]

@kbrock I completely agree. Silently ignoring data that is encrypted with a different key is the wrong approach.

Exporting encrypted data depends on the expected use. You would want to retain it for backup purposes, but not when importing into a different environment.

[mkanoor]

@nmaludy
I tried this with different versions of the product, they all seem to fail the same way as reported by the customer.

[----] E, [2017-03-02T20:23:51.259187 #7329:5cc8fe4] ERROR -- : The following error occurred during method evaluation:
[----] E, [2017-03-02T20:23:51.259627 #7329:5cc8fe4] ERROR -- : NoMethodError: undefined method decrypt' for nil:NilClass [----] E, [2017-03-02T20:23:51.260401 #7329:5cc8fe4] ERROR -- : <AEMethod bztest> [----] E, [2017-03-02T20:23:51.267769 #7329:11fa88c] ERROR -- : Method STDERR: <code: password = test.decrypt('var2')>:7:in

': undefined method `decrypt' for nil:NilClass (NoMethodError)

When the key is different during the initial decrypt for $evm.instantiate we get
OpenSSL::Cipher::CipherError Exception: bad decrypt
Which we translate into our own exception MiqPassword::MiqPasswordError and the instantiate process is aborted and we return a nil.

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[mkanoor]

Not implementing this solution, since the behavior has not changed when the key is incorrect