[RP-PPPoE] pppoe-server efficiency (was Re: IPv6 support)

[Insane Laughing Clown]

On 07/30/2013 12:18 PM, David F. Skoll wrote:
> On Tue, 30 Jul 2013 12:07:20 -0700
> Insane Laughing Clown<mike-rppppoe at tiedyenetworks.com>  wrote:
>
>> There's some general design issues. For example, a bad client or herd
>> of them all shitting out padi can overrun the server since there's no
>> sanity checking on the number of outstanding requests in flight.
>
> A PADI does not allocate any server-side resources.  The design of
> PPPoE is such that there is no way for a PPPoE server to prevent this
> attack; senders can fake their MAC addresses and blast PADIs as fast
> as they like.  The only countermeasure is not to allocate resources
> in response to a PADI.
>

I didn't say it allocated server-side issues, I just said it can get 
flooded. Your assessment is otherwise correct, it's just one of the 
avenues by which it can and does get hammered.

> By the time you get a PADR, you can be reasonably sure you are speaking
> to a real client because it had to have received a cookie.  PADR-time
> is when you can limit outstanding requests.

A client can jump into any state it wants to and frequently these 
fucking bullshit walmart brand home routers do the weirdest stuff you 
can imagine.

>
>> PADT processing pays the lookup performance penalty every time one
>> is received wether it's valid or not.
>
> No, that's not true.  Check the source code:
>
>      /* Get session's index */
>      i = ntohs(packet->session) - 1 - SessOffset;
>      if (i>= NumSessionSlots) return;
>      if (Sessions[i].sess != packet->session) {
> 	syslog(LOG_ERR, "Session index %u doesn't match session number %u",
> 	       (unsigned int) i, (unsigned int) ntohs(packet->session));
> 	return;
>      }
>
>      /* If source MAC does not match, do not kill session */
>      if (memcmp(packet->ethHdr.h_source, Sessions[i].eth, ETH_ALEN)) {
> 	syslog(LOG_WARNING, "PADT for session %u received from "... boring stuff removed ...);
> 	return;
>      }
>
> Those are constant-time operations.

I admit, I am wrong, I am not sure why I flagged that case but it's as 
you said.



>
>> PADR isn't limited in any way and
>> bad clients can flood the server causing unlimited numbers of pppd's
>> to be created until the tables are full.
>
> That's where the -x option would come in handy and I agree... it needs
> to be made more efficient.  All that's needed is to make
> count_sessions_from_mac more efficient and to use a doubly-linked list
> for quick deletion of busy sessions.
>

How would you implement that for the case where -x is greater than 1?


>> Personally, there should be both numerical limits on the number of
>> sessions as well as the rate as which any given client is allowed to
>> create or terminate sessions and the only way to do that would be
>> with a hash table.
>
> Yes, you could do that if you were ambitious. :)  From the man page:
>
>         Note that pppoe-server is meant mainly for testing PPPoE  clients.   It
>         is not a high-performance server meant for production use.
>
> :) That's my get-out-of-jail-free card. :)
>
>

Its fine code and the above is certainly accepted, I was just pointing 
out what I think are issues for larger scaled deployments.

-OLC