From f7434cecdc4a545570d286a20f31ceececaad491 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Tue, 4 Feb 2025 17:59:32 +0100 Subject: [PATCH] feat(fips): remove keystore subcommand and config handling 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. --- internal/beatcmd/cmd.go | 4 +- internal/beatcmd/config.go | 14 ++--- internal/beatcmd/config_fips.go | 30 +++++++++ internal/beatcmd/config_nofips.go | 34 ++++++++++ internal/beatcmd/config_nofips_test.go | 62 +++++++++++++++++++ internal/beatcmd/config_test.go | 39 +----------- internal/beatcmd/keystore_fips.go | 24 +++++++ .../{keystore.go => keystore_nofips.go} | 2 + 8 files changed, 161 insertions(+), 48 deletions(-) create mode 100644 internal/beatcmd/config_fips.go create mode 100644 internal/beatcmd/config_nofips.go create mode 100644 internal/beatcmd/config_nofips_test.go create mode 100644 internal/beatcmd/keystore_fips.go rename internal/beatcmd/{keystore.go => keystore_nofips.go} (99%) diff --git a/internal/beatcmd/cmd.go b/internal/beatcmd/cmd.go index 10ee94b966a..18d51b0e421 100644 --- a/internal/beatcmd/cmd.go +++ b/internal/beatcmd/cmd.go @@ -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)) diff --git a/internal/beatcmd/config.go b/internal/beatcmd/config.go index 8b84263e6ca..92fd91c9a9d 100644 --- a/internal/beatcmd/config.go +++ b/internal/beatcmd/config.go @@ -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" @@ -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) @@ -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 diff --git a/internal/beatcmd/config_fips.go b/internal/beatcmd/config_fips.go new file mode 100644 index 00000000000..052a2f7fdfc --- /dev/null +++ b/internal/beatcmd/config_fips.go @@ -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 +} diff --git a/internal/beatcmd/config_nofips.go b/internal/beatcmd/config_nofips.go new file mode 100644 index 00000000000..ab06fa5687f --- /dev/null +++ b/internal/beatcmd/config_nofips.go @@ -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()) +} diff --git a/internal/beatcmd/config_nofips_test.go b/internal/beatcmd/config_nofips_test.go new file mode 100644 index 00000000000..3df22cd4f12 --- /dev/null +++ b/internal/beatcmd/config_nofips_test.go @@ -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) +} diff --git a/internal/beatcmd/config_test.go b/internal/beatcmd/config_test.go index 712bc53af89..8d8c96a8f76 100644 --- a/internal/beatcmd/config_test.go +++ b/internal/beatcmd/config_test.go @@ -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" ) @@ -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{}{ @@ -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{} diff --git a/internal/beatcmd/keystore_fips.go b/internal/beatcmd/keystore_fips.go new file mode 100644 index 00000000000..58830af4b68 --- /dev/null +++ b/internal/beatcmd/keystore_fips.go @@ -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 diff --git a/internal/beatcmd/keystore.go b/internal/beatcmd/keystore_nofips.go similarity index 99% rename from internal/beatcmd/keystore.go rename to internal/beatcmd/keystore_nofips.go index 1ca75d15acd..9bf9a8a733e 100644 --- a/internal/beatcmd/keystore.go +++ b/internal/beatcmd/keystore_nofips.go @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +//go:build !requirefips + package beatcmd import (