Merge pull request #548 from sgotti/keeper_remove_password_trailing_n…

…ewlines

keeper: remove trailing new lines from passwords

cmd/keeper/cmd/keeper.go

@@ -165,7 +165,7 @@ func readPasswordFromFile(filepath string) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("unable to read password from file %s: %v", filepath, err)
 	}
-	return strings.TrimSpace(string(pwBytes)), nil
+	return string(pwBytes), nil
 }
 
 // walLevel returns the wal_level value to use.
@@ -1806,16 +1806,6 @@ func keeper(c *cobra.Command, args []string) {
 		log.Fatalf("only one of --pg-su-password or --pg-su-passwordfile must be provided")
 	}
 
-	if cfg.pgSUUsername == cfg.pgReplUsername {
-		log.Warn("superuser name and replication user name are the same. Different users are suggested.")
-		if cfg.pgReplAuthMethod != cfg.pgSUAuthMethod {
-			log.Fatalf("do not support different auth methods when utilizing superuser for replication.")
-		}
-		if cfg.pgSUPassword != cfg.pgReplPassword && cfg.pgSUAuthMethod != "trust" && cfg.pgReplAuthMethod != "trust" {
-			log.Fatalf("provided superuser name and replication user name are the same but provided passwords are different")
-		}
-	}
-
 	if cfg.pgReplPasswordFile != "" {
 		cfg.pgReplPassword, err = readPasswordFromFile(cfg.pgReplPasswordFile)
 		if err != nil {
@@ -1829,6 +1819,35 @@ func keeper(c *cobra.Command, args []string) {
 		}
 	}
 
+	// Trim trailing new lines from passwords
+	tp := strings.TrimRight(cfg.pgSUPassword, "\r\n")
+	if cfg.pgSUPassword != tp {
+		log.Warn("superuser password contain trailing new line, removing")
+		if tp == "" {
+			log.Fatalf("superuser password is empty after removing trailing new line")
+		}
+		cfg.pgSUPassword = tp
+	}
+
+	tp = strings.TrimRight(cfg.pgReplPassword, "\r\n")
+	if cfg.pgReplPassword != tp {
+		log.Warn("replication user password contain trailing new line, removing")
+		if tp == "" {
+			log.Fatalf("replication user password is empty after removing trailing new line")
+		}
+		cfg.pgReplPassword = tp
+	}
+
+	if cfg.pgSUUsername == cfg.pgReplUsername {
+		log.Warn("superuser name and replication user name are the same. Different users are suggested.")
+		if cfg.pgReplAuthMethod != cfg.pgSUAuthMethod {
+			log.Fatalf("do not support different auth methods when utilizing superuser for replication.")
+		}
+		if cfg.pgSUPassword != cfg.pgReplPassword && cfg.pgSUAuthMethod != "trust" && cfg.pgReplAuthMethod != "trust" {
+			log.Fatalf("provided superuser name and replication user name are the same but provided passwords are different")
+		}
+	}
+
 	// Open (and create if needed) the lock file.
 	// There is no need to clean up this file since we don't use the file as an actual lock. We get a lock
 	// on the file. So the lock get released when our process stops (or log.Fatalfs).

tests/integration/init_test.go

@@ -512,3 +512,90 @@ func TestExclusiveLock(t *testing.T) {
 		t.Fatalf("expecting keeper reporting that failed to take an exclusive lock on data dir")
 	}
 }
+
+func TestPasswordTrailingNewLine(t *testing.T) {
+	t.Parallel()
+
+	dir, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	tstore, err := NewTestStore(t, dir)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if err := tstore.Start(); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if err := tstore.WaitUp(10 * time.Second); err != nil {
+		t.Fatalf("error waiting on store up: %v", err)
+	}
+	storeEndpoints := fmt.Sprintf("%s:%s", tstore.listenAddress, tstore.port)
+	defer tstore.Stop()
+
+	clusterName := uuid.NewV4().String()
+
+	u := uuid.NewV4()
+	id := fmt.Sprintf("%x", u[:4])
+
+	pgSUPassword := "stolon_superuserpassword\n"
+	pgReplPassword := "stolon_replpassword\n"
+
+	tk, err := NewTestKeeperWithID(t, dir, id, clusterName, pgSUUsername, pgSUPassword, pgReplUsername, pgReplPassword, tstore.storeBackend, storeEndpoints)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if err := tk.StartExpect(); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	if err := tk.cmd.ExpectTimeout("superuser password contain trailing new line, removing", 30*time.Second); err != nil {
+		t.Fatalf(`expecting keeper reporting "superuser password contain trailing new line, removing"`)
+	}
+	if err := tk.cmd.ExpectTimeout("replication user password contain trailing new line, removing", 30*time.Second); err != nil {
+		t.Fatalf(`expecting keeper reporting "replication user password contain trailing new line, removing"`)
+	}
+	tk.Stop()
+
+	pgSUPassword = "stolon_superuserpassword\n"
+	pgReplPassword = "\n"
+
+	tk, err = NewTestKeeperWithID(t, dir, id, clusterName, pgSUUsername, pgSUPassword, pgReplUsername, pgReplPassword, tstore.storeBackend, storeEndpoints)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if err := tk.StartExpect(); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	if err := tk.cmd.ExpectTimeout("replication user password contain trailing new line, removing", 30*time.Second); err != nil {
+		t.Fatalf(`expecting keeper reporting "replication user password contain trailing new line, removing"`)
+	}
+	if err := tk.cmd.ExpectTimeout("replication user password is empty after removing trailing new line", 30*time.Second); err != nil {
+		t.Fatalf(`expecting keeper reporting "replication user password is empty after removing trailing new line"`)
+	}
+
+	tk.Stop()
+
+	pgSUPassword = "\n"
+	pgReplPassword = "stolon_replpassword\n"
+
+	tk, err = NewTestKeeperWithID(t, dir, id, clusterName, pgSUUsername, pgSUPassword, pgReplUsername, pgReplPassword, tstore.storeBackend, storeEndpoints)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if err := tk.StartExpect(); err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	if err := tk.cmd.ExpectTimeout("superuser password contain trailing new line, removing", 30*time.Second); err != nil {
+		t.Fatalf(`expecting keeper reporting "superuser password contain trailing new line, removing"`)
+	}
+	if err := tk.cmd.ExpectTimeout("superuser password is empty after removing trailing new line", 30*time.Second); err != nil {
+		t.Fatalf(`expecting keeper reporting "superuser password is empty after removing trailing new line"`)
+	}
+
+	tk.Stop()
+}