Bill Fischofer wrote:
> On Fri, May 12, 2017 at 6:27 AM, Peltonen, Janne (Nokia - FI/Espoo)
> <[email protected]> wrote:
> > Dmitry Eremin-Solenikov wrote:
> >> Another generic question regarding ODP. If the package gets passed to
> >> IPsec API, should I trust e.g. IP header values? IOW, can I assume, that
> >> ip->tot_len + l3_offset is equal to odp_packet_len(), or is that an error?
> >
> > Good question.
> >
> > I think there are several somewhat different cases that may merit
> > a bit different handling. In any case some clarification is needed
> > in the API spec.
> >
> > I-1) Inbound: IP length < packet length - L3 offset
> >
> > Received packets may contain some L2 data at the end (e.g. to pad
> > Ethernet frames to the minimum frame length).
>
> One of the advantages of using the ODP classifier is that the ODP
> parser is assumed to validate that the packet is well formed and to
> flag any that are not as errors. So if the error bits are clean coming
> out of classification you can assume that you're dealing with a
> well-formed packet. If you bypass the classifier, it's the
> application's responsibility to validate the packet structure.
>
> In the case of IPsec, the range of bytes over which encrypt/decrypt
> operations occurs is per the RFCs (the IP length). Any additional L2
> bytes are ignored and not propagated to the output. Remember that the
> range of operation for IPsec may not be the entire packet due to
> tunnel modes. See section 5 of RFC 4301 for details.
I am not sure if you agree or disagree that ODP should just ignore
the extra packet data and process the packet successfully in this case.
>
> >
> > I think it would make most sense to require ODP implementations to
> > discard the extra packet data (i.e. truncate the packet) before any
> > IPsec processing and then perform IPsec decapsulation normally.
> >
> > I do not think it would be very useful to require the application to
> > truncate the packet to the correct length since the ODP implementation
> > must anyway be able to handle the extra packet data in the inbound
> > inlined offload case.
> >
> > I-2) Inbound: IP length > packet length - L3 offset
> >
> > I think ODP should return an error (which one?) through the operation
> > status. Specifying undefined behavior may not be very useful since the
> > ODP implementation must anyway be able to gracefully handle this case
> > for inline processed packets.
>
> Various length underruns/overruns are classic attack vectors and are
> the reasons why parsers must validate packet structure. An inner
> section whose purported length is larger than its containing structure
> is always an error.
I think the API should say if such input results in undefined
behavior (e.g. crash) or an error report through the op status.
> If it is less than its containing structure this
> may or may not be a problem as padding bytes are generally benign,
> though can also be used as a covert channel.
>
> >
> > O-1) Outbound: IP length < packet length - L3 offset
> >
> > One possibility would be to discard the extra packet data at the end
> > before IPsec processing, just like in the inbound case. But here we
> > could also consider treating the extra packet data as TFC padding that
> > gets encrypted along with the actual payload (but that might be more
> > complex, especially with fragmentation offload).
> >
> > O-2) Outbound: IP length > packet length - L3 offset
> >
> > I think ODP should return an error in the op status as in the inbound
> > case, but I suppose undefined behavior could be considered too.
>
> For output since the application is supplying the packet it's the
> application's responsibility to ensure that output packets are well
> formed. Results are undefined if this is not the case.
The current API spec does not say the packet needs to be well formed,
so the API spec needs improvement if that is what is desired. And the
meaning of "well formed" should be made clear enough. Extra packet data
after the end of the IP packet does not make the IP packet ill-formed
and I think it would be reasonable to allow it in the API.
Janne
