Re: [ogpx] type-system : guidance for binary serialization implementers

["Robert G. Jakabosky" <[e-mail]>]

On Sunday 28, Meadhbh Hamrick wrote:
> one of the things we were trying to do was make a type system and
> serialization schemes that were resilient in the face of error. so i
> think there's a strong preference that we recover as best we can from
> an error instead of throwing up our hands and just saying "error!"

One primary source of decoding errors (i.e. LLSD message that doesn't follow 
the spec) will be from attackers (DOS attacks, remote code execution).  There 
is a big difference between handling unknown tags (to support future 
extentions) vs. trying to recover from a bad LLSD message and passing it on.  
The spec should allow for future extentions to be ignored by old 
implementations, but bad LLSD messages shouldn't be ignored, it will just 
hide normal bugs and make it harder to create new implementations (since you 
will not get useful decoding errors).

> here's a list of encoding / decoding errors i think about, along with
> suggestions for how to handle them.
>
> 1. the object count following the opening tag ('{' or '[') is less
> than the actual number of objects in the collection. (that is, the
> closing tag is not found after iterating through "object count" number
> of elements, but is found later in the stream.)

If closing tags are kept in the spec, then just reject the LLSD message as 
malformed.  What would you do if the same LLSD message was serialized in the 
XML format and the ending </array> or </map> tags where missing?

If the object count doesn't match the number of objects before the closing tag 
is found, then the LLSD message should be rejected with some error sent back 
to the sender.

> 2. the object count following the opening tag ('{' or '[') is greater
> than the actual number of objects in the collection. (that is, you get
> a closing tag before "object count" number of elements)

Same as case 1, just reject the LLSD message.

> 3. there is no closing tag. (that is, after "object count" number of
> elements, you find a tag that is not a '}' or a ']'.)
>
> this is more or less the same as error case number 1 above.
>
> though i wonder if it makes sense to differentiate the two cases. if
> we were, we would have to have the smarts enough to look forward in
> the stream, count the number of opening tags, then count the number of
> closing tags and see if they're unbalanced. ugh.

Doing this type of analysis would be fine for a verification tool, but not for 
a protocol decoder in a client/server program.  The more complex the decoder 
logic is the more likely it will have bugs that an attacker can exploit.

Also if the decoder should continue parsing the array/map until it finds the 
closing tag, then the "object count" is completely redundant and a waste of 
bandwidth.  Either drop the "object count" or the closing tags.

I would prefer the "object count" over the closing tags, also it would be nice 
if the "object count" was a "Base 128 Varint" instead of a fixed length 32bit 
integer:
http://code.google.com/apis/protocolbuffers/docs/encoding.html#varints

> 4. lack of 'k' elements in a map. (that is, you haven't reached the
> "object count" number of elements and are expecting a 'k' but get
> something else.)
>
> my druthers would be to say, if you encounter a type that can be
> converted to a string, you just do the conversion and use the result
> as the key.

Then why not just allow any type that can be converted to a string be used as 
a key in the maps?  How would you handle this for the XML encoding formats?

> another way to handle it would be to ignore it.

It should be a malformed error (with details about why it is malformed for 
easier debugging).

> 5. you encounter a bare 'k'. (that is, you encounter a 'k' tag outside
> the context of a map.)
>
> i vote for "convert to string"

This would just hide errors caused by a bad implementation of the LLSD binary 
encoder.  If you find a 'k' tag outside a map, then there might be some 
bigger problem.  Maybe the last map had more objects then the "object count" 
said.

> 6. duplicate keys in a map. (actually this applies to all serializations)
>
> i vote for "the last key in the map with the same name wins." that is,
> if you have two or more keys with the same name, the one found
> furthest in the stream is the one that's used.

This issue is not unique to the binary format, both the XML & JSON formats 
will have this issue too.  The JSON specs RFC4627 section 2.2 says that the 
names of object members SHOULD be unique, but it doesn't say how duplicate 
names are to be handled.

Section "2.2.2.  Map" of the LLSD draft says "Within a given Map value, each 
key must be unique, each with one value. ....".

Either that should be changed to a "each key MUST be unique", in which case 
duplicate keys would generate a malformed error with the whole LLSD message 
being rejected, or it should be changed to a "MUST"/"REQUIRED" rule on how to 
handle duplicates.

Having strict rules in the spec on how to handle malformed LLSD message is 
important, since a security hole can exist if two LLSD implementations 
interpret/decode the same LLSD message differently.  There has been a similar 
issue with HTTP proxies (sneaking a second HTTP request passed the proxy to a 
backend HTTP server).

Lets say you have a proxy/authenticator in front of a set of backend servers 
that will consume LLSD messages:

"VWRAP client(attacker)" -> "public frontend proxy/authenticator" -> "private 
backend server"

The "public frontend" proxy server would decode the LLSD message to apply some 
routing or authentication rules (i.e. check user id & credentials) before 
passing the original (not a re-encoded message) LLSD message to a "private 
backend" server for handling.  The attacker could trick the frontend server 
into routing a LLSD message with any user id they want to the backend server.  
This could easily happend if the proxy server was written/made by a different 
company (i.e. Cisco VWRAP load balancer), then the backend server and the two 
implementations of LLSD used different rules for how to handle duplicate keys 
("user id" or "credential" keys).  The same thing can happen if one of the 
two implementations ignore incorrect "object counts" or missing closing tags.

I will be commenting more on the other "type-system" e-mails soon.

P.S. will this mailling list change it's name to "vwrap" soon?  I know the 
vwrap jabber.ietf.org channel has already been created.

-- 
Robert G. Jakabosky