eclipse-tractusx / managed-identity-wallet

Apache License 2.0
9 stars 22 forks source link

Enhancement : Encryption algorithms should be used with secure mode and padding scheme #299

Open nitin-vavdiya opened 5 months ago

nitin-vavdiya commented 5 months ago

The issue:

in MIW, we store private keys in the database using AES encryption.

We did not use any padding scheme and secure mode in AES encryption. While scanning this repo with Sonar Cloud, it raises this issue with high severity.

Acceptance criteria

  1. Use AES algorithm with secure mode and padding scheme
  2. Write migration script for existing wallet private keys to be compiled with new AES encryption algorithm if needed. @OSchlienz or @borisrizov-zf Please confirm if we need migration for existing private keys in Catena-X
borisrizov-zf commented 5 months ago

Hi @nitin-vavdiya, thanks for commenting on this issue. It's already known that the AES implementation relies on default, which might change depending on the deployment system. Furthermore we were anticipating to use Vault to store secrets, but have pivoted and decided to keep the database solution for now (various reasons). We've already addressed part of the problem by eliminating defaults and being specific about the type of AES encryption, but without breaking current keys.

The final plan is to use GCM, but for that a larger code update is needed, as we'll have to change the way we store the encrypted data, i.a. we need the initialisation vector value, to be able to decrypt values from the database. Making a migration script is definitely part of the steps required to achieve this.

I'd suggest we schedule this update after the revocation service is part of the repo, so we can have a semi-final version of the code.

nitin-vavdiya commented 5 months ago

Thanks @borisrizov-zf for checking this out.

Sure we can address this issue once we have a basic multi-module application.