[kea-dev] Call for comments about the Decline design

[Tomek Mrugalski]

On 13.07.2015 17:41, Marcin Siodelski wrote:
> I had a quick look into the document. I will go review it more
> extensively, but for now I want to mostly focus on one general issue.
> 
> I would like to discuss the issue of using the special HW address or
> DUID to mark the lease as declined. I think that there are some flaws
> which should at least be documented. When the address gets declined for
> the certain client and the lease is updated to modify the HW address or
> DUID, you effectively loose the information who has declined this
> address. Sure, you can take it out from the logs, assuming that your
> logging level is set to the threshold when it logs such a message. But,
> you can't associate a declined lease with the client who actually
> declined it by looking into the lease database.
Ok, so let's assume for a second that an extra column is implemented
that governs whether a lease is declined or not. The declined lease is
no longer associated with a client that declined it. So what would you
put in the (mandatory) hwaddr or duid fields?

DUID is mandatory for all leases. It would be a lot of work to go
through the existing code and pick all the places that expect the lease
to have DUID. The same is true for hwaddr/client-id in v4.

> Another thing is that that it may be faster for the SQL databases to
> lookup leases in the database using a numeric value, rather than DUID.
> So, if you want to query for all declined leases, you could collect all
> expired leases by a "declined" flag rather than by the varbinary value.
This sort of query would happen only when processing
declined-addresses-list command. In my opinion it doesn't make sense at
all to optimize for something that happens very infrequently.

> I accept the argument that there will not be many declined leases,
> comparing to undeclined ones, so it may be premature to optimize
> queries, but one never knows if such optimizations will not be needed in
> the future. Then querying for declined leases by DUID may present some
> performance degradation.
In the very unlikely event that such optimizations are necessary, we
could add extra indexes to make such queries faster. But I'm pretty sure
that will never be needed.

> At this point however, I am mostly concerned about the representation of
> the declined leases in the database, which would make troubleshooting
> harder than if you had an additional field which value of 1 would
> indicate declined, and 0 undeclined, and didn't modify the client
> identifier.
On the contrary. If you add extra field, you bring in the following
additional tasks:
1. update schema for MySQL
2. update schema for Postgres
3. update memfile format
4. develop upgrade procedure for MySQL
5. develop upgrade procedure for Postgres
6. develop upgrade procedure for memfile
7. add extra checks to all existing reuseLeases code instances and write
extra checks for the new field.
8. existing code assumes in many places that lease6 always has DUID and
lease4 always has either hwaddr or client-id set. Neither would be true
anymore after that change. We'd have to go through all existing code and
add many sanity checks.

If we decide to go that way, here's what will happen. We decide that due
to time constraints we immediately drop tasks 2 and 5. We claim the
intention to do 1 and 4, but once the 1.0 deadline approaches, we'd
postpone them, too. They would be added to the infamous technical debt
that somehow tends to grow and does not want to shrink.

But that's not the biggest objection. The biggest problem is that once
the client declines an address, logically the client is not associated
with the lease anymore. Claiming that we should keep client's details,
it's sort of like wanting to keep part of the lease history in the lease
structure. We don't do such things. The information about who declined
the lease is kept in logs.

If you are concerned that it may be used as a resource exhaustion attack
vector, don't worry. There are similar attacks that can be performed
more easily. For the sake of DHCP security, I won't discuss them on a
public mailing list, though :) On a related note, we may consider adding
a per host statistic for the number of declined addresses. As we don't
have contexts support in statistics manager, I think this is out of
scope for 1.0. But one day we could develop it. There may be other
useful per host statistics that we may want to gather.

> I think some discussion in this respect would be useful in the document.
Agree. I'll put something in the doc tomorrow, but the majority of this
text belongs to the User's Guide.

> Though, if someone has already implemented the wrapper around database
> to use the DUID-based queries, the migration may cause him heart attack.
I tend to think of the database changes as the last resort and pick the
alternatives, if possible. Database schema change brings in unmeasurable
pain to unspecified number of deployments. We are hoping that number is
reasonably big. Also, our upgrade scripts are... less than perfect, so I
would tend to avoid using them. Finally, changing the database schema
means that we're effectively increasing the gap between memfile, MySQL
and Postgres, as we never have time to do things properly for Postgres.

Tomek