The present version of the EC2 metadata API (IMDSv2) requires a header
with a token to be present in all requests. The token is essentially a
cookie that's not actually a cookie that's obtained with a PUT call that
doesn't put anything. Apparently it's too easy to trick someone into
calling a GET method.
EC2 now supports IMDSv2 everywhere with IMDSv1 being optional, so let's
just use IMDSv2 unconditionally. Also, the presence of a token API can
be used to detect the AWS EC2 cloud.
Conflicts: variable alignments only
https://bugzilla.redhat.com/show_bug.cgi?id=2151986
(cherry picked from commit 8b7e12c2d6)
(cherry picked from commit 429f36cd81)
(cherry picked from commit e3ac982b32)
(cherry picked from commit c5a3e739b1)
(cherry picked from commit ee157ad48b)
(cherry picked from commit ae3ec36462)
(cherry picked from commit 865fe0732e)
We'll need to be able to issue PUT calls.
Conflicts: variable alignments only, missing nmcs-provider-aliyun
(cherry picked from commit cd74d75002)
(cherry picked from commit eff4372045)
(cherry picked from commit aaf66e9174)
(cherry picked from commit 3d94f4fdf9)
(cherry picked from commit 181466c6da)
(cherry picked from commit 7243307bb8)
(cherry picked from commit 1aa88024cb)
We're going to extend those to issue methods other than GET.
Also, "request" would've been too long, "req" looks nicer.
Conflicts: variable alignments, missing trivial commit in provider-azure
(cherry picked from commit 85ce088616)
(cherry picked from commit 6e8cfbae32)
(cherry picked from commit 20cd11ee49)
(cherry picked from commit 9ce530fa7a)
(cherry picked from commit d6d161a31d)
(cherry picked from commit 977fc2c8c5)
No need to do a deep clone. The strv array is not ever modified and we
pack it together in one memory allocation.
Conflicts: nm_strv_dup_packed is still called nm_utils_strv_dup_packed
(cherry picked from commit 599fe234ea)
(cherry picked from commit 3787eacac9)
(cherry picked from commit 89a6ce575d)
(cherry picked from commit d14dc95be3)
(cherry picked from commit 7e516418e0)
(cherry picked from commit becb47826a)
(cherry picked from commit 4704e14100)
It's not used anywhere.
Conflicts: variable alignments only
(cherry picked from commit ce225b2c06)
(cherry picked from commit 23b9514080)
(cherry picked from commit 36d417af60)
(cherry picked from commit d83537bff5)
(cherry picked from commit f584b9c97b)
(cherry picked from commit f59f629431)
(cherry picked from commit 1885ff2c65)
Let NMCSProviderGetConfigIfaceData.get_config_data have a pointer to the
NMCSProviderGetConfigTaskData. This will allow two things:
- at several places we pass on `nm_utils_user_data_pack(get_config_data,
config_iface_data)` as user data. We can avoid that, by just letting
config_iface_data have a pointer to get_config_data.
- NMCSProviderGetConfigIfaceData contains a provider specific field
"priv". That may also require special initialization or destruction,
depending on the type. We thus need access to the provider type,
which we have via iface_data->get_config_data->self.
Also let NMCSProviderGetConfigTaskData have a pointer "self" to the
NMCSProvider. While there was already the "task", which contains the
provider as source-object, this is more convenient.
Conflicts: missing nmcs-provider-aliyun
(cherry picked from commit 069946cda1)
(cherry picked from commit 061c05ca39)
(cherry picked from commit 1cf4fd0235)
(cherry picked from commit be1dce951e)
(cherry picked from commit 646bc7a10e)
The code never set "iface_get_config->cidr_addr", despite
setting "cidr_prefix" and "has_cidr". As a result, cloud-setup
would think that the subnet is "0.0.0.0/$PLEN", and calculate
the gateway as "0.0.0.1".
As a result it would add a default route to table 30400 via 0.0.0.1,
which is obviously wrong.
How to detect the right gateway? Let's try obtain the subnet also via
the meta data. That seems mostly correct, except that we only access
subnet at index 0. What if there are multiple ones? I don't know.
https://bugzilla.redhat.com/show_bug.cgi?id=1912236
(cherry picked from commit c2629f72b0)
Background
==========
Imagine you run a container on your machine. Then the routing table
might look like:
default via 10.0.10.1 dev eth0 proto dhcp metric 100
10.0.10.0/28 dev eth0 proto kernel scope link src 10.0.10.5 metric 100
[...]
10.42.0.0/24 via 10.42.0.0 dev flannel.1 onlink
10.42.1.2 dev cali02ad7e68ce1 scope link
10.42.1.3 dev cali8fcecf5aaff scope link
10.42.2.0/24 via 10.42.2.0 dev flannel.1 onlink
10.42.3.0/24 via 10.42.3.0 dev flannel.1 onlink
That is, there are another interfaces with subnets and specific routes.
If nm-cloud-setup now configures rules:
0: from all lookup local
30400: from 10.0.10.5 lookup 30400
32766: from all lookup main
32767: from all lookup default
and
default via 10.0.10.1 dev eth0 table 30400 proto static metric 10
10.0.10.1 dev eth0 table 30400 proto static scope link metric 10
then these other subnets will also be reached via the default route.
This container example is just one case where this is a problem. In
general, if you have specific routes on another interface, then the
default route in the 30400+ table will interfere badly.
The idea of nm-cloud-setup is to automatically configure the network for
secondary IP addresses. When the user has special requirements, then
they should disable nm-cloud-setup and configure whatever they want.
But the container use case is popular and important. It is not something
where the user actively configures the network. This case needs to work better,
out of the box. In general, nm-cloud-setup should work better with the
existing network configuration.
Change
======
Add new routing tables 30200+ with the individual subnets of the
interface:
10.0.10.0/24 dev eth0 table 30200 proto static metric 10
[...]
default via 10.0.10.1 dev eth0 table 30400 proto static metric 10
10.0.10.1 dev eth0 table 30400 proto static scope link metric 10
Also add more important routing rules with priority 30200+, which select
these tables based on the source address:
30200: from 10.0.10.5 lookup 30200
These will do source based routing for the subnets on these
interfaces.
Then, add a rule with priority 30350
30350: lookup main suppress_prefixlength 0
which processes the routes from the main table, but ignores the default
routes. 30350 was chosen, because it's in between the rules 30200+ and
30400+, leaving a range for the user to configure their own rules.
Then, as before, the rules 30400+ again look at the corresponding 30400+
table, to find a default route.
Finally, process the main table again, this time honoring the default
route. That is for packets that have a different source address.
This change means that the source based routing is used for the
subnets that are configured on the interface and for the default route.
Whereas, if there are any more specific routes in the main table, they will
be preferred over the default route.
Apparently Amazon Linux solves this differently, by not configuring a
routing table for addresses on interface "eth0". That might be an
alternative, but it's not clear to me what is special about eth0 to
warrant this treatment. It also would imply that we somehow recognize
this primary interface. In practise that would be doable by selecting
the interface with "iface_idx" zero.
Instead choose this approach. This is remotely similar to what WireGuard does
for configuring the default route ([1]), however WireGuard uses fwmark to match
the packets instead of the source address.
[1] https://www.wireguard.com/netns/#improved-rule-based-routing
(cherry picked from commit fe80b2d1ec)
(cherry picked from commit 58e58361bd)
The table number is chosen as 30400 + iface_idx. That is, the range is
limited and we shouldn't handle more than 100 devices. Add a check for
that and error out.
(cherry picked from commit b68d694b78)
(cherry picked from commit 292233e16e)
The routes/rules that are configured are independent of the
order in which we process the devices. That is, because they
use the "iface_idx" for cases where there is ambiguity.
Still, it feels nicer to always process them in a defined order.
(cherry picked from commit a95ea0eb29)
(cherry picked from commit 6302cd416d)
Sorted by iface_idx. The iface_idx is probably something useful and
stable, provided by the provider. E.g. it's the order in which
interfaces are exposed on the meta data.
(cherry picked from commit 1c5cb9d3c2)
(cherry picked from commit 0a2ed62703)
get-config() gives a NMCSProviderGetConfigResult structure, and the
main part of data is the GHashTable of MAC addresses and
NMCSProviderGetConfigIfaceData instances.
Let NMCSProviderGetConfigIfaceData also have a reference to the MAC
address. This way, I'll be able to create a (sorted) list of interface
datas, that also contain the MAC address.
(cherry picked from commit ec56fe60fb)
(cherry picked from commit cc289e5369)
nm-cloud-setup automatically configures the network. That may conflict
with what the user wants. In case the user configures some specific
setup, they are encouraged to disable nm-cloud-setup (and its
automatism).
Still, what we do by default matters, and should play as well with
user's expectations. Configuring policy routing and a higher priority
table (30400+) that hijacks the traffic can cause problems.
If the system only has one IPv4 address and one interface, then there
is no point in configuring policy routing at all. Detect that, and skip
the change in that case.
Note that of course we need to handle the case where previously multiple
IP addresses were configured and an update gives only one address. In
that case we need to clear the previously configured rules/routes. The
patch achieves this.
(cherry picked from commit 5f047968d7)
(cherry picked from commit 8bc8a0f56b)
nm-cloud-setup automatically detects routes, addresses and rules and configures them
on the device using the emphermal Reapply() API. That is, it does not modify the
existing profile (on disk), but changes the runtime configuration only.
As such, it used to wipe otherwise statically configured IP addresses, routes and
rules. That seems unnecessary. Let's keep the configuration from the (persistent)
configuration.
There is of course the problem that nm-cloud-setup doesn't really
understand the existing IP configuration, and it can only hope that
it can be meaningfully combined with what nm-cloud-setup wants to
configure. This should cover most simple cases, for more complex setups,
the user probably should disable nm-cloud-setup and configure the
network explicitly to their liking.
https://bugzilla.redhat.com/show_bug.cgi?id=1971527https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/893
(cherry picked from commit 4201ee5119)
(cherry picked from commit 9541b0bea4)
Now that we return a struct from get_config(), we can have system-wide
properties returned.
Let it count and cache the number of valid iface_datas.
Currently that is not yet used, but it will be.
(cherry picked from commit a3cd66d3fa)
(cherry picked from commit e74375fc3b)
Returning a struct seems easier to understand, because then the result
is typed.
Also, we might return additional results, which are system wide and not
per-interface.
(cherry picked from commit 323e182768)
(cherry picked from commit c94b1c43d4)
If previously the profile would track two addresses ("10.116.1.130/24",
"10.116.1.65/24"), and during an update the second address was removed
(leaving "10.116.1.130/24"), then the addresses of the profile were
wrongly not changed.
The effect is that removing a secondary IP address might not take
effect.
Fix that.
https://bugzilla.redhat.com/show_bug.cgi?id=1920838
Fixes: 69f048bf0c ('cloud-setup: add tool for automatic IP configuration in cloud')
We use the iface_idx for example to determine the routing table,
by using table 30400+iface_idx.
While the HTTP API for Azure has a index, it does not mean that we
should use that index as-is for our purpose.
Instead, treat those indexes separately and ensure that the
iface_idx that we return is numbering the interfaces starting
from zero.
While it's not clear whether we should be strict or forgiving
when fetching the HTTP meta data, we should be consistent.
On a parse error of the IP addresses we fail. Hence also
fail on a parse error for the subnet.
The API of mcs_provider_get_config() allows to explicitly request
for certain interfaces (MAC addresses), but it also allows to fetch
any.
That means, the result dictionary will be pre-populated with the
MAC addresses that were requested, but if we encounter an unknown
interface, then that is not a reason to fail.
Previously we would call
nmcs_utils_hwaddr_normalize(g_bytes_get_data(response, NULL), -1);
which treats the data in response as NUL terminated. That is not
entirely wrong, because the HTTP request's response is guaranteed
to have a NUL termination at the end. However, it doesn't seam to good
either.
For one, we already have the length. Use it. But also, if the response
contains any NUL bytes in the middle, then this would wrongly only
consider the first line. We should not accept "00:11:22:33:44:55\0bogus"
as valid.
While at it, reject NUL characters from nmcs_utils_hwaddr_normalize() --
except one NUL at the end.
First note that all three provider implementations are very similar.
That is why NMCSProvider's implementation does already some work that
is common to all implementations. For example, it provides the
NMCSProviderGetConfigTaskData structure to help tracking the data of
the request.
Also note that the GCP/Azure implementations didn't handle the
cancellation correctly. They always would pass
g_task_get_cancellable(get_config_data->task)
to the asynchronous requests. That is the GCancellable provider by the
caller. That is fine when there is only one async operation ongoing. But
that is not the case, we have parallel HTTP requests.
Then, when an error happened, the overall get_config() operations fails
and the still pending requests should all be aborted. However, we must
not cancel the GCancellable of the user (because that is not owned by us).
The correct solution is to use an internal cancellable in those cases.
Anyway. Since all of this is similar, we can extend the base class
to handle things for us. This also gets the cancellation right by having
a "get_config_data->intern_cancellable".
The code is not entirely straight forward. Consistent naming
is hence important.
In "nmcs-provider-ec2.c", variables of this kind are called
"get_config_data". That also matches to the type of the data
(NMCSProviderGetConfigTaskData).
Rename the variables to make naming consistent. Also, I find the
longer name to be clearer.
Currently libnm headers include <linux/if_{ether,infiniband,vlan}.h>.
These are public headers, that means we drag in the linux header to all
users of <NetworkManager.h>.
Often the linux headers work badly together with certain headers from libc.
Depending on the libc version, you have to order linux headers in the right
order with respect to libc headers.
We should do better about libnm headers. As a first step, assume that
the linux headers don't get included by libnm, and explicitly include
them where they are needed.
If we call g_cancellable_connect() on a GCancellable that is already
cancelled, then the callback is invoked synchronously. We need to
handle that.
However, we can slightly simplify the code. There is no change in
behavior, but we can always let the cancelled callback return the
result.
"iface_data->cancellable" is an internal cancellable for the parallel
HTTP requests. Once we encounter a failure, those requests are all
obsolete and must be cancelled.
There are two GCancellable at work: one is provided by the user
during nmcs_provider_get_config(), and one is used internally for the
individual HTTP GET requests.
In _get_config_fetch_done_cb(), if the error reason is "cancelled",
then it means that our internal iface_data->cancellable was cancelled.
Probably because an error happend (like a timeout or the user cancelled
the external GCancellable).
In that case, we must not report that the task completed with a
cancellation, because we need to preserve the error that was the
original cause.
