Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jhump opened this issue May 25, 2024 · 1 comment
Open

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

jhump opened this issue May 25, 2024 · 1 comment

Comments

@jhump
Copy link
Member

jhump commented May 25, 2024

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
Copy link
Member

akshayjshah commented May 25, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants