[RP-PPPoE] Unexpected connection termination - stray PADT

[Dianne Skoll]

On 2020-04-30 09:36, David Balažic wrote:

> It seems (as I am not a kernel hacker) OpenWRT uses the kernel pppoe module.
> That is the file linux-4.14.171/drivers/net/ppp/pppoe.c if I'm not mistaken.

> It seems, it does not check the DST addr of the received PADT packet
> (it checks src addr and session id for match).

Yes, that's how I read it too.

> What is the angle from the standards point of view? Should/must the
> dst addr be checked?

I would say yes, although the standard does not explicitly spell this out.
It says this:

   This packet may be sent anytime after a session is established to
   indicate that a PPPoE session has been terminated.  It may be sent by
   either the Host or the Access Concentrator.  The DESTINATION_ADDR
   field is a unicast Ethernet address, the CODE field is set to 0xa7
   and the SESSION_ID MUST be set to indicate which session is to be
   terminated.  No TAGs are required.

Since is says that the DESTINATION_ADDR field is a unicast Ethernet
address, I think that's highly suggestive that the receiver of a PADT
should ignore it if the destination address doesn't match its interface
address.  I cannot see any rationale for accepting a PADT that is *not*
directed towards our interface, because it may be for a completely
unrelated session.  The standard also says [Sec 4]:

   The SESSION_ID field is sixteen bits.  It is an unsigned value in
   network byte order.  It's value is defined below for Discovery
   packets.  The value is fixed for a given PPP session and, in fact,
   defines a PPP session along with the Ethernet SOURCE_ADDR and
   DESTINATION_ADDR.

> IOW: why does the rp-ppoe code do this? (following the standard, or
> fixing a user reported problem, something else?)

I did it that way because the standard says that all three of these
items are needed to define a session:  Session ID, SOURCE_ADDR *and*
DESTINATION_ADDR.  If the DESTINATION_ADDR does not match, then it's
by definition a different session.

Therefore, IMO, this is a bug in the Linux kernel.

Regards,

Dianne.