Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace logger with slogger in agent keys and in storage #1623

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions cmd/launcher/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ import (
"fmt"
"log/slog"

"github.com/go-kit/kit/log"
"github.com/kolide/launcher/ee/agent/types"
"github.com/kolide/launcher/pkg/contexts/ctxlog"
"github.com/kolide/launcher/pkg/osquery"
"github.com/kolide/launcher/pkg/service"
"github.com/kolide/launcher/pkg/traces"
Expand All @@ -17,12 +15,10 @@ func createExtensionRuntime(ctx context.Context, k types.Knapsack, launcherClien
ctx, span := traces.StartSpan(ctx)
defer span.End()

logger := log.With(ctxlog.FromContext(ctx), "caller", log.DefaultCaller)
slogger := k.Slogger().With("component", "osquery_extension_creator")

// create the osquery extension
extOpts := osquery.ExtensionOpts{
Logger: logger, // Preserved only for temporary use in agent.SetupKeys
LoggingInterval: k.LoggingInterval(),
RunDifferentialQueriesImmediately: k.EnableInitialRunner(),
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/launcher/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func runLauncher(ctx context.Context, cancel func(), multiSlogger, systemMultiSl
return fmt.Errorf("write launcher pid to file: %w", err)
}

stores, err := agentbbolt.MakeStores(ctx, logger, db)
stores, err := agentbbolt.MakeStores(ctx, slogger, db)
if err != nil {
return fmt.Errorf("failed to create stores: %w", err)
}
Expand Down
25 changes: 12 additions & 13 deletions ee/agent/flags/flag_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"
"time"

"github.com/go-kit/kit/log"
"github.com/kolide/launcher/ee/agent/flags/keys"
"github.com/kolide/launcher/ee/agent/storage"
storageci "github.com/kolide/launcher/ee/agent/storage/ci"
Expand Down Expand Up @@ -34,9 +33,9 @@ func TestControllerBoolFlags(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

store, err := storageci.NewStore(t, log.NewNopLogger(), storage.AgentFlagsStore.String())
store, err := storageci.NewStore(t, multislogger.NewNopLogger(), storage.AgentFlagsStore.String())
require.NoError(t, err)
fc := NewFlagController(multislogger.New().Logger, store)
fc := NewFlagController(multislogger.NewNopLogger(), store)
assert.NotNil(t, fc)

var value bool
Expand Down Expand Up @@ -110,9 +109,9 @@ func TestControllerStringFlags(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

store, err := storageci.NewStore(t, log.NewNopLogger(), storage.AgentFlagsStore.String())
store, err := storageci.NewStore(t, multislogger.NewNopLogger(), storage.AgentFlagsStore.String())
require.NoError(t, err)
fc := NewFlagController(multislogger.New().Logger, store)
fc := NewFlagController(multislogger.NewNopLogger(), store)
assert.NotNil(t, fc)

var value string
Expand Down Expand Up @@ -234,9 +233,9 @@ func TestControllerDurationFlags(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

store, err := storageci.NewStore(t, log.NewNopLogger(), storage.AgentFlagsStore.String())
store, err := storageci.NewStore(t, multislogger.NewNopLogger(), storage.AgentFlagsStore.String())
require.NoError(t, err)
fc := NewFlagController(multislogger.New().Logger, store)
fc := NewFlagController(multislogger.NewNopLogger(), store)
assert.NotNil(t, fc)

for i, valueToSet := range tt.valuesToSet {
Expand Down Expand Up @@ -267,9 +266,9 @@ func TestControllerNotify(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

store, err := storageci.NewStore(t, log.NewNopLogger(), storage.AgentFlagsStore.String())
store, err := storageci.NewStore(t, multislogger.NewNopLogger(), storage.AgentFlagsStore.String())
require.NoError(t, err)
fc := NewFlagController(multislogger.New().Logger, store)
fc := NewFlagController(multislogger.NewNopLogger(), store)
assert.NotNil(t, fc)

mockObserver := mocks.NewFlagsChangeObserver(t)
Expand Down Expand Up @@ -305,9 +304,9 @@ func TestControllerUpdate(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

store, err := storageci.NewStore(t, log.NewNopLogger(), storage.AgentFlagsStore.String())
store, err := storageci.NewStore(t, multislogger.NewNopLogger(), storage.AgentFlagsStore.String())
require.NoError(t, err)
fc := NewFlagController(multislogger.New().Logger, store)
fc := NewFlagController(multislogger.NewNopLogger(), store)
assert.NotNil(t, fc)

mockObserver := mocks.NewFlagsChangeObserver(t)
Expand Down Expand Up @@ -346,9 +345,9 @@ func TestControllerOverride(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

store, err := storageci.NewStore(t, log.NewNopLogger(), storage.AgentFlagsStore.String())
store, err := storageci.NewStore(t, multislogger.NewNopLogger(), storage.AgentFlagsStore.String())
require.NoError(t, err)
fc := NewFlagController(multislogger.New().Logger, store)
fc := NewFlagController(multislogger.NewNopLogger(), store)
assert.NotNil(t, fc)

mockObserver := mocks.NewFlagsChangeObserver(t)
Expand Down
23 changes: 14 additions & 9 deletions ee/agent/keys.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package agent

import (
"context"
"crypto"
"fmt"
"log/slog"
"runtime"
"time"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/kolide/launcher/ee/agent/keys"
"github.com/kolide/launcher/ee/agent/types"
"github.com/kolide/launcher/pkg/backoff"
Expand All @@ -29,13 +29,13 @@ func LocalDbKeys() keyInt {
return localDbKeys
}

func SetupKeys(logger log.Logger, store types.GetterSetterDeleter) error {
logger = log.With(logger, "component", "agentkeys")
func SetupKeys(slogger *slog.Logger, store types.GetterSetterDeleter) error {
slogger = slogger.With("component", "agentkeys")

var err error

// Always setup a local key
localDbKeys, err = keys.SetupLocalDbKey(logger, store)
localDbKeys, err = keys.SetupLocalDbKey(slogger, store)
if err != nil {
return fmt.Errorf("setting up local db keys: %w", err)
}
Expand All @@ -46,7 +46,7 @@ func SetupKeys(logger log.Logger, store types.GetterSetterDeleter) error {
}

err = backoff.WaitFor(func() error {
hwKeys, err := setupHardwareKeys(logger, store)
hwKeys, err := setupHardwareKeys(slogger, store)
if err != nil {
return err
}
Expand All @@ -56,7 +56,10 @@ func SetupKeys(logger log.Logger, store types.GetterSetterDeleter) error {

if err != nil {
// Use of hardware keys is not fully implemented as of 2023-02-01, so log an error and move on
level.Info(logger).Log("msg", "failed to setting up hardware keys", "err", err)
slogger.Log(context.TODO(), slog.LevelInfo,
"failed setting up hardware keys",
"err", err,
)
}

return nil
Expand Down Expand Up @@ -104,7 +107,9 @@ func storeKeyData(store types.Setter, pri, pub []byte) error {
// clearKeyData is used to clear the keys as part of error handling around new keys. It is not intended to be called
// regularly, and since the path that calls it is around DB errors, it has no error handling.
// nolint:unused
func clearKeyData(logger log.Logger, deleter types.Deleter) {
level.Info(logger).Log("msg", "Clearing keys")
func clearKeyData(slogger *slog.Logger, deleter types.Deleter) {
slogger.Log(context.TODO(), slog.LevelInfo,
"clearing keys",
)
_ = deleter.Delete([]byte(privateEccData), []byte(publicEccData))
}
19 changes: 13 additions & 6 deletions ee/agent/keys/local.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package keys

import (
"context"
"crypto/ecdsa"
"crypto/x509"
"fmt"
"log/slog"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/kolide/krypto/pkg/echelper"
"github.com/kolide/launcher/ee/agent/types"
)
Expand All @@ -26,14 +26,21 @@ func (k dbKey) Type() string {
return "local"
}

func SetupLocalDbKey(logger log.Logger, store types.GetterSetter) (*dbKey, error) {
func SetupLocalDbKey(slogger *slog.Logger, store types.GetterSetter) (*dbKey, error) {
if key, err := fetchKey(store); key != nil && err == nil {
level.Info(logger).Log("msg", "found local key in database")
slogger.Log(context.TODO(), slog.LevelInfo,
"found local key in database",
)
return &dbKey{key}, nil
} else if err != nil {
level.Info(logger).Log("msg", "Failed to parse key, regenerating", "err", err)
slogger.Log(context.TODO(), slog.LevelInfo,
"failed to parse key, regenerating",
"err", err,
)
} else if key == nil {
level.Info(logger).Log("msg", "No key found, generating new key")
slogger.Log(context.TODO(), slog.LevelInfo,
"no key found, generating new key",
)
}

// Time to regenerate!
Expand Down
10 changes: 5 additions & 5 deletions ee/agent/keys/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,28 @@ package keys
import (
"testing"

"github.com/go-kit/kit/log"
"github.com/kolide/launcher/ee/agent/storage"
storageci "github.com/kolide/launcher/ee/agent/storage/ci"
"github.com/kolide/launcher/pkg/log/multislogger"
"github.com/stretchr/testify/require"
)

func TestSetupLocalDbKey(t *testing.T) {
t.Parallel()

logger := log.NewNopLogger()
store, err := storageci.NewStore(t, log.NewNopLogger(), storage.ConfigStore.String())
slogger := multislogger.NewNopLogger()
store, err := storageci.NewStore(t, slogger, storage.ConfigStore.String())
require.NoError(t, err)

key, err := SetupLocalDbKey(logger, store)
key, err := SetupLocalDbKey(slogger, store)
require.NoError(t, err)
require.NotNil(t, key)

// Call a thing. Make sure this is a real key
require.NotNil(t, key.Public())

// If we call this _again_ do we get the same key back?
key2, err := SetupLocalDbKey(logger, store)
key2, err := SetupLocalDbKey(slogger, store)
require.NoError(t, err)
require.Equal(t, key.Public(), key2.Public())
}
4 changes: 2 additions & 2 deletions ee/agent/keys_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ package agent

import (
"errors"
"log/slog"

"github.com/go-kit/kit/log"
"github.com/kolide/launcher/ee/agent/types"
)

// nolint:unused
func setupHardwareKeys(logger log.Logger, store types.GetterSetterDeleter) (keyInt, error) {
func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) {
// We're seeing issues where launcher hangs (and does not complete startup) on the
// Sonoma Beta 2 release when trying to interact with the secure enclave below, on
// CreateKey. Since we don't expect this to work at the moment anyway, we are
Expand Down
14 changes: 8 additions & 6 deletions ee/agent/keys_tpm.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,35 @@
package agent

import (
"context"
"fmt"
"log/slog"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/kolide/krypto/pkg/tpm"
"github.com/kolide/launcher/ee/agent/types"
)

// nolint:unused
func setupHardwareKeys(logger log.Logger, store types.GetterSetterDeleter) (keyInt, error) {
func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) {
priData, pubData, err := fetchKeyData(store)
if err != nil {
return nil, err
}

if pubData == nil || priData == nil {
level.Info(logger).Log("msg", "Generating new keys")
slogger.Log(context.TODO(), slog.LevelInfo,
"generating new keys",
)

var err error
priData, pubData, err = tpm.CreateKey()
if err != nil {
clearKeyData(logger, store)
clearKeyData(slogger, store)
return nil, fmt.Errorf("creating key: %w", err)
}

if err := storeKeyData(store, priData, pubData); err != nil {
clearKeyData(logger, store)
clearKeyData(slogger, store)
return nil, fmt.Errorf("storing key: %w", err)
}
}
Expand Down
19 changes: 10 additions & 9 deletions ee/agent/reset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"testing"
"time"

"github.com/go-kit/kit/log"
"github.com/golang-jwt/jwt/v5"
"github.com/kolide/kit/fsutil"
"github.com/kolide/krypto/pkg/echelper"
Expand Down Expand Up @@ -252,15 +251,17 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
// the tests and they will continue to validate expected behavior.
tt.expectDatabaseWipe = false

slogger := multislogger.NewNopLogger()

// Set up dependencies: data store for hardware-identifying data
testHostDataStore, err := storageci.NewStore(t, log.NewNopLogger(), storage.PersistentHostDataStore.String())
testHostDataStore, err := storageci.NewStore(t, slogger, storage.PersistentHostDataStore.String())
require.NoError(t, err, "could not create test host data store")
mockKnapsack := typesmocks.NewKnapsack(t)
mockKnapsack.On("PersistentHostDataStore").Return(testHostDataStore)
testConfigStore, err := storageci.NewStore(t, log.NewNopLogger(), storage.ConfigStore.String())
testConfigStore, err := storageci.NewStore(t, slogger, storage.ConfigStore.String())
require.NoError(t, err, "could not create test config store")
mockKnapsack.On("ConfigStore").Return(testConfigStore).Maybe()
testServerProvidedDataStore, err := storageci.NewStore(t, log.NewNopLogger(), storage.ServerProvidedDataStore.String())
testServerProvidedDataStore, err := storageci.NewStore(t, slogger, storage.ServerProvidedDataStore.String())
require.NoError(t, err, "could not create test server provided data store")
mockKnapsack.On("ServerProvidedDataStore").Return(testServerProvidedDataStore).Maybe()
mockKnapsack.On("Stores").Return(map[storage.Store]types.KVStore{
Expand All @@ -270,7 +271,6 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
}).Maybe()

// Set up logger
slogger := multislogger.New().Logger
mockKnapsack.On("Slogger").Return(slogger)

// Set up dependencies: ensure that retrieved hardware data matches expectations
Expand Down Expand Up @@ -431,15 +431,17 @@ func TestDetectAndRemediateHardwareChange_SavesDataOverMultipleResets(t *testing

t.Skip("un-skip test once we decide to reset the database on hardware change")

slogger := multislogger.NewNopLogger()

// Set up dependencies: data store for hardware-identifying data
testHostDataStore, err := storageci.NewStore(t, log.NewNopLogger(), storage.PersistentHostDataStore.String())
testHostDataStore, err := storageci.NewStore(t, slogger, storage.PersistentHostDataStore.String())
require.NoError(t, err, "could not create test host data store")
mockKnapsack := typesmocks.NewKnapsack(t)
mockKnapsack.On("PersistentHostDataStore").Return(testHostDataStore)
testConfigStore, err := storageci.NewStore(t, log.NewNopLogger(), storage.ConfigStore.String())
testConfigStore, err := storageci.NewStore(t, slogger, storage.ConfigStore.String())
require.NoError(t, err, "could not create test config store")
mockKnapsack.On("ConfigStore").Return(testConfigStore)
testServerProvidedDataStore, err := storageci.NewStore(t, log.NewNopLogger(), storage.ServerProvidedDataStore.String())
testServerProvidedDataStore, err := storageci.NewStore(t, slogger, storage.ServerProvidedDataStore.String())
require.NoError(t, err, "could not create test server provided data store")
mockKnapsack.On("ServerProvidedDataStore").Return(testServerProvidedDataStore)
mockKnapsack.On("Stores").Return(map[storage.Store]types.KVStore{
Expand All @@ -449,7 +451,6 @@ func TestDetectAndRemediateHardwareChange_SavesDataOverMultipleResets(t *testing
})

// Set up logger
slogger := multislogger.New().Logger
mockKnapsack.On("Slogger").Return(slogger)

// Set up dependencies: ensure that retrieved hardware data matches expectations
Expand Down
Loading
Loading