[systemd-devel] [PATCH] V2 sd-ipv4ll: generate predictable addresses
Tom Gundersen
teg at jklm.no
Fri Mar 21 03:45:39 PDT 2014
On Fri, Mar 21, 2014 at 8:46 AM, Umut Tezduyar Lindskog
<umut at tezduyar.com> wrote:
> On Thu, Mar 20, 2014 at 7:16 PM, Tom Gundersen <teg at jklm.no> wrote:
>> 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?
>
> I am fine with it but we also need a solution in case there is an
> address collision and we need to pick up a new address. I don't think
> we can get rid of random_32().
Yeah, I didn't mean to use the mac address directly, only as the seed
for the address generation in case a seed has not been set externally.
> I have also been thinking and I am having yet another propose
> (inspired by the discussion). We have to have a unique MAC for ipv4ll
> to work anyways and without setting MAC, ipv4ll will not start. How
> about as soon as MAC is set, we siphash it and then initialize
> initstate_r in the library. This will drop the need of
> sd_ipv4ll_set_address_seed API and I think simplify things a lot.
I think we should still support the sd_ipv4ll_set_address_seed() API,
but fall back to using the mac address as you outline.
The reason is that we don't only want a unique seed (which the MAC
address must be), but if at all possible we want one which is
persistent between reboots. MAC addresses are not guaranteed to be
persistent, so I think we should only rely on them as a fallback.
Cheers,
Tom
More information about the systemd-devel
mailing list