Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nitin-vavdiya opened this issue May 1, 2024 · 2 comments

Comments

@nitin-vavdiya
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

Thanks @borisrizov-zf for checking this out.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants