[ARVADOS] created: 2.1.0-183-gcb74b7536

[Git user]

        at  cb74b75363ede0bb5e2ec603b6ccb0230392f42f (commit)


commit cb74b75363ede0bb5e2ec603b6ccb0230392f42f
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Thu Dec 3 17:23:47 2020 -0500

    17151: Reject site secrets that are short or have undesirable chars.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/config/deprecated_test.go b/lib/config/deprecated_test.go
index ca376ba0b..0dd03583d 100644
--- a/lib/config/deprecated_test.go
+++ b/lib/config/deprecated_test.go
@@ -150,7 +150,7 @@ func (s *LoadSuite) TestLegacyKeepWebConfig(c *check.C) {
 }
 `)
 	cluster, err := testLoadLegacyConfig(content, "-legacy-keepweb-config", c)
-	c.Check(err, check.IsNil)
+	c.Assert(err, check.IsNil)
 
 	c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com", Path: "/"})
 	c.Check(cluster.SystemRootToken, check.Equals, "abcdefg")
@@ -183,7 +183,7 @@ func (s *LoadSuite) TestLegacyKeepWebConfigDoesntDisableMissingItems(c *check.C)
 }
 `)
 	cluster, err := testLoadLegacyConfig(content, "-legacy-keepweb-config", c)
-	c.Check(err, check.IsNil)
+	c.Assert(err, check.IsNil)
 	// The resulting ManagementToken should be the one set up on the test server.
 	c.Check(cluster.ManagementToken, check.Equals, TestServerManagementToken)
 }
@@ -193,8 +193,8 @@ func (s *LoadSuite) TestLegacyKeepproxyConfig(c *check.C) {
 	content := []byte(fmtKeepproxyConfig("", true))
 	cluster, err := testLoadLegacyConfig(content, f, c)
 
-	c.Check(err, check.IsNil)
-	c.Check(cluster, check.NotNil)
+	c.Assert(err, check.IsNil)
+	c.Assert(cluster, check.NotNil)
 	c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com", Path: "/"})
 	c.Check(cluster.SystemRootToken, check.Equals, "abcdefg")
 	c.Check(cluster.ManagementToken, check.Equals, "xyzzy")
@@ -262,8 +262,8 @@ func (s *LoadSuite) TestLegacyArvGitHttpdConfig(c *check.C) {
 	f := "-legacy-git-httpd-config"
 	cluster, err := testLoadLegacyConfig(content, f, c)
 
-	c.Check(err, check.IsNil)
-	c.Check(cluster, check.NotNil)
+	c.Assert(err, check.IsNil)
+	c.Assert(cluster, check.NotNil)
 	c.Check(cluster.Services.Controller.ExternalURL, check.Equals, arvados.URL{Scheme: "https", Host: "example.com", Path: "/"})
 	c.Check(cluster.SystemRootToken, check.Equals, "abcdefg")
 	c.Check(cluster.ManagementToken, check.Equals, "xyzzy")
@@ -285,7 +285,7 @@ func (s *LoadSuite) TestLegacyArvGitHttpdConfigDoesntDisableMissingItems(c *chec
 }
 `)
 	cluster, err := testLoadLegacyConfig(content, "-legacy-git-httpd-config", c)
-	c.Check(err, check.IsNil)
+	c.Assert(err, check.IsNil)
 	// The resulting ManagementToken should be the one set up on the test server.
 	c.Check(cluster.ManagementToken, check.Equals, TestServerManagementToken)
 }
@@ -295,8 +295,8 @@ func (s *LoadSuite) TestLegacyKeepBalanceConfig(c *check.C) {
 	content := []byte(fmtKeepBalanceConfig(""))
 	cluster, err := testLoadLegacyConfig(content, f, c)
 
-	c.Check(err, check.IsNil)
-	c.Check(cluster, check.NotNil)
+	c.Assert(err, check.IsNil)
+	c.Assert(cluster, check.NotNil)
 	c.Check(cluster.ManagementToken, check.Equals, "xyzzy")
 	c.Check(cluster.Services.Keepbalance.InternalURLs[arvados.URL{Host: ":80"}], check.Equals, arvados.ServiceInstance{})
 	c.Check(cluster.Collections.BalanceCollectionBuffers, check.Equals, 1000)
diff --git a/lib/config/load.go b/lib/config/load.go
index be6181bbe..a72ed6eee 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -13,6 +13,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"regexp"
 	"strings"
 
 	"git.arvados.org/arvados.git/sdk/go/arvados"
@@ -270,6 +271,10 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
 	// Check for known mistakes
 	for id, cc := range cfg.Clusters {
 		for _, err = range []error{
+			checkUnacceptableToken(fmt.Sprintf("Clusters.%s.ManagementToken", id), cc.ManagementToken),
+			checkUnacceptableToken(fmt.Sprintf("Clusters.%s.SystemRootToken", id), cc.SystemRootToken),
+			checkUnacceptableToken(fmt.Sprintf("Clusters.%s.API.RailsSessionSecretToken", id), cc.API.RailsSessionSecretToken),
+			checkUnacceptableToken(fmt.Sprintf("Clusters.%s.Collections.BlobSigningKey", id), cc.Collections.BlobSigningKey),
 			checkKeyConflict(fmt.Sprintf("Clusters.%s.PostgreSQL.Connection", id), cc.PostgreSQL.Connection),
 			ldr.checkEmptyKeepstores(cc),
 			ldr.checkUnlistedKeepstores(cc),
@@ -282,6 +287,21 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
 	return &cfg, nil
 }
 
+var acceptableTokenRe = regexp.MustCompile(`^[a-zA-Z0-9]+$`)
+var acceptableTokenLength = 4
+
+func checkUnacceptableToken(label, token string) error {
+	if token == "" {
+		// It's not an error to have an empty (unconfigured)
+		// token here.
+	} else if !acceptableTokenRe.MatchString(token) {
+		return fmt.Errorf("%s: unacceptable characters in token (only a-z, A-Z, 0-9 are acceptable)", label)
+	} else if len(token) < acceptableTokenLength {
+		return fmt.Errorf("%s: token is too short (must be at least %d characters)", label, acceptableTokenLength)
+	}
+	return nil
+}
+
 func checkKeyConflict(label string, m map[string]string) error {
 	saw := map[string]bool{}
 	for k := range m {
diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 58ddf950e..13e420af2 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -279,6 +279,22 @@ func (s *LoadSuite) TestNoWarningsForDumpedConfig(c *check.C) {
 	c.Check(logbuf.String(), check.Equals, "")
 }
 
+func (s *LoadSuite) TestUnacceptableTokens(c *check.C) {
+	for _, trial := range []struct {
+		configPath string
+		example    string
+	}{
+		{"SystemRootToken", "SystemRootToken: a_b_c"},
+		{"ManagementToken", "ManagementToken: a b c"},
+		{"API.RailsSessionSecretToken", "API: {RailsSessionSecretToken: \"$abc\"}"},
+		{"Collections.BlobSigningKey", "Collections: {BlobSigningKey: \"⛵\"}"},
+	} {
+		c.Logf("trying bogus config: %s", trial.example)
+		_, err := testLoader(c, "Clusters:\n zzzzz:\n  "+trial.example, nil).Load()
+		c.Check(err, check.ErrorMatches, `Clusters.zzzzz.`+trial.configPath+`: unacceptable characters in token.*`)
+	}
+}
+
 func (s *LoadSuite) TestPostgreSQLKeyConflict(c *check.C) {
 	_, err := testLoader(c, `
 Clusters:

-----------------------------------------------------------------------


hooks/post-receive
--