From a657848bb274f729376683f7674ce2eb2fa1ed6e Mon Sep 17 00:00:00 2001 From: Mikilio Date: Fri, 31 Jul 2026 14:49:57 +0200 Subject: [PATCH 1/3] feat(network): persist and retrieve NM secrets via D-Bus Secret Service keyring Store WiFi/802-1x/VPN/WireGuard secrets in the freedesktop Secret Service keyring when the user opts to save, and clean them up when connections are deleted. - Add keyring store/lookup/delete via CreateItem (replace=true), SearchItems, and Item.Delete on the default collection - ensureUnlocked() unlocks the default collection before any keyring op - Extract NM formatting (VPN secrets dict, 802-1x key filtering) into formatKeyringSecrets; secret_service.go stays connection-type agnostic - DeleteSecrets/DeleteSecrets2 now remove keyring items keyed by connection UUID - Support arbitrary NM setting types via hints; warn on unknown settings instead of blindly persisting reply.Secrets - Improve DeleteSecrets/DeleteSecrets2 tests to verify UUID extraction --- .../server/network/agent_networkmanager.go | 151 ++++++++-- .../network/agent_networkmanager_test.go | 49 ++++ .../internal/server/network/secret_service.go | 264 ++++++++++++------ 3 files changed, 360 insertions(+), 104 deletions(-) diff --git a/core/internal/server/network/agent_networkmanager.go b/core/internal/server/network/agent_networkmanager.go index 4a3e1a341..643367d0d 100644 --- a/core/internal/server/network/agent_networkmanager.go +++ b/core/internal/server/network/agent_networkmanager.go @@ -141,14 +141,7 @@ func (a *SecretAgent) GetSecrets( 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 - } - } - } + connUuid := readConnUuid(conn) // Phase 1: Determine if this connection is ours and what fields we need. if a.backend != nil { @@ -369,8 +362,8 @@ func (a *SecretAgent) GetSecrets( a.backend.clearCachedWiFiSecret(connUuid) } } else { - if secretOut := a.trySecretService(connUuid, settingName, fields); secretOut != nil { - return secretOut, nil + if secrets := a.trySecretService(connUuid, settingName, fields); len(secrets) > 0 { + return formatKeyringSecrets(settingName, secrets), nil } switch settingName { @@ -530,8 +523,7 @@ func (a *SecretAgent) GetSecrets( case "802-1x": secretsOnly := nmVariantMap{} for k, v := range reply.Secrets { - switch k { - case "password", "private-key-password", "phase2-private-key-password", "pin": + if isDot1xSecretKey(k) { secretsOnly[k] = dbus.MakeVariant(v) } } @@ -563,6 +555,60 @@ func (a *SecretAgent) GetSecrets( log.Infof("[SecretAgent] Queued credentials persist for after connection succeeds") } + if reply.Save && connUuid != "" { + toStore := make(map[string]string) + switch settingName { + case "802-11-wireless-security": + for k, v := range reply.Secrets { + toStore[k] = v + } + case "802-1x": + for k, v := range reply.Secrets { + if isDot1xSecretKey(k) { + toStore[k] = v + } + } + case "vpn", "wireguard": + for k, v := range reply.Secrets { + if k != "username" { + toStore[k] = v + } + } + default: + var keys []string + for k := range reply.Secrets { + keys = append(keys, k) + } + log.Warnf("[SecretAgent] Unknown setting type %q — not storing secrets (keys: %v)", settingName, keys) + } + if len(toStore) > 0 { + if err := a.withSecretService(func(sess *secretServiceSession) error { + stored := 0 + for key, val := range toStore { + ident := displayName + if connType == "802-11-wireless" && ssid != "" { + ident = ssid + } + label := fmt.Sprintf("%s: %s: %s", connType, ident, key) + if err := sess.store(connUuid, settingName, key, val, label); err != nil { + log.Debugf("[SecretAgent] Failed to store %s/%s: %v", connUuid, key, err) + continue + } + stored++ + } + if stored > 0 { + log.Infof("[SecretAgent] Stored %d/%d secret(s) in keyring for %s/%s", stored, len(toStore), connUuid, settingName) + } + if stored < len(toStore) { + return fmt.Errorf("stored %d/%d secrets", stored, len(toStore)) + } + return nil + }); err != nil { + log.Warnf("[SecretAgent] Could not persist secrets to keyring: %v", err) + } + } + } + if a.backend != nil { switch settingName { case "802-11-wireless-security", "802-1x": @@ -575,12 +621,37 @@ func (a *SecretAgent) GetSecrets( func (a *SecretAgent) DeleteSecrets(conn map[string]nmVariantMap, path dbus.ObjectPath) *dbus.Error { ssid := readSSID(conn) - log.Infof("[SecretAgent] DeleteSecrets called: path=%s, SSID=%s", path, ssid) + connType, _, _ := readConnTypeAndName(conn) + connUuid := readConnUuid(conn) + + log.Infof("[SecretAgent] DeleteSecrets called: path=%s, SSID=%s, type=%s, uuid=%s", path, ssid, connType, connUuid) + a.deleteSecretsByConn(connUuid) + return nil } func (a *SecretAgent) DeleteSecrets2(path dbus.ObjectPath, setting string) *dbus.Error { log.Infof("[SecretAgent] DeleteSecrets2 (alternate) called: path=%s, setting=%s", path, setting) + + if a.conn == nil { + return nil + } + + var settings map[string]map[string]dbus.Variant + if err := a.conn.Object("org.freedesktop.NetworkManager", path). + Call("org.freedesktop.NetworkManager.Settings.Connection.GetSettings", 0). + Store(&settings); err != nil { + log.Warnf("[SecretAgent] DeleteSecrets2: GetSettings failed for %s: %v — orphaned keyring items may remain", path, err) + return nil + } + + settingsTyped := make(nmSettingMap, len(settings)) + for k, v := range settings { + settingsTyped[k] = v + } + connUuid := readConnUuid(settingsTyped) + a.deleteSecretsByConn(connUuid) + return nil } @@ -629,6 +700,25 @@ func (a *SecretAgent) save8021xIdentity(path dbus.ObjectPath, identity string) { log.Infof("[SecretAgent] Saved 802.1x identity to connection profile") } +func readConnUuid(conn map[string]nmVariantMap) string { + if c, ok := conn["connection"]; ok { + if v, ok := c["uuid"]; ok { + if s, ok2 := v.Value().(string); ok2 { + return s + } + } + } + return "" +} + +func isDot1xSecretKey(key string) bool { + switch key { + case "password", "private-key-password", "phase2-private-key-password", "pin": + return true + } + return false +} + func readSSID(conn map[string]nmVariantMap) string { if w, ok := conn["802-11-wireless"]; ok { if v, ok := w["ssid"]; ok { @@ -679,10 +769,8 @@ func fieldsNeeded(setting string, hints []string, conn map[string]nmVariantMap) return hints } return infer8021xFields(conn) - case "vpn", "wireguard": - return hints default: - return []string{} + return hints } } @@ -966,13 +1054,40 @@ func reasonFromFlags(flags uint32) string { return "required" } +func formatKeyringSecrets(settingName string, secrets map[string]string) nmSettingMap { + switch settingName { + case "vpn", "wireguard": + secretsDict := make(map[string]string) + for k, v := range secrets { + if k != "username" { + secretsDict[k] = v + } + } + vpnSec := nmVariantMap{"secrets": dbus.MakeVariant(secretsDict)} + return nmSettingMap{settingName: vpnSec} + case "802-1x": + secretsOnly := nmVariantMap{} + for k, v := range secrets { + if isDot1xSecretKey(k) { + secretsOnly[k] = dbus.MakeVariant(v) + } + } + return nmSettingMap{settingName: secretsOnly} + default: + sec := nmVariantMap{} + for k, v := range secrets { + sec[k] = dbus.MakeVariant(v) + } + return nmSettingMap{settingName: sec} + } +} + func buildWiFiSecretsResponse(settingName string, secrets map[string]string) nmSettingMap { sec := nmVariantMap{} switch settingName { case "802-1x": for k, v := range secrets { - switch k { - case "password", "private-key-password", "phase2-private-key-password", "pin": + if isDot1xSecretKey(k) { sec[k] = dbus.MakeVariant(v) } } diff --git a/core/internal/server/network/agent_networkmanager_test.go b/core/internal/server/network/agent_networkmanager_test.go index 2b7636420..59de77133 100644 --- a/core/internal/server/network/agent_networkmanager_test.go +++ b/core/internal/server/network/agent_networkmanager_test.go @@ -477,3 +477,52 @@ func TestNmVariantMap(t *testing.T) { value := settingMap["test-setting"]["test-key"].Value() assert.Equal(t, "test-value", value) } + +func TestDeleteSecretsByConn_EmptyConnUuid(t *testing.T) { + agent := &SecretAgent{} + agent.deleteSecretsByConn("") +} + +func TestDeleteSecrets_ExtractsConnUuid(t *testing.T) { + conn := map[string]nmVariantMap{ + "connection": { + "uuid": dbus.MakeVariant("test-uuid-123"), + "id": dbus.MakeVariant("TestNetwork"), + "type": dbus.MakeVariant("802-11-wireless"), + }, + "802-11-wireless": { + "ssid": dbus.MakeVariant("TestSSID"), + }, + } + + // Verify the UUID extraction DeleteSecrets relies on + assert.Equal(t, "test-uuid-123", readConnUuid(conn)) + + agent := &SecretAgent{} + err := agent.DeleteSecrets(conn, "/test/path") + assert.Nil(t, err) +} + +func TestDeleteSecrets2_ExtractsSettings(t *testing.T) { + // Simulate the settings map returned by GetSettings, as read in DeleteSecrets2 + settings := map[string]map[string]dbus.Variant{ + "connection": { + "uuid": dbus.MakeVariant("test-uuid-123"), + "type": dbus.MakeVariant("802-11-wireless"), + }, + } + + // Convert to nmSettingMap exactly as DeleteSecrets2 does + settingsTyped := make(nmSettingMap, len(settings)) + for k, v := range settings { + settingsTyped[k] = v + } + + // Verify the UUID extraction DeleteSecrets2 relies on + assert.Equal(t, "test-uuid-123", readConnUuid(settingsTyped)) + + // Verify nil-conn guard returns nil + agent := &SecretAgent{conn: nil} + err := agent.DeleteSecrets2("/test/path", "802-11-wireless-security") + assert.Nil(t, err) +} diff --git a/core/internal/server/network/secret_service.go b/core/internal/server/network/secret_service.go index 1a93d2eb2..b3debdd61 100644 --- a/core/internal/server/network/secret_service.go +++ b/core/internal/server/network/secret_service.go @@ -10,13 +10,22 @@ import ( ) const ( - secretServiceBusName = "org.freedesktop.secrets" - secretServicePath = "/org/freedesktop/secrets" - secretServiceIface = "org.freedesktop.Secret.Service" - secretItemIface = "org.freedesktop.Secret.Item" - secretPromptIface = "org.freedesktop.Secret.Prompt" + secretServiceBusName = "org.freedesktop.secrets" + secretServicePath = "/org/freedesktop/secrets" + secretServiceIface = "org.freedesktop.Secret.Service" + secretItemIface = "org.freedesktop.Secret.Item" + secretCollectionIface = "org.freedesktop.Secret.Collection" + secretPromptIface = "org.freedesktop.Secret.Prompt" + secretDefaultCollection = "/org/freedesktop/secrets/aliases/default" ) +type nmSecret struct { + Session dbus.ObjectPath + Parameters []byte + Value []byte + ContentType string +} + type secretServiceSession struct { conn *dbus.Conn svc dbus.BusObject @@ -49,6 +58,32 @@ func openSecretService() (*secretServiceSession, error) { }, nil } +func (s *secretServiceSession) close() { + s.conn.Close() +} + +func (s *secretServiceSession) searchItems(attrs map[string]string) ([]dbus.ObjectPath, error) { + var unlocked []dbus.ObjectPath + var locked []dbus.ObjectPath + call := s.svc.Call(secretServiceIface+".SearchItems", 0, attrs) + if call.Err != nil { + return nil, fmt.Errorf("SearchItems failed: %w", call.Err) + } + if err := call.Store(&unlocked, &locked); err != nil { + return nil, fmt.Errorf("failed to store SearchItems result: %w", err) + } + + if len(locked) > 0 { + if err := s.unlock(locked); err != nil { + log.Debugf("[SecretAgent] Failed to unlock items: %v", err) + return nil, err + } + unlocked = append(unlocked, locked...) + } + + return unlocked, nil +} + func (s *secretServiceSession) unlock(items []dbus.ObjectPath) error { var prompt dbus.ObjectPath var unlocked []dbus.ObjectPath @@ -116,47 +151,56 @@ func (s *secretServiceSession) unlock(items []dbus.ObjectPath) error { return nil } +func (s *secretServiceSession) ensureUnlocked() error { + collection := s.conn.Object(secretServiceBusName, dbus.ObjectPath(secretDefaultCollection)) + var locked bool + call := collection.Call("org.freedesktop.DBus.Properties.Get", 0, secretCollectionIface, "Locked") + if call.Err != nil { + log.Debugf("[SecretAgent] Could not read collection Locked property (may not exist yet): %v", call.Err) + return nil + } + var variant dbus.Variant + if err := call.Store(&variant); err != nil { + log.Debugf("[SecretAgent] Could not store collection Locked property: %v", err) + return nil + } + if v, ok := variant.Value().(bool); ok { + locked = v + } + if !locked { + return nil + } + + log.Debugf("[SecretAgent] Default collection is locked, unlocking...") + return s.unlock([]dbus.ObjectPath{dbus.ObjectPath(secretDefaultCollection)}) +} + func (s *secretServiceSession) lookup(connUuid, settingName, settingKey string) string { + if err := s.ensureUnlocked(); err != nil { + log.Debugf("[SecretAgent] lookup: failed to unlock collection: %v", err) + return "" + } + 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) + paths, err := s.searchItems(attrs) + if err != nil { + log.Debugf("[SecretAgent] searchItems failed for %s: %v", connUuid, 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 { + if len(paths) == 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) + item := s.conn.Object(secretServiceBusName, paths[0]) + var secret nmSecret + call := item.Call(secretItemIface+".GetSecret", 0, s.sessionPath) if call.Err != nil { log.Debugf("[SecretAgent] Secret service GetSecret failed: %v", call.Err) return "" @@ -176,15 +220,92 @@ func (s *secretServiceSession) lookup(connUuid, settingName, settingKey string) return secretValue } -func (s *secretServiceSession) close() { - s.conn.Close() +func (s *secretServiceSession) store(connUuid, settingName, settingKey, value, label string) error { + if err := s.ensureUnlocked(); err != nil { + return fmt.Errorf("failed to unlock collection: %w", err) + } + + attrs := map[string]string{ + "connection-uuid": connUuid, + "setting-name": settingName, + "setting-key": settingKey, + } + + secret := nmSecret{ + Session: s.sessionPath, + Value: []byte(value), + ContentType: "text/plain", + } + + props := map[string]dbus.Variant{ + "org.freedesktop.Secret.Item.Label": dbus.MakeVariant(label), + "org.freedesktop.Secret.Item.Attributes": dbus.MakeVariant(attrs), + } + + collection := s.conn.Object(secretServiceBusName, dbus.ObjectPath(secretDefaultCollection)) + call := collection.Call(secretCollectionIface+".CreateItem", 0, props, secret, true) + if call.Err != nil { + return fmt.Errorf("CreateItem failed: %w", call.Err) + } + + var itemPath dbus.ObjectPath + var promptPath dbus.ObjectPath + if err := call.Store(&itemPath, &promptPath); err != nil { + return fmt.Errorf("CreateItem returned invalid response: %w", err) + } + + if promptPath != "/" && promptPath != "" { + return fmt.Errorf("CreateItem requires prompt %s — secret not persisted for %s/%s", promptPath, connUuid, settingKey) + } + + log.Debugf("[SecretAgent] Stored secret for %s/%s (item=%s)", connUuid, settingKey, itemPath) + return nil +} + +func (s *secretServiceSession) deleteByUuid(connUuid string) error { + if err := s.ensureUnlocked(); err != nil { + return fmt.Errorf("failed to unlock collection: %w", err) + } + + attrs := map[string]string{ + "connection-uuid": connUuid, + } + + paths, err := s.searchItems(attrs) + if err != nil { + return err + } + + if len(paths) == 0 { + return nil + } + + for _, p := range paths { + item := s.conn.Object(secretServiceBusName, p) + if call := item.Call(secretItemIface+".Delete", 0); call.Err != nil { + log.Debugf("[SecretAgent] Failed to delete %s: %v", p, call.Err) + } + } + + log.Debugf("[SecretAgent] Deleted %d secret item(s) for %s", len(paths), connUuid) + return nil +} + +func (a *SecretAgent) withSecretService(fn func(*secretServiceSession) error) error { + sess, err := openSecretService() + if err != nil { + log.Debugf("[SecretAgent] Failed to open secret service session: %v", err) + return err + } + defer sess.close() + return fn(sess) } func (a *SecretAgent) trySecretService( connUuid string, settingName string, fields []string, -) nmSettingMap { +) map[string]string { if connUuid == "" { log.Debugf("[SecretAgent] trySecretService: connUuid is empty, skipping keyring lookup") return nil @@ -194,62 +315,33 @@ func (a *SecretAgent) trySecretService( 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 - } + var out map[string]string + err := a.withSecretService(func(sess *secretServiceSession) error { + 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 fmt.Errorf("missing field %s", field) + } + found[field] = val + } - sess, err := openSecretService() + out = found + return nil + }) 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) - } + return out +} - 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) +func (a *SecretAgent) deleteSecretsByConn(connUuid string) { + if connUuid == "" { + return } - return out + _ = a.withSecretService(func(sess *secretServiceSession) error { + return sess.deleteByUuid(connUuid) + }) } From 79420159c530eca80f49c4dc8b825b62e3d94705 Mon Sep 17 00:00:00 2001 From: Mikilio Date: Fri, 31 Jul 2026 15:41:29 +0200 Subject: [PATCH 2/3] refactor(network): flatten Phase 2 field resolution in SecretAgent Extract the password-flags decision chain out of GetSecrets into two helpers and group Phase 2 resolution by setting type instead of by operation: - readPasswordFlags maps setting name to the correct flag key (psk-flags/password-flags) and returns 0xFFFF when absent - handlePasswordFlags owns the whole flag decision chain, returning either resolved fields, an early response, or a *dbus.Error - Phase 2 is now a flat switch over settingName: vpn infers fields directly, wifi/802-1x delegate to the flag helpers Also fixes the broken VPN flow introduced by the earlier merge (VPN never set passwordFlags, so it always hit the 0xFFFF error path) and cleans up the stray indentation. --- .../server/network/agent_networkmanager.go | 151 ++++++++++-------- 1 file changed, 86 insertions(+), 65 deletions(-) diff --git a/core/internal/server/network/agent_networkmanager.go b/core/internal/server/network/agent_networkmanager.go index 643367d0d..3c9ffac92 100644 --- a/core/internal/server/network/agent_networkmanager.go +++ b/core/internal/server/network/agent_networkmanager.go @@ -171,79 +171,34 @@ func (a *SecretAgent) GetSecrets( // Phase 2: Resolve fields from hints or password-flags. if len(fields) == 0 { - if settingName == "vpn" { - if a.backend != nil { - a.backend.stateMutex.RLock() - isConnectingVPN := a.backend.state.IsConnectingVPN - a.backend.stateMutex.RUnlock() + switch settingName { + case "vpn": + if a.backend == nil { + log.Infof("[SecretAgent] VPN with empty hints - deferring to other agents for %s", vpnSvc) + return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) + } - if !isConnectingVPN { - log.Infof("[SecretAgent] VPN with empty hints - deferring to other agents for %s", vpnSvc) - return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) - } + a.backend.stateMutex.RLock() + isConnectingVPN := a.backend.state.IsConnectingVPN + a.backend.stateMutex.RUnlock() - fields = inferVPNFields(conn, vpnSvc) - log.Infof("[SecretAgent] VPN with empty hints but we're connecting - inferred fields: %v", fields) - } else { + if !isConnectingVPN { log.Infof("[SecretAgent] VPN with empty hints - deferring to other agents for %s", vpnSvc) return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) } - } - if len(fields) == 0 { - const ( - NM_SETTING_SECRET_FLAG_NONE = 0 - NM_SETTING_SECRET_FLAG_AGENT_OWNED = 1 - NM_SETTING_SECRET_FLAG_NOT_SAVED = 2 - NM_SETTING_SECRET_FLAG_NOT_REQUIRED = 4 - ) - - var passwordFlags uint32 = 0xFFFF - switch settingName { - case "802-11-wireless-security": - if wifiSecSettings, ok := conn["802-11-wireless-security"]; ok { - if flagsVariant, ok := wifiSecSettings["psk-flags"]; ok { - if pwdFlags, ok := flagsVariant.Value().(uint32); ok { - passwordFlags = pwdFlags - } - } - } - case "802-1x": - if dot1xSettings, ok := conn["802-1x"]; ok { - if flagsVariant, ok := dot1xSettings["password-flags"]; ok { - if pwdFlags, ok := flagsVariant.Value().(uint32); ok { - passwordFlags = pwdFlags - } - } - } - } + fields = inferVPNFields(conn, vpnSvc) + log.Infof("[SecretAgent] VPN with empty hints but we're connecting - inferred fields: %v", fields) - if passwordFlags == 0xFFFF { - log.Warnf("[SecretAgent] Could not determine password-flags for empty hints - returning NoSecrets error") - return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) - } else if passwordFlags&NM_SETTING_SECRET_FLAG_NOT_REQUIRED != 0 { - log.Infof("[SecretAgent] Secrets not required (flags=%d)", passwordFlags) - out := nmSettingMap{} - out[settingName] = nmVariantMap{} - return out, nil - } else if passwordFlags&NM_SETTING_SECRET_FLAG_AGENT_OWNED != 0 { - switch settingName { - case "802-11-wireless-security": - fields = []string{"psk"} - case "802-1x": - fields = infer8021xFields(conn) - default: - log.Warnf("[SecretAgent] Agent-owned secrets for unhandled setting %s (flags=%d)", settingName, passwordFlags) - 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] Secrets stored in NM config (flags=%d), deferring to system", passwordFlags) - return nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) + case "802-11-wireless-security", "802-1x": + newFields, response, err := handlePasswordFlags(settingName, conn, readPasswordFlags(settingName, conn)) + if err != nil { + return nil, err + } + if response != nil { + return response, nil } + fields = newFields } } @@ -760,6 +715,72 @@ func readConnTypeAndName(conn map[string]nmVariantMap) (string, string, string) return connType, name, svc } +func readPasswordFlags(settingName string, conn map[string]nmVariantMap) uint32 { + var flagKey string + switch settingName { + case "802-11-wireless-security": + flagKey = "psk-flags" + case "802-1x": + flagKey = "password-flags" + default: + return 0xFFFF + } + + settings, ok := conn[settingName] + if !ok { + return 0xFFFF + } + + flagsVariant, ok := settings[flagKey] + if !ok { + return 0xFFFF + } + + if pwdFlags, ok := flagsVariant.Value().(uint32); ok { + return pwdFlags + } + + return 0xFFFF +} + +func handlePasswordFlags(settingName string, conn map[string]nmVariantMap, passwordFlags uint32) ([]string, nmSettingMap, *dbus.Error) { + const ( + NM_SETTING_SECRET_FLAG_NONE = 0 + NM_SETTING_SECRET_FLAG_AGENT_OWNED = 1 + NM_SETTING_SECRET_FLAG_NOT_SAVED = 2 + NM_SETTING_SECRET_FLAG_NOT_REQUIRED = 4 + ) + + if passwordFlags == 0xFFFF { + log.Warnf("[SecretAgent] Could not determine password-flags for empty hints - returning NoSecrets error") + return nil, nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) + } else if passwordFlags&NM_SETTING_SECRET_FLAG_NOT_REQUIRED != 0 { + log.Infof("[SecretAgent] Secrets not required (flags=%d)", passwordFlags) + out := nmSettingMap{} + out[settingName] = nmVariantMap{} + return nil, out, nil + } else if passwordFlags&NM_SETTING_SECRET_FLAG_AGENT_OWNED != 0 { + var fields []string + switch settingName { + case "802-11-wireless-security": + fields = []string{"psk"} + case "802-1x": + fields = infer8021xFields(conn) + default: + log.Warnf("[SecretAgent] Agent-owned secrets for unhandled setting %s (flags=%d)", settingName, passwordFlags) + return nil, nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) + } + log.Infof("[SecretAgent] Agent-owned secrets, inferred fields: %v", fields) + return fields, nil, nil + } else if passwordFlags&NM_SETTING_SECRET_FLAG_NOT_SAVED != 0 { + log.Infof("[SecretAgent] Secrets not saved, will need to prompt (flags=%d)", passwordFlags) + return nil, nil, nil + } + + log.Infof("[SecretAgent] Secrets stored in NM config (flags=%d), deferring to system", passwordFlags) + return nil, nil, dbus.NewError("org.freedesktop.NetworkManager.SecretAgent.Error.NoSecrets", nil) +} + func fieldsNeeded(setting string, hints []string, conn map[string]nmVariantMap) []string { switch setting { case "802-11-wireless-security": From eedb7d72ac7ee06b418ccb88b898af0cd9fadee0 Mon Sep 17 00:00:00 2001 From: Mikilio Date: Fri, 31 Jul 2026 15:45:40 +0200 Subject: [PATCH 3/3] feat(network): log unhandled setting types at their dispatch points Add visibility into which NetworkManager connection types the SecretAgent does not handle explicitly, so missed types are easy to spot in logs. - GetSecrets response shaping: Warn when returning secrets as-is for an unhandled setting type - Backend caching, fieldsNeeded, and buildFieldsInfo: Debug when falling back to generic behavior for an unhandled setting type The keyring store path already warns; this extends the same idea to the remaining dispatch points. --- core/internal/server/network/agent_networkmanager.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/internal/server/network/agent_networkmanager.go b/core/internal/server/network/agent_networkmanager.go index 3c9ffac92..8f36ee53c 100644 --- a/core/internal/server/network/agent_networkmanager.go +++ b/core/internal/server/network/agent_networkmanager.go @@ -489,6 +489,7 @@ func (a *SecretAgent) GetSecrets( } log.Infof("[SecretAgent] Returning 802-1x enterprise secrets with %d fields", len(secretsOnly)) default: + log.Warnf("[SecretAgent] Shaping response for unhandled setting type %q - returning secrets as-is", settingName) out[settingName] = sec } if settingName == "vpn" && a.backend != nil && !isPKCS11 && (vpnUsername != "" || reply.Save) { @@ -568,6 +569,8 @@ func (a *SecretAgent) GetSecrets( switch settingName { case "802-11-wireless-security", "802-1x": a.backend.cacheWiFiSecret(connUuid, ssid, settingName, reply.Secrets) + default: + log.Debugf("[SecretAgent] No cache strategy for setting type %q", settingName) } } @@ -791,6 +794,7 @@ func fieldsNeeded(setting string, hints []string, conn map[string]nmVariantMap) } return infer8021xFields(conn) default: + log.Debugf("[SecretAgent] No field inference for setting type %q - falling back to hints", setting) return hints } } @@ -853,6 +857,7 @@ func buildFieldsInfo(setting string, fields []string, vpnService string) []Field case "vpn": info.Label, info.IsSecret = vpnFieldMeta(f, vpnService) default: + log.Debugf("[SecretAgent] No field metadata for setting type %q - using raw field name", setting) info.Label = f info.IsSecret = true }