Subject: Re: [sidr] rpki-rtr protocol maximum message length

From: Joe Touch <[email-masked]>
Date: Wed, 01 Jun 2011 09:07:30 -0700

Hi, all,

On 5/31/2011 11:57 PM, Hannes Gredler wrote:
> On Tue, May 31, 2011 at 11:26:05AM -0700, Joe Touch wrote:
> |
> |
> | On 5/31/2011 11:19 AM, Hannes Gredler wrote:
> |>On Tue, May 31, 2011 at 10:56:52AM -0700, Joe Touch wrote:
> |>|
> |>|
> |>| On 5/31/2011 10:52 AM, Paul Hoffman wrote:
> |>|>On May 31, 2011, at 10:23 AM, Randy Bush wrote:
> |>|>
> |>|>>>1. Maybe add to section 5.9 "An Error Report PDU MUST NOT be sent for
> |>|>>>an Error Report PDU.".
> |>|>>
> |>|>>makes sense
> |>|>>
> |>|>>>To answer the initial request, it seems reasonable to specify "the
> |>|>>>length of the Arbitrary Text of Error Diagnostic Message MUST NOT be
> |>|>>>longer than 200 octets". That keeps the length of the entire PDU under
> |>|>>>the semi-magic 256 octets for the current version of the protocol
> |>|>>
> |>|>>in what way is 256 octets semi-magic?
> |>|>
> |>|>C or assembly-based implementations will need to malloc (etc.) the
> |>|>buffer for the incoming message. Having that fit into 256 octets could
> |>|>easily have some value to some implementations. There is no security
> |>|>problem if the cache ignores such a MUST and makes the message longer
> |>|>because the length is given the PDU. (Having said that, I hope router
> |>|>implementers of this protocol are being robust in checking the lengths
> |>|>in this PDU; I can already see ways to craft malicious versions of this
> |>|>PDU that might trigger a buffer overflow...)
> |>|
> |>| These are good reasons for robust implementations to check for the
> |>| message length and read only as much as the currently allocated
> |>| buffer allows, reading more in as needed.
> |>
> |>wonder what the normative procedure is for "too-large" messages ?
> |>
> |>my pre-standard implementation has an inbound buffer of 4K
> |>and resets the session if it encounters a PDU length of>   4K.
> |
> | That makes an easy DOS attack.
>
> i fail to see why max-PDU size spelled out in a protocol-spec is
> a pre-cursor to a DOS attack. as long as the message integrity of
> your TCP session is preserved (e.g. MD5, AO) its perfectly letigimate
> to flap a session if i do not like what i see.

The potential for a DOS attack comes from apps thinking 'max-PDU' is 
enforced somewhere other than at the app. They write apps reading the 
entire socket into a (fixed-size) buffer, and that's how we get buffer 
overflow attacks.

If every app designer (and kernel protocol designer too) limited reads 
to the size of the incoming buffer, there would never be such an attack, 
yet they persist.

My concern with claiming a "max PDU" is that it encourages such complacency.

Finally, there's a misplaced notion that PDUs correspond to packets on 
the wire. In TCP, socket write boundaries don't correspond to segment 
boundaries anyway, so there's no real point in claiming such a limit as 
a "PDU" issue.

> | It's better to:
> |
> | - allocate a 4K (or whatever value you want) buffer
> | - read up to the first 4K
> | - check to see if there's more to read, and allocate more buffer
> | space or read it into the first buffer (after the first 4K has been
> | parsed)
>
> i am not so sure about wether the above is really "better", since
> there is a price to pay. in the above you are assuming that each
> input parser is a true "stream parser" such that the entire parsing
> context is saved when e.g. a PDU is scattered across two buffers.
>
> e.g. consider an IPV4 Prefix where the first part is in buffer #1 and the
> reminder is in buffer #2.

When you read from a TCP socket, you need to be prepared to deal with 
such boundaries anyway. As I note above, TCP doesn't preserve boundaries.

If you're parsing a bytestream (TCP) for an application protocol, yes, 
you need a parser anyway.

> lets further assume that between reading buffer 1 and buffer 2 there is
> a time lag (e.g. the socket just got flow blocked). now you need to save your
> previous parsing results up until the socket becomes readable again.

Yes. That's true anyway if you do nonblocking reads.

> with max-PDU sized recv buffers the whole story gets a whole lot simpler,
> as you can avoid the context saving for each and every input parser:
>
> you just need to ask.
>
> 1. do i have enough data in the buffer for reading the PDU header ?
> 2. if so does the message exceed the recv buffer ?
> 3. do i have enough data in the buffer for parsing an entire PDU ?

And what happens when you read more than the application PTU? You can't 
exactly put it back on the socket to read later.

There's no way around this - even if the message says "I'm 240 bytes", 
you can't know that the 240 bytes are there; the source could have errd. 
You still need to grab data from the socket into an intermediate space - 
which will be a few PDUs in length - and parse that, so you can get 
around such boundary issues and grab data off the socket without needing 
to put it back.

> | Either way, there's no reason for a *protocol action* to deal with a
> | receiver buffering issue.
>
> i find less code-complexity in input parsers a quite compelling argument.

If you think you're getting around this complexity issue, you have a 
parser that has a security hole.

Joe

---

From: Hannes Gredler <[email-masked]>
Date: Wed, 1 Jun 2011 20:44:11 +0200

On Wed, Jun 01, 2011 at 09:07:30AM -0700, Joe Touch wrote:
| Hi, all,
| 
| On 5/31/2011 11:57 PM, Hannes Gredler wrote:
| >On Tue, May 31, 2011 at 11:26:05AM -0700, Joe Touch wrote:
| >|
| >|
| >| On 5/31/2011 11:19 AM, Hannes Gredler wrote:
| >|>On Tue, May 31, 2011 at 10:56:52AM -0700, Joe Touch wrote:
| >|>|
| >|>|
| >|>| On 5/31/2011 10:52 AM, Paul Hoffman wrote:
| >|>|>On May 31, 2011, at 10:23 AM, Randy Bush wrote:
| >|>|>
| >|>|>>>1. Maybe add to section 5.9 "An Error Report PDU MUST NOT be sent for
| >|>|>>>an Error Report PDU.".
| >|>|>>
| >|>|>>makes sense
| >|>|>>
| >|>|>>>To answer the initial request, it seems reasonable to specify "the
| >|>|>>>length of the Arbitrary Text of Error Diagnostic Message MUST NOT be
| >|>|>>>longer than 200 octets". That keeps the length of the entire PDU under
| >|>|>>>the semi-magic 256 octets for the current version of the protocol
| >|>|>>
| >|>|>>in what way is 256 octets semi-magic?
| >|>|>
| >|>|>C or assembly-based implementations will need to malloc (etc.) the
| >|>|>buffer for the incoming message. Having that fit into 256 octets could
| >|>|>easily have some value to some implementations. There is no security
| >|>|>problem if the cache ignores such a MUST and makes the message longer
| >|>|>because the length is given the PDU. (Having said that, I hope router
| >|>|>implementers of this protocol are being robust in checking the lengths
| >|>|>in this PDU; I can already see ways to craft malicious versions of this
| >|>|>PDU that might trigger a buffer overflow...)
| >|>|
| >|>| These are good reasons for robust implementations to check for the
| >|>| message length and read only as much as the currently allocated
| >|>| buffer allows, reading more in as needed.
| >|>
| >|>wonder what the normative procedure is for "too-large" messages ?
| >|>
| >|>my pre-standard implementation has an inbound buffer of 4K
| >|>and resets the session if it encounters a PDU length of>   4K.
| >|
| >| That makes an easy DOS attack.
| >
| >i fail to see why max-PDU size spelled out in a protocol-spec is
| >a pre-cursor to a DOS attack. as long as the message integrity of
| >your TCP session is preserved (e.g. MD5, AO) its perfectly letigimate
| >to flap a session if i do not like what i see.
| 
| The potential for a DOS attack comes from apps thinking 'max-PDU' is
| enforced somewhere other than at the app. They write apps reading
| the entire socket into a (fixed-size) buffer, and that's how we get
| buffer overflow attacks.
|
| If every app designer (and kernel protocol designer too) limited
| reads to the size of the incoming buffer, there would never be such
| an attack, yet they persist.

hence my point explicit spelling out what the maximum PDU size should be;
the seasoned implementer will size its input buffer accordingly.
 
| My concern with claiming a "max PDU" is that it encourages such complacency.
| 
| Finally, there's a misplaced notion that PDUs correspond to packets
| on the wire. In TCP, socket write boundaries don't correspond to
| segment boundaries anyway, so there's no real point in claiming such
| a limit as a "PDU" issue.
|
| >| It's better to:
| >|
| >| - allocate a 4K (or whatever value you want) buffer
| >| - read up to the first 4K
| >| - check to see if there's more to read, and allocate more buffer
| >| space or read it into the first buffer (after the first 4K has been
| >| parsed)
| >
| >i am not so sure about wether the above is really "better", since
| >there is a price to pay. in the above you are assuming that each
| >input parser is a true "stream parser" such that the entire parsing
| >context is saved when e.g. a PDU is scattered across two buffers.
| >
| >e.g. consider an IPV4 Prefix where the first part is in buffer #1 and the
| >reminder is in buffer #2.
| 
| When you read from a TCP socket, you need to be prepared to deal
| with such boundaries anyway. As I note above, TCP doesn't preserve
| boundaries.

the properties of stream based protocols is understood;

| If you're parsing a bytestream (TCP) for an application protocol,
| yes, you need a parser anyway.
| 
| >lets further assume that between reading buffer 1 and buffer 2 there is
| >a time lag (e.g. the socket just got flow blocked). now you need to save your
| >previous parsing results up until the socket becomes readable again.
| 
| Yes. That's true anyway if you do nonblocking reads.
| 
| >with max-PDU sized recv buffers the whole story gets a whole lot simpler,
| >as you can avoid the context saving for each and every input parser:
| >
| >you just need to ask.
| >
| >1. do i have enough data in the buffer for reading the PDU header ?
| >2. if so does the message exceed the recv buffer ?
| >3. do i have enough data in the buffer for parsing an entire PDU ?
| 
| And what happens when you read more than the application PTU? You
| can't exactly put it back on the socket to read later.

one implementation choice could be to:
parse your input buffer up until you reach the "incomplete" PDU
which does not yet have enough payload. finally the incomplete part
gets copied back to the beginning of the buffer;
for the rpki protocol (with its small PDUs) that rebasing is a relatively
cheap operation.

| There's no way around this - even if the message says "I'm 240
| bytes", you can't know that the 240 bytes are there;

usually my buffer struct is a the buffer itself and two pointers
(start and end) for housekeeping purposes. therefore i know how
full my buffer is (buffer pointer get updated after every read
and parser call)

| the source could have errd. You still need to grab data from the socket into an
| intermediate space - which will be a few PDUs in length - and parse
| that, so you can get around such boundary issues and grab data off
| the socket without needing to put it back.

exactly .. all i am asking for is a constant for storing 1 PDU such that
i can appropriately size the "intermediate space"

| >| Either way, there's no reason for a *protocol action* to deal with a
| >| receiver buffering issue.
| >
| >i find less code-complexity in input parsers a quite compelling argument.
| 
| If you think you're getting around this complexity issue,
| you have a parser that has a security hole.

i have running (security audited) I/O code which works like described
above for more than a decade in the internet.
IOW i have proof point that your assertion is not correct;

---

From: Joe Touch <[email-masked]>
Date: Wed, 01 Jun 2011 14:12:50 -0700

Hi, Hannes,

On 6/1/2011 11:44 AM, Hannes Gredler wrote:
...
> full my buffer is (buffer pointer get updated after every read

(see below)

> i can appropriately size the "intermediate space"

OK, so we're talking about the same thing. I was objecting to the 
concept of "PDU" - what we're talking about is the maximum length of a 
self-contained parseable message. (PDUs tend to mean "packet" at L1-L4).

>
> |>| Either way, there's no reason for a *protocol action* to deal with a
> |>| receiver buffering issue.
> |>
> |>i find less code-complexity in input parsers a quite compelling argument.
> |

Your example above *is* the complexity you claimed you were trying to 
avoid...

Joe

---

From: Hannes Gredler <[email-masked]>
Date: Thu, 2 Jun 2011 09:42:42 +0200

On Wed, Jun 01, 2011 at 02:12:50PM -0700, Joe Touch wrote:
| Hi, Hannes,
| 
| On 6/1/2011 11:44 AM, Hannes Gredler wrote:
| ...
| >one implementation choice could be to:
| >parse your input buffer up until you reach the "incomplete" PDU
| >which does not yet have enough payload. finally the incomplete part
| >gets copied back to the beginning of the buffer;
| >for the rpki protocol (with its small PDUs) that rebasing is a relatively
| >cheap operation.
| >
| >| There's no way around this - even if the message says "I'm 240
| >| bytes", you can't know that the 240 bytes are there;
| >
| >usually my buffer struct is a the buffer itself and two pointers
| >(start and end) for housekeeping purposes. therefore i know how
| >full my buffer is (buffer pointer get updated after every read
| >and parser call)
| 
| (see below)
| 
| >| the source could have errd. You still need to grab data from the socket into an
| >| intermediate space - which will be a few PDUs in length - and parse
| >| that, so you can get around such boundary issues and grab data off
| >| the socket without needing to put it back.
| >
| >exactly .. all i am asking for is a constant for storing 1 PDU such that
| >i can appropriately size the "intermediate space"
| 
| OK, so we're talking about the same thing. I was objecting to the
| concept of "PDU" - what we're talking about is the maximum length of
| a self-contained parseable message. (PDUs tend to mean "packet" at
| L1-L4).

good :-)

| >|>| Either way, there's no reason for a *protocol action* to deal with a
| >|>| receiver buffering issue.
| >|>
| >|>i find less code-complexity in input parsers a quite compelling argument.
| >|
| >| If you think you're getting around this complexity issue,
| >| you have a parser that has a security hole.
| >
| >i have running (security audited) I/O code which works like described
| >above for more than a decade in the internet.
| >IOW i have proof point that your assertion is not correct;
| 
| Your example above *is* the complexity you claimed you were trying
| to avoid...

even with the the above i cannot handle cases like processing a 4100 byte
message as it never fits into the "intermediate" space of 4K;

that is the main reason i was asking for a max-PDU size constant in the spec;
(alternatively i continue what i do (flap the session) and write
an error PDU with some more descriptive text to tell the operator whats
going on)

tx,

/hannes