Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Support custom keyfile in Sabre CLIs #85

Merged
merged 5 commits into from
Apr 2, 2021

Conversation

arsulegai
Copy link
Contributor

@arsulegai arsulegai commented Nov 10, 2019

  1. Add support for keyfile (relative/absolute) path read from the CLI.
  2. Update deprecated env with dirs, to get the user's home directory
  3. Add IDE files in git ignore list.

Copy link
Contributor

@vaporos vaporos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current approach, you can specify the key very simply with "-k first_key" and "-k second_key" which is useful for quickly swapping between different keys. This expand to $HOME/.sawtooth/first_key.priv and $HOME/.sawtooth/second_key.priv.

We should probably keep this behavior.

I think we could additionally support the argument for -k being a filename and a path. We would then have three options for how the argument for -k is to be considered:

a) the short "name" which translates to $HOME/.sawtooth/{K}.priv
b) a filename which translates to $HOME/.sawtooth/{K}
c) a path which translates to just {K}

I think the best way to implement this is to first check for "/" in K; if so, then it is (c). Next, we construct the path for (a) and see if it exists on the filesystem; if it does, use it. If it doesn't, then we construct (b) and look for it on the filesystem; if it does, use it. If none of these work out, then we were unable to load the key because it is not found.

Thoughts?

@arsulegai
Copy link
Contributor Author

first_key or second_key has assumptions, such that there is expectation on the place where keys are stored, the filename format of the keys. This works out if the sawtooth CLI or any other CLI used for keygen behaved the same.

The use case where user do not prefer to store the keys in $HOME is now forced to save all the keys in that directory. Another use case is that it could be possible that somebody writes an automation script to perform some of these tasks and that requires the script and the keys be packaged in some other directory.

So, we definitely need an option to read the keyfile path from the CLI.

Most common tools (even the ones in Hyperledger Sawtooth) have used the option -k to specify the keyfile path and not just the keyfile name. Having multiple definitions of reading an input value could turn to be ambiguous. Accepting the user keys with something like -u would be good. Having said that, I don't really have hard comments on either keeping the same option -k or changing it to something like -u.

What's your take on that?

@vaporos
Copy link
Contributor

vaporos commented Jan 16, 2020

Yes, it has assumptions, that sawtooth CLI keys are stored in $HOME/.sawtooth/..., but that is appropriate for this CLI as long as it is part of Sawtooth.

We really can't change the behavior without reworking documentation and scripts, which makes this a backward compatibility issue. But we can expand upon it.

More generally... Really what we are finding is that we want a separate CLI some_new_prog that is a keygen tool and a place for the keys in $HOME/.some_new_prog/ so it isn't sawtooth agnostic.

@arsulegai
Copy link
Contributor Author

It makes sense to have backward compatibility, I will get to the logic you proposed. Have multiple definition to the use of -k argument.

@arsulegai
Copy link
Contributor Author

In the current approach, you can specify the key very simply with "-k first_key" and "-k second_key" which is useful for quickly swapping between different keys. This expand to $HOME/.sawtooth/first_key.priv and $HOME/.sawtooth/second_key.priv.

We should probably keep this behavior.

I think we could additionally support the argument for -k being a filename and a path. We would then have three options for how the argument for -k is to be considered:

a) the short "name" which translates to $HOME/.sawtooth/{K}.priv
b) a filename which translates to $HOME/.sawtooth/{K}
c) a path which translates to just {K}

I think the best way to implement this is to first check for "/" in K; if so, then it is (c). Next, we construct the path for (a) and see if it exists on the filesystem; if it does, use it. If it doesn't, then we construct (b) and look for it on the filesystem; if it does, use it. If none of these work out, then we were unable to load the key because it is not found.

Thoughts?

Agree, added the logic that follows the explanation you gave to the input parameter.

agunde406
agunde406 previously approved these changes Apr 8, 2020
@agunde406
Copy link
Contributor

@arsulegai Can you rebase this PR to handle merge conflicts.

Signed-off-by: S m, Aruna <aruna.mohan@walmartlabs.com>
1. Support absolute keyfile read with extension other than `.priv`
2. Support relative keyfile read with any file extension.

Signed-off-by: S m, Aruna <aruna.mohan@walmartlabs.com>
Use `dirs` to get the user's home directory, remove currently
used deprecated `env`.

Signed-off-by: S m, Aruna <aruna.mohan@walmartlabs.com>
1. Support absolute keyfile read with extension other than `.priv`
2. Support relative keyfile read with any file extension.

Signed-off-by: S m, Aruna <aruna.mohan@walmartlabs.com>
1. Support absolute keyfile read with extension other than `.priv`
2. Support relative keyfile read with any file extension.

Signed-off-by: S m, Aruna <aruna.mohan@walmartlabs.com>
@agunde406 agunde406 merged commit c7f14a1 into hyperledger-archives:master Apr 2, 2021
@arsulegai arsulegai deleted the cli-keyname branch April 2, 2021 14:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants