symmetric encryption fixed

[jeugregg]

security issue

Fixes # .

Changes proposed in this PR:
I was trying to test some function in the repo :
https://github.com/oceanprotocol/ocean.py/blob/main/ocean_lib/ocean/crypto.py

I wanted to test symmetric encryption and test the result with wrong key...
And all keys works !

if you look at the code of calc_symkey at line 21 and 22 :
hash_b = sha256(base_b)
symkey_b = b64encode(str(hash_b).encode("ascii"))[:43] + b"=" # bytes

Actually the result of "str(hash_b)" is not a hash but the string representation of the hash object.
e.g. : <sha256 _hashlib.HASH object @ 0x11d3aaa30>

To have a hash, you need to just replace line 21 by :
hash_b = sha256(base_b).hexdigest()

e.g. : eadd5118ccd09b09649ff613c3cb457c10f686187d7c26103dadb64772f8b03b

With the current code, I think that all symmetric key with calc_symkey give the same encryption !

I have tried, and I can decrypt with any symmetric key issued with calc_symkey something encrypted.

[trentmc]

Thanks for taking a look at this code.

With these changes, are the unit tests passing?

Why do you say "fixed"? It worked before; the unit tests passed. It looks more like a refactor, specifically a code cleanup. (Not a "fix".)

Thanks,

[jeugregg]

Like I said, I think a new test is needed : "I wanted to test symmetric encryption and test the result with wrong key. And all keys works !"
What I mean here : "You can use any key, it will work" and it is NOT GOOD AT ALL!
Consequently, if someone use a key1 to symmetric encrypt something, actually, the current code is always encrypt with the same key0.
And if someone use the code to decrypt, with a random key2, actually it will decrypt with key0 and can read any encrypted message with this code.

So to be shorter, the code always encrypts with a key0 (a fixed absolute key) and always decrypt with it, and even with any key as input ! No message can be protected !

A new test is to check if the decrypt function is not working with a wrong key.

Possible test :

sym_key = calc_symkey("123")
wrong_sym_key = calc_symkey("456")
assert wrong_sym_key != sym_key, "NOK : wrong_sym_key is the same as sym_key"

I have updated the crypto test

[trentmc]

Thank you for the thorough response. We will review the code in detail, aiming for the next 24 hours. Best,

[calina-c]

I think in line 22, we no longer need the str cast, since hash_b is already a string

[calina-c]

@jeugregg I added some fixes wrt. the undefined variables in the test. @trentmc I'm fine with merging this fix as long as everyone is content with the result.

Note that the remaining tests are failing due to permission on our private keys for external contributors, so totally unrelated.

[trentmc]

@trentmc I'm fine with merging this fix as long as everyone is content with the result.

OK go ahead and merge it. Thanks!