Skip to content

Commit

Permalink
Replace deprecated gopass with golang.org/x/term (#250)
Browse files Browse the repository at this point in the history
- Replace deprecated github.com/howeyc/gopass with golang.org/x/term
- Add length validation for password input to keep the gopass behaviour
  • Loading branch information
tuannh99 authored Dec 7, 2024
1 parent 43a1619 commit 10b9ce0
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 21 deletions.
17 changes: 14 additions & 3 deletions cmd/keymaster-unlocker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"crypto/tls"
"errors"
"flag"
"fmt"
"io/ioutil"
Expand All @@ -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 (
Expand All @@ -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()
Expand All @@ -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)
}
Expand Down
21 changes: 18 additions & 3 deletions cmd/keymasterd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -208,6 +208,7 @@ const (
defaultRSAKeySize = 3072
defaultSecsBetweenDependencyChecks = 60
defaultOktaUsernameFilterRegexp = "@.*"
maxPasswordLength = 512
)

func (state *RuntimeState) loadTemplates() (err error) {
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/keymasterd/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-----`
Expand Down
5 changes: 2 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
57 changes: 52 additions & 5 deletions lib/client/util/util.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package util

import (
"bufio"
"bytes"
"crypto"
"crypto/rand"
"crypto/rsa"
"crypto/tls"
"crypto/x509"
"encoding/pem"
"errors"
"fmt"
"io/ioutil"
"net/http"
Expand All @@ -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
}

Expand Down
67 changes: 67 additions & 0 deletions lib/client/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io/ioutil"
"net"
"os"
"strings"
"testing"

"github.com/Cloud-Foundations/golib/pkg/log/testlogger"
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 10b9ce0

Please sign in to comment.