diff --git a/Gopkg.lock b/Gopkg.lock index 4525dec859f4e..518517872b2ff 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -197,11 +197,13 @@ name = "github.com/grpc-ecosystem/go-grpc-middleware" packages = [ ".", + "auth", "logging", "logging/logrus", "logging/logrus/ctxlogrus", "tags", - "tags/logrus" + "tags/logrus", + "util/metautils" ] revision = "bc372cc64f55abd91995ba3f219b380ffbc59e9d" @@ -774,6 +776,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "322a07c0669a9e0865463bd2976ab0cc3828cf63e43ada0b14ea65933d011529" + inputs-digest = "04546dfcb4a6b5756f85134841cba8a155e5dbdc5036f870b3d963f27c97059c" solver-name = "gps-cdcl" solver-version = 1 diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index e5f7017c21cb7..a588bf86a885f 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -13,6 +13,7 @@ import ( argoappv1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/server/application" "github.com/argoproj/argo-cd/util" + "github.com/argoproj/argo-cd/util/cli" "github.com/argoproj/argo-cd/util/diff" "github.com/spf13/cobra" "github.com/yudai/gojsondiff/formatter" @@ -61,20 +62,15 @@ func NewApplicationAddCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com } var app argoappv1.Application if fileURL != "" { - var ( - fileContents []byte - err error - ) - _, err = url.ParseRequestURI(fileURL) + _, err := url.ParseRequestURI(fileURL) if err != nil { - fileContents, err = readLocalFile(fileURL) + err = cli.UnmarshalLocalFile(fileURL, &app) } else { - fileContents, err = readRemoteFile(fileURL) + err = cli.UnmarshalRemoteFile(fileURL, &app) } if err != nil { log.Fatal(err) } - unmarshalApplication(fileContents, &app) } else { // all these params are required if we're here diff --git a/cmd/argocd/commands/login.go b/cmd/argocd/commands/login.go new file mode 100644 index 0000000000000..7d14f32d89f65 --- /dev/null +++ b/cmd/argocd/commands/login.go @@ -0,0 +1,79 @@ +package commands + +import ( + "bufio" + "context" + "fmt" + "log" + "os" + "strings" + "syscall" + + "github.com/argoproj/argo-cd/errors" + argocdclient "github.com/argoproj/argo-cd/pkg/apiclient" + "github.com/argoproj/argo-cd/server/session" + "github.com/argoproj/argo-cd/util" + util_config "github.com/argoproj/argo-cd/util/config" + "github.com/spf13/cobra" + "golang.org/x/crypto/ssh/terminal" +) + +// NewLoginCommand returns a new instance of `argocd login` command +func NewLoginCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command { + var ( + username string + password string + ) + var command = &cobra.Command{ + Use: "login", + Short: "Log in to Argo CD", + Long: "Log in to Argo CD", + Run: func(c *cobra.Command, args []string) { + for username == "" { + reader := bufio.NewReader(os.Stdin) + fmt.Print("Username: ") + usernameRaw, err := reader.ReadString('\n') + if err != nil { + log.Fatal(err) + } + username = strings.TrimSpace(usernameRaw) + } + for password == "" { + fmt.Print("Password: ") + passwordRaw, err := terminal.ReadPassword(syscall.Stdin) + if err != nil { + log.Fatal(err) + } + password = string(passwordRaw) + if password == "" { + fmt.Print("\n") + } + } + + conn, sessionIf := argocdclient.NewClientOrDie(clientOpts).NewSessionClientOrDie() + defer util.Close(conn) + + sessionRequest := session.SessionRequest{ + Username: username, + Password: password, + } + createdSession, err := sessionIf.Create(context.Background(), &sessionRequest) + errors.CheckError(err) + fmt.Printf("user %q logged in successfully\n", username) + + // now persist the new token + localConfig, err := util_config.ReadLocalConfig() + if err != nil { + log.Fatal(err) + } + localConfig.Sessions[clientOpts.ServerAddr] = createdSession.Token + err = util_config.WriteLocalConfig(localConfig) + if err != nil { + log.Fatal(err) + } + + }, + } + command.Flags().StringVar(&username, "username", "", "the username of an account to authenticate") + return command +} diff --git a/cmd/argocd/commands/root.go b/cmd/argocd/commands/root.go index 1410bc45b1872..c73b695eb7e5d 100644 --- a/cmd/argocd/commands/root.go +++ b/cmd/argocd/commands/root.go @@ -25,6 +25,7 @@ func NewCommand() *cobra.Command { command.AddCommand(cli.NewVersionCmd(cliName)) command.AddCommand(NewClusterCommand(&clientOpts, pathOpts)) command.AddCommand(NewApplicationCommand(&clientOpts)) + command.AddCommand(NewLoginCommand(&clientOpts)) command.AddCommand(NewRepoCommand(&clientOpts)) command.AddCommand(NewInstallCommand()) command.AddCommand(NewUninstallCommand()) diff --git a/cmd/argocd/commands/util.go b/cmd/argocd/commands/util.go deleted file mode 100644 index 5892364c760e9..0000000000000 --- a/cmd/argocd/commands/util.go +++ /dev/null @@ -1,49 +0,0 @@ -package commands - -import ( - "encoding/json" - "io/ioutil" - "log" - "net/http" - - argoappv1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1" - "github.com/ghodss/yaml" -) - -// unmarshalApplication tries to convert a YAML or JSON byte array into an Application struct. -func unmarshalApplication(data []byte, app *argoappv1.Application) { - // first, try unmarshaling as JSON - // Based on technique from Kubectl, which supports both YAML and JSON: - // https://mlafeldt.github.io/blog/teaching-go-programs-to-love-json-and-yaml/ - // http://ghodss.com/2014/the-right-way-to-handle-yaml-in-golang/ - // Short version: JSON unmarshaling won't zero out null fields; YAML unmarshaling will. - // This may have unintended effects or hard-to-catch issues when populating our application object. - data, err := yaml.YAMLToJSON(data) - if err != nil { - log.Fatal("Could not decode valid JSON or YAML Kubernetes manifest") - } - err = json.Unmarshal(data, &app) - if err != nil { - log.Fatalf("Could not unmarshal Kubernetes manifest: %s", string(data)) - } -} - -// readLocalFile reads a file from disk and returns its contents as a byte array. -// The caller is responsible for checking error return values. -func readLocalFile(path string) (data []byte, err error) { - data, err = ioutil.ReadFile(path) - return -} - -// readRemoteFile issues a GET request to retrieve the contents of the specified URL as a byte array. -// The caller is responsible for checking error return values. -func readRemoteFile(url string) (data []byte, err error) { - resp, err := http.Get(url) - if err == nil { - defer func() { - _ = resp.Body.Close() - }() - data, err = ioutil.ReadAll(resp.Body) - } - return -} diff --git a/cmd/argocd/commands/util_test.go b/cmd/argocd/commands/util_test.go deleted file mode 100644 index cf76c6768784c..0000000000000 --- a/cmd/argocd/commands/util_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package commands - -import ( - "fmt" - "io/ioutil" - "net" - "net/http" - "os" - "testing" -) - -func TestReadLocalFile(t *testing.T) { - sentinel := "Hello, world!" - - file, err := ioutil.TempFile(os.TempDir(), "") - if err != nil { - panic(err) - } - defer func() { - _ = os.Remove(file.Name()) - }() - - _, _ = file.WriteString(sentinel) - _ = file.Sync() - - data, err := readLocalFile(file.Name()) - if string(data) != sentinel { - t.Errorf("Test data did not match (err = %v)! Expected \"%s\" and received \"%s\"", err, sentinel, string(data)) - } -} - -func TestReadRemoteFile(t *testing.T) { - sentinel := "Hello, world!" - - serve := func(c chan<- string) { - // listen on first available dynamic (unprivileged) port - listener, err := net.Listen("tcp", ":0") - if err != nil { - panic(err) - } - - // send back the address so that it can be used - c <- listener.Addr().String() - - http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - // return the sentinel text at root URL - fmt.Fprint(w, sentinel) - }) - - panic(http.Serve(listener, nil)) - } - - c := make(chan string, 1) - - // run a local webserver to test data retrieval - go serve(c) - - address := <-c - data, err := readRemoteFile("http://" + address) - t.Logf("Listening at address: %s", address) - if string(data) != sentinel { - t.Errorf("Test data did not match (err = %v)! Expected \"%s\" and received \"%s\"", err, sentinel, string(data)) - } -} diff --git a/pkg/apiclient/apiclient.go b/pkg/apiclient/apiclient.go index 35cee4d2dc6e2..694c1bce5d82e 100644 --- a/pkg/apiclient/apiclient.go +++ b/pkg/apiclient/apiclient.go @@ -12,6 +12,7 @@ import ( "github.com/argoproj/argo-cd/server/application" "github.com/argoproj/argo-cd/server/cluster" "github.com/argoproj/argo-cd/server/repository" + "github.com/argoproj/argo-cd/server/session" grpc_util "github.com/argoproj/argo-cd/util/grpc" log "github.com/sirupsen/logrus" "google.golang.org/grpc" @@ -31,6 +32,8 @@ type ServerClient interface { NewClusterClientOrDie() (*grpc.ClientConn, cluster.ClusterServiceClient) NewApplicationClient() (*grpc.ClientConn, application.ApplicationServiceClient, error) NewApplicationClientOrDie() (*grpc.ClientConn, application.ApplicationServiceClient) + NewSessionClient() (*grpc.ClientConn, session.SessionServiceClient, error) + NewSessionClientOrDie() (*grpc.ClientConn, session.SessionServiceClient) } type ClientOptions struct { @@ -143,3 +146,20 @@ func (c *client) NewApplicationClientOrDie() (*grpc.ClientConn, application.Appl } return conn, repoIf } + +func (c *client) NewSessionClient() (*grpc.ClientConn, session.SessionServiceClient, error) { + conn, err := c.NewConn() + if err != nil { + return nil, nil, err + } + sessionIf := session.NewSessionServiceClient(conn) + return conn, sessionIf, nil +} + +func (c *client) NewSessionClientOrDie() (*grpc.ClientConn, session.SessionServiceClient) { + conn, sessionIf, err := c.NewSessionClient() + if err != nil { + log.Fatalf("Failed to establish connection to %s: %v", c.ServerAddr, err) + } + return conn, sessionIf +} diff --git a/util/cli/config.go b/util/cli/config.go new file mode 100644 index 0000000000000..31315317d5051 --- /dev/null +++ b/util/cli/config.go @@ -0,0 +1,74 @@ +package cli + +import ( + "encoding/json" + "io/ioutil" + "net/http" + + "github.com/ghodss/yaml" +) + +// unmarshalObject tries to convert a YAML or JSON byte array into the provided type. +func unmarshalObject(data []byte, obj interface{}) error { + // first, try unmarshaling as JSON + // Based on technique from Kubectl, which supports both YAML and JSON: + // https://mlafeldt.github.io/blog/teaching-go-programs-to-love-json-and-yaml/ + // http://ghodss.com/2014/the-right-way-to-handle-yaml-in-golang/ + // Short version: JSON unmarshaling won't zero out null fields; YAML unmarshaling will. + // This may have unintended effects or hard-to-catch issues when populating our application object. + jsonData, err := yaml.YAMLToJSON(data) + if err != nil { + return err + } + + err = json.Unmarshal(jsonData, &obj) + if err != nil { + return err + } + + return err +} + +// MarshalLocalYAMLFile writes JSON or YAML to a file on disk. +// The caller is responsible for checking error return values. +func MarshalLocalYAMLFile(path string, obj interface{}) error { + yamlData, err := yaml.Marshal(obj) + if err == nil { + err = ioutil.WriteFile(path, yamlData, 0600) + } + return err +} + +// UnmarshalLocalFile retrieves JSON or YAML from a file on disk. +// The caller is responsible for checking error return values. +func UnmarshalLocalFile(path string, obj interface{}) error { + data, err := ioutil.ReadFile(path) + if err == nil { + err = unmarshalObject(data, obj) + } + return err +} + +// UnmarshalRemoteFile retrieves JSON or YAML through a GET request. +// The caller is responsible for checking error return values. +func UnmarshalRemoteFile(url string, obj interface{}) error { + data, err := readRemoteFile(url) + if err == nil { + err = unmarshalObject(data, obj) + } + return err +} + +// ReadRemoteFile issues a GET request to retrieve the contents of the specified URL as a byte array. +// The caller is responsible for checking error return values. +func readRemoteFile(url string) ([]byte, error) { + var data []byte + resp, err := http.Get(url) + if err == nil { + defer func() { + _ = resp.Body.Close() + }() + data, err = ioutil.ReadAll(resp.Body) + } + return data, err +} diff --git a/util/cli/config_test.go b/util/cli/config_test.go new file mode 100644 index 0000000000000..866d5c1d464a6 --- /dev/null +++ b/util/cli/config_test.go @@ -0,0 +1,94 @@ +package cli + +import ( + "fmt" + "io/ioutil" + "net" + "net/http" + "os" + "testing" +) + +func TestUnmarshalLocalFile(t *testing.T) { + const ( + field1 = "Hello, world!" + field2 = 42 + ) + sentinel := fmt.Sprintf("---\nfield1: %q\nfield2: %d", field1, field2) + + file, err := ioutil.TempFile(os.TempDir(), "") + if err != nil { + panic(err) + } + defer func() { + _ = os.Remove(file.Name()) + }() + + _, _ = file.WriteString(sentinel) + _ = file.Sync() + + var testStruct struct { + Field1 string + Field2 int + } + err = UnmarshalLocalFile(file.Name(), &testStruct) + if err != nil { + t.Errorf("Could not unmarshal test data: %s", err) + } + + if testStruct.Field1 != field1 || testStruct.Field2 != field2 { + t.Errorf("Test data did not match! Expected {%s %d} but got: %v", field1, field2, testStruct) + } +} + +func TestUnmarshalRemoteFile(t *testing.T) { + const ( + field1 = "Hello, world!" + field2 = 42 + ) + sentinel := fmt.Sprintf("---\nfield1: %q\nfield2: %d", field1, field2) + + serve := func(c chan<- string) { + // listen on first available dynamic (unprivileged) port + listener, err := net.Listen("tcp", ":0") + if err != nil { + panic(err) + } + + // send back the address so that it can be used + c <- listener.Addr().String() + + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + // return the sentinel text at root URL + fmt.Fprint(w, sentinel) + }) + + panic(http.Serve(listener, nil)) + } + + c := make(chan string, 1) + + // run a local webserver to test data retrieval + go serve(c) + + address := <-c + t.Logf("Listening at address: %s", address) + + data, err := readRemoteFile("http://" + address) + if string(data) != sentinel { + t.Errorf("Test data did not match (err = %v)! Expected %q and received %q", err, sentinel, string(data)) + } + + var testStruct struct { + Field1 string + Field2 int + } + err = UnmarshalRemoteFile("http://"+address, &testStruct) + if err != nil { + t.Errorf("Could not unmarshal test data: %s", err) + } + + if testStruct.Field1 != field1 || testStruct.Field2 != field2 { + t.Errorf("Test data did not match! Expected {%s %d} but got: %v", field1, field2, testStruct) + } +} diff --git a/util/config/local.go b/util/config/local.go new file mode 100644 index 0000000000000..51bc01e73e12f --- /dev/null +++ b/util/config/local.go @@ -0,0 +1,59 @@ +package config + +import ( + "os" + "os/user" + "path" + + "github.com/argoproj/argo-cd/util/cli" +) + +// LocalConfig holds all local session config. +type LocalConfig struct { + Sessions map[string]string `json:"sessions"` +} + +// LocalConfigDir returns the local configuration path for settings such as cached authentication tokens. +func localConfigDir() (string, error) { + usr, err := user.Current() + if err != nil { + return "", err + } + return path.Join(usr.HomeDir, ".argocd"), nil +} + +// LocalConfigPath returns the local configuration path for settings such as cached authentication tokens. +func localConfigPath() (string, error) { + dir, err := localConfigDir() + if err != nil { + return "", err + } + return path.Join(dir, "config"), nil +} + +// ReadLocalConfig loads up the local configuration file. +func ReadLocalConfig() (LocalConfig, error) { + config := LocalConfig{ + Sessions: make(map[string]string), + } + + path, err := localConfigPath() + if err == nil { + err = cli.UnmarshalLocalFile(path, &config) + if os.IsNotExist(err) { + err = nil + } + } + + return config, err +} + +// WriteLocalConfig writes a new local configuration file. +func WriteLocalConfig(config LocalConfig) error { + path, err := localConfigPath() + if err == nil { + err = cli.MarshalLocalYAMLFile(path, config) + } + + return err +} diff --git a/util/password/password.go b/util/password/password.go index dea3473cf3149..21ed21e11f4f0 100644 --- a/util/password/password.go +++ b/util/password/password.go @@ -2,6 +2,7 @@ package password import ( "crypto/subtle" + "fmt" "golang.org/x/crypto/bcrypt" ) @@ -30,14 +31,22 @@ var preferredHashers = []PasswordHasher{ // HashPasswordWithHashers hashes an entered password using the first hasher in the provided list of hashers. func hashPasswordWithHashers(password string, hashers []PasswordHasher) (string, error) { + // Even though good hashers will disallow blank passwords, let's be explicit that ALL BLANK PASSWORDS ARE INVALID. Full stop. + if password == "" { + return "", fmt.Errorf("blank passwords are not allowed") + } return hashers[0].HashPassword(password) } // VerifyPasswordWithHashers verifies an entered password against a hashed password using one or more algorithms. It returns whether the hash is "stale" (i.e., was verified using something other than the FIRST hasher specified). -func verifyPasswordWithHashers(password, hashedPassword string, hashers []PasswordHasher) (valid, stale bool) { - // Be explicit here for security, even though zero values can be assumed. - valid = false - stale = false +func verifyPasswordWithHashers(password, hashedPassword string, hashers []PasswordHasher) (bool, bool) { + // Even though good hashers will disallow blank passwords, let's be explicit that ALL BLANK PASSWORDS ARE INVALID. Full stop. + if password == "" { + return false, false + } + + valid, stale := false, false + for idx, hasher := range hashers { if hasher.VerifyPassword(password, hashedPassword) { valid = true @@ -47,7 +56,8 @@ func verifyPasswordWithHashers(password, hashedPassword string, hashers []Passwo break } } - return + + return valid, stale } // HashPassword hashes against the current preferred hasher. diff --git a/util/password/password_test.go b/util/password/password_test.go index 2047f11a11cb8..c4d222e719f22 100644 --- a/util/password/password_test.go +++ b/util/password/password_test.go @@ -12,10 +12,10 @@ func testPasswordHasher(t *testing.T, h PasswordHasher) { ) hashedPassword, _ := h.HashPassword(defaultPassword) if !h.VerifyPassword(defaultPassword, hashedPassword) { - t.Errorf("Password \"%s\" should have validated against hash \"%s\"", defaultPassword, hashedPassword) + t.Errorf("Password %q should have validated against hash %q", defaultPassword, hashedPassword) } if h.VerifyPassword(defaultPassword, pollution+hashedPassword) { - t.Errorf("Password \"%s\" should NOT have validated against hash \"%s\"", defaultPassword, pollution+hashedPassword) + t.Errorf("Password %q should NOT have validated against hash %q", defaultPassword, pollution+hashedPassword) } } @@ -33,6 +33,7 @@ func TestDummyPasswordHasher(t *testing.T) { func TestPasswordHashing(t *testing.T) { const ( defaultPassword = "Hello, world!" + blankPassword = "" ) hashers := []PasswordHasher{ BcryptPasswordHasher{0}, @@ -42,17 +43,26 @@ func TestPasswordHashing(t *testing.T) { hashedPassword, _ := hashPasswordWithHashers(defaultPassword, hashers) valid, stale := verifyPasswordWithHashers(defaultPassword, hashedPassword, hashers) if !valid { - t.Errorf("Password \"%s\" should have validated against hash \"%s\"", defaultPassword, hashedPassword) + t.Errorf("Password %q should have validated against hash %q", defaultPassword, hashedPassword) } if stale { - t.Errorf("Password \"%s\" should not have been marked stale against hash \"%s\"", defaultPassword, hashedPassword) + t.Errorf("Password %q should not have been marked stale against hash %q", defaultPassword, hashedPassword) } valid, stale = verifyPasswordWithHashers(defaultPassword, defaultPassword, hashers) if !valid { - t.Errorf("Password \"%s\" should have validated against itself with dummy hasher", defaultPassword) + t.Errorf("Password %q should have validated against itself with dummy hasher", defaultPassword) } if !stale { - t.Errorf("Password \"%s\" should have been acknowledged stale against itself with dummy hasher", defaultPassword) + t.Errorf("Password %q should have been acknowledged stale against itself with dummy hasher", defaultPassword) } + hashedPassword, err := hashPasswordWithHashers(blankPassword, hashers) + if err == nil { + t.Errorf("Blank password should have produced error, rather than hash %q", hashedPassword) + } + + valid, _ = verifyPasswordWithHashers(blankPassword, "", hashers) + if valid != false { + t.Errorf("Blank password should have failed verification") + } } diff --git a/util/session/sessionmanager.go b/util/session/sessionmanager.go index e6b0a645c7602..e5a6b0c5d7ba8 100644 --- a/util/session/sessionmanager.go +++ b/util/session/sessionmanager.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + util_password "github.com/argoproj/argo-cd/util/password" jwt "github.com/dgrijalva/jwt-go" ) @@ -16,6 +17,10 @@ type SessionManager struct { const ( // sessionManagerClaimsIssuer fills the "iss" field of the token. sessionManagerClaimsIssuer = "argocd" + + // invalidLoginError, for security purposes, doesn't say whether the username or password was invalid. This does not mitigate the potential for timing attacks to determine which is which. + invalidLoginError = "Invalid username or password" + blankPasswordError = "Blank passwords are not allowed" ) // SessionManagerTokenClaims holds claim metadata for a token. @@ -82,3 +87,30 @@ func MakeSignature(size int) ([]byte, error) { } return b, err } + +// LoginLocalUser checks if a username/password combo is correct and creates a new token if so. +// [TODO] This may belong elsewhere. +func (mgr SessionManager) LoginLocalUser(username, password string, users map[string]string) (string, error) { + if password == "" { + err := fmt.Errorf(blankPasswordError) + return "", err + } + + passwordHash, ok := users[username] + if !ok { + // Username was not found in local user store. + // Ensure we still send password to hashing algorithm for comparison. + // This mitigates potential for timing attacks that benefit from short-circuiting, + // provided the hashing library/algorithm in use doesn't itself short-circuit. + passwordHash = "" + } + + if valid, _ := util_password.VerifyPassword(password, passwordHash); valid { + token, err := mgr.Create(username) + if err == nil { + return token, nil + } + } + + return "", fmt.Errorf(invalidLoginError) +}