From 0dfa95ffe40d20a89df07d8bc80f1b6bfa79bd50 Mon Sep 17 00:00:00 2001 From: Kilian Mio <86004375+Mikilio@users.noreply.github.com> Date: Wed, 13 May 2026 15:27:44 +0200 Subject: [PATCH] feat: retrieve WiFi/VPN secrets from D-Bus Secret Service with unlock support (#2402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: preserve pre-existing connection profiles on cancelled WiFi activation When a pre-existing WiFi connection activation is cancelled, do not remove the connection profile — only remove profiles for newly created connections. * feat: retrieve WiFi/VPN secrets from D-Bus secret service with unlock Wireless, 802.1x, VPN, and WireGuard secrets are now looked up from org.freedesktop.Secret before prompting the user. If the keyring is locked, the unlock prompt is triggered via Prompt.Prompt() and the lookup retried after the vault is unlocked. --- .gitignore | 3 + .../server/network/agent_networkmanager.go | 107 +++++--- .../network/agent_networkmanager_test.go | 35 +++ core/internal/server/network/backend.go | 1 + .../network/backend_networkmanager_wifi.go | 33 ++- .../internal/server/network/secret_service.go | 255 ++++++++++++++++++ 6 files changed, 387 insertions(+), 47 deletions(-) create mode 100644 core/internal/server/network/secret_service.go diff --git a/.gitignore b/.gitignore index dc9e4ef9..e62cfc7c 100644 --- a/.gitignore +++ b/.gitignore @@ -107,6 +107,9 @@ vim/ bin/ +# Core dumps +core.* + # direnv .envrc .direnv/ diff --git a/core/internal/server/network/agent_networkmanager.go b/core/internal/server/network/agent_networkmanager.go index 088b7fac..e0dd42d7 100644 --- a/core/internal/server/network/agent_networkmanager.go +++ b/core/internal/server/network/agent_networkmanager.go @@ -23,6 +23,13 @@ const ( agentIdentifier = "com.danklinux.NMAgent" ) +const ( + nmSecretAgentFlagAllowInteraction = 0x1 + nmSecretAgentFlagRequestNew = 0x2 + nmSecretAgentFlagUserRequested = 0x4 + nmSecretAgentFlagOnlySystem = 0x80000000 +) + type SecretAgent struct { conn *dbus.Conn objPath dbus.ObjectPath @@ -129,6 +136,21 @@ func (a *SecretAgent) GetSecrets( log.Infof("[SecretAgent] connType=%s, name=%s, vpnSvc=%s, fields=%v, flags=%d, vpnPasswordFlags=%d", connType, displayName, vpnSvc, fields, flags, vpnPasswordFlags) + if flags&nmSecretAgentFlagOnlySystem != 0 { + log.Infof("[SecretAgent] ONLY_SYSTEM flag set, deferring to system secret storage") + return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) + } + + var connUuid string + if c, ok := conn["connection"]; ok { + if v, ok := c["uuid"]; ok { + if s, ok2 := v.Value().(string); ok2 { + connUuid = s + } + } + } + + // Phase 1: Determine if this connection is ours and what fields we need. if a.backend != nil { a.backend.stateMutex.RLock() isConnecting := a.backend.state.IsConnecting @@ -145,15 +167,6 @@ func (a *SecretAgent) GetSecrets( return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) } case "vpn", "wireguard": - var connUuid string - if c, ok := conn["connection"]; ok { - if v, ok := c["uuid"]; ok { - if s, ok2 := v.Value().(string); ok2 { - connUuid = s - } - } - } - // If we're connecting to a VPN, only respond if it's the one we're connecting to // This prevents interfering with nmcli/other tools when our app isn't connecting if isConnectingVPN && connUuid != connectingVPNUUID { @@ -163,6 +176,7 @@ func (a *SecretAgent) GetSecrets( } } + // Phase 2: Resolve fields from hints or password-flags. if len(fields) == 0 { if settingName == "vpn" { if a.backend != nil { @@ -230,41 +244,19 @@ func (a *SecretAgent) GetSecrets( return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) } log.Infof("[SecretAgent] Agent-owned secrets, inferred fields: %v", fields) + } else if passwordFlags&NM_SETTING_SECRET_FLAG_NOT_SAVED != 0 { + log.Infof("[SecretAgent] Secrets not saved, will need to prompt (flags=%d)", passwordFlags) + // Fall through — fields remain empty, prompt will be required. } else { - log.Infof("[SecretAgent] No secrets needed, using system stored secrets (flags=%d)", passwordFlags) - out := nmSettingMap{} - out[settingName] = nmVariantMap{} - return out, nil - } - } - } - - reason := reasonFromFlags(flags) - if a.manager != nil && connType == "802-11-wireless" && a.manager.WasRecentlyFailed(ssid) { - reason = "wrong-password" - } - if settingName == "vpn" && isPKCS11Auth(conn, vpnSvc) { - reason = "pkcs11" - } - - var connId, connUuid string - if c, ok := conn["connection"]; ok { - if v, ok := c["id"]; ok { - if s, ok2 := v.Value().(string); ok2 { - connId = s - } - } - if v, ok := c["uuid"]; ok { - if s, ok2 := v.Value().(string); ok2 { - connUuid = s + log.Infof("[SecretAgent] Secrets stored in NM config (flags=%d), deferring to system", passwordFlags) + return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) } } } + // Phase 3: Cached VPN credentials — user-provided, take priority. if settingName == "vpn" && a.backend != nil { - // Check for cached PKCS11 PIN first - isPKCS11Request := len(fields) == 1 && fields[0] == "key_pass" - if isPKCS11Request { + if isPKCS11Request := len(fields) == 1 && fields[0] == "key_pass"; isPKCS11Request { a.backend.cachedPKCS11Mu.Lock() cached := a.backend.cachedPKCS11PIN if cached != nil && cached.ConnectionUUID == connUuid { @@ -283,7 +275,6 @@ func (a *SecretAgent) GetSecrets( a.backend.cachedPKCS11Mu.Unlock() } - // Check for cached VPN password a.backend.cachedVPNCredsMu.Lock() cached := a.backend.cachedVPNCreds if cached != nil && cached.ConnectionUUID == connUuid { @@ -314,6 +305,7 @@ func (a *SecretAgent) GetSecrets( a.backend.cachedGPSamlMu.Lock() cachedGPSaml := a.backend.cachedGPSamlCookie if cachedGPSaml != nil && cachedGPSaml.ConnectionUUID == connUuid { + a.backend.cachedGPSamlCookie = nil a.backend.cachedGPSamlMu.Unlock() log.Infof("[SecretAgent] Using cached GlobalProtect SAML cookie for %s", connUuid) @@ -369,6 +361,37 @@ func (a *SecretAgent) GetSecrets( } } + // Phase 4: Non-interactive secret retrieval (keyring). + // Always try the keyring even when REQUEST_NEW is set — the vault may have + // been unlocked by a prior call's Prompt flow, making the lookup non-interactive. + if secretOut := a.trySecretService(connUuid, settingName, fields); secretOut != nil { + return secretOut, nil + } + + // Phase 5: If interaction is not allowed, we're done. + if flags&nmSecretAgentFlagAllowInteraction == 0 { + log.Infof("[SecretAgent] ALLOW_INTERACTION not set, cannot prompt user") + return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) + } + + // Phase 6: Prepare prompt. + reason := reasonFromFlags(flags) + if a.manager != nil && connType == "802-11-wireless" && a.manager.WasRecentlyFailed(ssid) { + reason = "wrong-password" + } + if settingName == "vpn" && isPKCS11Auth(conn, vpnSvc) { + reason = "pkcs11" + } + + var connId string + if c, ok := conn["connection"]; ok { + if v, ok := c["id"]; ok { + if s, ok2 := v.Value().(string); ok2 { + connId = s + } + } + } + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() @@ -403,6 +426,7 @@ func (a *SecretAgent) GetSecrets( wasConnectingVPN := a.backend.state.IsConnectingVPN cancelledSSID := a.backend.state.ConnectingSSID cancelledVPNUUID := a.backend.state.ConnectingVPNUUID + connPreExisting := a.backend.state.ConnectingPreExisting if wasConnecting || wasConnectingVPN { log.Infof("[SecretAgent] Clearing connecting state due to cancelled prompt") a.backend.state.IsConnecting = false @@ -414,7 +438,8 @@ func (a *SecretAgent) GetSecrets( // If this was a WiFi connection that was just cancelled, remove the connection profile // (it was created with AddConnection but activation was cancelled) - if wasConnecting && cancelledSSID != "" && connType == "802-11-wireless" { + // Only do this for newly created connections, not pre-existing ones. + if wasConnecting && cancelledSSID != "" && connType == "802-11-wireless" && !connPreExisting { log.Infof("[SecretAgent] Removing connection profile for cancelled WiFi connection: %s", cancelledSSID) if err := a.backend.ForgetWiFiNetwork(cancelledSSID); err != nil { log.Warnf("[SecretAgent] Failed to remove cancelled connection profile: %v", err) @@ -623,7 +648,7 @@ func fieldsNeeded(setting string, hints []string, conn map[string]nmVariantMap) return hints } return infer8021xFields(conn) - case "vpn": + case "vpn", "wireguard": return hints default: return []string{} diff --git a/core/internal/server/network/agent_networkmanager_test.go b/core/internal/server/network/agent_networkmanager_test.go index 20409865..6ae6c8f4 100644 --- a/core/internal/server/network/agent_networkmanager_test.go +++ b/core/internal/server/network/agent_networkmanager_test.go @@ -339,6 +339,41 @@ func TestInferVPNFields_GPSaml(t *testing.T) { } } +func TestSecretAgent_GetSecrets_OnlySystemFlag(t *testing.T) { + agent := &SecretAgent{} + conn := map[string]nmVariantMap{ + "connection": { + "id": dbus.MakeVariant("TestWiFi"), + "type": dbus.MakeVariant("802-11-wireless"), + }, + "802-11-wireless": { + "ssid": dbus.MakeVariant("TestSSID"), + }, + } + + _, err := agent.GetSecrets(conn, "/test/path", "802-11-wireless-security", nil, 0x80000000) + assert.Error(t, err) + assert.Contains(t, err.Error(), "NoSecrets") +} + +func TestSecretAgent_GetSecrets_NoInteractionFlag(t *testing.T) { + agent := &SecretAgent{} + conn := map[string]nmVariantMap{ + "connection": { + "id": dbus.MakeVariant("TestWiFi"), + "type": dbus.MakeVariant("802-11-wireless"), + }, + "802-11-wireless": { + "ssid": dbus.MakeVariant("TestSSID"), + }, + } + + // flags=0 means ALLOW_INTERACTION is not set + _, err := agent.GetSecrets(conn, "/test/path", "802-11-wireless-security", nil, 0x0) + assert.Error(t, err) + assert.Contains(t, err.Error(), "NoSecrets") +} + func TestNmVariantMap(t *testing.T) { // Test that nmVariantMap and nmSettingMap work correctly settingMap := make(nmSettingMap) diff --git a/core/internal/server/network/backend.go b/core/internal/server/network/backend.go index f1bb04d9..2ada0f15 100644 --- a/core/internal/server/network/backend.go +++ b/core/internal/server/network/backend.go @@ -74,6 +74,7 @@ type BackendState struct { IsConnecting bool ConnectingSSID string ConnectingDevice string + ConnectingPreExisting bool IsConnectingVPN bool ConnectingVPNUUID string LastError string diff --git a/core/internal/server/network/backend_networkmanager_wifi.go b/core/internal/server/network/backend_networkmanager_wifi.go index 6f593b12..5cb5c09a 100644 --- a/core/internal/server/network/backend_networkmanager_wifi.go +++ b/core/internal/server/network/backend_networkmanager_wifi.go @@ -245,18 +245,34 @@ func (b *NetworkManagerBackend) GetWiFiQRCodeContent(ssid string) (string, error return "", fmt.Errorf("QR code generation only supports WPA connections, `%s` uses %s", ssid, securityType) } + var psk string + secrets, err := conn.GetSecrets("802-11-wireless-security") if err != nil { - return "", fmt.Errorf("failed to retrieve connection secrets for `%s`: %w", ssid, err) + log.Debugf("[GetWiFiQRCodeContent] conn.GetSecrets failed: %v, falling back to secret service", err) + } else if secSecrets, ok := secrets["802-11-wireless-security"]; ok { + if s, ok := secSecrets["psk"].(string); ok { + psk = s + } } - secSecrets, ok := secrets["802-11-wireless-security"] - if !ok { - return "", fmt.Errorf("failed to retrieve password for `%s`", ssid) + if psk == "" { + uuid := "" + if connMeta, ok := connSettings["connection"]; ok { + if u, ok := connMeta["uuid"].(string); ok { + uuid = u + } + } + if uuid != "" { + sess, err := openSecretService() + if err == nil { + psk = sess.lookup(uuid, "802-11-wireless-security", "psk") + sess.close() + } + } } - psk, ok := secSecrets["psk"].(string) - if !ok { + if psk == "" { return "", fmt.Errorf("failed to retrieve password for `%s`", ssid) } @@ -281,6 +297,7 @@ func (b *NetworkManagerBackend) ConnectWiFi(req ConnectionRequest) error { b.state.IsConnecting = true b.state.ConnectingSSID = req.SSID b.state.ConnectingDevice = req.Device + b.state.ConnectingPreExisting = false b.state.LastError = "" b.stateMutex.Unlock() @@ -292,6 +309,9 @@ func (b *NetworkManagerBackend) ConnectWiFi(req ConnectionRequest) error { existingConn, err := b.findConnection(req.SSID) if err == nil && existingConn != nil { + b.stateMutex.Lock() + b.state.ConnectingPreExisting = true + b.stateMutex.Unlock() _, err := nm.ActivateConnection(existingConn, devInfo.device, nil) if err != nil { log.Warnf("[ConnectWiFi] Failed to activate existing connection: %v", err) @@ -607,6 +627,7 @@ func (b *NetworkManagerBackend) findConnection(ssid string) (gonetworkmanager.Co if bytes.Equal(candidateSSID, ssidBytes) { return conn, nil } + log.Debugf("[findConnection] SSID mismatch: stored=%q, request=%q", string(candidateSSID), ssid) } } } diff --git a/core/internal/server/network/secret_service.go b/core/internal/server/network/secret_service.go new file mode 100644 index 00000000..1a93d2eb --- /dev/null +++ b/core/internal/server/network/secret_service.go @@ -0,0 +1,255 @@ +package network + +import ( + "context" + "fmt" + "time" + + "github.com/AvengeMedia/DankMaterialShell/core/internal/log" + "github.com/godbus/dbus/v5" +) + +const ( + secretServiceBusName = "org.freedesktop.secrets" + secretServicePath = "/org/freedesktop/secrets" + secretServiceIface = "org.freedesktop.Secret.Service" + secretItemIface = "org.freedesktop.Secret.Item" + secretPromptIface = "org.freedesktop.Secret.Prompt" +) + +type secretServiceSession struct { + conn *dbus.Conn + svc dbus.BusObject + sessionPath dbus.ObjectPath +} + +func openSecretService() (*secretServiceSession, error) { + c, err := dbus.ConnectSessionBus() + if err != nil { + return nil, err + } + + svc := c.Object(secretServiceBusName, dbus.ObjectPath(secretServicePath)) + + var sessionPath dbus.ObjectPath + call := svc.Call(secretServiceIface+".OpenSession", 0, "plain", dbus.MakeVariant("")) + if call.Err != nil { + c.Close() + return nil, call.Err + } + if err := call.Store(new(dbus.Variant), &sessionPath); err != nil { + c.Close() + return nil, err + } + + return &secretServiceSession{ + conn: c, + svc: svc, + sessionPath: sessionPath, + }, nil +} + +func (s *secretServiceSession) unlock(items []dbus.ObjectPath) error { + var prompt dbus.ObjectPath + var unlocked []dbus.ObjectPath + call := s.svc.Call(secretServiceIface+".Unlock", 0, items) + if call.Err != nil { + return call.Err + } + if err := call.Store(&unlocked, &prompt); err != nil { + return err + } + if prompt == "/" { + return nil + } + + if err := s.conn.AddMatchSignal( + dbus.WithMatchInterface(secretPromptIface), + dbus.WithMatchObjectPath(prompt), + ); err != nil { + return err + } + defer s.conn.RemoveMatchSignal( + dbus.WithMatchInterface(secretPromptIface), + dbus.WithMatchObjectPath(prompt), + ) + + ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) + defer cancel() + + ch := make(chan *dbus.Signal, 10) + s.conn.Signal(ch) + + go func() { + defer s.conn.RemoveSignal(ch) + for { + select { + case v := <-ch: + if v.Path == prompt && v.Name == secretPromptIface+".Completed" { + if len(v.Body) < 2 { + log.Debugf("[SecretAgent] Unlock prompt Completed signal has %d body element(s), expected >= 2", len(v.Body)) + } else { + if dismissed, ok := v.Body[0].(bool); ok && dismissed { + log.Debugf("[SecretAgent] Unlock prompt dismissed by user") + } + } + cancel() + return + } + case <-ctx.Done(): + return + } + } + }() + + promptObj := s.conn.Object(secretServiceBusName, prompt) + if err := promptObj.Call(secretPromptIface+".Prompt", 0, "").Store(); err != nil { + cancel() + return err + } + + <-ctx.Done() + if ctx.Err() == context.DeadlineExceeded { + promptObj.Call(secretPromptIface+".Dismiss", 0) + return fmt.Errorf("timed out waiting for unlock prompt") + } + return nil +} + +func (s *secretServiceSession) lookup(connUuid, settingName, settingKey string) string { + attrs := map[string]string{ + "connection-uuid": connUuid, + "setting-name": settingName, + "setting-key": settingKey, + } + + var unlocked []dbus.ObjectPath + var locked []dbus.ObjectPath + call := s.svc.Call(secretServiceIface+".SearchItems", 0, attrs) + if call.Err != nil { + log.Debugf("[SecretAgent] Secret service SearchItems failed: %v", call.Err) + return "" + } + if err := call.Store(&unlocked, &locked); err != nil { + log.Debugf("[SecretAgent] Failed to store SearchItems result: %v", err) + return "" + } + + if len(unlocked) == 0 && len(locked) > 0 { + log.Debugf("[SecretAgent] Attempting to unlock %d locked item(s) for %s", len(locked), connUuid) + if err := s.unlock(locked); err != nil { + log.Debugf("[SecretAgent] Failed to unlock items: %v", err) + return "" + } + unlocked = locked + } + + if len(unlocked) == 0 { + log.Debugf("[SecretAgent] No secret service items found for %s", connUuid) + return "" + } + + item := s.conn.Object(secretServiceBusName, unlocked[0]) + var secret struct { + Session dbus.ObjectPath + Parameters []byte + Value []byte + ContentType string + } + call = item.Call(secretItemIface+".GetSecret", 0, s.sessionPath) + if call.Err != nil { + log.Debugf("[SecretAgent] Secret service GetSecret failed: %v", call.Err) + return "" + } + if err := call.Store(&secret); err != nil { + log.Debugf("[SecretAgent] Failed to store GetSecret result: %v", err) + return "" + } + + secretValue := string(secret.Value) + if secretValue == "" { + log.Debugf("[SecretAgent] Secret service returned empty value for %s/%s", connUuid, settingKey) + return "" + } + + log.Infof("[SecretAgent] Retrieved secret from secret service for %s/%s", connUuid, settingKey) + return secretValue +} + +func (s *secretServiceSession) close() { + s.conn.Close() +} + +func (a *SecretAgent) trySecretService( + connUuid string, + settingName string, + fields []string, +) nmSettingMap { + if connUuid == "" { + log.Debugf("[SecretAgent] trySecretService: connUuid is empty, skipping keyring lookup") + return nil + } + if len(fields) == 0 { + log.Debugf("[SecretAgent] trySecretService: no fields requested, skipping keyring lookup") + return nil + } + + switch settingName { + case "802-11-wireless-security", "802-1x", "vpn", "wireguard": + default: + log.Debugf("[SecretAgent] trySecretService: setting %s not supported for keyring lookup", settingName) + return nil + } + + sess, err := openSecretService() + if err != nil { + log.Debugf("[SecretAgent] Failed to open secret service session: %v", err) + return nil + } + defer sess.close() + + found := make(map[string]string) + for _, field := range fields { + val := sess.lookup(connUuid, settingName, field) + if val == "" { + log.Debugf("[SecretAgent] Secret service missing field '%s' for %s", field, connUuid) + return nil + } + found[field] = val + } + + out := nmSettingMap{} + sec := nmVariantMap{} + for k, v := range found { + sec[k] = dbus.MakeVariant(v) + } + + switch settingName { + case "vpn": + secretsDict := make(map[string]string) + for k, v := range found { + if k != "username" { + secretsDict[k] = v + } + } + vpnSec := nmVariantMap{} + vpnSec["secrets"] = dbus.MakeVariant(secretsDict) + out[settingName] = vpnSec + log.Infof("[SecretAgent] Returning VPN secrets from secret service with %d fields", len(secretsDict)) + case "802-1x": + secretsOnly := nmVariantMap{} + for k, v := range found { + switch k { + case "password", "private-key-password", "phase2-private-key-password", "pin": + secretsOnly[k] = dbus.MakeVariant(v) + } + } + out[settingName] = secretsOnly + log.Infof("[SecretAgent] Returning 802-1x secrets from secret service with %d fields", len(secretsOnly)) + default: + out[settingName] = sec + log.Infof("[SecretAgent] Returning %s secrets from secret service", settingName) + } + + return out +}