Subject: Re: Bugs in PF_KEY marshalling, socket-buffer overflow
To: Michael Richardson <mcr@sandelman.ottawa.on.ca>
From: Jonathan Stone <jonathan@dsg.stanford.edu>
List: tech-net
Date: 05/19/2004 21:01:17
In message <18785.1085017753@marajade.sandelman.ottawa.on.ca>Michael Richardson
 writes

>  This discussion is interesting...
>  Linux IPsec (FreeS/WAN, Openswan)


Oh well. Its your time, so you can waste it flogging a dead horse if
you wish.  One might ask, why, if you choose to spend your time on a
B-grade codebase ( /proc? Since when were IPsec messages processes?
4K limits?  Spare us!), you come here to discuss the limitations of
that codebase.

> has a similiar problem with ACQUIREs
>they are not reliable under memory exhaustion. To solve this problem,
>one must scan a /proc system, which has a 4k page problem.

I dont get that. How is grabbing pages at random any more reliable
than socket buffers (nevermind the 4k limit braindamage)?  Don't feel
obliged to answer that, though: I may not want to hear the answer.

>  The plan to fix things is to have the keying deamon send requests down
>to the kernel that would get returned with ACQUIRE's. If one can't
>allocate an available ACQUIRE, the packet that caused it would get
>dropped. 

Seems reasonable so far. The IKE daemon can decide (or be configured)
on the limits it expects to handle; request and pin pages appropriately;
then hand them down to the kernel.  The kernel then guarantees at least
that much buffering for ACQUIREs.  Sounds OK to me.


>  Basically, unreliable PF_KEY is a bad idea. 

Yep.  Remember: rough consensus and working code.

If ACQUIREs aren't expected to work reliably, then IPsec cannot be
expected to work reliably. (I encourage you to quote that in the IEFT
IPSEC WG, should RFC-2367 ever come up). And I for one refuse to
beleive anyone from the KAME project would say that KAME IPsec is
not expectd to work reliably.

And for the record, RFC-2367 was never more than Informational; see
RFC-2026 for what that means.


>  The idea of making it routing-socket like (with the broadcast
>property) was a bad idea. Get rid of it.


In this context (kernel-generated ACQUIREs) that was never actually
required.  Maybe its time you re-read RFC-2367.  Kernel-generated
messages are ``special'', and they need not be broadcast to every
listener:

   Some messages are generated by the operating system to indicate that
   actions need to be taken, and are not necessarily in response to any
   message sent down by the user.  Such messages are not received by all
   PF_KEY sockets, but by sockets which have indicated that kernel-
   originated messages are to be received.  These messages are special
   because of the expected frequency at which they will occur. [...]

That seems pretty plain to me. kernel-generated ACQUIREs are special,
and should be handled differently than normal, unreliable, broadcast
PF_KEY messages.


>  So, the acquires should get dropped as a unit, not part of them.

You mean each ACQUIRE should be either received in its entirety, or
dropped in its entireity?  Sure, doesn't that go without saying?
RFC-2367, sec 1.4, page 7, last paragraph, is quite unambiguous on
that point:

    [...]  an implementation MUST check for
    a properly formed message before returning it to the appropriate
    listeners.

The context is checking user-supplied messages, but it applies equally
well to kernel-generated messages, and to socket-buffer limits causing
truncation of a message. If socket-buffer limits cause partial
ACQUIREs to be passed up by th kernel, that's just broken.
(Tho' I still suspect something in my private tree may be
responsible for that one).