Set DisableSSCTokens=true as the default config

This changes the default behavior of OpenBao to disable SSCTs, modifying tests &c to align with this. Internal state required for SSCTs does not exist in OpenBao (as it is a Vault Enterprise feature) and thus while existing SSCT wrapped tokens from Vault would work in OpenBao, they are not populated with consistency information and so no benefit is seen from keeping them enabled by default. Signed-off-by: Alexander Scheel <[email protected]>
openbao · Jun 12, 2024 · 53ff6ef · 53ff6ef
1 parent 15c4855
commit 53ff6ef
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 22 deletions.
diff --git a/changelog/298.txt b/changelog/298.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+core: re-introduce Server Side Consistent Tokens (SSCTs) from upstream, defaulting to disabled
+```
diff --git a/command/server.go b/command/server.go
@@ -2657,7 +2657,7 @@ func createCoreConfig(c *ServerCommand, config *server.Config, backend physical.
  SecureRandomReader: secureRandomReader,
  EnableResponseHeaderHostname: config.EnableResponseHeaderHostname,
  EnableResponseHeaderRaftNodeID: config.EnableResponseHeaderRaftNodeID,
- DisableSSCTokens: config.DisableSSCTokens,
+ DisableSSCTokens: *config.DisableSSCTokens,
  AdministrativeNamespacePath: config.AdministrativeNamespacePath,
  }
 

diff --git a/command/server/config.go b/command/server/config.go
@@ -108,7 +108,7 @@ type Config struct {
  EnableResponseHeaderRaftNodeID bool `hcl:"-"`
  EnableResponseHeaderRaftNodeIDRaw interface{} `hcl:"enable_response_header_raft_node_id"`
 
- DisableSSCTokens bool `hcl:"-"`
+ DisableSSCTokens *bool `hcl:"-"`
 }
 
 const (
@@ -677,6 +677,13 @@ func ParseConfig(d, source string) (*Config, error) {
  }
  }
 
+ // We default to disabling SSCTs if it is not enabled in the
+ // configuration explicitly.
+ if result.DisableSSCTokens == nil {
+ disableSSCTokens := true
+ result.DisableSSCTokens = &disableSSCTokens
+ }
+
  list, ok := obj.Node.(*ast.ObjectList)
  if !ok {
  return nil, fmt.Errorf("error parsing: file doesn't contain a root object")

diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go
@@ -34,6 +34,8 @@ func testConfigRaftRetryJoin(t *testing.T) {
  if err != nil {
  t.Fatal(err)
  }
+
+ disableSSCTs := true
  retryJoinConfig := `[{"leader_api_addr":"http:https://127.0.0.1:8200"},{"leader_api_addr":"http:https://127.0.0.2:8200"},{"leader_api_addr":"http:https://127.0.0.3:8200"}]`
  expected := &Config{
  SharedConfig: &configutil.SharedConfig{
@@ -55,6 +57,8 @@ func testConfigRaftRetryJoin(t *testing.T) {
  "retry_join": retryJoinConfig,
  },
  },
+
+ DisableSSCTokens: &disableSSCTs,
  }
  config.Prune()
  if diff := deep.Equal(config, expected); diff != nil {
@@ -68,6 +72,7 @@ func testLoadConfigFile_topLevel(t *testing.T, entropy *configutil.Entropy) {
  t.Fatalf("err: %s", err)
  }
 
+ disableSSCTs := true
  expected := &Config{
  SharedConfig: &configutil.SharedConfig{
  Listeners: []*configutil.Listener{
@@ -161,6 +166,8 @@ func testLoadConfigFile_topLevel(t *testing.T, entropy *configutil.Entropy) {
 
  APIAddr: "top_level_api_addr",
  ClusterAddr: "top_level_cluster_addr",
+
+ DisableSSCTokens: &disableSSCTs,
  }
  addExpectedEntConfig(expected, []string{})
 
@@ -176,6 +183,8 @@ func testLoadConfigFile_json2(t *testing.T, entropy *configutil.Entropy) {
  t.Fatalf("err: %s", err)
  }
 
+ disableSSCTs := true
+
  expected := &Config{
  SharedConfig: &configutil.SharedConfig{
  Listeners: []*configutil.Listener{
@@ -249,6 +258,8 @@ func testLoadConfigFile_json2(t *testing.T, entropy *configutil.Entropy) {
 
  DisableSealWrap: true,
  DisableSealWrapRaw: true,
+
+ DisableSSCTokens: &disableSSCTs,
  }
  addExpectedEntConfig(expected, []string{"http"})
 
@@ -272,6 +283,8 @@ func testLoadConfigFileIntegerAndBooleanValuesCommon(t *testing.T, path string)
  t.Fatalf("err: %s", err)
  }
 
+ disableSSCTs := true
+
  expected := &Config{
  SharedConfig: &configutil.SharedConfig{
  Listeners: []*configutil.Listener{
@@ -302,6 +315,8 @@ func testLoadConfigFileIntegerAndBooleanValuesCommon(t *testing.T, path string)
  DisableCacheRaw: true,
  EnableUI: true,
  EnableUIRaw: true,
+
+ DisableSSCTokens: &disableSSCTs,
  }
 
  config.Prune()
@@ -316,6 +331,8 @@ func testLoadConfigFile(t *testing.T) {
  t.Fatalf("err: %s", err)
  }
 
+ disableSSCTs := true
+
  expected := &Config{
  SharedConfig: &configutil.SharedConfig{
  Listeners: []*configutil.Listener{
@@ -397,6 +414,8 @@ func testLoadConfigFile(t *testing.T) {
  EnableResponseHeaderHostnameRaw: true,
  EnableResponseHeaderRaftNodeID: true,
  EnableResponseHeaderRaftNodeIDRaw: true,
+
+ DisableSSCTokens: &disableSSCTs,
  }
 
  addExpectedEntConfig(expected, []string{})
@@ -525,6 +544,8 @@ func testLoadConfigFile_json(t *testing.T) {
  t.Fatalf("err: %s", err)
  }
 
+ disableSSCTs := true
+
  expected := &Config{
  SharedConfig: &configutil.SharedConfig{
  Listeners: []*configutil.Listener{
@@ -592,6 +613,7 @@ func testLoadConfigFile_json(t *testing.T) {
  EnableRawEndpointRaw: true,
  DisableSealWrap: true,
  DisableSealWrapRaw: true,
+ DisableSSCTokens: &disableSSCTs,
  }
 
  addExpectedEntConfig(expected, []string{})
@@ -935,6 +957,8 @@ EOF
  t.Fatal(err)
  }
 
+ disableSSCTs := true
+
  expected := &Config{
  APIAddr: "127.0.0.1",
  SharedConfig: &configutil.SharedConfig{
@@ -948,6 +972,7 @@ EOF
  },
  },
  },
+ DisableSSCTokens: &disableSSCTs,
  }
  config.Prune()
  if diff := deep.Equal(config, expected); diff != nil {
@@ -974,6 +999,8 @@ ha_storage "consul" {
  t.Fatal(err)
  }
 
+ disableSSCTs := true
+
  expected := &Config{
  Storage: &Storage{
  Type: "consul",
@@ -990,7 +1017,8 @@ ha_storage "consul" {
  "max_parallel": "128",
  },
  },
- SharedConfig: &configutil.SharedConfig{},
+ SharedConfig: &configutil.SharedConfig{},
+ DisableSSCTokens: &disableSSCTs,
  }
  config.Prune()
  if diff := deep.Equal(config, expected); diff != nil {
@@ -1005,6 +1033,8 @@ func testParseSeals(t *testing.T) {
  }
  config.Listeners[0].RawConfig = nil
 
+ disableSSCTs := true
+
  expected := &Config{
  Storage: &Storage{
  Type: "consul",
@@ -1052,6 +1082,7 @@ func testParseSeals(t *testing.T) {
  },
  },
  },
+ DisableSSCTokens: &disableSSCTs,
  }
  addExpectedDefaultEntConfig(expected)
  config.Prune()
@@ -1064,6 +1095,8 @@ func testLoadConfigFileLeaseMetrics(t *testing.T) {
  t.Fatalf("err: %s", err)
  }
 
+ disableSSCTs := true
+
  expected := &Config{
  SharedConfig: &configutil.SharedConfig{
  Listeners: []*configutil.Listener{
@@ -1138,6 +1171,7 @@ func testLoadConfigFileLeaseMetrics(t *testing.T) {
  MaxLeaseTTLRaw: "10h",
  DefaultLeaseTTL: 10 * time.Hour,
  DefaultLeaseTTLRaw: "10h",
+ DisableSSCTokens: &disableSSCTs,
  }
 
  addExpectedEntConfig(expected, []string{})
@@ -1154,6 +1188,7 @@ func testConfigRaftAutopilot(t *testing.T) {
  t.Fatal(err)
  }
 
+ disableSSCTs := true
  autopilotConfig := `[{"cleanup_dead_servers":true,"last_contact_threshold":"500ms","max_trailing_logs":250,"min_quorum":3,"server_stabilization_time":"10s"}]`
  expected := &Config{
  SharedConfig: &configutil.SharedConfig{
@@ -1174,6 +1209,7 @@ func testConfigRaftAutopilot(t *testing.T) {
  "autopilot": autopilotConfig,
  },
  },
+ DisableSSCTokens: &disableSSCTs,
  }
  config.Prune()
  if diff := deep.Equal(config, expected); diff != nil {

diff --git a/vault/expiration.go b/vault/expiration.go
@@ -1619,7 +1619,7 @@ func (m *ExpirationManager) RegisterAuth(ctx context.Context, te *logical.TokenE
  m.updatePending(&le)
  if strings.HasPrefix(auth.ClientToken, consts.ServiceTokenPrefix) {
  generatedTokenEntry := logical.TokenEntry{Policies: auth.Policies}
- tok := m.tokenStore.GenerateSSCTokenID(auth.ClientToken, logical.IndexStateFromContext(ctx), &generatedTokenEntry)
+ tok := m.tokenStore.GenerateSSCTokenID(auth.ClientToken, &generatedTokenEntry)
  te.ExternalID = tok
  }
 

diff --git a/vault/request_handling.go b/vault/request_handling.go
@@ -629,9 +629,6 @@ func (c *Core) handleCancelableRequest(ctx context.Context, req *logical.Request
  // should receive a 403 bad token error like they do for all other invalid tokens, unless the error
  // specifies that we should forward the request or retry the request.
  if err != nil {
- if errors.Is(err, logical.ErrMissingRequiredState) {
- return nil, err
- }
  return logical.ErrorResponse("bad token"), logical.ErrPermissionDenied
  }
  req.Data["token"] = token
@@ -2112,9 +2109,6 @@ func (c *Core) PopulateTokenEntry(ctx context.Context, req *logical.Request) err
  // should receive a 403 bad token error like they do for all other invalid tokens, unless the error
  // specifies that we should forward the request or retry the request.
  if err != nil {
- if errors.Is(err, logical.ErrMissingRequiredState) {
- return err
- }
  return logical.ErrPermissionDenied
  }
  }

diff --git a/vault/token_store.go b/vault/token_store.go
@@ -1138,7 +1138,7 @@ func (ts *TokenStore) create(ctx context.Context, entry *logical.TokenEntry) err
  }
  entry.ExternalID = entry.ID
  if !userSelectedID && !ts.core.DisableSSCTokens() {
- entry.ExternalID = ts.GenerateSSCTokenID(entry.ID, logical.IndexStateFromContext(ctx), entry)
+ entry.ExternalID = ts.GenerateSSCTokenID(entry.ID, entry)
  }
  return nil
 
@@ -1220,7 +1220,7 @@ func (ts *TokenStore) create(ctx context.Context, entry *logical.TokenEntry) err
 // minted service tokens. This function is meant to be robust so as to allow vault
 // to continue operating even in the case where IDs can't be generated. Thus it logs
 // errors as opposed to throwing them.
-func (ts *TokenStore) GenerateSSCTokenID(innerToken string, walState *logical.WALState, te *logical.TokenEntry) string {
+func (ts *TokenStore) GenerateSSCTokenID(innerToken string, te *logical.TokenEntry) string {
  // Set up the prefix prepending function. This should really only be used in
  // the token ID generation code itself.
  prependServicePrefix := func(externalToken string) string {
@@ -1239,14 +1239,6 @@ func (ts *TokenStore) GenerateSSCTokenID(innerToken string, walState *logical.WA
  return prependServicePrefix(innerToken)
  }
 
- // If there is no WAL state, do not throw an error as it may be a single
- // node cluster, or an OSS core. Instead, log that this has happened and
- // create a walState with nil values to signify that these values should
- // be ignored
- if walState == nil {
- ts.logger.Debug("no wal state found when generating token")
- walState = &logical.WALState{}
- }
  if te.IsRoot() {
  return prependServicePrefix(innerToken)
  }
@@ -1255,10 +1247,9 @@ func (ts *TokenStore) GenerateSSCTokenID(innerToken string, walState *logical.WA
  // that root tokens are always fixed size. This is required because during root token
  // generation, the size needs to be known to create the OTP.
 
- localIndex := walState.LocalIndex
  tokenGenerationCounter := uint32(ts.GetSSCTokensGenerationCounter())
 
- t := tokens.Token{Random: innerToken, LocalIndex: localIndex, IndexEpoch: tokenGenerationCounter}
+ t := tokens.Token{Random: innerToken, LocalIndex: 0, IndexEpoch: tokenGenerationCounter}
  marshalledToken, err := proto.Marshal(&t)
  if err != nil {
  ts.logger.Error("unable to marshal token", "error", err)