Subject: [P2PSIP] Possible problem with the extension mechanism on RELOAD? (and other places lengths might help)

From: "David A. Bryan" <[email-masked]>
Date: Thu, 8 Jul 2010 10:58:59 -0400

So looking at the extension mechanism in 5.3.3...is there a reason
that the MessageExtension object (5.3.3) has no length? I may have
just missed something obvious, but it seems it needs to have one, if
you want to allow multiple extensions where intermediate peers might
not understand the opaque data. If for example we had two extensions,
a message (I'll use a join for this example) might look like:

Forwarding Header
Message Contents
 message_code
 message_body
   JoinReq
      joining_peer_id (NodeID)
      overlay_specific_data (variable length)
  extensions
      ....extension 1....
      ....extension 2....
Security Block

Let's take a hypothetical case where a peer receiving this understands
extension 2, but not extension 1. Even if extension 1 isn't critical
and it knows the type, since what follows is opaque data, if it
doesn't understand the extension, how does it know how long that block
of opaque data is before getting to extension 2?

Perhaps we need to change MessageExtension in 5.3.3. to include a
length prior to the opaque data?

While we're on this, there are several places where the parser seems
to need to know about overlay specific/generic data just to to parse.
The one above seems like it truly won't work without a length (since
we explicitly want to allow for non-critical extensions), but others,
for example the variable overlay_specific_data in some messages (for
example this join) seem to imply that while it will work, the low
level parser itself needs to know about the overlay specific data
(since this can vary by overlay, not implementation). A parser that
doesn't support that overlay parameter can't even parse the message (I
don't think it can even know where the security block starts, so it
technically can't even verify the signature, unless it reads backwards
from the end or something, correct?)

It seems like you need to have the extension-specific/overlay-specific
code inside the basic parser...or am I missing something? (i.e., you
can't write a generic parser that can parse any message, it needs to
have contextual awareness) If this was a deliberate design decision it
seems problematic. I personally don't really like the idea that a
parser needs to understand the overlay specific data just to parse the
message. Obviously you can do this with plug in code or something, but
then we are forcing a particular implementation/design in the design
of the protocol, and the extra bits (lengths) here and there that
would allow a parser to process a message without the extensions
simply aren't that heavy. It seems like a better design to allow a
basic RELOAD parser to parse any message, passing specific data up to
be decoded by higher levels.

David (as individual)

---

From: "David A. Bryan" <[email-masked]>
Date: Thu, 8 Jul 2010 15:34:12 -0400

So having a conversation with myself here as I think more about these
issues...

I take back what I said about the overlay specific data. This info is coming
from the config, without which a peer can't join anyway, and otherwise many
things (including peerids) will need lengths, so I think the overlay
specific data not having a size is fine as is, so long as the length is in
the config. Love to hear other thoughts on it, however.

Still think there is an issue with extension needing length, however...

Thanks,

David (as individual)

On Jul 8, 2010 10:58 AM, "David A. Bryan" <[email-masked]> wrote:

Forwarding Header
Message Contents
 message_code
 message_body
  JoinReq
     joining_peer_id (NodeID)

David (as individual)

---

From: Cullen Jennings <[email-masked]>
Date: Thu, 8 Jul 2010 14:11:16 -0600

Agree with what you are getting at here but I think the draft is already pretty much what you want and this is just a syntax confusion. 

When the spec has a typedef like 

     typedef opaque       ResourceId<0..2^8-1>;

This results in bits on the wire that look like an 8 bit integer as the length followed by that many byes of data. So any time we have a variably length element generated by the < > operators, it includes the length as well as the data. 

So with that in mind, looking at section 5.3.3, the 

   MessageExtensions      extensions<0..2^32-1>;

will contain the length all *all* the extensions if a parser just wanted to skip over them all. If it did not skip them all, the 

  opaque                extension_contents<0..2^32-1>;

will start with the length of the extension data so the parser can skip over an extenuation it does not understand. 

So given all that, does draft look reasonable. I completely agree an peer needs to be able to know the length of an unknown extortion to be able to step over it in parsing so if we can't do that, we need to fix it. 

On Jul 8, 2010, at 8:58 AM, David A. Bryan wrote:

> _______________________________________________
> P2PSIP mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/p2psip

Cullen Jennings
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html

---

From: "David A. Bryan" <[email-masked]>
Date: Thu, 8 Jul 2010 16:27:41 -0400

In this case, definitely that's fine. Can you point out where in the spec it
mentions opaque data encoding a length? I even looked for that in the TLS
style syntax description again, since everything I was worried about in the
draft made since if that was the case, but couldn't find it.

I think we may have the very minor bug of needing to make the fact the
length is encoded a bit more clear in the draft (or I'm just blind), but
given this, my bigger concern is a non-issue. Thanks, that clears it up.

David (as individual)

On Jul 8, 2010 4:11 PM, "Cullen Jennings" <[email-masked]> wrote:

Agree with what you are getting at here but I think the draft is already
pretty much what you want and this is just a syntax confusion.

This results in bits on the wire that look like an 8 bit integer as the
length followed by that many byes of data. So any time we have a variably
length element generated by the < > operators, it includes the length as
well as the data.

will contain the length all *all* the extensions if a parser just wanted to
skip over them all. If it did not skip them all, the

will start with the length of the extension data so the parser can skip over
an extenuation it does not understand.

So given all that, does draft look reasonable. I completely agree an peer
needs to be able to know the length of an unknown extortion to be able to
step over it in parsing so if we can't do that, we need to fix it.

On Jul 8, 2010, at 8:58 AM, David A. Bryan wrote:

> So looking at the extension mechanism in 5.3...

Cullen Jennings

---

From: Roni Even <[email-masked]>
Date: Fri, 09 Jul 2010 00:18:00 +0300

David,

It is in section 5.3.1.1 but I can agree that it is not clear enough and
maybe some specific text would make it better.

I assume that for  fix length there opaque there is no length field and for
variable length there is a length filed with size that will allow to list
the actual size of the string.

Roni Even

From: [email-masked] [mailto:[email-masked]] On Behalf Of
David A. Bryan
Sent: Thursday, July 08, 2010 11:28 PM
To: Cullen Jennings
Cc: P2PSIP WG
Subject: Re: [P2PSIP] Possible problem with the extension mechanism on
RELOAD? (and other places lengths might help)

David (as individual)

Cullen Jennings

---

From: Cullen Jennings <[email-masked]>
Date: Thu, 8 Jul 2010 15:39:40 -0600

On Jul 8, 2010, at 2:27 PM, David A. Bryan wrote:

> I think we may have the very minor bug of needing to make the fact the length is encoded a bit more clear in the draft (or I'm just blind),

I tried to clear that up in the next version of the draft.