libnm: avoid parsing IP addresses twice in NMIPAddress/NMIPRoute API

Usually the normalization (canonicalize) and validation of the IP
address string both requires to parse the string. As we always do
validation first, we can use the parsed address and don't need to parse
it a second time.
This commit is contained in:
Thomas Haller
2022-02-03 23:10:48 +01:00
parent 6208a1bb84
commit 00e4f21629

View File

@@ -54,56 +54,24 @@ const NMUtilsDNSOptionDesc _nm_utils_dns_option_descs[] = {
{NULL, FALSE, FALSE}}; {NULL, FALSE, FALSE}};
static char * static char *
canonicalize_ip(int family, const char *ip, gboolean null_any) canonicalize_ip_binary(int family, const NMIPAddr *ip, gboolean null_any)
{ {
guint8 addr_bytes[sizeof(struct in6_addr)];
char addr_str[NM_UTILS_INET_ADDRSTRLEN];
int ret;
if (!ip) { if (!ip) {
if (null_any) if (null_any)
return NULL; return NULL;
if (family == AF_INET) if (NM_IS_IPv4(family))
return g_strdup("0.0.0.0"); return g_strdup("0.0.0.0");
if (family == AF_INET6) return g_strdup("::");
return g_strdup("::");
g_return_val_if_reached(NULL);
} }
ret = inet_pton(family, ip, addr_bytes); if (null_any && nm_ip_addr_is_null(family, ip))
g_return_val_if_fail(ret == 1, NULL); return NULL;
if (null_any) { return nm_utils_inet_ntop_dup(family, ip);
if (!memcmp(addr_bytes, &in6addr_any, nm_utils_addr_family_to_size(family)))
return NULL;
}
return g_strdup(inet_ntop(family, addr_bytes, addr_str, sizeof(addr_str)));
}
static char *
canonicalize_ip_binary(int family, gconstpointer ip, gboolean null_any)
{
char string[NM_UTILS_INET_ADDRSTRLEN];
if (!ip) {
if (null_any)
return NULL;
if (family == AF_INET)
return g_strdup("0.0.0.0");
if (family == AF_INET6)
return g_strdup("::");
g_return_val_if_reached(NULL);
}
if (null_any) {
if (!memcmp(ip, &in6addr_any, nm_utils_addr_family_to_size(family)))
return NULL;
}
return g_strdup(inet_ntop(family, ip, string, sizeof(string)));
} }
static gboolean static gboolean
valid_ip(int family, const char *ip, GError **error) valid_ip(int family, const char *ip, NMIPAddr *addr, GError **error)
{ {
if (!ip) { if (!ip) {
g_set_error(error, g_set_error(error,
@@ -112,7 +80,7 @@ valid_ip(int family, const char *ip, GError **error)
family == AF_INET ? _("Missing IPv4 address") : _("Missing IPv6 address")); family == AF_INET ? _("Missing IPv4 address") : _("Missing IPv6 address"));
return FALSE; return FALSE;
} }
if (!nm_utils_ipaddr_is_valid(family, ip)) { if (!nm_utils_parse_inaddr_bin(family, ip, NULL, addr)) {
g_set_error(error, g_set_error(error,
NM_CONNECTION_ERROR, NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_FAILED, NM_CONNECTION_ERROR_FAILED,
@@ -120,8 +88,9 @@ valid_ip(int family, const char *ip, GError **error)
: _("Invalid IPv6 address '%s'"), : _("Invalid IPv6 address '%s'"),
ip); ip);
return FALSE; return FALSE;
} else }
return TRUE;
return TRUE;
} }
static gboolean static gboolean
@@ -192,21 +161,23 @@ NMIPAddress *
nm_ip_address_new(int family, const char *addr, guint prefix, GError **error) nm_ip_address_new(int family, const char *addr, guint prefix, GError **error)
{ {
NMIPAddress *address; NMIPAddress *address;
NMIPAddr addr_bin;
g_return_val_if_fail(family == AF_INET || family == AF_INET6, NULL); g_return_val_if_fail(family == AF_INET || family == AF_INET6, NULL);
g_return_val_if_fail(addr != NULL, NULL); g_return_val_if_fail(addr != NULL, NULL);
if (!valid_ip(family, addr, error)) if (!valid_ip(family, addr, &addr_bin, error))
return NULL; return NULL;
if (!valid_prefix(family, prefix, error)) if (!valid_prefix(family, prefix, error))
return NULL; return NULL;
address = g_slice_new0(NMIPAddress); address = g_slice_new(NMIPAddress);
address->refcount = 1; *address = (NMIPAddress){
.refcount = 1,
address->family = family; .family = family,
address->address = canonicalize_ip(family, addr, FALSE); .address = canonicalize_ip_binary(family, &addr_bin, FALSE),
address->prefix = prefix; .prefix = prefix,
};
return address; return address;
} }
@@ -228,7 +199,6 @@ NMIPAddress *
nm_ip_address_new_binary(int family, gconstpointer addr, guint prefix, GError **error) nm_ip_address_new_binary(int family, gconstpointer addr, guint prefix, GError **error)
{ {
NMIPAddress *address; NMIPAddress *address;
char string[NM_UTILS_INET_ADDRSTRLEN];
g_return_val_if_fail(family == AF_INET || family == AF_INET6, NULL); g_return_val_if_fail(family == AF_INET || family == AF_INET6, NULL);
g_return_val_if_fail(addr != NULL, NULL); g_return_val_if_fail(addr != NULL, NULL);
@@ -236,12 +206,13 @@ nm_ip_address_new_binary(int family, gconstpointer addr, guint prefix, GError **
if (!valid_prefix(family, prefix, error)) if (!valid_prefix(family, prefix, error))
return NULL; return NULL;
address = g_slice_new0(NMIPAddress); address = g_slice_new(NMIPAddress);
address->refcount = 1; *address = (NMIPAddress){
.refcount = 1,
address->family = family; .family = family,
address->address = g_strdup(inet_ntop(family, addr, string, sizeof(string))); .address = nm_utils_inet_ntop_dup(family, addr),
address->prefix = prefix; .prefix = prefix,
};
return address; return address;
} }
@@ -277,9 +248,8 @@ nm_ip_address_unref(NMIPAddress *address)
address->refcount--; address->refcount--;
if (address->refcount == 0) { if (address->refcount == 0) {
g_free(address->address); g_free(address->address);
if (address->attributes) nm_g_hash_table_unref(address->attributes);
g_hash_table_unref(address->attributes); nm_g_slice_free(address);
g_slice_free(NMIPAddress, address);
} }
} }
@@ -442,12 +412,18 @@ nm_ip_address_get_address(NMIPAddress *address)
void void
nm_ip_address_set_address(NMIPAddress *address, const char *addr) nm_ip_address_set_address(NMIPAddress *address, const char *addr)
{ {
NMIPAddr addr_bin;
g_return_if_fail(address != NULL); g_return_if_fail(address != NULL);
g_return_if_fail(addr != NULL);
g_return_if_fail(nm_utils_ipaddr_is_valid(address->family, addr)); if (!valid_ip(address->family, addr, &addr_bin, NULL)) {
g_return_if_fail(addr != NULL);
g_return_if_fail(nm_utils_ipaddr_is_valid(address->family, addr));
nm_assert_not_reached();
}
g_free(address->address); g_free(address->address);
address->address = canonicalize_ip(address->family, addr, FALSE); address->address = canonicalize_ip_binary(address->family, &addr_bin, FALSE);
} }
/** /**
@@ -480,13 +456,11 @@ nm_ip_address_get_address_binary(NMIPAddress *address, gpointer addr)
void void
nm_ip_address_set_address_binary(NMIPAddress *address, gconstpointer addr) nm_ip_address_set_address_binary(NMIPAddress *address, gconstpointer addr)
{ {
char string[NM_UTILS_INET_ADDRSTRLEN];
g_return_if_fail(address != NULL); g_return_if_fail(address != NULL);
g_return_if_fail(addr != NULL); g_return_if_fail(addr != NULL);
g_free(address->address); g_free(address->address);
address->address = g_strdup(inet_ntop(address->family, addr, string, sizeof(string))); address->address = nm_utils_inet_ntop_dup(address->family, addr);
} }
/** /**
@@ -642,27 +616,30 @@ nm_ip_route_new(int family,
GError **error) GError **error)
{ {
NMIPRoute *route; NMIPRoute *route;
NMIPAddr dest_bin;
NMIPAddr next_hop_bin;
g_return_val_if_fail(family == AF_INET || family == AF_INET6, NULL); g_return_val_if_fail(family == AF_INET || family == AF_INET6, NULL);
g_return_val_if_fail(dest, NULL); g_return_val_if_fail(dest, NULL);
if (!valid_ip(family, dest, error)) if (!valid_ip(family, dest, &dest_bin, error))
return NULL; return NULL;
if (!valid_prefix(family, prefix, error)) if (!valid_prefix(family, prefix, error))
return NULL; return NULL;
if (next_hop && !valid_ip(family, next_hop, error)) if (next_hop && !valid_ip(family, next_hop, &next_hop_bin, error))
return NULL; return NULL;
if (!valid_metric(metric, error)) if (!valid_metric(metric, error))
return NULL; return NULL;
route = g_slice_new0(NMIPRoute); route = g_slice_new(NMIPRoute);
route->refcount = 1; *route = (NMIPRoute){
.refcount = 1,
route->family = family; .family = family,
route->dest = canonicalize_ip(family, dest, FALSE); .dest = canonicalize_ip_binary(family, &dest_bin, FALSE),
route->prefix = prefix; .prefix = prefix,
route->next_hop = canonicalize_ip(family, next_hop, TRUE); .next_hop = canonicalize_ip_binary(family, next_hop ? &next_hop_bin : NULL, TRUE),
route->metric = metric; .metric = metric,
};
return route; return route;
} }
@@ -700,14 +677,15 @@ nm_ip_route_new_binary(int family,
if (!valid_metric(metric, error)) if (!valid_metric(metric, error))
return NULL; return NULL;
route = g_slice_new0(NMIPRoute); route = g_slice_new0(NMIPRoute);
route->refcount = 1; *route = (NMIPRoute){
.refcount = 1,
route->family = family; .family = family,
route->dest = canonicalize_ip_binary(family, dest, FALSE); .dest = canonicalize_ip_binary(family, dest, FALSE),
route->prefix = prefix; .prefix = prefix,
route->next_hop = canonicalize_ip_binary(family, next_hop, TRUE); .next_hop = canonicalize_ip_binary(family, next_hop, TRUE),
route->metric = metric; .metric = metric,
};
return route; return route;
} }
@@ -744,9 +722,8 @@ nm_ip_route_unref(NMIPRoute *route)
if (route->refcount == 0) { if (route->refcount == 0) {
g_free(route->dest); g_free(route->dest);
g_free(route->next_hop); g_free(route->next_hop);
if (route->attributes) nm_g_hash_table_unref(route->attributes);
g_hash_table_unref(route->attributes); nm_g_slice_free(route);
g_slice_free(NMIPRoute, route);
} }
} }
@@ -911,11 +888,17 @@ nm_ip_route_get_dest(NMIPRoute *route)
void void
nm_ip_route_set_dest(NMIPRoute *route, const char *dest) nm_ip_route_set_dest(NMIPRoute *route, const char *dest)
{ {
NMIPAddr dest_bin;
g_return_if_fail(route != NULL); g_return_if_fail(route != NULL);
g_return_if_fail(nm_utils_ipaddr_is_valid(route->family, dest));
if (!valid_ip(route->family, dest, &dest_bin, NULL)) {
g_return_if_fail(nm_utils_ipaddr_is_valid(route->family, dest));
nm_assert_not_reached();
}
g_free(route->dest); g_free(route->dest);
route->dest = canonicalize_ip(route->family, dest, FALSE); route->dest = canonicalize_ip_binary(route->family, &dest_bin, FALSE);
} }
/** /**
@@ -948,13 +931,11 @@ nm_ip_route_get_dest_binary(NMIPRoute *route, gpointer dest)
void void
nm_ip_route_set_dest_binary(NMIPRoute *route, gconstpointer dest) nm_ip_route_set_dest_binary(NMIPRoute *route, gconstpointer dest)
{ {
char string[NM_UTILS_INET_ADDRSTRLEN];
g_return_if_fail(route != NULL); g_return_if_fail(route != NULL);
g_return_if_fail(dest != NULL); g_return_if_fail(dest != NULL);
g_free(route->dest); g_free(route->dest);
route->dest = g_strdup(inet_ntop(route->family, dest, string, sizeof(string))); route->dest = nm_utils_inet_ntop_dup(route->family, dest);
} }
/** /**
@@ -1022,11 +1003,17 @@ nm_ip_route_get_next_hop(NMIPRoute *route)
void void
nm_ip_route_set_next_hop(NMIPRoute *route, const char *next_hop) nm_ip_route_set_next_hop(NMIPRoute *route, const char *next_hop)
{ {
NMIPAddr next_hop_bin;
g_return_if_fail(route != NULL); g_return_if_fail(route != NULL);
g_return_if_fail(!next_hop || nm_utils_ipaddr_is_valid(route->family, next_hop));
if (next_hop && !valid_ip(route->family, next_hop, &next_hop_bin, NULL)) {
g_return_if_fail(!next_hop || nm_utils_ipaddr_is_valid(route->family, next_hop));
nm_assert_not_reached();
}
g_free(route->next_hop); g_free(route->next_hop);
route->next_hop = canonicalize_ip(route->family, next_hop, TRUE); route->next_hop = canonicalize_ip_binary(route->family, next_hop ? &next_hop_bin : NULL, TRUE);
} }
/** /**
@@ -1694,7 +1681,7 @@ nm_ip_routing_rule_unref(NMIPRoutingRule *self)
g_free(self->iifname); g_free(self->iifname);
g_free(self->oifname); g_free(self->oifname);
g_slice_free(NMIPRoutingRule, self); nm_g_slice_free(self);
} }
/** /**
@@ -4002,25 +3989,31 @@ gboolean
nm_setting_ip_config_add_dns(NMSettingIPConfig *setting, const char *dns) nm_setting_ip_config_add_dns(NMSettingIPConfig *setting, const char *dns)
{ {
NMSettingIPConfigPrivate *priv; NMSettingIPConfigPrivate *priv;
char *dns_canonical; int addr_family;
NMIPAddr dns_bin;
char dns_canonical[NM_UTILS_INET_ADDRSTRLEN];
guint i; guint i;
g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), FALSE); g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), FALSE);
g_return_val_if_fail(dns != NULL, FALSE);
g_return_val_if_fail(nm_utils_ipaddr_is_valid(NM_SETTING_IP_CONFIG_GET_FAMILY(setting), dns), addr_family = NM_SETTING_IP_CONFIG_GET_FAMILY(setting);
FALSE);
if (!valid_ip(addr_family, dns, &dns_bin, NULL)) {
g_return_val_if_fail(dns != NULL, FALSE);
g_return_val_if_fail(nm_utils_ipaddr_is_valid(addr_family, dns), FALSE);
nm_assert_not_reached();
}
priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting);
dns_canonical = canonicalize_ip(NM_SETTING_IP_CONFIG_GET_FAMILY(setting), dns, FALSE); nm_utils_inet_ntop(addr_family, &dns_bin, dns_canonical);
for (i = 0; i < priv->dns->len; i++) { for (i = 0; i < priv->dns->len; i++) {
if (!strcmp(dns_canonical, priv->dns->pdata[i])) { if (nm_streq(dns_canonical, priv->dns->pdata[i]))
g_free(dns_canonical);
return FALSE; return FALSE;
}
} }
g_ptr_array_add(priv->dns, dns_canonical); g_ptr_array_add(priv->dns, g_strdup(dns_canonical));
_notify(setting, PROP_DNS); _notify(setting, PROP_DNS);
return TRUE; return TRUE;
} }
@@ -4059,26 +4052,32 @@ gboolean
nm_setting_ip_config_remove_dns_by_value(NMSettingIPConfig *setting, const char *dns) nm_setting_ip_config_remove_dns_by_value(NMSettingIPConfig *setting, const char *dns)
{ {
NMSettingIPConfigPrivate *priv; NMSettingIPConfigPrivate *priv;
char *dns_canonical; int addr_family;
NMIPAddr dns_bin;
char dns_canonical[NM_UTILS_INET_ADDRSTRLEN];
guint i; guint i;
g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), FALSE); g_return_val_if_fail(NM_IS_SETTING_IP_CONFIG(setting), FALSE);
g_return_val_if_fail(dns != NULL, FALSE);
g_return_val_if_fail(nm_utils_ipaddr_is_valid(NM_SETTING_IP_CONFIG_GET_FAMILY(setting), dns), addr_family = NM_SETTING_IP_CONFIG_GET_FAMILY(setting);
FALSE);
if (!valid_ip(addr_family, dns, &dns_bin, NULL)) {
g_return_val_if_fail(dns != NULL, FALSE);
g_return_val_if_fail(nm_utils_ipaddr_is_valid(addr_family, dns), FALSE);
nm_assert_not_reached();
}
priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting);
dns_canonical = canonicalize_ip(NM_SETTING_IP_CONFIG_GET_FAMILY(setting), dns, FALSE); nm_utils_inet_ntop(addr_family, &dns_bin, dns_canonical);
for (i = 0; i < priv->dns->len; i++) { for (i = 0; i < priv->dns->len; i++) {
if (!strcmp(dns_canonical, priv->dns->pdata[i])) { if (nm_streq(dns_canonical, priv->dns->pdata[i])) {
g_ptr_array_remove_index(priv->dns, i); g_ptr_array_remove_index(priv->dns, i);
_notify(setting, PROP_DNS); _notify(setting, PROP_DNS);
g_free(dns_canonical);
return TRUE; return TRUE;
} }
} }
g_free(dns_canonical);
return FALSE; return FALSE;
} }