[ARVADOS] created: 2.1.0-183-g519a5ea91
Git user
git at public.arvados.org
Mon Dec 7 19:53:26 UTC 2020
at 519a5ea91370c4aed4be9a676bea25aef48d7ea7 (commit)
commit 519a5ea91370c4aed4be9a676bea25aef48d7ea7
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Dec 7 14:53:05 2020 -0500
Warn about missing/short secrets. Delete Rails session key.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index 3f622112e..86ac50962 100644
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -39,6 +39,12 @@ h2(#main). development main (as of 2020-10-28)
"Upgrading from 2.1.0":#v2_1_0
+h3. System token requirements
+
+System services now log a warning at startup if any of the system tokens (@ManagementToken@, @SystemRootToken@, and @Collections.BlobSigningKey@) are less than 32 characters, or contain characters other than a-z, A-Z, and 0-9. After upgrading, run @arvados-server config-check@ and update your configuration file if needed to resolve any warnings.
+
+The @API.RailsSessionSecretToken@ configuration key has been removed. Delete this entry from your configuration file after upgrading.
+
h3. Centos7 Python 3 dependency upgraded to python3
Now that Python 3 is part of the base repository in CentOS 7, the Python 3 dependency for Centos7 Arvados packages was changed from SCL rh-python36 to python3.
diff --git a/doc/install/install-api-server.html.textile.liquid b/doc/install/install-api-server.html.textile.liquid
index b8442eb06..2893111e3 100644
--- a/doc/install/install-api-server.html.textile.liquid
+++ b/doc/install/install-api-server.html.textile.liquid
@@ -48,8 +48,6 @@ h3. Tokens
<notextile>
<pre><code> SystemRootToken: <span class="userinput">"$system_root_token"</span>
ManagementToken: <span class="userinput">"$management_token"</span>
- API:
- RailsSessionSecretToken: <span class="userinput">"$rails_secret_token"</span>
Collections:
BlobSigningKey: <span class="userinput">"blob_signing_key"</span>
</code></pre>
@@ -59,8 +57,6 @@ h3. Tokens
@ManagementToken@ is used to authenticate access to system metrics.
- at API.RailsSessionSecretToken@ is required by the API server.
-
@Collections.BlobSigningKey@ is used to control access to Keep blocks.
You can generate a random token for each of these items at the command line like this:
diff --git a/lib/boot/supervisor.go b/lib/boot/supervisor.go
index 1e8e83ff3..0e4472b8a 100644
--- a/lib/boot/supervisor.go
+++ b/lib/boot/supervisor.go
@@ -611,9 +611,6 @@ func (super *Supervisor) autofillConfig(cfg *arvados.Config) error {
if cluster.ManagementToken == "" {
cluster.ManagementToken = randomHexString(64)
}
- if cluster.API.RailsSessionSecretToken == "" {
- cluster.API.RailsSessionSecretToken = randomHexString(64)
- }
if cluster.Collections.BlobSigningKey == "" {
cluster.Collections.BlobSigningKey = randomHexString(64)
}
diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index 74c3cc969..bb8d7dca1 100644
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -49,10 +49,12 @@ func (s *CommandSuite) TestCheck_NoWarnings(c *check.C) {
in := `
Clusters:
z1234:
- ManagementToken: xyzzy
- SystemRootToken: xyzzy
+ ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
API:
MaxItemsPerResponse: 1234
+ Collections:
+ BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
PostgreSQL:
Connection:
sslmode: require
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 005d2738d..5763b595d 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -212,11 +212,6 @@ Clusters:
# serving a single incoming multi-cluster (federated) request.
MaxRequestAmplification: 4
- # RailsSessionSecretToken is a string of alphanumeric characters
- # used by Rails to sign session tokens. IMPORTANT: This is a
- # site secret. It should be at least 50 characters.
- RailsSessionSecretToken: ""
-
# Maximum wall clock time to spend handling an incoming request.
RequestTimeout: 5m
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/export.go b/lib/config/export.go
index 0735354b1..e4917032f 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -69,7 +69,6 @@ var whitelist = map[string]bool{
"API.MaxKeepBlobBuffers": false,
"API.MaxRequestAmplification": false,
"API.MaxRequestSize": true,
- "API.RailsSessionSecretToken": false,
"API.RequestTimeout": true,
"API.SendTimeout": true,
"API.WebsocketClientEventQueue": false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 885bb4b8c..aeae214ef 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -218,11 +218,6 @@ Clusters:
# serving a single incoming multi-cluster (federated) request.
MaxRequestAmplification: 4
- # RailsSessionSecretToken is a string of alphanumeric characters
- # used by Rails to sign session tokens. IMPORTANT: This is a
- # site secret. It should be at least 50 characters.
- RailsSessionSecretToken: ""
-
# Maximum wall clock time to spend handling an incoming request.
RequestTimeout: 5m
diff --git a/lib/config/load.go b/lib/config/load.go
index be6181bbe..7eb403910 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,9 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
// Check for known mistakes
for id, cc := range cfg.Clusters {
for _, err = range []error{
+ ldr.checkToken(fmt.Sprintf("Clusters.%s.ManagementToken", id), cc.ManagementToken),
+ ldr.checkToken(fmt.Sprintf("Clusters.%s.SystemRootToken", id), cc.SystemRootToken),
+ ldr.checkToken(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 +286,20 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
return &cfg, nil
}
+var acceptableTokenRe = regexp.MustCompile(`^[a-zA-Z0-9]+$`)
+var acceptableTokenLength = 32
+
+func (ldr *Loader) checkToken(label, token string) error {
+ if token == "" {
+ ldr.Logger.Warnf("%s: secret token is not set (use %d+ random characters from a-z, A-Z, 0-9)", label, acceptableTokenLength)
+ } 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 {
+ ldr.Logger.Warnf("%s: token is too short (should 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..c9ed37b83 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -192,6 +192,10 @@ func (s *LoadSuite) TestDeprecatedOrUnknownWarning(c *check.C) {
_, err := testLoader(c, `
Clusters:
zzzzz:
+ ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ Collections:
+ BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
postgresql: {}
BadKey: {}
Containers: {}
@@ -261,6 +265,10 @@ func (s *LoadSuite) TestNoUnrecognizedKeysInDefaultConfig(c *check.C) {
err = yaml.Unmarshal(buf, &loaded)
c.Assert(err, check.IsNil)
+ c.Check(logbuf.String(), check.Matches, `(?ms).*SystemRootToken: secret token is not set.*`)
+ c.Check(logbuf.String(), check.Matches, `(?ms).*ManagementToken: secret token is not set.*`)
+ c.Check(logbuf.String(), check.Matches, `(?ms).*Collections.BlobSigningKey: secret token is not set.*`)
+ logbuf.Reset()
loader.logExtraKeys(loaded, supplied, "")
c.Check(logbuf.String(), check.Equals, "")
}
@@ -269,7 +277,13 @@ func (s *LoadSuite) TestNoWarningsForDumpedConfig(c *check.C) {
var logbuf bytes.Buffer
logger := logrus.New()
logger.Out = &logbuf
- cfg, err := testLoader(c, `{"Clusters":{"zzzzz":{}}}`, &logbuf).Load()
+ cfg, err := testLoader(c, `
+Clusters:
+ zzzzz:
+ ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ Collections:
+ BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`, &logbuf).Load()
c.Assert(err, check.IsNil)
yaml, err := yaml.Marshal(cfg)
c.Assert(err, check.IsNil)
@@ -279,6 +293,31 @@ func (s *LoadSuite) TestNoWarningsForDumpedConfig(c *check.C) {
c.Check(logbuf.String(), check.Equals, "")
}
+func (s *LoadSuite) TestUnacceptableTokens(c *check.C) {
+ for _, trial := range []struct {
+ short bool
+ configPath string
+ example string
+ }{
+ {false, "SystemRootToken", "SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_b_c"},
+ {false, "ManagementToken", "ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa b c"},
+ {false, "ManagementToken", "ManagementToken: \"$aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabc\""},
+ {false, "Collections.BlobSigningKey", "Collections: {BlobSigningKey: \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa⛵\"}"},
+ {true, "SystemRootToken", "SystemRootToken: a_b_c"},
+ {true, "ManagementToken", "ManagementToken: a b c"},
+ {true, "ManagementToken", "ManagementToken: \"$abc\""},
+ {true, "Collections.BlobSigningKey", "Collections: {BlobSigningKey: \"⛵\"}"},
+ } {
+ c.Logf("trying bogus config: %s", trial.example)
+ _, err := testLoader(c, "Clusters:\n zzzzz:\n "+trial.example, nil).Load()
+ if trial.short {
+ c.Check(err, check.ErrorMatches, `Clusters.zzzzz.`+trial.configPath+`: unacceptable characters in token.*`)
+ } else {
+ c.Check(err, check.ErrorMatches, `Clusters.zzzzz.`+trial.configPath+`: unacceptable characters in token.*`)
+ }
+ }
+}
+
func (s *LoadSuite) TestPostgreSQLKeyConflict(c *check.C) {
_, err := testLoader(c, `
Clusters:
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index a8d601d5f..9dc9e17dd 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -86,7 +86,6 @@ type Cluster struct {
MaxKeepBlobBuffers int
MaxRequestAmplification int
MaxRequestSize int
- RailsSessionSecretToken string
RequestTimeout Duration
SendTimeout Duration
WebsocketClientEventQueue int
diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 0cb4151ac..4562a0654 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -762,7 +762,6 @@ def setup_config():
"SystemRootToken": auth_token('system_user'),
"API": {
"RequestTimeout": "30s",
- "RailsSessionSecretToken": "e24205c490ac07e028fd5f8a692dcb398bcd654eff1aef5f9fe6891994b18483",
},
"Login": {
"SSO": {
diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb
index 69b20420a..5327713f6 100644
--- a/services/api/config/arvados_config.rb
+++ b/services/api/config/arvados_config.rb
@@ -93,7 +93,6 @@ arvcfg.declare_config "API.MaxRequestSize", Integer, :max_request_size
arvcfg.declare_config "API.MaxIndexDatabaseRead", Integer, :max_index_database_read
arvcfg.declare_config "API.MaxItemsPerResponse", Integer, :max_items_per_response
arvcfg.declare_config "API.AsyncPermissionsUpdateInterval", ActiveSupport::Duration, :async_permissions_update_interval
-arvcfg.declare_config "API.RailsSessionSecretToken", NonemptyString, :secret_token
arvcfg.declare_config "Users.AutoSetupNewUsers", Boolean, :auto_setup_new_users
arvcfg.declare_config "Users.AutoSetupNewUsersWithVmUUID", String, :auto_setup_new_users_with_vm_uuid
arvcfg.declare_config "Users.AutoSetupNewUsersWithRepository", Boolean, :auto_setup_new_users_with_repository
@@ -297,5 +296,9 @@ Server::Application.configure do
# Rails.configuration.API["Blah"]
ConfigLoader.copy_into_config $arvados_config, config
ConfigLoader.copy_into_config $remaining_config, config
- secrets.secret_key_base = $arvados_config["API"]["RailsSessionSecretToken"]
+
+ # We don't rely on cookies for authentication, so instead of
+ # requiring a signing key in config, we assign a new random one at
+ # startup.
+ secrets.secret_key_base = rand(1<<255).to_s(36)
end
diff --git a/tools/arvbox/lib/arvbox/docker/cluster-config.sh b/tools/arvbox/lib/arvbox/docker/cluster-config.sh
index 708af17d5..2a286637f 100755
--- a/tools/arvbox/lib/arvbox/docker/cluster-config.sh
+++ b/tools/arvbox/lib/arvbox/docker/cluster-config.sh
@@ -125,8 +125,6 @@ Clusters:
password: ${database_pw}
dbname: arvados_${database_env}
client_encoding: utf8
- API:
- RailsSessionSecretToken: $secret_token
Collections:
BlobSigningKey: $blob_signing_key
DefaultReplication: 1
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list