feat: migration pre-upgrade script #104
Conversation
Signed-off-by: blu3beri <blu3beri@proton.me>
src/migration/mod.rs
Outdated
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
This test only works locally, so I will remove it when we merge it or we need to find a way to create a simple indy-sdk wallet.
There was a problem hiding this comment.
Maybe you can push your local indy-wallet and use that to run the tests?
Then we ofc. need to have a way regenerate it in some way, but I don't suspect we'd need to change the input very often
There was a problem hiding this comment.
Yeah I think that would be fine.
|
How does this work compare/relate to the work being done in the hyperledger/aries-acapy-wallet-upgrade project? |
There is some overlap between the two. For the migration we have to do a generic (non-(aca-py|afj|af-go)-specific) conversion. This mainly converting the data to the new tables, but not the record transformation. The record transformation is dependent upon the framework it will be used in, so that part is not implemented here and still has to be done in the framework itself. The script from aca-py also handles postgres wallets and multi-tenancy properly, this one does not. However, when those fixes are applied in the future, the aca-py script can depend on this function for the preprocessing. Hope that helps! |
src/migration/mod.rs
Outdated
| } | ||
|
|
||
| impl IndySdkToAriesAskarMigration { | ||
| pub async fn new(spec_uri: &str, wallet_name: &str, wallet_key: &str) -> Result<Self, Error> { |
There was a problem hiding this comment.
We probably need to add support for the keyDerivationMethod here?
There was a problem hiding this comment.
Yeah I had that in the back of my head the whole time. Let me check if the python script has some logic for this.
EDIT: python script does not have any logic for this. Time to check the indy-sdk.
There was a problem hiding this comment.
If you can use the Askar key derivcation logic for this, we have this mapping in AFJ. Left is indy, right is Askar
const correspondenceTable = {
[KeyDerivationMethod.Raw]: StoreKeyMethod.Raw,
[KeyDerivationMethod.Argon2IInt]: `${StoreKeyMethod.Kdf}:argon2i:int`,
[KeyDerivationMethod.Argon2IMod]: `${StoreKeyMethod.Kdf}:argon2i:mod`,
}
There was a problem hiding this comment.
export enum KeyDerivationMethod {
/** default value in indy-sdk. Will be used when no value is provided */
Argon2IMod = 'ARGON2I_MOD',
/** less secure, but faster */
Argon2IInt = 'ARGON2I_INT',
/** raw wallet master key */
Raw = 'RAW',
}
There was a problem hiding this comment.
export declare enum StoreKeyMethod {
Raw = "raw",
Kdf = "kdf",
None = "none"
}
There was a problem hiding this comment.
Oh thanks! Yeah I try my best to use askar existing methods. I got some working now with custom kdf, but I will transform it now using askar existing functionality.
There was a problem hiding this comment.
Latest commit adds argon2i MOD and INT and also RAW.
The tests are also included now and use the RAW method to make it a lot quicker.
Signed-off-by: blu3beri <blu3beri@proton.me>
Signed-off-by: blu3beri <blu3beri@proton.me>
Signed-off-by: blu3beri <blu3beri@proton.me>
|
@andrewwhitehead There might be some duplicate code (related to kdf and more), but I wanted to modify as little of the rest of the library as possible. This means that removing the script would revert perfectly back to the state before the script without having to remove some code in specific places. If there is some functionality I can use from askar, without changing anything, let me know. |
|
Running into some issues with the tags. Shouldn't merge this yet. |
|
I'm working on a few updates for this as well |
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
Upgrade updates
|
@andrewwhitehead I merged your PR. Just one question about the name removal of |
|
@blu3beri I removed the |
Signed-off-by: Andrew Whitehead <cywolf@gmail.com>
|
#89 may need to be merged first to fix the test failure. |
|
I'm not sure why the sqlite rekey test seems to be failing sometimes on Linux. It's running into a database locked error when provisioning a new database with a random filename. It's not related to these changes. |
Co-authored-by: Andrew Whitehead <cywolf@gmail.com> Signed-off-by: blu3beri <blu3beri@proton.me>
I added the migration but not the conversion script to aries-askar itself.
It is added as a feature (default right now, but can be removed). After a while we can probably remove it from the default features as most people have migrated, but then it can always be opted-in of course.
I was working on the migration script for React Native Sqlite wallets and I was running into a lof of issues regarding sqlite and cryptography dependencies. Every sqlite dependency came with a list of issues and it since we can not patch them (as the sqlite dependency has to be a direct dependency of the wallet), it seemed unfeasible.
I tried to depend on the values / structs / functions from aries-askar itself as much as possible, but I might have created some duplicate code. @andrewwhitehead you can probably spot this a lot better than me here.
The python script could also depend on this function, but it is not required.
The script also only supports indy-sdk sqlite -> aries-askar sqlite. There is no additional support for multiple wallets (although the script can be be called multiple times for this to work, however untested). Postgres has been omitted for now as it did not fit the direct use case. I don't think that adding support for Postgres would be all too complex.
Curious to hear if there are any objections to this PR or if there is a very specific reason why it was not done for the Python script.
cc: @TimoGlastra @andrewwhitehead @swcurran
NOTE: It does not really have a test, yet. I would have to create an indy-sdk wallet and I am not too sure what the quickest way is to do that.