Skip to content

Version negotiation responses are never recognized, so the protocol version can never be renegotiated #285

Description

@plorenz

Summary

A dialer can never renegotiate the protocol version, because it never recognizes the response that
would tell it to. No version change is pending, so nothing is broken today, but the mechanism that
exists for one does not work and would have to be fixed before it could be used.

Detail

WriteUnknownVersionResponse writes 16 bytes: the magic, a version count, and two versions. A
listener writes exactly that and then closes the connection (classic_listener.go).

ReadV2 reads a full 20-byte message frame before it inspects the magic, so those 16 bytes plus a
close surface as unexpected EOF. The magicUnknownVersion branch is never reached, so
GetRetryVersion never returns a version, and the dialer's retry in CreateWithHeaders redials with
the version that just failed.

Measured against a listener that sends a real response: err type=*errors.errorString,
GetRetryVersion -> 2, false. Padding the response to 20 bytes produces
channel.UnsupportedVersionError and GetRetryVersion -> 2, true, so the rest of the mechanism works
once the response is recognized.

Two further defects sit behind that one, so a recognized response would still parse incorrectly:

  • When the version list has not fully arrived, the continuation read fills a buf declared inside the
    if, shadowing the outer one. The bytes read are discarded and only the short leftover is parsed.
  • The parse loop runs to the end of the buffer rather than stopping at the declared count, so trailing
    bytes are reported as supported versions.

Separately, the count sizes an allocation and arrives from a peer that has not been authenticated, with
no bound on it.

Proposal

Classify the frame once the magic has been read rather than after a whole frame, reading at least the
magic so a frame that has already arrived in full still takes a single read. Parse the version list
through the bytes buffered with the magic followed by the wire, honor the declared count, and bound it.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions