[systemd-devel] [PATCH] V2 sd-ipv4ll: generate predictable addresses
Tom Gundersen
teg at jklm.no
Thu Mar 20 11:16:26 PDT 2014
On Thu, Mar 20, 2014 at 6:52 PM, Tom Gundersen <teg at jklm.no> wrote:
> On Wed, Mar 19, 2014 at 2:36 PM, Umut Tezduyar Lindskog
> <umut.tezduyar at axis.com> wrote:
>> ---
>> src/libsystemd-network/sd-ipv4ll.c | 86 +++++++++++++++++++++-------
>> src/network/networkd-link.c | 12 +++-
>> src/network/networkd.h | 1 +
>> src/shared/net-util.c | 39 +++++++++++++
>> src/shared/net-util.h | 2 +
>> src/systemd/sd-ipv4ll.h | 1 +
>> src/udev/net/link-config.c | 27 +--------
>> 7 files changed, 119 insertions(+), 49 deletions(-)
>>
>> diff --git a/src/libsystemd-network/sd-ipv4ll.c b/src/libsystemd-network/sd-ipv4ll.c
>> index c6f6e01..fbe3efd 100644
>> --- a/src/libsystemd-network/sd-ipv4ll.c
>> +++ b/src/libsystemd-network/sd-ipv4ll.c
>> @@ -76,6 +76,8 @@ struct sd_ipv4ll {
>> usec_t defend_window;
>> int next_wakeup_valid;
>> be32_t address;
>> + struct random_data *random_data;
>> + char *random_data_state;
>> /* External */
>> be32_t claimed_address;
>> struct ether_addr mac_addr;
>> @@ -130,30 +132,27 @@ static int ipv4ll_stop(sd_ipv4ll *ll, int event) {
>> return 0;
>> }
>>
>> -static be32_t ipv4ll_pick_address(sd_ipv4ll *ll) {
>> +static int ipv4ll_pick_address(sd_ipv4ll *ll, be32_t *address) {
>> be32_t addr;
>> + int r;
>> + int32_t random;
>>
>> assert(ll);
>> + assert(address);
>> + assert(ll->random_data);
>>
>> - if (ll->address) {
>> - do {
>> - uint32_t r = random_u32() & 0x0000FFFF;
>> - addr = htonl(IPV4LL_NETWORK | r);
>> - } while (addr == ll->address ||
>> - (ntohl(addr) & IPV4LL_NETMASK) != IPV4LL_NETWORK ||
>> - (ntohl(addr) & 0x0000FF00) == 0x0000 ||
>> - (ntohl(addr) & 0x0000FF00) == 0xFF00);
>> - } else {
>> - uint32_t a = 1;
>> - int i;
>> -
>> - for (i = 0; i < ETH_ALEN; i++)
>> - a += ll->mac_addr.ether_addr_octet[i]*i;
>> - a = (a % 0xFE00) + 0x0100;
>> - addr = htonl(IPV4LL_NETWORK | (uint32_t) a);
>> - }
>> + do {
>> + r = random_r(ll->random_data, &random);
>> + if (r < 0)
>> + return r;
>> + addr = htonl((random & 0x0000FFFF) | IPV4LL_NETWORK);
>> + } while (addr == ll->address ||
>> + (ntohl(addr) & IPV4LL_NETMASK) != IPV4LL_NETWORK ||
>> + (ntohl(addr) & 0x0000FF00) == 0x0000 ||
>> + (ntohl(addr) & 0x0000FF00) == 0xFF00);
>>
>> - return addr;
>> + *address = addr;
>> + return 0;
>> }
>>
>> static int ipv4ll_timer(sd_event_source *s, uint64_t usec, void *userdata) {
>> @@ -306,7 +305,9 @@ static void ipv4ll_run_state_machine(sd_ipv4ll *ll, IPv4LLTrigger trigger, void
>> ll->claimed_address = 0;
>>
>> /* Pick a new address */
>> - ll->address = ipv4ll_pick_address(ll);
>> + r = ipv4ll_pick_address(ll, &ll->address);
>> + if (r < 0)
>> + goto out;
>> ll->conflict++;
>> ll->defend_window = 0;
>> ipv4ll_set_state(ll, IPV4LL_STATE_WAITING_PROBE, 1);
>> @@ -435,6 +436,36 @@ int sd_ipv4ll_get_address(sd_ipv4ll *ll, struct in_addr *address){
>> return 0;
>> }
>>
>> +int sd_ipv4ll_set_address_seed (sd_ipv4ll *ll, uint64_t entropy) {
>> + int r;
>> +
>> + assert_return(ll, -EINVAL);
>> +
>> + free(ll->random_data);
>> + free(ll->random_data_state);
>> +
>> + ll->random_data = new0(struct random_data, 1);
>> + ll->random_data_state = new0(char, 128);
>> +
>> + if (!ll->random_data || !ll->random_data_state) {
>> + r = -ENOMEM;
>> + goto error;
>> + }
>> +
>> + r = initstate_r((unsigned int)entropy, ll->random_data_state, 128, ll->random_data);
>> + if (r < 0)
>> + goto error;
>> +
>> +error:
>> + if (r < 0){
>> + free(ll->random_data);
>> + free(ll->random_data_state);
>> + ll->random_data = NULL;
>> + ll->random_data_state = NULL;
>> + }
>> + return r;
>> +}
>> +
>> int sd_ipv4ll_start (sd_ipv4ll *ll) {
>> int r;
>>
>> @@ -453,8 +484,17 @@ int sd_ipv4ll_start (sd_ipv4ll *ll) {
>> ll->defend_window = 0;
>> ll->claimed_address = 0;
>>
>> - if (ll->address == 0)
>> - ll->address = ipv4ll_pick_address(ll);
>> + if (!ll->random_data) {
>> + r = sd_ipv4ll_set_address_seed(ll, random_u64());
>> + if (r < 0)
>> + goto out;
>> + }
>> +
>> + if (ll->address == 0) {
>> + r = ipv4ll_pick_address(ll, &ll->address);
>> + if (r < 0)
>> + goto out;
>> + }
>>
>> ipv4ll_set_state (ll, IPV4LL_STATE_INIT, 1);
>>
>> @@ -493,6 +533,8 @@ void sd_ipv4ll_free (sd_ipv4ll *ll) {
>> sd_ipv4ll_stop(ll);
>> sd_ipv4ll_detach_event(ll);
>>
>> + free(ll->random_data);
>> + free(ll->random_data_state);
>> free(ll);
>> }
>>
>> diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
>> index fdc351f..7a589a5 100644
>> --- a/src/network/networkd-link.c
>> +++ b/src/network/networkd-link.c
>> @@ -72,6 +72,8 @@ int link_new(Manager *manager, struct udev_device *device, Link **ret) {
>> if (r < 0)
>> return r;
>>
>> + link->udev_device = udev_device_ref(device);
>> +
>> *ret = link;
>> link = NULL;
>>
>> @@ -94,6 +96,8 @@ void link_free(Link *link) {
>> free(link->ifname);
>> free(link->state_file);
>>
>> + udev_device_unref(link->udev_device);
>> +
>> free(link);
>> }
>>
>> @@ -893,10 +897,16 @@ static int link_acquire_conf(Link *link) {
>>
>> if (link->network->ipv4ll) {
>> if (!link->ipv4ll) {
>> + uint64_t seed = 0;
>> r = sd_ipv4ll_new(&link->ipv4ll);
>> if (r < 0)
>> return r;
>> -
>> + r = net_get_unique_predictable_data(link->udev_device, &seed);
>> + if (r >= 0) {
>> + r = sd_ipv4ll_set_address_seed(link->ipv4ll, seed);
>> + if (r < 0)
>> + return r;
>> + }
>
> I thought a bit more about this case, and I think we should try a bit
> harder here to get a predictable seed.
>
> The situation I have in mind is in the container, where we will not
> have any udev properties available, so
> net_get_unique_predictable_data() will always fail. However, we may
> still have a predictable mac address. Or it could be random of course,
> we don't know, but at this point the alternative is random data
> anyway, so might as well use the mac address. At least for the devices
> we create ourselves from nspawn the address will be a predictable one.
>
> So I'd suggest replacing the if block above with
>
> if (r < 0) {
> siphash24((uint64_t*)&seed, ETH_ALEN,
> &link->mac.ether_addr_octet, HASH_KEY.bytes)
> }
Actually, as the mac address is available also inside the library,
maybe best to simply put this logic inside the library so the users
don't have to care. I.e., leave the caller in networkd as you have it
now, just replace the use of random_u32() when the seed is not set
inside the library with a hash of the mac. What do you think?
> (not tested)
>
> [You could then add back the requirement to sd_ipv4ll_start() that
> set_address_seed() must have been called, and drop the call with
> random data from within the library (as that would be dead code now).]
>
>> r = sd_ipv4ll_attach_event(link->ipv4ll, NULL, 0);
>> if (r < 0)
>> return r;
>> diff --git a/src/network/networkd.h b/src/network/networkd.h
>> index 0c01719..b998b62 100644
>> --- a/src/network/networkd.h
>> +++ b/src/network/networkd.h
>> @@ -196,6 +196,7 @@ struct Link {
>> char *ifname;
>> char *state_file;
>> struct ether_addr mac;
>> + struct udev_device *udev_device;
>>
>> unsigned flags;
>>
>> diff --git a/src/shared/net-util.c b/src/shared/net-util.c
>> index 50cfa2c..0b96032 100644
>> --- a/src/shared/net-util.c
>> +++ b/src/shared/net-util.c
>> @@ -24,6 +24,9 @@
>> #include <arpa/inet.h>
>> #include <fnmatch.h>
>>
>> +#include "strv.h"
>> +#include "siphash24.h"
>> +#include "libudev-private.h"
>> #include "net-util.h"
>> #include "log.h"
>> #include "utf8.h"
>> @@ -31,6 +34,42 @@
>> #include "conf-parser.h"
>> #include "condition.h"
>>
>> +#define HASH_KEY SD_ID128_MAKE(d3,1e,48,fa,90,fe,4b,4c,9d,af,d5,d7,a1,b1,2e,8a)
>> +
>> +int net_get_unique_predictable_data(struct udev_device *device, uint64_t *result) {
>> + size_t l, sz = 0;
>> + const char *name, *field = NULL;
>> + int r;
>> + uint8_t *v;
>> +
>> + /* fetch some persistent data unique (on this machine) to this device */
>> + FOREACH_STRING(field, "ID_NET_NAME_ONBOARD", "ID_NET_NAME_SLOT", "ID_NET_NAME_PATH", "ID_NET_NAME_MAC") {
>> + name = udev_device_get_property_value(device, field);
>> + if (name)
>> + break;
>> + }
>> +
>> + if (!name)
>> + return -ENOENT;
>> +
>> + l = strlen(name);
>> + sz = sizeof(sd_id128_t) + l;
>> + v = alloca(sz);
>> +
>> + /* fetch some persistent data unique to this machine */
>> + r = sd_id128_get_machine((sd_id128_t*) v);
>> + if (r < 0)
>> + return r;
>> + memcpy(v + sizeof(sd_id128_t), name, l);
>> +
>> + /* Let's hash the machine ID plus the device name. We
>> + * use a fixed, but originally randomly created hash
>> + * key here. */
>> + siphash24(result, v, sz, HASH_KEY.bytes);
>> +
>> + return 0;
>> +}
>> +
>> bool net_match_config(const struct ether_addr *match_mac,
>> const char *match_path,
>> const char *match_driver,
>> diff --git a/src/shared/net-util.h b/src/shared/net-util.h
>> index 99479e1..01a6b72 100644
>> --- a/src/shared/net-util.h
>> +++ b/src/shared/net-util.h
>> @@ -62,3 +62,5 @@ int config_parse_ifalias(const char *unit, const char *filename, unsigned line,
>> int ltype, const char *rvalue, void *data, void *userdata);
>>
>> int net_parse_inaddr(const char *address, unsigned char *family, void *dst);
>> +
>> +int net_get_unique_predictable_data(struct udev_device *device, uint64_t *result);
>> diff --git a/src/systemd/sd-ipv4ll.h b/src/systemd/sd-ipv4ll.h
>> index 0207006..2397d43 100644
>> --- a/src/systemd/sd-ipv4ll.h
>> +++ b/src/systemd/sd-ipv4ll.h
>> @@ -42,6 +42,7 @@ int sd_ipv4ll_get_address(sd_ipv4ll *ll, struct in_addr *address);
>> int sd_ipv4ll_set_callback(sd_ipv4ll *ll, sd_ipv4ll_cb_t cb, void *userdata);
>> int sd_ipv4ll_set_mac(sd_ipv4ll *ll, const struct ether_addr *addr);
>> int sd_ipv4ll_set_index(sd_ipv4ll *ll, int interface_index);
>> +int sd_ipv4ll_set_address_seed (sd_ipv4ll *ll, uint64_t entropy);
>> int sd_ipv4ll_start (sd_ipv4ll *ll);
>> int sd_ipv4ll_stop (sd_ipv4ll *ll);
>> void sd_ipv4ll_free (sd_ipv4ll *ll);
>> diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
>> index d3f1aff..7049181 100644
>> --- a/src/udev/net/link-config.c
>> +++ b/src/udev/net/link-config.c
>> @@ -303,36 +303,11 @@ static int get_mac(struct udev_device *device, bool want_random, struct ether_ad
>> if (want_random)
>> random_bytes(mac->ether_addr_octet, ETH_ALEN);
>> else {
>> - const char *name;
>> uint8_t result[8];
>> - size_t l, sz;
>> - uint8_t *v;
>> -
>> - /* fetch some persistent data unique (on this machine) to this device */
>> - name = udev_device_get_property_value(device, "ID_NET_NAME_ONBOARD");
>> - if (!name) {
>> - name = udev_device_get_property_value(device, "ID_NET_NAME_SLOT");
>> - if (!name) {
>> - name = udev_device_get_property_value(device, "ID_NET_NAME_PATH");
>> - if (!name)
>> - return -ENOENT;
>> - }
>> - }
>>
>> - l = strlen(name);
>> - sz = sizeof(sd_id128_t) + l;
>> - v = alloca(sz);
>> -
>> - /* fetch some persistent data unique to this machine */
>> - r = sd_id128_get_machine((sd_id128_t*) v);
>> + r = net_get_unique_predictable_data(device, (uint64_t*)result);
>> if (r < 0)
>> return r;
>> - memcpy(v + sizeof(sd_id128_t), name, l);
>> -
>> - /* Let's hash the machine ID plus the device name. We
>> - * use a fixed, but originally randomly created hash
>> - * key here. */
>> - siphash24(result, v, sz, HASH_KEY.bytes);
>>
>> assert_cc(ETH_ALEN <= sizeof(result));
>> memcpy(mac->ether_addr_octet, result, ETH_ALEN);
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> systemd-devel mailing list
>> systemd-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list