Re: [dtn] [EXTERNAL] Erik Kline's No Objection on draft-ietf-dtn-bpbis-29: (with COMMENT)

[Magnus Westerlund <[e-mail]>]

Hi Scott,

So the below indicates that you actualy want to allow something that IPv6
forbidds. And I do think you have reasons for allowing this. However, have you
read RFC 5722 and considered if there are attacks on BP nodes between source and
destination that could be fooled by manipulating the content of fragments. I
think this attack is not an issue assuming two things:

1. No on-path node will actually look into the data fragments. (This is clearly
not true on the Internet). 

2. That each on path node treat each BP fragment independently and don't create
state. Becasue if you create state from one fragment, that will affect the
processing of subsequent fragments then you could fool the network to believe
that you are doing one thing and do something else. 

So I think some more thought is needed here to ensure that no security issue is
created. 

Cheers

Magnus


On Mon, 2020-12-07 at 14:49 +0000, Burleigh, Scott C (US 312B) wrote:
> Hi, Magnus.  The ">" problem is new to me, but I guess I can use "[SBx]"
> instead.  As for "salient", sure, let's substitute "material".
> 
> On reassembly: dropping the whole bundle if any fragment overlaps with
> previously received fragments is not at all what we want to do.  Fragment
> overlaps are not anomalous in any way, as it is entirely nominal for multiple
> copies of a bundle to be concurrently taking different paths to the
> destination (this is how most terrestrial DTN forwarding works) and the
> fragmentation that occurs on one path might be wildly different from that
> which occurs on another path.
> 
> The only thing we fragment is the payload, which should be immutable from
> source to destination, so the order of superposition of overlaps should be
> irrelevant.  If it is not -- that is, if you get different results from
> reassembling in different ways -- then one or more fragments have been
> corrupted (i.e., changed) and the effect is the same as if the payload had
> been corrupted in any other way.  The CRC or BIB on the payload block should
> report the transmission failure and the application should act accordingly. 
> 
> I believe that the way I wrote this up is the way we want it to work.
> 
> Scott
> 
> -----Original Message-----
> From: Magnus Westerlund <magnus.westerlund=[e-mail]> 
> Sent: Monday, December 7, 2020 1:48 AM
> To: [e-mail]; Burleigh, Scott C (US 312B) <
> [e-mail]>; [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail]; 
> [e-mail]
> Subject: Re: [EXTERNAL] Erik Kline's No Objection on draft-ietf-dtn-bpbis-29:
> (with COMMENT)
> 
> Hi Scott,
> 
> WG, please pay attention here as there are text proposals. 
> 
> Some comments inline
> 
> Scott, I would note that your usage of > to mark the lines you added likely
> have
> the reverse effect that you intended in a number of email software, including
> Evolution. It is interpreted as a quote from previous email. Which has the
> interesting effect that the text I should read was greyed while what was
> quoted
> is black. 
> 
> 
> On Fri, 2020-12-04 at 21:48 +0000, Burleigh, Scott C (US 312B) wrote:
> > Hi, Erik.  Some thoughts on your comments, in-line below.
> > 
> > Scott
> > 
> > -----Original Message-----
> > From: Erik Kline via Datatracker <[e-mail]> 
> > Sent: Wednesday, December 2, 2020 5:36 PM
> > To: The IESG <[e-mail]>
> > Cc: [e-mail]; [e-mail]; [e-mail]; Fred
> > Templin <[e-mail]>; [e-mail]
> > Subject: [EXTERNAL] Erik Kline's No Objection on draft-ietf-dtn-bpbis-29:
> > (with COMMENT)
> > 
> > Erik Kline has entered the following ballot position for
> > draft-ietf-dtn-bpbis-29: No Objection
> > 
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > I was going to ballot as Discuss, but I lack some context on the history
> > of this document and its evolution.  Nevertheless, hopefully the "discuss"
> > points below leave a record of things about which I had some concern.
> > 
> > 
> > [[ discuss ]]
> > 
> > [ section 4.1.5.1.2 ]
> > 
> > * The encoding considerations mentions "dtn" scheme but this is the "ipn"
> >   scheme section so...should this be "ipn"?
> > 
> > > 	Excellent catch.  Thank you.
> > 
> > [ section 5.2 ]
> > 
> > * Should step 2 be step 1 of 5.3 (not 5.4)?
> > 
> > > 	No, in the case of a locally sourced node we definitely DO NOT want to
> > > exclude the possibility of the node forwarding the bundle to itself (BP
> > > loopback).  The procedure described in 5.3 applies only to a bundle that
> > > has
> > > been forwarded by some node and has been received at the local node; the
> > > only entry into the procedure in 5.3 is from Step 5 of 5.6 (Bundle
> > > Reception).
> > > 	It could well be argued that the content of 5.3 should simply be
> > > inserted at the end of 5.6.  The current organization was retained from
> > > RFC5050, for continuity, but I would be fine with making that change. 
> > 
> > [ section 5.4.2 ]
> > 
> > * It seems to me that the behaviour in step 1, which I think does make some
> >   sense, pretty easily sets up the potential for ping-ponging (AIUI).
> > 
> >   If true, should there be some text (perhaps in 5.4) acknowledging that
> >   forwarding a given bundle to a node for the 2nd time might warrant, at
> > least,
> >   some reassessment of the routing/reachability state in the node?
> > 
> >   I fully understand that extensive routing discussion is out of scope
> >   for this document.
> > 
> > > 	The question, I think, is how worried we are about ping-ponging.  If the
> > > node that is performing 5.4.2 decides to send the bundle back to the
> > > Previous Node, and that node then decides to reforward it again in the
> > > same
> > > way (based on its current information about the network topology), it's
> > > possible that the second forwarding attempt at this node will succeed
> > > because of changes in configuration or connectivity.
> > > 	Obviously we can't tolerate infinite ping-ponging, but bundle lifetime
> > > and/or Hop Count limit will defend against that, even if routing
> > > implementations are simple-minded.  We've thought about this quite a bit,
> > > and I'd like to leave this text as is.
> > 
> > [ section 5.6 ]
> > 
> > * Does "cannot process" include "does not understand"?
> > 
> >   If so it might be good to use a different reason value from
> >   "Block unintelligible" so that some other node can understand that, for
> >   example, the CRC (if sent) was valid.
> > 
> >   Basically, consider differentiating between "understood but malformed" and
> >   "not understood".
> > 
> > > 	I agree.  I would propose to add a new "Block unsupported" reason code
> > > to Figure 4 and substitute "Block unsupported" for "Block unintelligible"
> > > in
> > > 5.6.
> > 
> > [ section 5.9 ]
> > 
> > * The mention of non-overlapping fragments brings a few issues to mind that
> >   have been encountered in IPv6 when considering fragment handling:
> > 
> >     [1] How are overlapping fragments to be handled?  Are they ignored
> >     and/or cause any specific error to be generated? (vis. IPv6: RFC 5722)
> > 
> > > 	A darn good point.  On reviewing this section I think it needs to be
> > > rewritten as follows:
> > 
> > Note that the bundle fragmentation procedure described in 5.8 above may
> > result
> > in the replacement of a single original bundle with an arbitrarily large
> > number of fragmentary bundles.  In order to be delivered at a destination
> > node, the original bundle's payload must be reassembled from the payloads of
> > those fragments.
> > 
> > The "salient extents" of a received fragment's payload are all continuous
> > sequences of bytes in that payload that do not overlap with the salient
> > extents of the payloads of any previously received fragments with the same
> > source node ID and creation timestamp.  If the concatenation – as informed
> > by
> > fragment offsets and payload lengths – of the salient extents of the
> > payloads
> > of this fragment and  all previously received fragments with the same source
> > node ID and creation timestamp as this fragment forms a continuous byte
> > array
> > whose length is equal to the total application data unit length noted in the
> > fragment’s primary block, then:
> > •	This byte array -- the reassembled application data unit -- MUST replace
> > the payload of that fragment whose salient extents include the extent at
> > offset zero.  Note that this will enable delivery of the reconstituted
> > original bundle as described in Step 1 of 5.7.
> > •	The "Reassembly pending" retention constraint MUST be removed from every
> > other fragment with the same source node ID and creation timestamp as this
> > fragment.
> > 
> > Note: reassembly of application data units from fragments occurs at the
> > nodes
> > that are members of destination endpoints as necessary; an application data
> > unit MAY also be reassembled at some other node on the path to the
> > destination.
> > 
> 
> I would like you to consider using another word than "salient". First of all I
> had to look its definition up, even if I did remember one of its meanings.
> Secondly, I don't know in this text if you want the "important" (adjective) or
> the "pointing outwards" or protruding meaning (adjective and noun). And I
> would
> note that pointing outwards appears to imply that it is angled in relation to
> a
> base which makes no sense in this context where we are talking about a byte
> array. 
> 
> I have trouble understanding if there are any specific aspects in BP that
> require you to have more than the rule in RFC 8200 to drop the whole bunle if
> any fragment overlaps with previous received fragments. There is the extra
> wrinkle of handling duplication of fragments and if you should track that to
> avoid a duplicated fragment as overlapping with already received. But beyond
> that I don't know if you have any additional considerations here? 
> 
> 
> Cheers
> 
> Magnus
> 
> >     [2] What about "atomic fragments", i.e. a fragment that starts at
> >     offset zero and has a total payload length equal to the actual length
> >     of the payload (i.e. a fragmented payload consisting of a single
> > fragment)?
> >     (vis. IPv6: RFC 6949)
> > 
> >     I'm guessing these should be silently accepted, but there might be some
> >     text saying they MUST/SHOULD NOT be generated.
> > 
> > > 	A conformant implementation will never create such a fragment, because
> > > of the definition of "fragmenting" in the second paragraph of 5.8: the
> > > length of one of the fragments would be 0 and the length of the other
> > > would
> > > be M, both invalid.  But we can fortify this constraint by changing "are"
> > > to
> > > "MUST be".  I think the fragment acceptance procedure in 5.9 would handle
> > > such a fragment correctly if a non-conformant implementation created one.
> > 
> > [[ comments ]]
> > 
> > [ section 4.2.2 ]
> > 
> > * The Creation Timestamp element description seems to copy a bunch of text
> >   from 4.1.7.  Might be able to shorten the document a bit by referring to
> >   4.1.7 for extended discussion of creation timestamp operations.
> > 
> > > 	Sure.
> > 
> > [[ questions ]]
> > 
> > [ section 4.3.3 ]
> > 
> > * What's the value of using a {limit, incrementing_value} pair like this
> > over
> >   just a single decrements_to_zero integer?  I'm not suggesting this be
> >   changed (it's probably too late anyway), but I was just curious as to the
> >   motivation.
> > 
> > > 	There was a very brief discussion of this in January of 2017.  Having
> > > the two fields does provide some additional information - if you only
> > > decrement hop limit to zero you have no idea what value it started out
> > > with,
> > > which might be important for some routers.  It wasn't a major point of
> > > contention and could have gone either way.
> > 
> > 
> >