RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1691619 - fix libnm and NetworkManager's handling of team configuration (strict JSON parsing)
Summary: fix libnm and NetworkManager's handling of team configuration (strict JSON pa...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: NetworkManager
Version: 8.0
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: rc
: 8.0
Assignee: Thomas Haller
QA Contact: Desktop QE
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-03-22 05:50 UTC by Thomas Haller
Modified: 2020-11-14 06:33 UTC (History)
8 users (show)

Fixed In Version: NetworkManager-1.20.0-1.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-11-05 22:29:37 UTC
Type: Bug
Target Upstream Version:
Embargoed:
pm-rhel: mirror+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2019:3623 0 None None None 2019-11-05 22:29:56 UTC

Description Thomas Haller 2019-03-22 05:50:54 UTC
Originally, NetworkManager (on D-Bus and libnm) only supported a plain "team.config" setting for team connection profiles. This was the JSON setting, that was opaque and not understandable to NM.

This "API" of having opaque JSON was cumbersome, and there was the goal to add a more convenient, typed API.

---

How it should be:


NM has a very strict understanding of what a team configuration is. A supported team configuration in NetworkManager consists of a well-known set of parameters, that are strictly verified to have value values and these values make sense together. It should start from a small set of supported configurations, and gradually support more settings -- always being very strict about what is valid and what not. Let's call this structured setting NMTeamConfig.

Converting a valid NMTeamConfig to JSON is always possible -- and uniquely so. Meaning, one particular NMTeamConfig has a normalized, preferred JSON representation: the "normalized" JSON.

We also need to strictly parse arbitrary JSON into NMTeamConfig. This parsing must be strict, there must be only JSON elements that we understand, that are valid, and nothing else. At the very least, we must be able to parse a "normalized" JSON back. Optionally, slight variations may be accepted (like, ignoring the order of dictionary keys). But that's not necessary. It's only for convenience that we may also accept some sets of non-normalized JSON (that are well understood). The stricter the better.


Now it's simple:

(1) if the user uses new-style API to configure a NMTeamConfig, the configuration must validate. As always, nm_connection_verify() rejects invalid configurations. A valid configuration can be converted to a normalized JSON config.

(2) if the user sets "team.config" directly, then we try to strictly parse it as NMTeamConfig. If we fail, then new-style NMTeamConfig is un-initialized and we keep using the opaque JSON string (that we were unable to understand). We cannot fail nm_connection_verify() in this case to not break backwards compatibility.
If the JSON can be strictly parsed as NMTeamConfig, then it's good too. Then new-style API is also available. Now, "team.config" may be in normalized form or not. It doesn't matter, we can keep the unnormalized form that the user wrote by hand (e.g. if it has additional whitespaces).

(3) on D-Bus or from keyfile, if only "team.config" is present, then we do (2). If only new-style parameters are present, we do (1). If both are present, then the new-style parameters must be valid, but also "team.config" must be parsable to the exact saem NMTeamConfig. Again, "team.config" may be in non-normalized form or not, but it must be valid.

(4) when libnm or NetworkManager serialize a team config to D-Bus or keyfile, then it either only serializes the unparsable JSON or it serializes the valid JSON together with the valid new-style NMTeamConfig.


---

How it is instead:


libnm/NetworkManager has not a strong sense of what makes a valid (new-style) configuration. The checks in verify() [1] are not sufficiently strict.

Also, libnm does not strictly parse the JSON. Instead, it picks individual fields on a one-by-one basis [2].

Also, libnm does not generate one entire JSON configuration out of the settings. Instead, it sets individual fields [3] inside the existing JSON, without consideration for the context.

The mistake that libnm makes is trying to merge JSON config that is not fully parsed/validate with additional settings on a one by one basis. When using new-style API, then the settings MUST validate and nothing else.

This needs to be fixed. We may avoid breaking backwards compatibility in many cases because when we accepted something in the past that we would no longer accept, we can just still generate the same JSON (that we do not consider as valid new-style NMTeamConfig).

NMSettingTeam must keep track whether of the preferred style ("team.config" or new-style NMTeamConfig). Basically: what API did the user use last. Regardless of the preferred style, if the "team.config" JSON or the new-style NMTeamConfig is valid/parsable, then the alternative style can be generated without loss. If it's invalid, then we may still generate something on a best effort basis (though, it would not validate).


Sending over DBUS:

- When libnm client-side generates a GVariant, it only sends the JSON. That is always the best choice. If the preferred style is new-style's NMTeamConfig and the configuration is invalid, then we still want to try as good as possible to craft a JSON. NM server side would either understand it (unlikely), or it would accept it as opaque old-style value.

- When libnm server side sends a message to the clients, then it includes both JSON and new-style fields. Since NM only keeps connections that verify, it means it either includes new-style and JSON with identical meaning; or it only sends the non-validating old-style JSON.

- when libnm client side parses fields, it always makes the best of it and never rejects profiles right away (it doesn't call nm_connection_verify()). If the data parses as valid new-style, then the preferred style is new-style. Otherwise, the preferred style is old-style.

- when libnm server side parses a GVariant that only contains "team.config" JSON, it accepts it (either as parsable new-style or as non-parsable blob). When the message on D-Bus only new-style fields (either alone or along "team.config" JSON), they must validate or are rejected.




[1] https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/libnm-core/nm-setting-team.c?id=72bdeebd73f05c831084202f6b983ee3f9b830d6#n1192
[2] https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/libnm-core/nm-setting-team.c?id=72bdeebd73f05c831084202f6b983ee3f9b830d6#n1349
[3] https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/libnm-core/nm-setting-team.c?id=72bdeebd73f05c831084202f6b983ee3f9b830d6#n1595

Comment 1 Thomas Haller 2019-03-22 09:58:39 UTC
See also https://github.com/NetworkManager/NetworkManager/pull/318

Comment 4 Vladimir Benes 2019-08-05 09:10:55 UTC
team config reflects changes in other team properties and team properties (I've tried many of them) are changed accordingly if team.config json changed. Marking as sanityOnly.

Comment 6 errata-xmlrpc 2019-11-05 22:29:37 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2019:3623


Note You need to log in before you can comment on or make changes to this bug.