On Tue, Mar 05, 2024 at 05:14:46PM -0800, Jacob Champion wrote:
> On Tue, Mar 5, 2024 at 1:51 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> I don't have a strong opinion about making this configurable, but I do
>> think we should consider making this a hash table so that we can set
>> MAX_CONN_RECORDS much higher.
>
> I'm curious why? It seems like the higher you make MAX_CONN_RECORDS,
> the easier it is to put off the brute-force protection. (My assumption
> is that anyone mounting a serious attack is not going to be doing this
> from their own computer; they'll be doing it from many devices they
> don't own -- a botnet, or a series of proxies, or something.)
Assuming you are referring to the case where we run out of free slots in
acr_array, I'm not sure I see that as desirable behavior. Once you run out
of slots, all failed authentication attempts are now subject to the maximum
delay, which is arguably a denial-of-service scenario, albeit not a
particularly worrisome one.
I also think the linear search approach artifically constrains the value of
MAX_CONN_RECORDS, so even if a user wanted to bump it up substantially for
their own build, they'd potentially begin noticing the effects of the O(n)
behavior. AFAICT this is really the only reason this value is set so low
at the moment, as I don't think the memory usage or code complexity of a
hash table with thousands of slots would be too bad. In fact, it might
even be simpler to use hash_search() instead of hard-coding linear searches
in multiple places.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com