Skip to content

Commit

Permalink
feat(fips): remove keystore subcommand and config handling
Browse files Browse the repository at this point in the history
the keystore is providing obfuscation of data on disk by using
an empty password by default.
This fails in fips only mode with the following error:

crypto/hmac: use of keys shorter than 112 bits is not allowed in FIPS 140-only mode

Disable the keystore subcommand and do not try to create one
when loading the config.
  • Loading branch information
kruskall committed Feb 4, 2025
1 parent 9350ef5 commit f7434ce
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 48 deletions.
4 changes: 3 additions & 1 deletion internal/beatcmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ func NewRootCommand(beatParams BeatParams) *cobra.Command {
// Register subcommands.
rootCommand.AddCommand(runCommand)
rootCommand.AddCommand(exportCommand)
rootCommand.AddCommand(keystoreCommand)
if keystoreCommand != nil {
rootCommand.AddCommand(keystoreCommand)
}
rootCommand.AddCommand(versionCommand)
rootCommand.AddCommand(genTestCmd(beatParams))

Expand Down
14 changes: 5 additions & 9 deletions internal/beatcmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/elastic/beats/v7/libbeat/cfgfile"
"github.com/elastic/beats/v7/libbeat/cloudid"
"github.com/elastic/beats/v7/libbeat/common"
"github.com/elastic/beats/v7/libbeat/pprof"
"github.com/elastic/elastic-agent-libs/config"
libkeystore "github.com/elastic/elastic-agent-libs/keystore"
Expand Down Expand Up @@ -94,10 +93,14 @@ func LoadConfig(opts ...LoadConfigOption) (*Config, *config.C, libkeystore.Keyst
configOpts = append(configOpts, ucfg.ResolveNOOP)
} else {
configOpts = append(configOpts,
ucfg.Resolve(libkeystore.ResolverWrap(keystore)),
ucfg.ResolveEnv,
ucfg.VarExp,
)
if keystore != nil {
configOpts = append(configOpts,
ucfg.Resolve(libkeystore.ResolverWrap(keystore)),
)
}
}
config.OverwriteConfigOpts(configOpts)

Expand Down Expand Up @@ -138,13 +141,6 @@ func WithMergeConfig(cfg ...*config.C) LoadConfigOption {
}
}

// loadKeystore returns the appropriate keystore based on the configuration.
func loadKeystore(cfg *config.C) (libkeystore.Keystore, error) {
keystoreCfg, _ := cfg.Child("keystore", -1)
defaultPathConfig := paths.Resolve(paths.Data, "apm-server.keystore")
return libkeystore.Factory(keystoreCfg, defaultPathConfig, common.IsStrictPerms())
}

func initPaths(cfg *config.C) error {
// To Fix the chicken-egg problem with the Keystore and the loading of the configuration
// files we are doing a partial unpack of the configuration file and only take into consideration
Expand Down
30 changes: 30 additions & 0 deletions internal/beatcmd/config_fips.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

//go:build requirefips

package beatcmd

import (
"github.com/elastic/elastic-agent-libs/config"
libkeystore "github.com/elastic/elastic-agent-libs/keystore"
)

// loadKeystore returns the appropriate keystore based on the configuration.
func loadKeystore(cfg *config.C) (libkeystore.Keystore, error) {
return nil, nil
}
34 changes: 34 additions & 0 deletions internal/beatcmd/config_nofips.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

//go:build !requirefips

package beatcmd

import (
"github.com/elastic/beats/v7/libbeat/common"
"github.com/elastic/elastic-agent-libs/config"
libkeystore "github.com/elastic/elastic-agent-libs/keystore"
"github.com/elastic/elastic-agent-libs/paths"
)

// loadKeystore returns the appropriate keystore based on the configuration.
func loadKeystore(cfg *config.C) (libkeystore.Keystore, error) {
keystoreCfg, _ := cfg.Child("keystore", -1)
defaultPathConfig := paths.Resolve(paths.Data, "apm-server.keystore")
return libkeystore.Factory(keystoreCfg, defaultPathConfig, common.IsStrictPerms())
}
62 changes: 62 additions & 0 deletions internal/beatcmd/config_nofips_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

package beatcmd

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/elastic/elastic-agent-libs/keystore"
)

func TestLoadConfigKeystore(t *testing.T) {
initCfgfile(t, `
apm-server:
auth.secret_token: ${APM_SECRET_TOKEN}
`)

cfg, _, _, err := LoadConfig(WithDisableConfigResolution())
require.NoError(t, err)
assertConfigEqual(t, map[string]interface{}{
"auth": map[string]interface{}{
"secret_token": "${APM_SECRET_TOKEN}",
},
}, cfg.APMServer)

cfg, _, ks, err := LoadConfig()
require.NoError(t, err)

err = cfg.APMServer.Unpack(new(map[string]interface{}))
require.Error(t, err)
assert.Contains(t, err.Error(), `missing field accessing 'apm-server.auth'`)

wks, err := keystore.AsWritableKeystore(ks)
require.NoError(t, err)
err = wks.Store("APM_SECRET_TOKEN", []byte("abc123"))
require.NoError(t, err)
err = wks.Save()
require.NoError(t, err)

assertConfigEqual(t, map[string]interface{}{
"auth": map[string]interface{}{
"secret_token": "abc123",
},
}, cfg.APMServer)
}
39 changes: 1 addition & 38 deletions internal/beatcmd/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (

"github.com/elastic/beats/v7/libbeat/cfgfile"
"github.com/elastic/elastic-agent-libs/config"
"github.com/elastic/elastic-agent-libs/keystore"
"github.com/elastic/elastic-agent-libs/paths"
)

Expand All @@ -37,11 +36,10 @@ func TestLoadConfig(t *testing.T) {
apm-server:
host: :8200
`)
cfg, rawConfig, keystore, err := LoadConfig()
cfg, rawConfig, _, err := LoadConfig()
require.NoError(t, err)
assert.NotNil(t, cfg)
assert.NotNil(t, rawConfig)
assert.NotNil(t, keystore)

assertConfigEqual(t, map[string]interface{}{
"apm-server": map[string]interface{}{
Expand Down Expand Up @@ -75,41 +73,6 @@ apm-server:
}, cfg.APMServer)
}

func TestLoadConfigKeystore(t *testing.T) {
initCfgfile(t, `
apm-server:
auth.secret_token: ${APM_SECRET_TOKEN}
`)

cfg, _, _, err := LoadConfig(WithDisableConfigResolution())
require.NoError(t, err)
assertConfigEqual(t, map[string]interface{}{
"auth": map[string]interface{}{
"secret_token": "${APM_SECRET_TOKEN}",
},
}, cfg.APMServer)

cfg, _, ks, err := LoadConfig()
require.NoError(t, err)

err = cfg.APMServer.Unpack(new(map[string]interface{}))
require.Error(t, err)
assert.Contains(t, err.Error(), `missing field accessing 'apm-server.auth'`)

wks, err := keystore.AsWritableKeystore(ks)
require.NoError(t, err)
err = wks.Store("APM_SECRET_TOKEN", []byte("abc123"))
require.NoError(t, err)
err = wks.Save()
require.NoError(t, err)

assertConfigEqual(t, map[string]interface{}{
"auth": map[string]interface{}{
"secret_token": "abc123",
},
}, cfg.APMServer)
}

func assertConfigEqual(t testing.TB, expected map[string]interface{}, actual *config.C) {
t.Helper()
var m map[string]interface{}
Expand Down
24 changes: 24 additions & 0 deletions internal/beatcmd/keystore_fips.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

//go:build requirefips

package beatcmd

import "github.com/spf13/cobra"

var keystoreCommand *cobra.Command = nil
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.

//go:build !requirefips

package beatcmd

import (
Expand Down

0 comments on commit f7434ce

Please sign in to comment.