Settings.Connection.Update() round-trip failure

[paulburlumi]

I'm trying to change the SSID for a connection and have written the following code:

func setSSID(conn gonetworkmanager.Connection, ssid string) error {
    settings, err := conn.GetSettings()
    if err != nil {
        return fmt.Errorf("GetSettings failed: %v", err)
    }

    for k1, v1 := range settings {
        for k2, v2 := range v1 {
            log.Printf("[\"%s\"][\"%s\"] = \"%T\", \"%v\"", k1, k2, v2, v2)
        }
    }

    settings["802-11-wireless"]["ssid"] = []byte(ssid)

    // TODO: determine why these keys don't round trip correctly.
    delete(settings["ipv6"], "addresses")
    delete(settings["ipv6"], "routes")

    err = conn.Update(settings)
    if err != nil {
        return fmt.Errorf("Update failed: %v", err)
    }

    return nil
}

If I remove the two lines marked TODO I get this failure:

2020/06/28 15:07:16 Update failed: ipv6.addresses: can't set property of type 'a(ayuay)' from value of type 'aav'

or this failure:

2020/06/28 15:19:39 Update failed: ipv6.routes: can't set property of type 'a(ayuayu)' from value of type 'aav'

The output from the program is:

2020/06/28 16:36:16 ["802-11-wireless-security"]["proto"] = "[]string", "[rsn]"
2020/06/28 16:36:16 ["802-11-wireless-security"]["group"] = "[]string", "[ccmp]"
2020/06/28 16:36:16 ["802-11-wireless-security"]["key-mgmt"] = "string", "wpa-psk"
2020/06/28 16:36:16 ["802-11-wireless-security"]["pairwise"] = "[]string", "[ccmp]"
2020/06/28 16:36:16 ["ipv4"]["address-data"] = "[]map[string]dbus.Variant", "[]"
2020/06/28 16:36:16 ["ipv4"]["addresses"] = "[][]uint32", "[]"
2020/06/28 16:36:16 ["ipv4"]["dns"] = "[]uint32", "[]"
2020/06/28 16:36:16 ["ipv4"]["dns-search"] = "[]string", "[]"
2020/06/28 16:36:16 ["ipv4"]["method"] = "string", "shared"
2020/06/28 16:36:16 ["ipv4"]["route-data"] = "[]map[string]dbus.Variant", "[]"
2020/06/28 16:36:16 ["ipv4"]["routes"] = "[][]uint32", "[]"
2020/06/28 16:36:16 ["ipv6"]["address-data"] = "[]map[string]dbus.Variant", "[]"
2020/06/28 16:36:16 ["ipv6"]["addresses"] = "[][]interface {}", "[]"
2020/06/28 16:36:16 ["ipv6"]["dns"] = "[][]uint8", "[]"
2020/06/28 16:36:16 ["ipv6"]["dns-search"] = "[]string", "[]"
2020/06/28 16:36:16 ["ipv6"]["method"] = "string", "ignore"
2020/06/28 16:36:16 ["ipv6"]["route-data"] = "[]map[string]dbus.Variant", "[]"
2020/06/28 16:36:16 ["ipv6"]["routes"] = "[][]interface {}", "[]"
2020/06/28 16:36:16 ["connection"]["timestamp"] = "uint64", "1593362134"
2020/06/28 16:36:16 ["connection"]["uuid"] = "string", "833d47c1-fb7f-4dc8-9a73-51fc4337deb3"
2020/06/28 16:36:16 ["connection"]["type"] = "string", "802-11-wireless"
2020/06/28 16:36:16 ["connection"]["id"] = "string", "Hotspot"
2020/06/28 16:36:16 ["connection"]["permissions"] = "[]string", "[]"
2020/06/28 16:36:16 ["802-11-wireless"]["seen-bssids"] = "[]string", "[DC:A6:32:81:8C:72]"
2020/06/28 16:36:16 ["802-11-wireless"]["ssid"] = "[]uint8", "[84 69 77 83 45 67 84 82 76 45 72 85 66 45 66 65 76 69 78 65]"
2020/06/28 16:36:16 ["802-11-wireless"]["mac-address-blacklist"] = "[]string", "[]"
2020/06/28 16:36:16 ["802-11-wireless"]["mode"] = "string", "ap"
2020/06/28 16:36:16 ["802-11-wireless"]["security"] = "string", "802-11-wireless-security"

Looking at the documentation it appears ["ipv6"]["addresses"] and ["ipv6"]["routes"] are both marked Deprecated.

I'm not familiar with the code, but I suspect a bug somewhere.

[mullerch]

The errors you have come from dbus. It's saying that the objects don't contain the correct type.

Here is a type reference : https://pythonhosted.org/txdbus/dbus_overview.html

It's unlikely a problem of Connection.Update() seeing the implementation.

I would print the struct after the delete, there's probably something wrong with it. Sending a settings map without 'addresses' and 'routes' should work fine.

[paulburlumi]

OK. I'll try and reproduce with the underlying dbus library used by gonetworkmanager and raise the problem there instead.

[mullerch]

Maybe I didn't catch totally the problem, but at this point I have the feeling that this is a usage or compatibility problem between your code and NetworkManager. NM expects a(ayuayu) while you send aav. I would first double check what exactly you are sending and what your NM version expects. I've left those parameters empty many times and I never got this problem. Which NM version do you have?

[paulburlumi]

/usr/sbin/NetworkManager --version
1.20.2

For reference this is on a RPi 4 running linux/arm64.

[paulburlumi]

What I really want to do is simply change the SSID for a connection and nothing else.

Ideally I would use Connection.Update like this:

settings := make(gonetworkmanager.ConnectionSettings)
settings["802-11-wireless"] = make(map[string]interface{})
settings["802-11-wireless"]["ssid"] = []byte(ssid)

err = conn.Update(settings)
if err != nil {
    return fmt.Errorf("Update failed: %v", err)
}

However this won't work as the Update documentation states it will do a replace: Update the connection with new settings and properties (replacing all previous settings and properties)

So I need the complete set of settings from Connection.GetSettings like this:

settings, err := conn.GetSettings()
if err != nil {
    return fmt.Errorf("GetSettings failed: %v", err)
}

settings["802-11-wireless"]["ssid"] = []byte(ssid)

err = conn.Update(settings)
if err != nil {
    return fmt.Errorf("Update failed: %v", err)
}

However this doesn't work as I see the following errors from Connection.Update:

2020/06/28 15:07:16 Update failed: ipv6.addresses: can't set property of type 'a(ayuay)' from value of type 'aav'
2020/06/28 15:19:39 Update failed: ipv6.routes: can't set property of type 'a(ayuayu)' from value of type 'aav'

I can work around these errors by changing the code like this:

settings, err := conn.GetSettings()
if err != nil {
    return fmt.Errorf("GetSettings failed: %v", err)
}

settings["802-11-wireless"]["ssid"] = []byte(ssid)

delete(settings["ipv6"], "addresses")
delete(settings["ipv6"], "routes")

err = conn.Update(settings)
if err != nil {
    return fmt.Errorf("Update failed: %v", err)
}

This is a bit hacky, but it appears to work for the moment.

The issue I'm trying to raise is that the following code doesn't work for me on NM v1.20.2:

settings, err := conn.GetSettings()
if err != nil {
    return fmt.Errorf("GetSettings failed: %v", err)
}

err = conn.Update(settings)
if err != nil {
    return fmt.Errorf("Update failed: %v", err)
}

So.. would you expect this code to work or not? Would you expect the output from Connection.GetSettings to be valid input to Connection.Update?

If yes (it should work), then there is an issue, probably with GetSettings. 🙁 If no (it should fail for the reasons described), we can just close this issue. 😀

[mullerch]

At this time there is no checks regarding the content of settings in the library, neither there are some helper function to make those kind of simple updates.

I really see this library as a very simple layer to avoid all the boring and error prone stuff (path definition, types conversion, ...).

A second layer that brings more verification and helpers would be great. Maybe this second layer should have gonetworkmanager as a connection option (and another with the libnm?)

[mullerch]

I just got some time do dig into this. As you said there is something special with 'addresses' and 'routes', they have been replaced with 'address-data' and 'route-data' and are now marked 'deprecated'. Still nothing says it will fail if specified. I would expect the output from Connection.GetSettings to be valid input to Connection.Update, but the addresses from the '-data' properties would then be ignored if there are some in the legacy properties.

Strange thing is that the types errors reported do exactly match what would occur if we send the 'address-data' data format into the 'addresses' field. Also I noticed the problem only occurs on ipv6 settings. To me it looks like a bug in the NetworkManager dbus server.

I did the same trick you did in my library usage to ensure the '-data' fields are taken in account, as it's not specified in which case legacy fields are filled. Still some cases (half settings in legacy, half in new) would be problematic.

[paulburlumi]

If it is any help I can confirm that what I have described above does work on NM v1.20.2 with the python-networkmanager package. So I don't believe NetworkManager itself is at fault here.

def set_ssid(connection_path, new_ssid):
    """Sets the new SSID"""
    print("Setting WiFi SSID to:" + new_ssid)
    conn = NetworkManager.Connection(connection_path)
    config = conn.GetSettings()
    config['802-11-wireless']['ssid'] = new_ssid
    conn.Update(config)
    print("SSID Updated")

[gerow]

I ran into something similar, but using the dbus go lib directly instead of gonetworkmanager. The types of the "address" and "routes" fields are pretty strange, so I would guess that the root issue here is in the dbus library instead of gonetworkmanager.

Considering these fields are deprecated anyway I'm happy with the workaround of just deleting them, but if someone wanted to pursue a proper fix I would guess the godbus maintainers would be the right next step. https://github.com/godbus/dbus