From 10b9ce05171e9ff808d2d2220b2724075ea014cb Mon Sep 17 00:00:00 2001 From: Tuan Nguyen Huy Date: Sun, 8 Dec 2024 06:10:05 +0700 Subject: [PATCH] Replace deprecated gopass with golang.org/x/term (#250) - Replace deprecated github.com/howeyc/gopass with golang.org/x/term - Add length validation for password input to keep the gopass behaviour --- cmd/keymaster-unlocker/main.go | 17 +++++++-- cmd/keymasterd/config.go | 21 +++++++++-- cmd/keymasterd/config_test.go | 2 +- go.mod | 5 +-- go.sum | 10 ++--- lib/client/util/util.go | 57 ++++++++++++++++++++++++++--- lib/client/util/util_test.go | 67 ++++++++++++++++++++++++++++++++++ 7 files changed, 158 insertions(+), 21 deletions(-) diff --git a/cmd/keymaster-unlocker/main.go b/cmd/keymaster-unlocker/main.go index 8f5a9e7b..a85c5d2d 100644 --- a/cmd/keymaster-unlocker/main.go +++ b/cmd/keymaster-unlocker/main.go @@ -3,6 +3,7 @@ package main import ( "context" "crypto/tls" + "errors" "flag" "fmt" "io/ioutil" @@ -16,7 +17,7 @@ import ( "github.com/Cloud-Foundations/Dominator/lib/log/cmdlogger" "github.com/Cloud-Foundations/golib/pkg/log" - "github.com/howeyc/gopass" + "golang.org/x/term" ) var ( @@ -32,6 +33,8 @@ var ( retryInterval = flag.Duration("retryInterval", 0, "If > 0: retry") ) +const maxPasswordLength = 512 + func Usage() { fmt.Fprintf(os.Stderr, "Usage of %s (version %s):\n", os.Args[0], Version) flag.PrintDefaults() @@ -40,10 +43,18 @@ func Usage() { func getPassword(password string) (string, error) { if password == "" { fmt.Printf("Password for unlocking %s: ", *keymasterHostname) - passwd, err := gopass.GetPasswd() + passwd, err := term.ReadPassword(int(os.Stdin.Fd())) + + // Add a newline after the password input + fmt.Println() + if err != nil { return "", err - // Handle gopass.ErrInterrupted or getch() read error + } + + // Check password length + if len(password) > maxPasswordLength { + return "", errors.New("maximum length exceeded") } password = string(passwd) } diff --git a/cmd/keymasterd/config.go b/cmd/keymasterd/config.go index bd456539..4c4d64fa 100644 --- a/cmd/keymasterd/config.go +++ b/cmd/keymasterd/config.go @@ -41,11 +41,11 @@ import ( "github.com/Cloud-Foundations/keymaster/lib/server/aws_identity_cert" "github.com/Cloud-Foundations/keymaster/lib/vip" "github.com/duo-labs/webauthn/webauthn" - "github.com/howeyc/gopass" "golang.org/x/crypto/openpgp" "golang.org/x/crypto/openpgp/armor" "golang.org/x/crypto/ssh" "golang.org/x/oauth2" + "golang.org/x/term" "golang.org/x/time/rate" "gopkg.in/yaml.v2" ) @@ -208,6 +208,7 @@ const ( defaultRSAKeySize = 3072 defaultSecsBetweenDependencyChecks = 60 defaultOktaUsernameFilterRegexp = "@.*" + maxPasswordLength = 512 ) func (state *RuntimeState) loadTemplates() (err error) { @@ -795,16 +796,30 @@ func generateArmoredEncryptedCAPrivateKey(passphrase []byte, func getPassphrase() ([]byte, error) { ///matching := false for { + // Prompt for the passphrase 1 fmt.Printf("Please enter your passphrase:\n") - passphrase1, err := gopass.GetPasswd() + passphrase1, err := term.ReadPassword(int(os.Stdin.Fd())) + // Add a newline after the password input + fmt.Println() + if err != nil { return nil, err } + + // Prompt for the passphrase 2 fmt.Printf("Please re-enter your passphrase:\n") - passphrase2, err := gopass.GetPasswd() + passphrase2, err := term.ReadPassword(int(os.Stdin.Fd())) + // Add a newline after the password input + fmt.Println() + if err != nil { return nil, err } + + // Check passphrases length + if len(passphrase1) > maxPasswordLength || len(passphrase2) > maxPasswordLength { + return nil, errors.New("maximum length exceeded") + } if bytes.Equal(passphrase1, passphrase2) { return passphrase1, nil } diff --git a/cmd/keymasterd/config_test.go b/cmd/keymasterd/config_test.go index a73237b2..46b35f57 100644 --- a/cmd/keymasterd/config_test.go +++ b/cmd/keymasterd/config_test.go @@ -11,7 +11,7 @@ import ( "github.com/Cloud-Foundations/golib/pkg/log/testlogger" ) -//openssl genpkey -algorithm ED25519 -out key.pem +// openssl genpkey -algorithm ED25519 -out key.pem const pkcs8Ed25519PrivateKey = `-----BEGIN PRIVATE KEY----- MC4CAQAwBQYDK2VwBCIEIHoHbl2RwHwmyWtXVLroUZEI+d/SqL3RKmECM5P7o7D5 -----END PRIVATE KEY-----` diff --git a/go.mod b/go.mod index d5e8458c..7f5b7a57 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,6 @@ require ( github.com/flynn/u2f v0.0.0-20180613185708-15554eb68e5d github.com/foomo/htpasswd v0.0.0-20200116085101-e3a90e78da9c github.com/go-jose/go-jose/v4 v4.0.4 - github.com/howeyc/gopass v0.0.0-20210920133722-c8aef6fb66ef github.com/lib/pq v1.10.9 github.com/marshallbrekka/go-u2fhost v0.0.0-20210111072507-3ccdec8c8105 github.com/mattn/go-sqlite3 v1.14.24 @@ -34,7 +33,7 @@ require ( golang.org/x/crypto v0.28.0 golang.org/x/net v0.30.0 golang.org/x/oauth2 v0.23.0 - golang.org/x/term v0.25.0 + golang.org/x/term v0.26.0 gopkg.in/ldap.v2 v2.5.1 gopkg.in/yaml.v2 v2.4.0 mvdan.cc/sh/v3 v3.10.0 @@ -91,7 +90,7 @@ require ( github.com/prometheus/common v0.60.1 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/x448/float16 v0.8.4 // indirect - golang.org/x/sys v0.26.0 // indirect + golang.org/x/sys v0.27.0 // indirect golang.org/x/time v0.7.0 google.golang.org/protobuf v1.35.1 // indirect gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect diff --git a/go.sum b/go.sum index 15625792..421c3fad 100644 --- a/go.sum +++ b/go.sum @@ -151,8 +151,6 @@ github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmg github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= -github.com/howeyc/gopass v0.0.0-20210920133722-c8aef6fb66ef h1:A9HsByNhogrvm9cWb28sjiS3i7tcKCkflWFEkHfuAgM= -github.com/howeyc/gopass v0.0.0-20210920133722-c8aef6fb66ef/go.mod h1:lADxMC39cJJqL93Duh1xhAs4I2Zs8mKS89XWXFGp9cs= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= @@ -331,15 +329,15 @@ golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= -golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s= +golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= -golang.org/x/term v0.25.0 h1:WtHI/ltw4NvSUig5KARz9h521QvRC8RmF/cuYqifU24= -golang.org/x/term v0.25.0/go.mod h1:RPyXicDX+6vLxogjjRxjgD2TKtmAO6NZBsBRfrOLu7M= +golang.org/x/term v0.26.0 h1:WEQa6V3Gja/BhNxg540hBip/kkaYtRg3cxg4oXSw4AU= +golang.org/x/term v0.26.0/go.mod h1:Si5m1o57C5nBNQo5z1iq+XDijt21BDBDp2bK0QI8e3E= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= diff --git a/lib/client/util/util.go b/lib/client/util/util.go index ed91f81a..a81ff59c 100644 --- a/lib/client/util/util.go +++ b/lib/client/util/util.go @@ -1,6 +1,7 @@ package util import ( + "bufio" "bytes" "crypto" "crypto/rand" @@ -8,6 +9,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/pem" + "errors" "fmt" "io/ioutil" "net/http" @@ -21,20 +23,65 @@ import ( "github.com/Cloud-Foundations/golib/pkg/log" "github.com/Cloud-Foundations/keymaster/lib/client/net" - "github.com/howeyc/gopass" "golang.org/x/crypto/ssh" "golang.org/x/net/publicsuffix" + "golang.org/x/term" ) const rsaKeySize = 2048 +const maxPasswordLength = 512 + func getUserCreds(userName string) (password []byte, err error) { fmt.Printf("Password for %s: ", userName) - password, err = gopass.GetPasswd() - if err != nil { - return nil, err - // Handle gopass.ErrInterrupted or getch() read error + + if term.IsTerminal(int(os.Stdin.Fd())) { + password, err = term.ReadPassword(int(os.Stdin.Fd())) + + // Always print newline, even on error + fmt.Println() + + if err != nil { + return nil, fmt.Errorf("failed to read password: %w", err) + } + } else { + // Read password from stdin without terminal operations + var pass []byte + reader := bufio.NewReader(os.Stdin) + + for counter := 0; counter <= maxPasswordLength; counter++ { + b, err := reader.ReadByte() + if err != nil { + return nil, fmt.Errorf("failed to read password: %w", err) + } + + switch b { + case 0: // NULL byte - ignore + continue + case 127, 8: // Backspace + if len(pass) > 0 { + pass = pass[:len(pass)-1] + } + continue // Skip the current iteration after removing character + case 13, 10: // Enter/newline + password = pass + return password, nil + case 3: // Ctrl-C + return nil, errors.New("interrupted") + default: + pass = append(pass, b) + } + } + + // If we get here, we've exceeded maxPasswordLength + return nil, errors.New("maximum length exceeded") } + + // Check password length + if len(password) > maxPasswordLength { + return nil, errors.New("maximum length exceeded") + } + return password, nil } diff --git a/lib/client/util/util_test.go b/lib/client/util/util_test.go index 64b3333b..bfe21216 100644 --- a/lib/client/util/util_test.go +++ b/lib/client/util/util_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "net" "os" + "strings" "testing" "github.com/Cloud-Foundations/golib/pkg/log/testlogger" @@ -84,6 +85,72 @@ func TestGetParseURLEnvVariable(t *testing.T) { } +func TestGetUserCreds(t *testing.T) { + tests := []struct { + name string + input string + want string + wantErr bool + errMsg string + }{ + { + name: "basic password", + input: "password\n", + want: "password", + }, + { + name: "password with backspace", + input: "passworrd\x08\x08d\n", // \x08 is backspace + want: "password", + }, + { + name: "ignore null bytes", + input: "pass\x00word\n", + want: "password", + }, + { + name: "ctrl-c interruption", + input: "pass\x03word\n", + wantErr: true, + errMsg: "interrupted", + }, + { + name: "max length exceeded", + input: string(make([]byte, maxPasswordLength+1)) + "\n", + wantErr: true, + errMsg: "maximum length exceeded", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := pipeToStdin(tt.input) + if err != nil { + t.Fatal(err) + } + + password, err := getUserCreds("username") + + if tt.wantErr { + if err == nil { + t.Errorf("expected error containing %q, got nil", tt.errMsg) + } else if !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("expected error containing %q, got %q", tt.errMsg, err.Error()) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if string(password) != tt.want { + t.Errorf("got password %q, want %q", string(password), tt.want) + } + }) + } +} + // ------------WARN-------- Next name copied from https://github.com/howeyc/gopass/blob/master/pass_test.go for using // // gopass checks