-
Notifications
You must be signed in to change notification settings - Fork 23
Signing CLI #566
Signing CLI #566
Changes from all commits
ccdf5d2
ee03339
3ad2a63
2a32271
8856b8c
1e10d33
8aa57cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,281 @@ | ||
| package cmd | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/hex" | ||
| "fmt" | ||
| "io/ioutil" | ||
| "os" | ||
| "sync" | ||
| "time" | ||
|
|
||
| "github.com/ipfs/go-log" | ||
|
|
||
| "github.com/keep-network/keep-core/pkg/net" | ||
| "github.com/keep-network/keep-core/pkg/net/key" | ||
| "github.com/keep-network/keep-core/pkg/net/local" | ||
| "github.com/keep-network/keep-ecdsa/pkg/ecdsa" | ||
| "github.com/keep-network/keep-ecdsa/pkg/ecdsa/tss" | ||
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/keep-network/keep-common/pkg/persistence" | ||
| "github.com/keep-network/keep-ecdsa/internal/config" | ||
| "github.com/keep-network/keep-ecdsa/pkg/registry" | ||
| "github.com/urfave/cli" | ||
| ) | ||
|
|
||
| // SigningCommand contains the definition of the `signing` command-line | ||
| // subcommand and its own subcommands. | ||
| var SigningCommand cli.Command | ||
|
|
||
| func init() { | ||
| SigningCommand = cli.Command{ | ||
| Name: "signing", | ||
| Usage: "Provides several tools useful for out-of-band signing", | ||
| Before: func(c *cli.Context) error { | ||
| _ = log.SetLogLevel("*", "fatal") // disable the regular logger | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts on going through our existing
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in the follow-up PR: #568 |
||
| return nil | ||
| }, | ||
| Subcommands: []cli.Command{ | ||
| { | ||
| Name: "decrypt-key-share", | ||
| Usage: "Decrypts the key share of the operator for the given keep", | ||
| ArgsUsage: "[keep-address]", | ||
| Action: DecryptKeyShare, | ||
| Flags: []cli.Flag{ | ||
| cli.StringFlag{ | ||
| Name: "output-file,o", | ||
| Usage: "Output file for the decrypted key share", | ||
| }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a flag or just a parameter? Feels like |
||
| }, | ||
| }, | ||
| { | ||
| Name: "sign-digest", | ||
| Usage: "Sign a given digest using provided key shares", | ||
| Action: SignDigest, | ||
| ArgsUsage: "[unprefixed-hex-digest] [key-shares-dir]", | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // DecryptKeyShare decrypt key shares for given keep using provided operator config. | ||
| func DecryptKeyShare(c *cli.Context) error { | ||
| config, err := config.ReadConfig(c.GlobalString("config")) | ||
| if err != nil { | ||
| return fmt.Errorf("failed while reading config file: [%v]", err) | ||
| } | ||
|
|
||
| keepAddressHex := c.Args().First() | ||
| if !common.IsHexAddress(keepAddressHex) { | ||
| return fmt.Errorf("invalid keep address") | ||
| } | ||
|
|
||
| keepAddress := common.HexToAddress(keepAddressHex) | ||
|
|
||
| handle, err := persistence.NewDiskHandle(config.Storage.DataDir) | ||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "failed while creating a storage disk handler: [%v]", | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| persistence := persistence.NewEncryptedPersistence( | ||
| handle, | ||
| config.Ethereum.Account.KeyFilePassword, | ||
| ) | ||
|
|
||
| keepRegistry := registry.NewKeepsRegistry(persistence) | ||
|
|
||
| keepRegistry.LoadExistingKeeps() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually a random note I missed making: we will need to make this work more flexibly. Right now you need a full config file and to have your path set up just right to decrypt the keyshare. We should be able to just point this to a keyshare and, if we have the password configured properly, have that keyshare decrypted to stdout. Otherwise if we have a snapshot that didn't get properly persisted, for example, you'd have to move it into the For followup work though.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree but from the other side, seems we won't be able to use the existing registry code as we do here and we will be forced to copy-paste relevant parts.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I figured structuring it all right will be a bit more complicated. |
||
|
|
||
| signers, err := keepRegistry.GetSigners(keepAddress) | ||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "no signers for keep [%s]: [%v]", | ||
| keepAddress.String(), | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| signer := signers[0] | ||
|
|
||
| signerBytes, err := signer.Marshal() | ||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "failed to marshall signer for keep [%s]: [%v]", | ||
| keepAddress.String(), | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| _, err = os.Stdout.Write(signerBytes) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we do support
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in the follow-up PR: #568 |
||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "could not write signer bytes to stdout: [%v]", | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| outputFilePath := c.String("output-file") | ||
|
|
||
| if len(outputFilePath) > 0 { | ||
| if _, err := os.Stat(outputFilePath); !os.IsNotExist(err) { | ||
| return fmt.Errorf( | ||
| "could not write shares to file; file [%s] already exists", | ||
| outputFilePath, | ||
| ) | ||
| } | ||
|
|
||
| err = ioutil.WriteFile(outputFilePath, signerBytes, 0444) // read-only | ||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "failed to write to file [%s]: [%v]", | ||
| outputFilePath, | ||
| err, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // SignDigest signs a given digest using key shares from the provided directory. | ||
| func SignDigest(c *cli.Context) error { | ||
| digest := c.Args().First() | ||
| if len(digest) == 0 { | ||
| return fmt.Errorf("invalid digest") | ||
| } | ||
|
|
||
| keySharesDir := c.Args().Get(1) | ||
| if len(keySharesDir) == 0 { | ||
| return fmt.Errorf("invalid key shares directory name") | ||
| } | ||
|
|
||
| keySharesFiles, err := ioutil.ReadDir(keySharesDir) | ||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "could not read key shares directory: [%v]", | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| signers := make([]tss.ThresholdSigner, len(keySharesFiles)) | ||
| networkProviders := make([]net.Provider, len(keySharesFiles)) | ||
|
|
||
| for i, keyShareFile := range keySharesFiles { | ||
| keyShareBytes, err := ioutil.ReadFile( | ||
| fmt.Sprintf("%s/%s", keySharesDir, keyShareFile.Name()), | ||
| ) | ||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "could not read key share file [%v]: [%v]", | ||
| keyShareFile.Name(), | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| var signer tss.ThresholdSigner | ||
| err = signer.Unmarshal(keyShareBytes) | ||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "could not unmarshal signer from file [%v]: [%v]", | ||
| keyShareFile.Name(), | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| operatorPublicKey, err := signer.MemberID().PublicKey() | ||
| if err != nil { | ||
| return fmt.Errorf( | ||
| "could not get operator public key: [%v]", | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| networkKey := key.NetworkPublic(*operatorPublicKey) | ||
| networkProvider := local.ConnectWithKey(&networkKey) | ||
|
|
||
| signers[i] = signer | ||
| networkProviders[i] = networkProvider | ||
| } | ||
|
|
||
| digestBytes, err := hex.DecodeString(digest) | ||
| if err != nil { | ||
| return fmt.Errorf("could not decode digest string: [%v]", err) | ||
| } | ||
|
|
||
| ctx, cancelCtx := context.WithTimeout( | ||
| context.Background(), | ||
| 1*time.Minute, | ||
| ) | ||
| defer cancelCtx() | ||
|
|
||
| var waitGroup sync.WaitGroup | ||
| waitGroup.Add(len(signers)) | ||
|
|
||
| type signingOutcome struct { | ||
| signerIndex int | ||
| signature *ecdsa.Signature | ||
| err error | ||
| } | ||
|
|
||
| signingOutcomesChannel := make(chan *signingOutcome, len(signers)) | ||
|
|
||
| for i := range signers { | ||
| go func(signerIndex int) { | ||
| defer waitGroup.Done() | ||
|
|
||
| signature, err := signers[signerIndex].CalculateSignature( | ||
| ctx, | ||
| digestBytes, | ||
| networkProviders[signerIndex], | ||
| ) | ||
|
|
||
| signingOutcomesChannel <- &signingOutcome{ | ||
| signerIndex, | ||
| signature, | ||
| err, | ||
| } | ||
| }(i) | ||
| } | ||
|
|
||
| waitGroup.Wait() | ||
| close(signingOutcomesChannel) | ||
|
|
||
| signatures := make(map[string]int) | ||
|
|
||
| for signingOutcome := range signingOutcomesChannel { | ||
| if signingOutcome.err != nil { | ||
| _, _ = fmt.Fprintf( | ||
| os.Stderr, | ||
| "signer with index [%v] returned an error: [%v]", | ||
| signingOutcome.signerIndex, | ||
| signingOutcome.err, | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, this feels like something we should do as not-a-log, printing to stderr with something solely for human consumption. |
||
| continue | ||
| } | ||
|
|
||
| signature := fmt.Sprintf("%+v", signingOutcome.signature) | ||
| signatures[signature]++ | ||
| } | ||
|
|
||
| if len(signatures) != 1 { | ||
| return fmt.Errorf( | ||
| "signing failed; a single signature should be produced", | ||
| ) | ||
| } | ||
|
|
||
| for signature, signersCount := range signatures { | ||
| if signersCount != len(signers) { | ||
| return fmt.Errorf( | ||
| "signing failed; all signers should support the signature", | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turns out I was totally wrong about returning an error. It does do us the favor of exiting with a non-zero status, but it prints the error out to a logger. This… Feels like a mistake, but probably not one we need to block this PR on. In particular, it feels like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in the follow-up PR: #568 |
||
| } | ||
|
|
||
| fmt.Printf(signature) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also use
Beforehere and below to check theArgslength and make sure it matches what we expect, returning anerrorif it does not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, but I feel it's a little bit clearer if we validate the arguments in the subcommand right in the place where we use them.