Re: [RFC] ipv6: use a random ifid for headerless devices

[Hannes Frederic Sowa <hannes@stressinduktion.org>]

Hello,

On 08.12.2015 19:57, Bjørn Mork wrote:
> Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
>> On 05.12.2015 20:02, Bjørn Mork wrote:
>>> Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
>>>> On Thu, Dec 3, 2015, at 20:29, Bjørn Mork wrote:
>>>>
>>>>> After looking more at addrconf, I started wondering if we couldn't abuse
>>>>> ipv6_generate_stable_address() for this purpose?  We could add a new
>>>>> addr_gen_mode which would trigger automatic generation of a secret if
>>>>> stable_secret is uninitialized.  This would be good enough to ensure
>>>>> stability until the interface is destroyed.  And it would still allow
>>>>> the adminstrator to select IN6_ADDR_GEN_MODE_STABLE_PRIVACY by entering
>>>>> a new secret.
>>>>
>>>> I am fine with your proposal but I would really like to see it only
>>>> happen on the per-interface stable_secret instance.
>>>
>>> Do you think something like the patch below will be OK?
>>
>> I wouldn't call it IN6_ADDR_GEN_MODE_AUTO, this doesn't say anything.
>> But the idea is already good.
> 
> No, I didn't like that name either.  I just couldn't come up with
> anything descriptive, short and non-redundant. "random", "generated",
> "stable" are even worse.  And that's about where my imagination ended.
> We need a child here :)

Sorry for answering so late...

What do you think about simply using IN6_ADDR_GEN_MODE_RANDOM?

>>> Or would it be better to drop the additional mode and just generate a
>>> random secret if the mode is IN6_ADDR_GEN_MODE_STABLE_PRIVACY and the
>>> secrets are missing?  Or would that be changing the userspace ABI?  This
>>> is not clear to me...
>>
>> I would not like to do that somehow. The problem is that the stable
>> secrets get written by user space probably during boot-up, but we don't
>> know when. That's why I would also not set the ->initialized flag, so
>> user can overwrite it to the final secret later on. We block it otherwise.
> 
> I am not sure I follow...  There is nothing preventing userspace from
> initializing the secret before or after generation of the random secret.

I actually missed that. Shortly before sending the patch I decided to
allow to reinitialize the stable_secret. Before I had a check in there
to not being able to rewrite the stable_secret after it became
initialized. So we are good here. Sorry for the confusion.

> Writing to /proc/sys/net/ipv6/conf/<iface>/stable_secret will update the
> secret and set the mode to IN6_ADDR_GEN_MODE_STABLE_PRIVACY as before,
> even if we have generated a random secret first.  I have verified that
> this part works as expected.

Thanks!

> I guess we should check &net->ipv6.devconf_dflt->stable_secret too
> before choosing the default mode.  IN6_ADDR_GEN_MODE_STABLE_PRIVACY is a
> more approproate default if a default secret is set.  IMHO, this should
> really be the case without the proposed change too, but it isn't. The
> current behaviour confuses me: Setting 'default' changes all existing
> interfaces, but does not change the default for new interfaces. Is that
> right?

Nope, that is a good point. I think we should do that unconditionally.
If we have a stable secret set, which we can use, we always should use
this address generation mode. Can you send the addition of this as a
separate patch so we can propose it for stable? Otherwise I can do that,
too.

>> My proposal would be to use the stable privacy generator in case the
>> device does not have a device address for EUI-48 generation with a
>> secret which we simply generate on the stack. Let's factor out the part
>> of the generator which depends on the inet6_dev and cnf bits for that.
> 
> Not sure I get this part either.  The point was to have stable addresses
> for the lifetime of the netdev.  We can generate the secret on the
> stack, but we will still need to stash it somewhere.  That could of
> course be to a new field.  But I don't see the point since there is no
> way you can combine this mode with IN6_ADDR_GEN_MODE_STABLE_PRIVACY.
> Only one mode can be active at, and that mode can then own the secret.

Ok, your argument makes sense.

> As long as we can manage to introduce this without changing any existing
> behaviour, of course.

Besides the naming I think your patch looks fine.

Thank you,
Hannes