search

connectrpc / connectrpc.com

Docs, governance, and RFCs for Connect: Protobuf RPC that works.
https://connectrpc.com
Apache License 2.0
15 stars 19 forks source link

spec: codec suffix for Connect streaming protocol violates RFC 6838 #164

Open jhump opened 4 months ago

jhump commented 4 months ago

Section 4.2.8 of RFC 6838 discusses the use of “+…” suffixes on media types (called “Structured Syntax Name Suffixes”). In particular, it includes two statements that make Connect's use of such suffixes in its streaming protocol incorrect:

  1. Media types MUST NOT be given names incorporating suffixes for structured syntaxes they do not actually employ.

    Connect’s potential use of +json falls into this category. This suffix indicates that the content is structurally JSON. But that is not actually true for application/connect+json. Instead, the content is a stream of messages, each with a 5-byte prefix and then a chunk of data that is structurally JSON. This is also true of any other suffix that might be used, so Connect's stream format would not be structurally compatible with any other structured syntax (except, possibly, by luck or accident).

  2. "+suffix" constructs for as-yet unregistered structured syntaxes SHOULD NOT be used, given the possibility of conflicts with future suffix definitions.

    Connect's potential use of any other suffix (including +proto) falls into this camp: +proto is not a registered suffix and thus should not be used.

Admittedly, this has not caused any known issues in practice, so priority of changing it is likely low, especially given how disruptive such a change could be (and the fact that server implementations would likely have to support the old media type formats for a long time, for long-term compatibility).

However, a change should at least be considered as there is a realistic chance, even if small, that this could pose issues in the future. One possible scenario: a serious vulnerability is discovered in a widely-used server-side JSON parser implementation. In that event, it is likely that a rule for a WAF would be developed that attempts to analyze JSON payloads to prevent problematic data from being sent to potentially vulnerable backends. Such a rule could cause serious issues for applications that use Connect if there is also any traffic that employs the application/connect+json media type flowing through the same WAF, since the media type is likely to be mistaken for JSON content and possibly rejected due to its structural issues.

akshayjshah commented 4 months ago

Agree that we're not in compliance with RFC 6838, and also agree that changing this is low-priority. Since neither protobuf nor Connect streaming have registered Content-Types, our options for becoming fully compliant seem limited.