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

Key rotation operations #525

Merged

Conversation

sklump
Copy link
Contributor

@sklump sklump commented May 26, 2020

Please have a look. I think:

  • the public DID key rotation should hang off the ledger API - it's defensive design against an off-line agent trying to the rotate key pair for a DID that's public. Ledgers need wallets but wallets don't need ledgers; the operation initializes at the wallet, updates the ledger, then returns to the wallet to apply the update, and it uses the same wallet operations as for local DID key pair rotation below.
  • the local DID key pair rotation should hang off the wallet because it only needs the wallet. There are two calls: start and apply; the routes.py calls one and then the other. The wallet needs to retain both calls to accommodate public DID key pair rotation as above.
  • the wallet master encryption key rotation is a provision operation. Could you look this over in particular? On some level I can't believe it's this simple.

I'm open to the idea of renaming stuff in routes.py for ledger, wallet. I was toying with the idea of 'nym' instead of 'public-did' for symmetry, but we've used public-did everywhere else to refer to public DIDs, and properly the keys belong to the DID not the NYM.

@sklump sklump requested review from nrempel and andrewwhitehead May 26, 2020 15:18
@codecov-commenter
Copy link

codecov-commenter commented May 26, 2020

Codecov Report

Merging #525 into master will increase coverage by 0.03%.
The diff coverage is 99.28%.

@@            Coverage Diff             @@
##           master     #525      +/-   ##
==========================================
+ Coverage   96.21%   96.24%   +0.03%     
==========================================
  Files         237      237              
  Lines       12202    12332     +130     
==========================================
+ Hits        11740    11869     +129     
- Misses        462      463       +1     

@andrewwhitehead
Copy link
Contributor

The wallet rekey isn't working for me so far, here's what I'm trying:

docker run --rm -ti --entrypoint bash aries-cloudagent-run

aca-py provision --wallet-type indy --wallet-key ORIG_KEY --wallet-name test --seed 0000000000000000000000000000Test

aca-py start --wallet-type indy --wallet-key ORIG_KEY --wallet-name test -it http 0.0.0.0 8000 -ot http

aca-py provision --wallet-type indy --wallet-key ORIG_KEY --wallet-name test --wallet-rekey NEW_KEY

# fails
aca-py start --wallet-type indy --wallet-key NEW_KEY --wallet-name test -it http 0.0.0.0 8000 -ot http

# succeeds
aca-py start --wallet-type indy --wallet-key ORIG_KEY --wallet-name test -it http 0.0.0.0 8000 -ot http

I think there should be some output if it has performed a wallet rekey, but I'm not seeing anything.

@sklump
Copy link
Contributor Author

sklump commented May 28, 2020

Thanks - I'm working on it

…ccess credentials in case of deletion

Signed-off-by: sklump <[email protected]>
@andrewwhitehead andrewwhitehead merged commit b5f733e into openwallet-foundation:master May 28, 2020
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

Successfully merging this pull request may close these issues.

4 participants