Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: add password-stdin flag with U2F enforcement #256

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion cmd/keymaster/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ var (
cliUsername = flag.String("username", "", "username for keymaster")
printVersion = flag.Bool("version", false,
"Print version and exit")
passwordStdin = flag.Bool("password-stdin", false,
"Read password from stdin (requires U2F as second factor, disables TOTP and VIPAccess)")
webauthBrowser = flag.String("webauthBrowser", "",
"Browser command to use for webauth")

Expand Down Expand Up @@ -362,10 +364,21 @@ func setupCerts(
}
} else {
// Authenticate using password and possible 2nd factor.
password, err := util.GetUserCreds(userName)
password, err := util.GetUserCreds(userName, *passwordStdin)
if err != nil {
return err
}

// Enforce U2F when using password-stdin
if *passwordStdin {
twofa.SetNoTOTP(true)
twofa.SetNoVIPAccess(true)

if twofa.GetNoU2F() {
return fmt.Errorf("U2F must be enabled when using --password-stdin")
}
}

baseUrl, err = twofa.AuthenticateToTargetUrls(userName, password,
targetURLs, false, client,
userAgentString, logger)
Expand Down
53 changes: 53 additions & 0 deletions cmd/keymaster/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/Cloud-Foundations/golib/pkg/log/testlogger"
"github.com/Cloud-Foundations/keymaster/lib/certgen"
"github.com/Cloud-Foundations/keymaster/lib/client/config"
"github.com/Cloud-Foundations/keymaster/lib/client/twofa"
"github.com/Cloud-Foundations/keymaster/lib/client/twofa/u2f"
"github.com/Cloud-Foundations/keymaster/lib/client/util"
"github.com/Cloud-Foundations/keymaster/lib/webapi/v0/proto"
Expand Down Expand Up @@ -310,3 +311,55 @@ func TestMainSimple(t *testing.T) {
b.Reset()

}

func TestPasswordStdinWithU2F(t *testing.T) {
logger := testlogger.New(t)
var b bytes.Buffer

// Test case 1: password-stdin with U2F disabled should fail
*passwordStdin = true
twofa.SetNoU2F(true)
twofa.SetNoTOTP(false)
twofa.SetNoVIPAccess(false)

err := mainWithError(&b, logger)
if err == nil {
t.Fatal("Expected error when using password-stdin with U2F disabled")
}
if err.Error() != "U2F must be enabled when using --password-stdin" {
t.Fatalf("Unexpected error message: %s", err.Error())
}

// Test case 2: password-stdin with U2F enabled should enforce TOTP and VIPAccess disabled
twofa.SetNoU2F(false)
twofa.SetNoTOTP(false)
twofa.SetNoVIPAccess(false)

// Pipe a password to stdin for the test
_, err = pipeToStdin("testpassword\n")
if err != nil {
t.Fatal(err)
}

err = mainWithError(&b, logger)
if err != nil {
// The error should be from trying to connect to the server, not from the U2F validation
if err.Error() == "U2F must be enabled when using --password-stdin" {
t.Fatal("U2F validation failed when it should have passed")
}
}

// Verify TOTP and VIPAccess were disabled
if !twofa.GetNoTOTP() {
t.Error("TOTP should be disabled when using password-stdin")
}
if !twofa.GetNoVIPAccess() {
t.Error("VIPAccess should be disabled when using password-stdin")
}

// Reset the flags
*passwordStdin = false
twofa.SetNoU2F(false)
twofa.SetNoTOTP(false)
twofa.SetNoVIPAccess(false)
}
25 changes: 25 additions & 0 deletions lib/client/twofa/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,31 @@ var (
noVIPAccess = flag.Bool("noVIPAccess", false, "Don't use VIPAccess as second factor")
)

// Getter and setter functions for the flags
func SetNoU2F(value bool) {
*noU2F = value
}

func GetNoU2F() bool {
return *noU2F
}

func SetNoTOTP(value bool) {
*noTOTP = value
}

func GetNoTOTP() bool {
return *noTOTP
}

func SetNoVIPAccess(value bool) {
*noVIPAccess = value
}

func GetNoVIPAccess() bool {
return *noVIPAccess
}

// AuthenticateToTargetUrls does an authentication to the keymasted server
// it performs 2fa if needed using the server side specified methods
// it assumes the http client has a valid cookiejar
Expand Down
4 changes: 2 additions & 2 deletions lib/client/util/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
)

// GetUserCreds prompts the user for their password and returns it.
func GetUserCreds(userName string) (password []byte, err error) {
return getUserCreds(userName)
func GetUserCreds(userName string, useStdin bool) (password []byte, err error) {
return getUserCreds(userName, useStdin)
}

// GetUserNameAndHomeDir gets the user name and home directory.
Expand Down
16 changes: 15 additions & 1 deletion lib/client/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"encoding/pem"
"errors"
"fmt"
"io"
"io/ioutil"
"net/http"
"net/http/cookiejar"
Expand All @@ -32,7 +33,20 @@ const rsaKeySize = 2048

const maxPasswordLength = 512

func getUserCreds(userName string) (password []byte, err error) {
func getUserCreds(userName string, useStdin bool) (password []byte, err error) {
if useStdin {
// Read from stdin
password, err = io.ReadAll(os.Stdin)

if err != nil {
return nil, fmt.Errorf("failed to read password from stdin: %w", err)
}
// Trim any trailing newline
password = bytes.TrimSpace(password)

return password, nil
}

fmt.Printf("Password for %s: ", userName)

if term.IsTerminal(int(os.Stdin.Fd())) {
Expand Down
33 changes: 31 additions & 2 deletions lib/client/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func TestGetUserCreds(t *testing.T) {
t.Fatal(err)
}

password, err := getUserCreds("username")
password, err := getUserCreds("username", false)

if tt.wantErr {
if err == nil {
Expand All @@ -151,6 +151,35 @@ func TestGetUserCreds(t *testing.T) {
}
}

func TestGetUserCredsFromStdin(t *testing.T) {
// Save old stdin
oldStdin := os.Stdin
defer func() { os.Stdin = oldStdin }()

// Create a pipe
r, w, err := os.Pipe()
if err != nil {
t.Fatal(err)
}
os.Stdin = r

// Write test password to pipe
testPassword := "test-password-123\n"
go func() {
w.Write([]byte(testPassword))
w.Close()
}()

password, err := getUserCreds("username", true)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if string(password) != "test-password-123" {
t.Errorf("got password %q, want %q", string(password), "test-password-123")
}
}

// ------------WARN-------- Next name copied from https://github.com/howeyc/gopass/blob/master/pass_test.go for using
//
// gopass checks
Expand All @@ -159,7 +188,7 @@ func TestPipe(t *testing.T) {
if err != nil {
t.Fatal(err)
}
password, err := GetUserCreds("userame")
password, err := GetUserCreds("userame", false)
if err != nil {
t.Fatal(err)
}
Expand Down
Loading