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

Add ockam sub command to rotate identity keys #3685

Open
mrinalwadhwa opened this issue Oct 19, 2022 · 9 comments
Open

Add ockam sub command to rotate identity keys #3685

mrinalwadhwa opened this issue Oct 19, 2022 · 9 comments

Comments

@mrinalwadhwa
Copy link
Member

mrinalwadhwa commented Oct 19, 2022

Currently

The following will create and identity

ockam node create n1
ockam identity show --node n1

The code for it is

Desired

It is possible to rotate identity keys
https://github.com/build-trust/ockam/blob/develop/implementations/rust/ockam/ockam_identity/src/identity.rs#L181

We want to support this as a command.


We love helping new contributors! ❤️
If you have questions or need help as you explore, please join us on Discord. If you're looking for other issues to contribute to, please checkout our good first issues.

@noyez
Copy link
Contributor

noyez commented Oct 20, 2022

Hello -- Do you envision the CLI to be something along the lines of ockam identity rotate --node n1 --key key-label? Other ideas:

  • have rotate-key be a subcommand of ockam node ...
  • have roate-key be a top-level command, like ockam rotate ...

@mrinalwadhwa
Copy link
Member Author

@noyez Thank you for looking into this!!
I was thinking of something like this:

ockam identity rotate --node n1 --key key-label

@noyez
Copy link
Contributor

noyez commented Oct 26, 2022

Hello -- I've been looking at this issue and I have a few questions.

  • What is the end result of rotating the keys? should I be seeing a different set of keys with the ockam identity show --full output? Or is the only observable effect in the default_value.json file?
  • To rotate the keys, it seems I need to create an Identity struct. I think i can piece that together by getting the IdentityChangeHistory, and the Vault from the ~/.config/ockam-cli/default_vault.json. But there appears to be done two different ways to create the Identity struct.
    1. Identity::new( )
    2. IdentityBuilder::build( )

Is one preferred over the other? It seems that IdentityBuilder::build() does a lot more then eventually calls Identity::new().

Thanks for any input and clarifications!

@SanjoDeundiak
Copy link
Member

Hey @noyez . You don't need to create a new Identity, we want to be able to rotate a key inside an already existed Identity instance. That instance can be found in NodeManager struct. New key will be automatically added to the Vault. But Identity itself should be resaved to the NodeConfig. Do not hesitate to ask more questions if you need

@noyez
Copy link
Contributor

noyez commented Nov 4, 2022

Hey @noyez . You don't need to create a new Identity, we want to be able to rotate a key inside an already existed Identity instance.

Got it. i was hoping my code path was loading the existing identity and not creating a new one, but it probably was creating a new one.

That instance can be found in NodeManager struct. New key will be automatically added to the Vault. But Identity itself should be resaved to the NodeConfig.

Please correct me if i'm wrong in my understanding. There is a NodeManager and that gets paired with NodeManagerWorker and for those sturcts to be useful, they need to be "started". However in order to start NodeManager, it gets consumed by NodeManagerWorker, and that gets consumed by start_worker. So once NodeManager and NodeManagerWorker are started they appear to be inaccessible since they are consumed. (I'm using start_embeded_node() as a bases for my assumptions.)

So to access the data from NodeManager should i be getting it from the node service somehow or with the messaging API?

Note that when i try creating a stand-alone NodeManager (i.e. w/o "starting" it) and calling rotate_key() on the identity, it hangs here https://github.com/build-trust/ockam/blob/develop/implementations/rust/ockam/ockam_identity/src/identity.rs#L149 when waiting on the await part of the change_history member. That experience coupled with the start_embeded_node function pushed me to the conclusion that the NodeManager and NodeManagerWorker needed to be "started". But i can't figure out how to access the NodeManager once it has been "started".

Please advise, thanks!

@SanjoDeundiak
Copy link
Member

SanjoDeundiak commented Nov 16, 2022

@noyez yes, NodeManagerWorker is a worker, it's running asynchronously in the background. In this case you don't need to access its internal state from the outside, because rotate command should be handled by the NodeManagerWorker itself. Please explore how Identity create command is handled as an example:
Request from the CLI is sent here, that request is handled here

@noyez
Copy link
Contributor

noyez commented Nov 17, 2022

@SanjoDeundiak ooooooh, of course! thank you! 🤦 Your last comment resonated and really helped me comprehend the interactions, and now it seems so simple.

I have a PR Draft (#3841). It is marked as a Draft because i'm observing errors when rotating a key only after identity show commands are run. For example, if i spin up a fresh node, then rotate keys, it works. But if i spin up a fresh node and ask for ockam identity show --node n1, then rotate key, i get an error. I'm not sure its related to the PR, but its worth mentioning. I added a test in the command.bats file specifically for this error since it probably shouldn't be failing so it is worth testing, so in my PR that test will fail.

Just a brief look at the flow and what works and what errors:

  ockam identity rotate-keys --label OCKAM_RK --node n1    # works
  ockam message send hello --to /node/n1/service/uppercase
  ockam identity rotate-keys --label OCKAM_RK --node n1    # works
  ockam node list
  ockam identity rotate-keys --label OCKAM_RK --node n1    # works
  ockam identity show --node n1
  ockam identity rotate-keys --label OCKAM_RK --node n1    # fails (and any rotate-key command thereafter fails)

Below is a more detailed log of what's happening:

Ockam Commands
ubuntu-myth-vm :: rust/ockam » ockam identity rotate-keys --label OCKAM_RK --node n1 
key rotated!
ubuntu-myth-vm :: rust/ockam » ockam identity rotate-keys --label OCKAM_RK --node n1
key rotated!
ubuntu-myth-vm :: rust/ockam » ockam identity show --node n1 --full
Change History:
  Change[0]:
    identifier: cdb5565163e5b1278eb31e6dbd213066e335da0c3e5d8ffed3789ce130523391
    change:
      prev_change_identifier: 0547c93239ba3d818ec26c9cdadd2a35cbdf1fa3b6d1a731e06164b1079fb7b8
      label:        OCKAM_RK
      public_key:   Ed25519 e7417e5ea17b05684cb56171f6f37ac92d37a03587fa43f66663bfda878a1322
    signatures:
      [0]: SelfSign 5e48f7a133ac1d9218f9fb7185cf890af6bbe9ca5ca58a7417710be2f1929549483827ac9e34544379610e29e1806778a9ae9204bf7fbc8adbc00138b2756a09
  Change[1]:
    identifier: 333b5257fae967883712100e44c1821a3b68584385c846af36d1f332e40425ca
    change:
      prev_change_identifier: cdb5565163e5b1278eb31e6dbd213066e335da0c3e5d8ffed3789ce130523391
      label:        OCKAM_RK
      public_key:   Ed25519 b1c63ab2af7d1e7353455024d2bef3314d94c842ce2035eed83476ca5490262b
    signatures:
      [0]: SelfSign 8ed727c43a15e968e3b70a7c9b14e0165e83406486e0238e8e7017f4be697cf2bda48bd6de95726eadc57f37ace52a918f36db498cc0792275cdb3a80cd81b08
      [1]: RootSign 15e3433e4f0c09d6d82e48e833ce1585ae076152680b686be414c0060c5541a858556e1fb944faa7583cf029ed40a2ac747bcc0fdb43099a99378b7891299c0f
      [2]: PrevSign 15e3433e4f0c09d6d82e48e833ce1585ae076152680b686be414c0060c5541a858556e1fb944faa7583cf029ed40a2ac747bcc0fdb43099a99378b7891299c0f
  Change[2]:
    identifier: 8b479f264cf60e89c5c41acc5213b8a0eb183d65d243c4c51ddaf115d1c8fbd3
    change:
      prev_change_identifier: 333b5257fae967883712100e44c1821a3b68584385c846af36d1f332e40425ca
      label:        OCKAM_RK
      public_key:   Ed25519 f0145720328890379550e40ac04765e84c9bed3edb091b0364b921427cdfb6e3
    signatures:
      [0]: SelfSign a68bfadabf235c9761cdede375757211baa7a1c7ad2c564fdfaf014a7da81b70e7aa8ad0ce2224a86b39610a808ba2277b701acfcdea6e5a2eba409d4ca14f0e
      [1]: RootSign 62d20fe9fab175b1d53e318a0eecf8a24c63642a1f446d234766061ceb4ef561632185ee17391d319196161e34babfea4a279654a325f3ab0a4e1bead662920a
      [2]: PrevSign 62d20fe9fab175b1d53e318a0eecf8a24c63642a1f446d234766061ceb4ef561632185ee17391d319196161e34babfea4a279654a325f3ab0a4e1bead662920a

ubuntu-myth-vm :: rust/ockam » ockam identity rotate-keys --label OCKAM_RK --node n1
Version 0.77.0, hash: 5e5a9ddc557daa2e5183d83fb95a821062c2efcf
An error occurred while processing the request. Status code: 500 InternalServerError. Message: failed to handle request: invalid storage
Ockam Service Output The relevant service log:
2022-11-17T17:59:47.336956Z DEBUG ockam_api::nodemanager::service: request id=29b088fc method=Some(Post) path=/node/identity/actions/rotate_key body=true
2022-11-17T17:59:49.228109Z ERROR ockam_api::nodemanager::service: failed to handle request re=29b088fc method=Some(Post) path=/node/identity/actions/rotate_key code=[Origin::Vault; Kind::Invalid] cause=Some(StorageError)
2022-11-17T17:59:49.228380Z DEBUG ockam_api::nodemanager::service: responding re=29b088fc method=Some(Post) path=/node/identity/actions/rotate_key

@saputkin
Copy link
Contributor

saputkin commented May 3, 2023

@noyez are you still working on this one? if not I would like to help :)

@noyez
Copy link
Contributor

noyez commented May 5, 2023

New PR for this issue: #4876. Previous bug mentioned above appears to be fixed, although i have not done extensive testing.

@mrinalwadhwa mrinalwadhwa removed the hacktoberfest Apply to issues you want contributors to help with label Sep 24, 2023
@nazmulidris nazmulidris added hacktoberfest Apply to issues you want contributors to help with Type: User Experience labels Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants