aries-cloudagent-python icon indicating copy to clipboard operation
aries-cloudagent-python copied to clipboard

When running version 0.12.0, an error occurs if --wallet-key flag is missing

Open kukgini opened this issue 1 year ago • 3 comments

I'm tying to upgrade aca-py from 0.8.2 to 0.12.1

Before release 0.12.0, the --wallet-key parameter was not required on askar wallet type (as far as i know, it is required only on indy wallet type). However, it is required on version 0.12.0.

I see that the change was made by Ian Contanzo in 12/5/23 but not described in Breaking Changes. (commit hash: bfa0a5ec91ad18f2e9cddc3bdf876908dc271cc0)

I didn't set the parameter before, but instead set the --wallet-key-derivation-method to "kdf:argon2i:mod". So, I guess it was set to DEFAULT_KEY

kukgini avatar May 31 '24 00:05 kukgini

However, I realized that my service was set DEFAULT_KEY which is empty string because I didn't set --wallet-key flag.

IMHO, The reason why DEFAULT_KEY cannot be used when --wallet-key-derivation-method is RAW is because the wallet key must be 32 bits. And if it is set to Argon KDF, even if it is an empty string, it can be used as a wallet key because it is converted to 32bit. So, It seems like the restriction is technical reason.

However, In any cases, I think It is nessicery to set --wallet-key for persistance storage for security reason. So, basically I think your (@ianco) change is correct. The reason that the original source code requires a wallet-key only for indy is probably to distinguish it from in-memory. So, The mistake have been made when askar was firstly added.

The problem is that it seems like rekey feature is blocked, I can't change my current key which is empty string to another one.

kukgini avatar May 31 '24 01:05 kukgini

I think the --rekey parameter needs to support the use case where the existing key is blank (the DEFAULT_KEY).

As to whether --wallet-key must be provided for askar wallets I thought it was a requirement, I didn't realize there was a DEFAULT_KEY value available.

I'm going to tag @swcurran and @andrewwhitehead on this issue for their input. At the very least we need to support the 0.8.2 to 0.12.1 upgrade

ianco avatar May 31 '24 15:05 ianco

I don’t quite get the nuances/interactions of the options, but from the sounds of it we need to do three things:

  1. Make the —rekey update as per @kukgini / @ianco agreement.
  2. Update the breaking changes documentation in the release notes, and perhaps call out the issue further in the https://aca-py.org/latest/deploying/UpgradingACA-Py/ documentation.
  3. Get a 0.12.2 release out there that is a patch on 0.12.1 (not based on latest main).

Does that make sense?

swcurran avatar May 31 '24 16:05 swcurran