From 875e160893c46cbc1a6606cac27488d3010e590a Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 21 Mar 2022 21:43:24 +0100 Subject: [PATCH 1/7] gen-seed+gen-password: don't print newline To avoid a newline being added to any password or seed secret created with lndinit, we shouldn't print one to the console. --- cmd_gen_password.go | 2 +- cmd_gen_seed.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd_gen_password.go b/cmd_gen_password.go index ba81659..bb523e0 100644 --- a/cmd_gen_password.go +++ b/cmd_gen_password.go @@ -73,7 +73,7 @@ func (x *genPasswordCommand) Execute(_ []string) error { } } - fmt.Printf("%s\n", passwordString) + fmt.Printf("%s", passwordString) return nil } diff --git a/cmd_gen_seed.go b/cmd_gen_seed.go index 46a6cb1..15babe6 100644 --- a/cmd_gen_seed.go +++ b/cmd_gen_seed.go @@ -124,7 +124,7 @@ func (x *genSeedCommand) Execute(_ []string) error { } } - fmt.Printf("%s\n", seedWords) + fmt.Printf("%s", seedWords) return nil } From e2fcbc9e5286f980fe92f0cba1bca8791a9eb035 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 21 Mar 2022 21:43:25 +0100 Subject: [PATCH 2/7] k8s: don't always trim newline --- k8s.go | 8 ++------ main.go | 7 +------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/k8s.go b/k8s.go index fa1fd2a..a9d46cc 100644 --- a/k8s.go +++ b/k8s.go @@ -93,13 +93,9 @@ func readK8s(opts *k8sSecretOptions) (string, *jsonK8sObject, error) { opts.SecretKeyName) } - // Remove any newlines at the end of the file. We won't ever write a - // newline ourselves but maybe the file was provisioned by another - // process or user. - content := stripNewline(string(secret.Data[opts.SecretKeyName])) - // There is an additional layer of base64 encoding applied to each of // the secrets. Try to de-code it now. + content := string(secret.Data[opts.SecretKeyName]) if opts.Base64 { decoded, err := base64.StdEncoding.DecodeString(content) if err != nil { @@ -108,7 +104,7 @@ func readK8s(opts *k8sSecretOptions) (string, *jsonK8sObject, error) { opts.SecretKeyName, err) } - content = stripNewline(string(decoded)) + content = string(decoded) } return content, &jsonK8sObject{ diff --git a/main.go b/main.go index 8ec5072..e41f5ce 100644 --- a/main.go +++ b/main.go @@ -150,12 +150,7 @@ func readFile(fileName string) (string, error) { err) } - content := string(byteContent) - - // Remove any newlines at the end of the file. We won't ever write a - // newline ourselves but maybe the file was provisioned by another - // process or user. - return stripNewline(content), nil + return string(byteContent), nil } func stripNewline(str string) string { From cb9c6f06a187ad95dc8bf4cf1c865c5061c68657 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 21 Mar 2022 21:43:26 +0100 Subject: [PATCH 3/7] init-wallet: trim newline in wallet secrets only --- cmd_init_wallet.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cmd_init_wallet.go b/cmd_init_wallet.go index 2f76b13..43575e3 100644 --- a/cmd_init_wallet.go +++ b/cmd_init_wallet.go @@ -179,6 +179,13 @@ func (x *initWalletCommand) Execute(_ []string) error { } } + // The seed, its passphrase and the wallet password should all never + // have a newline at their end, otherwise that might lead to errors + // further down the line. + seed = stripNewline(seed) + seedPassPhrase = stripNewline(seedPassPhrase) + walletPassword = stripNewline(walletPassword) + switch x.InitType { case typeFile: cipherSeed, err := checkSeed(seed, seedPassPhrase) From 5c6c515b92222a88fcacf1a27a08674d3d78b1d5 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 21 Mar 2022 21:43:28 +0100 Subject: [PATCH 4/7] mod: add testify library --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 073f5fa..e2719cc 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/kkdai/bstream v1.0.0 github.com/lightninglabs/protobuf-hex-display v1.4.3-hex-display github.com/lightningnetwork/lnd v0.14.2-beta + github.com/stretchr/testify v1.7.0 google.golang.org/grpc v1.38.0 k8s.io/api v0.18.3 k8s.io/apimachinery v0.18.3 @@ -92,7 +93,6 @@ require ( github.com/sirupsen/logrus v1.7.0 // indirect github.com/soheilhy/cmux v0.1.5 // indirect github.com/spf13/pflag v1.0.5 // indirect - github.com/stretchr/testify v1.7.0 // indirect github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802 // indirect github.com/ulikunitz/xz v0.5.10 // indirect github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect From 2d1c9856bd9bef5f4f211ee97e1381e11a687b2d Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 21 Mar 2022 21:43:29 +0100 Subject: [PATCH 5/7] multi: refactor for testability, add unit tests --- Makefile | 6 +- cmd_init_wallet.go | 165 +++++++++++++++++++++------------------- cmd_init_wallet_test.go | 38 +++++++++ k8s.go | 33 +++++--- k8s_test.go | 78 +++++++++++++++++++ 5 files changed, 232 insertions(+), 88 deletions(-) create mode 100644 cmd_init_wallet_test.go create mode 100644 k8s_test.go diff --git a/Makefile b/Makefile index 9fdae4b..ecabab5 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ COMMIT_HASH := $(shell git rev-parse HEAD) GOBUILD := go build -v GOINSTALL := go install -v -GOTEST := go test +GOTEST := go test -v DOCKER_TOOLS := docker run -v $$(pwd):/build lndinit-tools GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*") @@ -103,6 +103,10 @@ scratch: build # UTILITIES # ========= +unit: + @$(call print, "Running unit tests.") + $(GOTEST) ./... + fmt: $(GOIMPORTS_BIN) @$(call print, "Fixing imports.") gosimports -w $(GOFILES_NOVENDOR) diff --git a/cmd_init_wallet.go b/cmd_init_wallet.go index 43575e3..21781c5 100644 --- a/cmd_init_wallet.go +++ b/cmd_init_wallet.go @@ -107,85 +107,11 @@ func (x *initWalletCommand) Execute(_ []string) error { requireSeed := (x.InitType == typeFile) || (x.InitType == typeRpc && !x.InitRpc.WatchOnly) - // First find out where we want to read the secrets from. - var ( - seed string - seedPassPhrase string - walletPassword string - err error - ) - switch x.SecretSource { - // Read all secrets from individual files. - case storageFile: - if requireSeed { - log("Reading seed from file") - seed, err = readFile(x.File.Seed) - if err != nil { - return err - } - } - - // The seed passphrase is optional. - if x.File.SeedPassphrase != "" { - log("Reading seed passphrase from file") - seedPassPhrase, err = readFile(x.File.SeedPassphrase) - if err != nil { - return err - } - } - - log("Reading wallet password from file") - walletPassword, err = readFile(x.File.WalletPassword) - if err != nil { - return err - } - - // Read passphrase from Kubernetes secret. - case storageK8s: - k8sSecret := &k8sSecretOptions{ - Namespace: x.K8s.Namespace, - SecretName: x.K8s.SecretName, - Base64: x.K8s.Base64, - } - k8sSecret.SecretKeyName = x.K8s.SeedKeyName - - if requireSeed { - log("Reading seed from k8s secret %s (namespace %s)", - x.K8s.SecretName, x.K8s.Namespace) - seed, _, err = readK8s(k8sSecret) - if err != nil { - return err - } - } - - // The seed passphrase is optional. - if x.K8s.SeedPassphraseKeyName != "" { - log("Reading seed passphrase from k8s secret %s "+ - "(namespace %s)", x.K8s.SecretName, - x.K8s.Namespace) - k8sSecret.SecretKeyName = x.K8s.SeedPassphraseKeyName - seedPassPhrase, _, err = readK8s(k8sSecret) - if err != nil { - return err - } - } - - log("Reading wallet password from k8s secret %s (namespace %s)", - x.K8s.SecretName, x.K8s.Namespace) - k8sSecret.SecretKeyName = x.K8s.WalletPasswordKeyName - walletPassword, _, err = readK8s(k8sSecret) - if err != nil { - return err - } + seed, seedPassPhrase, walletPassword, err := x.readInput(requireSeed) + if err != nil { + return fmt.Errorf("error reading input parameters: %v", err) } - // The seed, its passphrase and the wallet password should all never - // have a newline at their end, otherwise that might lead to errors - // further down the line. - seed = stripNewline(seed) - seedPassPhrase = stripNewline(seedPassPhrase) - walletPassword = stripNewline(walletPassword) - switch x.InitType { case typeFile: cipherSeed, err := checkSeed(seed, seedPassPhrase) @@ -275,6 +201,91 @@ func (x *initWalletCommand) Execute(_ []string) error { } } +func (x *initWalletCommand) readInput(requireSeed bool) (string, string, string, + error) { + + // First find out where we want to read the secrets from. + var ( + seed string + seedPassPhrase string + walletPassword string + err error + ) + switch x.SecretSource { + // Read all secrets from individual files. + case storageFile: + if requireSeed { + log("Reading seed from file") + seed, err = readFile(x.File.Seed) + if err != nil { + return "", "", "", err + } + } + + // The seed passphrase is optional. + if x.File.SeedPassphrase != "" { + log("Reading seed passphrase from file") + seedPassPhrase, err = readFile(x.File.SeedPassphrase) + if err != nil { + return "", "", "", err + } + } + + log("Reading wallet password from file") + walletPassword, err = readFile(x.File.WalletPassword) + if err != nil { + return "", "", "", err + } + + // Read passphrase from Kubernetes secret. + case storageK8s: + k8sSecret := &k8sSecretOptions{ + Namespace: x.K8s.Namespace, + SecretName: x.K8s.SecretName, + Base64: x.K8s.Base64, + } + k8sSecret.SecretKeyName = x.K8s.SeedKeyName + + if requireSeed { + log("Reading seed from k8s secret %s (namespace %s)", + x.K8s.SecretName, x.K8s.Namespace) + seed, _, err = readK8s(k8sSecret) + if err != nil { + return "", "", "", err + } + } + + // The seed passphrase is optional. + if x.K8s.SeedPassphraseKeyName != "" { + log("Reading seed passphrase from k8s secret %s "+ + "(namespace %s)", x.K8s.SecretName, + x.K8s.Namespace) + k8sSecret.SecretKeyName = x.K8s.SeedPassphraseKeyName + seedPassPhrase, _, err = readK8s(k8sSecret) + if err != nil { + return "", "", "", err + } + } + + log("Reading wallet password from k8s secret %s (namespace %s)", + x.K8s.SecretName, x.K8s.Namespace) + k8sSecret.SecretKeyName = x.K8s.WalletPasswordKeyName + walletPassword, _, err = readK8s(k8sSecret) + if err != nil { + return "", "", "", err + } + } + + // The seed, its passphrase and the wallet password should all never + // have a newline at their end, otherwise that might lead to errors + // further down the line. + seed = stripNewline(seed) + seedPassPhrase = stripNewline(seedPassPhrase) + walletPassword = stripNewline(walletPassword) + + return seed, seedPassPhrase, walletPassword, nil +} + func createWalletFile(cipherSeed *aezeed.CipherSeed, walletPassword, walletDir, network string, validatePassword bool) error { diff --git a/cmd_init_wallet_test.go b/cmd_init_wallet_test.go new file mode 100644 index 0000000..1245e33 --- /dev/null +++ b/cmd_init_wallet_test.go @@ -0,0 +1,38 @@ +package main + +import ( + "io/ioutil" + "testing" + + "github.com/stretchr/testify/require" +) + +var ( + testSeedWithNewline = []byte("seed phrase with newline\n") + testPasswordWithNewline = []byte("p4ssw0rd\r\n\n\r\r\n") +) + +// TestReadInput makes sure input files are always trimmed so we don't have any +// newline characters left over. +func TestReadInput(t *testing.T) { + cmd := newInitWalletCommand() + + cmd.File.Seed = writeToTempFile(t, testSeedWithNewline) + cmd.File.WalletPassword = writeToTempFile(t, testPasswordWithNewline) + + seed, seedPassphrase, walletPassword, err := cmd.readInput(true) + require.NoError(t, err) + require.Equal(t, "seed phrase with newline", seed) + require.Equal(t, "", seedPassphrase) + require.Equal(t, "p4ssw0rd", walletPassword) +} + +func writeToTempFile(t *testing.T, data []byte) string { + tempFileName, err := ioutil.TempFile("", "*.txt") + require.NoError(t, err) + + err = ioutil.WriteFile(tempFileName.Name(), data, 0600) + require.NoError(t, err) + + return tempFileName.Name() +} diff --git a/k8s.go b/k8s.go index a9d46cc..45dceb4 100644 --- a/k8s.go +++ b/k8s.go @@ -95,16 +95,12 @@ func readK8s(opts *k8sSecretOptions) (string, *jsonK8sObject, error) { // There is an additional layer of base64 encoding applied to each of // the secrets. Try to de-code it now. - content := string(secret.Data[opts.SecretKeyName]) - if opts.Base64 { - decoded, err := base64.StdEncoding.DecodeString(content) - if err != nil { - return "", nil, fmt.Errorf("failed to base64 decode "+ - "secret %s key %s: %v", opts.SecretName, - opts.SecretKeyName, err) - } - - content = string(decoded) + content, err := secretToString( + secret.Data[opts.SecretKeyName], opts.Base64, + ) + if err != nil { + return "", nil, fmt.Errorf("failed to decode raw secret %s "+ + "key %s: %v", opts.SecretName, opts.SecretKeyName, err) } return content, &jsonK8sObject{ @@ -239,3 +235,20 @@ func createSecretK8s(client *kubernetes.Clientset, opts *k8sSecretOptions, return nil } + +// secretToString turns the raw bytes of a secret into a string, removing the +// additional layer of base64 encoding if there is expected to be one. +func secretToString(rawSecret []byte, doubleBase64 bool) (string, error) { + content := string(rawSecret) + if doubleBase64 { + decoded, err := base64.StdEncoding.DecodeString(content) + if err != nil { + return "", fmt.Errorf("failed to base64 decode: %v", + err) + } + + content = string(decoded) + } + + return content, nil +} diff --git a/k8s_test.go b/k8s_test.go new file mode 100644 index 0000000..29616fe --- /dev/null +++ b/k8s_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "testing" + + "encoding/base64" + "github.com/stretchr/testify/require" +) + +var ( + dummyString = []byte("This is a simple string") + dummyStringB64 = base64.StdEncoding.EncodeToString(dummyString) + dummyStringNewline = []byte("This is a simple string newline\n") + dummyStringNewlineB64 = base64.StdEncoding.EncodeToString( + dummyStringNewline, + ) +) + +// TestSecretToString makes sure that a raw secret can be turned into a string +// correctly. +func TestSecretToString(t *testing.T) { + testCases := []struct { + name string + input []byte + base64 bool + expectErr bool + result string + }{{ + name: "plain string", + input: dummyString, + result: string(dummyString), + }, { + name: "plain base64", + input: []byte(dummyStringB64), + base64: true, + result: string(dummyString), + }, { + name: "invalid base64", + input: dummyString, + base64: true, + expectErr: true, + }, { + name: "plain base64 with newline in encoded", + input: []byte(dummyStringB64 + "\r\n"), + base64: true, + result: string(dummyString), + }, { + name: "string with newline", + input: dummyStringNewline, + result: string(dummyStringNewline), + }, { + name: "base64 with newline in original", + input: []byte(dummyStringNewlineB64), + base64: true, + result: string(dummyStringNewline), + }, { + name: "base64 with newline in encoded", + input: []byte(dummyStringNewlineB64 + "\r\n"), + base64: true, + result: string(dummyStringNewline), + }} + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(tt *testing.T) { + result, err := secretToString(tc.input, tc.base64) + + if tc.expectErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + require.Equal(t, tc.result, result) + }) + } +} From bd83e4b049c42b4cd48acb4d9f9864a79e13fd59 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 21 Mar 2022 21:43:30 +0100 Subject: [PATCH 6/7] github: add unit test to CI pipeline --- .github/workflows/main.yml | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 522fb9b..a9497c3 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -17,6 +17,8 @@ env: GOCACHE: /home/runner/work/go/pkg/build GOPATH: /home/runner/work/go + GO_VERSION: 1.17.6 + jobs: ######################## # lint code @@ -32,3 +34,32 @@ jobs: - name: lint run: make lint + + ######################## + # run unit tests + ######################## + unit-test: + name: run unit tests + runs-on: ubuntu-latest + steps: + - name: git checkout + uses: actions/checkout@v2 + + - name: go cache + uses: actions/cache@v1 + with: + path: /home/runner/work/go + key: lnd-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }} + restore-keys: | + lndinit-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}-${{ hashFiles('**/go.sum') }} + lndinit-${{ runner.os }}-go-${{ env.GO_VERSION }}-${{ github.job }}- + lndinit-${{ runner.os }}-go-${{ env.GO_VERSION }}- + lndinit-${{ runner.os }}-go- + + - name: setup go ${{ env.GO_VERSION }} + uses: actions/setup-go@v2 + with: + go-version: '${{ env.GO_VERSION }}' + + - name: run unit test + run: make unit From fd7a6d33f8c08f1b711628b3f2c26a96f4b8ff9c Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 21 Mar 2022 21:43:32 +0100 Subject: [PATCH 7/7] version: bump to v0.1.2-beta --- version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.go b/version.go index cb7f3bf..7cfdf98 100644 --- a/version.go +++ b/version.go @@ -32,7 +32,7 @@ const ( AppMinor uint = 1 // AppPatch defines the application patch for this binary. - AppPatch uint = 1 + AppPatch uint = 2 // AppPreRelease MUST only contain characters from semanticAlphabet // per the semantic versioning spec.