diff --git a/go.mod b/go.mod index b0f752d5a..2aad16e7f 100644 --- a/go.mod +++ b/go.mod @@ -299,6 +299,6 @@ require ( go.opentelemetry.io/otel/trace v1.37.0 golang.org/x/crypto v0.41.0 // indirect golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0 // indirect - golang.org/x/sys v0.35.0 // indirect + golang.org/x/sys v0.35.0 k8s.io/client-go v0.33.4 ) diff --git a/pkg/secrets/factory.go b/pkg/secrets/factory.go index 6866360b7..8b94b77cf 100644 --- a/pkg/secrets/factory.go +++ b/pkg/secrets/factory.go @@ -8,13 +8,14 @@ import ( "errors" "fmt" "os" + "sync" "github.com/adrg/xdg" - "github.com/zalando/go-keyring" "golang.org/x/term" "github.com/stacklok/toolhive/pkg/logger" "github.com/stacklok/toolhive/pkg/process" + "github.com/stacklok/toolhive/pkg/secrets/keyring" ) const ( @@ -27,6 +28,18 @@ const ( keyringService = "toolhive" ) +var ( + keyringProvider keyring.Provider + keyringOnce sync.Once +) + +func getKeyringProvider() keyring.Provider { + keyringOnce.Do(func() { + keyringProvider = keyring.NewCompositeProvider() + }) + return keyringProvider +} + // ProviderType represents an enum of the types of available secrets providers. type ProviderType string @@ -157,20 +170,10 @@ var ErrKeyringNotAvailable = errors.New("OS keyring is not available. " + "Please use a different secrets provider (e.g., 1password) " + "or ensure your system has a keyring service available") -// IsKeyringAvailable tests if the OS keyring is available by attempting to set and delete a test value. +// IsKeyringAvailable tests if any keyring backend is available func IsKeyringAvailable() bool { - testKey := "toolhive-keyring-test" - testValue := "test" - - // Try to set a test value - if err := keyring.Set(keyringService, testKey, testValue); err != nil { - return false - } - - // Clean up the test value - _ = keyring.Delete(keyringService, testKey) - - return true + provider := getKeyringProvider() + return provider.IsAvailable() } // CreateSecretProvider creates the specified type of secrets provider. @@ -214,14 +217,15 @@ func CreateSecretProviderWithPassword(managerType ProviderType, password string) // If optionalPassword is provided and keyring is not yet setup, it uses that password and stores it. // Otherwise, it uses the current functionality (read from keyring or stdin). func GetSecretsPassword(optionalPassword string) ([]byte, error) { - // Attempt to load the password from the OS keyring. - keyringSecret, err := keyring.Get(keyringService, keyringService) + provider := getKeyringProvider() + + // Attempt to load the password from the keyring + keyringSecret, err := provider.Get(keyringService, keyringService) if err == nil { return []byte(keyringSecret), nil } - // We need to determine if the error is due to a lack of keyring on the - // system or if the keyring is available but nothing was stored. + // Handle key not found if errors.Is(err, keyring.ErrNotFound) { var password []byte @@ -233,9 +237,6 @@ func GetSecretsPassword(optionalPassword string) ([]byte, error) { password = []byte(optionalPassword) } else { // Keyring is available but no password stored - this should only happen during setup - // We cannot ask for a password in a detached process. - // We should never trigger this, but this ensures that if there's a bug - // then it's easier to find. if process.IsDetached() { return nil, fmt.Errorf("detached process detected, cannot ask for password") } @@ -248,10 +249,9 @@ func GetSecretsPassword(optionalPassword string) ([]byte, error) { } } - // TODO GET function should not be saving anything into keyring // Store the password in the keyring for future use - logger.Info("writing password to os keyring") - err = keyring.Set(keyringService, keyringService, string(password)) + logger.Info(fmt.Sprintf("writing password to %s", provider.Name())) + err = provider.Set(keyringService, keyringService, string(password)) if err != nil { return nil, fmt.Errorf("failed to store password in keyring: %w", err) } @@ -260,7 +260,7 @@ func GetSecretsPassword(optionalPassword string) ([]byte, error) { } // Assume any other keyring error means keyring is not available - return nil, fmt.Errorf("OS keyring is not available: %w", err) + return nil, fmt.Errorf("keyring is not available: %w", err) } func readPasswordStdin() ([]byte, error) { @@ -281,7 +281,8 @@ func readPasswordStdin() ([]byte, error) { // ResetKeyringSecret clears out the secret from the keystore (if present). func ResetKeyringSecret() error { - return keyring.DeleteAll(keyringService) + provider := getKeyringProvider() + return provider.DeleteAll(keyringService) } // GenerateSecurePassword generates a cryptographically secure random password diff --git a/pkg/secrets/keyring/composite.go b/pkg/secrets/keyring/composite.go new file mode 100644 index 000000000..cb5f85dfc --- /dev/null +++ b/pkg/secrets/keyring/composite.go @@ -0,0 +1,116 @@ +// Package keyring provides a composite keyring provider that supports multiple backends. +// It supports macOS Keychain, Windows Credential Manager, and Linux D-Bus Secret Service, +// with keyctl as a fallback on Linux systems. +package keyring + +import ( + "fmt" + "runtime" + + "github.com/stacklok/toolhive/pkg/logger" +) + +const linuxOS = "linux" + +type compositeProvider struct { + providers []Provider + active Provider +} + +// NewCompositeProvider creates a new composite keyring provider that combines multiple backends. +// It uses zalando/go-keyring as the primary provider and keyctl as a fallback on Linux. +func NewCompositeProvider() Provider { + var providers []Provider + + // Add zalando/go-keyring as primary provider + // Handles macOS, Windows, and Linux D-Bus natively + zkProvider := NewZalandoKeyringProvider() + providers = append(providers, zkProvider) + + // Add keyctl provider as fallback ONLY on Linux + if runtime.GOOS == linuxOS { + if keyctl, err := NewKeyctlProvider(); err == nil { + providers = append(providers, keyctl) + } + } + + return &compositeProvider{ + providers: providers, + } +} + +func (c *compositeProvider) getActiveProvider() Provider { + if c.active != nil && c.active.IsAvailable() { + return c.active + } + + for _, provider := range c.providers { + if provider.IsAvailable() { + if c.active == nil || c.active.Name() != provider.Name() { + // Log provider selection if logger is available + // Use fmt.Printf as fallback since logger might not be initialized in tests + c.logProviderSelection(provider.Name()) + } + c.active = provider + return provider + } + } + + return nil +} + +// logProviderSelection safely logs the provider selection +func (*compositeProvider) logProviderSelection(providerName string) { + // Try to use the logger, but don't panic if it's not available + defer func() { + if r := recover(); r != nil { + // Logger not available, use fallback + fmt.Printf("Using keyring provider: %s\n", providerName) + } + }() + + logger.Info(fmt.Sprintf("Using keyring provider: %s", providerName)) +} + +func (c *compositeProvider) Set(service, key, value string) error { + provider := c.getActiveProvider() + if provider == nil { + return fmt.Errorf("no keyring provider available") + } + return provider.Set(service, key, value) +} + +func (c *compositeProvider) Get(service, key string) (string, error) { + provider := c.getActiveProvider() + if provider == nil { + return "", fmt.Errorf("no keyring provider available") + } + return provider.Get(service, key) +} + +func (c *compositeProvider) Delete(service, key string) error { + provider := c.getActiveProvider() + if provider == nil { + return fmt.Errorf("no keyring provider available") + } + return provider.Delete(service, key) +} + +func (c *compositeProvider) DeleteAll(service string) error { + provider := c.getActiveProvider() + if provider == nil { + return fmt.Errorf("no keyring provider available") + } + return provider.DeleteAll(service) +} + +func (c *compositeProvider) IsAvailable() bool { + return c.getActiveProvider() != nil +} + +func (c *compositeProvider) Name() string { + if provider := c.getActiveProvider(); provider != nil { + return provider.Name() + } + return "None Available" +} diff --git a/pkg/secrets/keyring/composite_test.go b/pkg/secrets/keyring/composite_test.go new file mode 100644 index 000000000..c9ee0af6a --- /dev/null +++ b/pkg/secrets/keyring/composite_test.go @@ -0,0 +1,315 @@ +package keyring + +import ( + "os" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// isRunningInCI detects if we're running in a CI environment +// by checking for common CI environment variables +func isRunningInCI() bool { + ciEnvVars := []string{ + "GITHUB_ACTIONS", + "CI", + "GITLAB_CI", + "CIRCLECI", + "TRAVIS", + "BUILDKITE", + "DRONE", + "CONTINUOUS_INTEGRATION", + } + + for _, envVar := range ciEnvVars { + if os.Getenv(envVar) != "" { + return true + } + } + return false +} + +// mockProvider is a test implementation of the Provider interface +type mockProvider struct { + name string + available bool + setErr error + getErr error + deleteErr error + storage map[string]map[string]string // service -> key -> value +} + +func newMockProvider(name string, available bool) *mockProvider { + return &mockProvider{ + name: name, + available: available, + storage: make(map[string]map[string]string), + } +} + +func (m *mockProvider) Set(service, key, value string) error { + if m.setErr != nil { + return m.setErr + } + if m.storage[service] == nil { + m.storage[service] = make(map[string]string) + } + m.storage[service][key] = value + return nil +} + +func (m *mockProvider) Get(service, key string) (string, error) { + if m.getErr != nil { + return "", m.getErr + } + if serviceMap, exists := m.storage[service]; exists { + if value, exists := serviceMap[key]; exists { + return value, nil + } + } + return "", ErrNotFound +} + +func (m *mockProvider) Delete(service, key string) error { + if m.deleteErr != nil { + return m.deleteErr + } + if serviceMap, exists := m.storage[service]; exists { + delete(serviceMap, key) + if len(serviceMap) == 0 { + delete(m.storage, service) + } + } + return nil +} + +func (m *mockProvider) DeleteAll(service string) error { + delete(m.storage, service) + return nil +} + +func (m *mockProvider) IsAvailable() bool { + return m.available +} + +func (m *mockProvider) Name() string { + return m.name +} + +func TestNewCompositeProvider(t *testing.T) { + t.Parallel() + provider := NewCompositeProvider() + require.NotNil(t, provider) + + composite, ok := provider.(*compositeProvider) + require.True(t, ok, "provider should be a compositeProvider") + + // Should always have at least one provider (zalando wrapper) + assert.GreaterOrEqual(t, len(composite.providers), 1) + + // First provider should always be zalando wrapper + firstProvider := composite.providers[0] + require.NotNil(t, firstProvider) + + // Test platform-specific behavior + switch runtime.GOOS { + case "linux": + // On Linux, first provider should be D-Bus Secret Service + assert.Equal(t, "D-Bus Secret Service", firstProvider.Name()) + // On Linux, we might have keyctl as fallback (if available) + // Length could be 1 (only zalando) or 2 (zalando + keyctl) + assert.GreaterOrEqual(t, len(composite.providers), 1) + assert.LessOrEqual(t, len(composite.providers), 2) + + if len(composite.providers) == 2 { + // If keyctl is available, it should be second + assert.Equal(t, "Linux Keyctl", composite.providers[1].Name()) + } + case "darwin": + // On macOS, should have macOS Keychain + assert.Equal(t, "macOS Keychain", firstProvider.Name()) + // Should have exactly one provider on macOS + assert.Equal(t, 1, len(composite.providers)) + case "windows": + // On Windows, should have Windows Credential Manager + assert.Equal(t, "Windows Credential Manager", firstProvider.Name()) + // Should have exactly one provider on Windows + assert.Equal(t, 1, len(composite.providers)) + default: + // On other platforms, should have generic name + assert.Equal(t, "Platform Keyring", firstProvider.Name()) + } + + // Verify the composite provider implements all interface methods + assert.NotNil(t, composite.IsAvailable) + assert.NotNil(t, composite.Name) + assert.NotNil(t, composite.Get) + assert.NotNil(t, composite.Set) + assert.NotNil(t, composite.Delete) + assert.NotNil(t, composite.DeleteAll) +} + +func TestCompositeProvider_GetActiveProvider(t *testing.T) { + t.Parallel() + tests := []struct { + name string + primaryAvailable bool + secondaryAvailable bool + expectedProvider string + expectedNil bool + }{ + { + name: "primary available, use primary", + primaryAvailable: true, + secondaryAvailable: true, + expectedProvider: "primary", + expectedNil: false, + }, + { + name: "primary unavailable, use secondary", + primaryAvailable: false, + secondaryAvailable: true, + expectedProvider: "secondary", + expectedNil: false, + }, + { + name: "both unavailable, return nil", + primaryAvailable: false, + secondaryAvailable: false, + expectedProvider: "", + expectedNil: true, + }, + { + name: "only primary available", + primaryAvailable: true, + secondaryAvailable: false, + expectedProvider: "primary", + expectedNil: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + primary := newMockProvider("primary", tt.primaryAvailable) + secondary := newMockProvider("secondary", tt.secondaryAvailable) + + composite := &compositeProvider{ + providers: []Provider{primary, secondary}, + } + + activeProvider := composite.getActiveProvider() + + if tt.expectedNil { + assert.Nil(t, activeProvider) + } else { + require.NotNil(t, activeProvider) + assert.Equal(t, tt.expectedProvider, activeProvider.Name()) + // Verify the active provider is cached + assert.Equal(t, activeProvider, composite.active) + } + }) + } +} + +func TestCompositeProvider_Operations_WithAvailableProvider(t *testing.T) { + t.Parallel() + mockProv := newMockProvider("test-provider", true) + composite := &compositeProvider{ + providers: []Provider{mockProv}, + } + + // Test Set + err := composite.Set("test-service", "test-key", "test-value") + assert.NoError(t, err) + + // Test Get + value, err := composite.Get("test-service", "test-key") + assert.NoError(t, err) + assert.Equal(t, "test-value", value) + + // Test Delete + err = composite.Delete("test-service", "test-key") + assert.NoError(t, err) + + // Verify deletion + _, err = composite.Get("test-service", "test-key") + assert.ErrorIs(t, err, ErrNotFound) + + // Test DeleteAll + _ = composite.Set("test-service", "key1", "value1") + _ = composite.Set("test-service", "key2", "value2") + err = composite.DeleteAll("test-service") + assert.NoError(t, err) +} + +func TestCompositeProvider_Operations_NoProviderAvailable(t *testing.T) { + t.Parallel() + mockProv := newMockProvider("test-provider", false) + composite := &compositeProvider{ + providers: []Provider{mockProv}, + } + + // Test Set + err := composite.Set("test-service", "test-key", "test-value") + assert.Error(t, err) + assert.Contains(t, err.Error(), "no keyring provider available") + + // Test Get + _, err = composite.Get("test-service", "test-key") + assert.Error(t, err) + assert.Contains(t, err.Error(), "no keyring provider available") + + // Test Delete + err = composite.Delete("test-service", "test-key") + assert.Error(t, err) + assert.Contains(t, err.Error(), "no keyring provider available") + + // Test DeleteAll + err = composite.DeleteAll("test-service") + assert.Error(t, err) + assert.Contains(t, err.Error(), "no keyring provider available") +} + +// Integration test with actual runtime behavior +func TestCompositeProvider_RealProviders(t *testing.T) { + t.Parallel() + provider := NewCompositeProvider() + require.NotNil(t, provider) + + // Should always have at least one provider (zalando wrapper) + composite, ok := provider.(*compositeProvider) + require.True(t, ok) + assert.GreaterOrEqual(t, len(composite.providers), 1) + + // Test that the provider selection works + _ = composite.getActiveProvider() + + // On any platform, we should get some kind of provider name + name := provider.Name() + assert.NotEmpty(t, name) + + // In CI environments, keyring services may not be available, so we skip this assertion + if !isRunningInCI() { + assert.NotEqual(t, "None Available", name, "should have at least one working provider") + } else { + t.Logf("Skipping keyring availability assertion in CI environment (provider: %s)", name) + } + + // Test basic availability + available := provider.IsAvailable() + if available { + // If available, basic operations should work + err := provider.Set("toolhive-test", "integration-test", "test-value") + if err == nil { + // Only test Get/Delete if Set worked + value, err := provider.Get("toolhive-test", "integration-test") + if err == nil { + assert.Equal(t, "test-value", value) + } + _ = provider.Delete("toolhive-test", "integration-test") + } + } +} diff --git a/pkg/secrets/keyring/dbus_wrapper.go b/pkg/secrets/keyring/dbus_wrapper.go new file mode 100644 index 000000000..e3ef851a5 --- /dev/null +++ b/pkg/secrets/keyring/dbus_wrapper.go @@ -0,0 +1,61 @@ +package keyring + +import ( + "errors" + "runtime" + + "github.com/zalando/go-keyring" +) + +type dbusWrapperProvider struct{} + +// NewZalandoKeyringProvider creates a new provider that wraps zalando/go-keyring. +// This provider supports macOS Keychain, Windows Credential Manager, and Linux D-Bus Secret Service. +func NewZalandoKeyringProvider() Provider { + return &dbusWrapperProvider{} +} + +func (*dbusWrapperProvider) Set(service, key, value string) error { + return keyring.Set(service, key, value) +} + +func (*dbusWrapperProvider) Get(service, key string) (string, error) { + value, err := keyring.Get(service, key) + if errors.Is(err, keyring.ErrNotFound) { + return "", ErrNotFound + } + return value, err +} + +func (*dbusWrapperProvider) Delete(service, key string) error { + return keyring.Delete(service, key) +} + +func (*dbusWrapperProvider) DeleteAll(service string) error { + return keyring.DeleteAll(service) +} + +func (*dbusWrapperProvider) IsAvailable() bool { + testKey := "toolhive-keyring-test" + testValue := "test" + + if err := keyring.Set("toolhive-test", testKey, testValue); err != nil { + return false + } + + _ = keyring.Delete("toolhive-test", testKey) + return true +} + +func (*dbusWrapperProvider) Name() string { + switch runtime.GOOS { + case "darwin": + return "macOS Keychain" + case "windows": + return "Windows Credential Manager" + case linuxOS: + return "D-Bus Secret Service" + default: + return "Platform Keyring" + } +} diff --git a/pkg/secrets/keyring/interface.go b/pkg/secrets/keyring/interface.go new file mode 100644 index 000000000..ebddca5cd --- /dev/null +++ b/pkg/secrets/keyring/interface.go @@ -0,0 +1,21 @@ +package keyring + +import "errors" + +// ErrNotFound is returned when a requested key is not found in the keyring +var ErrNotFound = errors.New("key not found") + +// Provider defines the interface for keyring backends +type Provider interface { + Set(service, key, value string) error + + Get(service, key string) (string, error) + + Delete(service, key string) error + + DeleteAll(service string) error + + IsAvailable() bool + + Name() string +} diff --git a/pkg/secrets/keyring/keyctl_linux.go b/pkg/secrets/keyring/keyctl_linux.go new file mode 100644 index 000000000..a13059f15 --- /dev/null +++ b/pkg/secrets/keyring/keyctl_linux.go @@ -0,0 +1,142 @@ +//go:build linux +// +build linux + +package keyring + +import ( + "fmt" + "sync" + + "golang.org/x/sys/unix" +) + +type keyctlProvider struct { + ringID int + mu sync.RWMutex + keys map[string]map[string]int // service -> key -> keyid mapping +} + +// NewKeyctlProvider creates a new keyring provider using Linux keyctl. +// It initializes access to the user keyring for persistence across process invocations. +func NewKeyctlProvider() (Provider, error) { + // Use user keyring for persistence across process invocations + ringID, err := unix.KeyctlGetKeyringID(unix.KEY_SPEC_USER_KEYRING, false) + if err != nil { + return nil, fmt.Errorf("could not get user keyring: %w", err) + } + + // Link to thread keyring for reads + _, err = unix.KeyctlInt(unix.KEYCTL_LINK, ringID, unix.KEY_SPEC_THREAD_KEYRING, 0, 0) + if err != nil { + return nil, fmt.Errorf("unable to link user keyring to thread keyring: %w", err) + } + + return &keyctlProvider{ + ringID: ringID, + keys: make(map[string]map[string]int), + }, nil +} + +func (k *keyctlProvider) Set(service, key, value string) error { + k.mu.Lock() + defer k.mu.Unlock() + + keyName := fmt.Sprintf("%s:%s", service, key) + keyID, err := unix.AddKey("user", keyName, []byte(value), k.ringID) + if err != nil { + return fmt.Errorf("failed to set key '%s' in user keyring: %w", keyName, err) + } + + if k.keys[service] == nil { + k.keys[service] = make(map[string]int) + } + k.keys[service][key] = keyID + + return nil +} + +func (k *keyctlProvider) Get(service, key string) (string, error) { + k.mu.RLock() + defer k.mu.RUnlock() + + keyName := fmt.Sprintf("%s:%s", service, key) + keyID, err := unix.KeyctlSearch(k.ringID, "user", keyName, 0) + if err != nil { + return "", ErrNotFound + } + + bufSize := 2048 + buf := make([]byte, bufSize) + readBytes, err := unix.KeyctlBuffer(unix.KEYCTL_READ, keyID, buf, bufSize) + if err != nil { + return "", fmt.Errorf("read of key '%s' failed: %w", keyName, err) + } + + if readBytes > bufSize { + return "", fmt.Errorf("buffer too small for keyring payload") + } + + return string(buf[:readBytes]), nil +} + +func (k *keyctlProvider) Delete(service, key string) error { + k.mu.Lock() + defer k.mu.Unlock() + + keyName := fmt.Sprintf("%s:%s", service, key) + keyID, err := unix.KeyctlSearch(k.ringID, "user", keyName, 0) + if err != nil { + return nil + } + + _, err = unix.KeyctlInt(unix.KEYCTL_REVOKE, keyID, 0, 0, 0) + if err != nil { + return fmt.Errorf("failed to delete key '%s': %w", keyName, err) + } + + // Remove from tracking + if serviceKeys, exists := k.keys[service]; exists { + delete(serviceKeys, key) + if len(serviceKeys) == 0 { + delete(k.keys, service) + } + } + + return nil +} + +func (k *keyctlProvider) DeleteAll(service string) error { + k.mu.Lock() + defer k.mu.Unlock() + + serviceKeys, exists := k.keys[service] + if !exists { + return nil + } + + var lastErr error + for key := range serviceKeys { + if err := k.Delete(service, key); err != nil { + lastErr = err + } + } + + delete(k.keys, service) + return lastErr +} + +func (k *keyctlProvider) IsAvailable() bool { + testKey := "toolhive-keyring-test" + testValue := "test" + + if err := k.Set("toolhive-test", testKey, testValue); err != nil { + return false + } + + _ = k.Delete("toolhive-test", testKey) + return true +} + +func (*keyctlProvider) Name() string { + return "Linux Keyctl" +} diff --git a/pkg/secrets/keyring/keyctl_other.go b/pkg/secrets/keyring/keyctl_other.go new file mode 100644 index 000000000..32bd9c017 --- /dev/null +++ b/pkg/secrets/keyring/keyctl_other.go @@ -0,0 +1,11 @@ +//go:build !linux +// +build !linux + +package keyring + +import "fmt" + +// NewKeyctlProvider creates a new keyctl provider. This provider is only available on Linux. +func NewKeyctlProvider() (Provider, error) { + return nil, fmt.Errorf("keyctl provider is only available on Linux") +}