From a6dfc00aeb2cef75dbf1da850da823b0a14ef365 Mon Sep 17 00:00:00 2001 From: Jie Chen Date: Mon, 24 Aug 2026 11:13:35 -0700 Subject: [PATCH] gcs: negotiate a guest/host contract version to detect drift The HCS<->GCS bridge negotiates only a protocol version that is frozen at 4 (prot.PvV4), so a host and GCS built from source commits whose message contract has drifted still connect successfully and then fail later in confusing ways (RPC timeouts, JSON errors). GCS ships separately from the host, as an RPM embedded in the UVM image, so this mispairing is easy to hit. Add a guest/host contract version, independent of the frozen protocol version, that both sides advertise during NegotiateProtocol and the host enforces at connect: - internal/gcscompat holds the single source of truth (GuestHostContractVersion, MinCompatibleContractVersion) plus a Compatible() range-overlap check. Both the Windows host and the Linux guest compile the same constants, so the values can only differ at runtime when the two binaries came from incompatible commits. - Each side advertises its [min..max] range as additive, omitempty fields (host in NegotiateProtocolRequest, guest in GcsCapabilities) plus a source commit for diagnostics. Peers that predate the contract advertise no range and are skipped, so already-deployed UVM images keep working. - If the ranges do not overlap, the host fails connect() with an actionable message naming both commits and ranges, and the guest rejects negotiation. The mispairing now surfaces at the first GCS connection instead of as a downstream failure. Bump GuestHostContractVersion in any change to the guest/host contract that both sides must agree on. Signed-off-by: Jie Chen --- internal/gcs/guestconnection.go | 27 ++++++- internal/gcs/guestconnection_test.go | 86 +++++++++++++++++++++++ internal/gcs/prot/protocol.go | 17 +++++ internal/gcscompat/contract.go | 33 +++++++++ internal/gcscompat/contract_test.go | 56 +++++++++++++++ internal/guest/bridge/bridge_unit_test.go | 67 ++++++++++++++++++ internal/guest/bridge/bridge_v2.go | 20 ++++++ internal/guest/prot/protocol.go | 16 +++++ 8 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 internal/gcscompat/contract.go create mode 100644 internal/gcscompat/contract_test.go diff --git a/internal/gcs/guestconnection.go b/internal/gcs/guestconnection.go index 2a60cc23ec..121f20f120 100644 --- a/internal/gcs/guestconnection.go +++ b/internal/gcs/guestconnection.go @@ -19,9 +19,11 @@ import ( "github.com/Microsoft/go-winio/pkg/guid" "github.com/Microsoft/hcsshim/internal/cow" "github.com/Microsoft/hcsshim/internal/gcs/prot" + "github.com/Microsoft/hcsshim/internal/gcscompat" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/logfields" "github.com/Microsoft/hcsshim/internal/ot" + "github.com/Microsoft/hcsshim/internal/version" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -121,8 +123,11 @@ func (gc *GuestConnection) Protocol() uint32 { // It should be false for subsequent connections (e.g. if reconnecting to an existing UVM). func (gc *GuestConnection) connect(ctx context.Context, isColdStart bool, initGuestState *InitialGuestState) (err error) { req := prot.NegotiateProtocolRequest{ - MinimumVersion: protocolVersion, - MaximumVersion: protocolVersion, + MinimumVersion: protocolVersion, + MaximumVersion: protocolVersion, + MinContractVersion: gcscompat.MinCompatibleContractVersion, + MaxContractVersion: gcscompat.GuestHostContractVersion, + HostCommit: version.Commit, } var resp prot.NegotiateProtocolResponse err = gc.brdg.RPC(ctx, prot.RPCNegotiateProtocol, &req, &resp, true) @@ -133,6 +138,24 @@ func (gc *GuestConnection) connect(ctx context.Context, isColdStart bool, initGu return fmt.Errorf("unexpected version %d returned", resp.Version) } + // Enforce guest/host contract compatibility. A guest that predates the + // contract advertises no range (MaxContractVersion == 0); skip enforcement + // for it so already-deployed UVM images keep working. Otherwise require the + // host and guest contract ranges to overlap, and fail fast with an + // actionable message naming both sides if they do not. + if guestMax := resp.Capabilities.MaxContractVersion; guestMax != 0 { + if !gcscompat.Compatible( + gcscompat.MinCompatibleContractVersion, gcscompat.GuestHostContractVersion, + resp.Capabilities.MinContractVersion, guestMax, + ) { + return fmt.Errorf( + "GCS/hcsshim contract mismatch: host contract [%d..%d] (commit %s) cannot interoperate with guest contract [%d..%d] (commit %s); the GCS in this UVM was built from an incompatible hcsshim revision", + gcscompat.MinCompatibleContractVersion, gcscompat.GuestHostContractVersion, version.Commit, + resp.Capabilities.MinContractVersion, guestMax, resp.Capabilities.GcsCommit, + ) + } + } + gc.os = strings.ToLower(resp.Capabilities.RuntimeOsType) if gc.os == "" { gc.os = "windows" diff --git a/internal/gcs/guestconnection_test.go b/internal/gcs/guestconnection_test.go index c5b5543239..7dd5023b0e 100644 --- a/internal/gcs/guestconnection_test.go +++ b/internal/gcs/guestconnection_test.go @@ -16,6 +16,7 @@ import ( "github.com/Microsoft/go-winio" "github.com/Microsoft/go-winio/pkg/guid" "github.com/Microsoft/hcsshim/internal/gcs/prot" + "github.com/Microsoft/hcsshim/internal/gcscompat" "github.com/Microsoft/hcsshim/internal/ot" "github.com/sirupsen/logrus" "go.opentelemetry.io/otel" @@ -192,6 +193,91 @@ func TestGcsConnect(t *testing.T) { defer gc.Close() } +// answerNegotiateOnce serves exactly one NegotiateProtocol handshake on rw, +// replying with the supplied capabilities, then returns. It lets a test drive +// the host-side contract compatibility check in connect() with a chosen guest +// contract range. +func answerNegotiateOnce(t *testing.T, rw io.ReadWriteCloser, caps prot.GcsCapabilities) { + t.Helper() + defer rw.Close() + id, typ, _, err := readMessage(rw) + if err != nil { + t.Error(err) + return + } + if proc := prot.RPCProc(typ &^ prot.MsgTypeRequest); proc != prot.RPCNegotiateProtocol { + t.Errorf("first RPC = %v, want NegotiateProtocol", proc) + return + } + if err := sendJSON(t, rw, prot.MsgTypeResponse|prot.MsgType(prot.RPCNegotiateProtocol), id, &prot.NegotiateProtocolResponse{ + Version: protocolVersion, + Capabilities: caps, + }); err != nil { + t.Error(err) + } +} + +// TestGcsConnectContractMismatch verifies that connect() fails fast with an +// actionable error when the guest advertises a contract range that cannot +// overlap the host's. This is how a mispaired GCS surfaces at the first +// connection instead of as a confusing downstream failure. +func TestGcsConnectContractMismatch(t *testing.T) { + s, c := pipeConn() + badMin := gcscompat.GuestHostContractVersion + 100 + done := make(chan struct{}) + go func() { + defer close(done) + answerNegotiateOnce(t, c, prot.GcsCapabilities{ + RuntimeOsType: "linux", + MinContractVersion: badMin, + MaxContractVersion: badMin + 1, + GcsCommit: "deadbeefcafe", + }) + }() + t.Cleanup(func() { <-done }) + + gcc := &GuestConnectionConfig{ + Conn: s, + Log: logrus.NewEntry(logrus.StandardLogger()), + IoListen: npipeIoListen, + } + _, err := gcc.Connect(context.Background(), true) + if err == nil { + t.Fatal("expected a contract mismatch error, got nil") + } + if !strings.Contains(err.Error(), "contract mismatch") { + t.Fatalf("error should mention contract mismatch, got: %v", err) + } +} + +// TestGcsConnectContractCompatible verifies that a guest advertising a +// compatible contract range connects normally. +func TestGcsConnectContractCompatible(t *testing.T) { + s, c := pipeConn() + done := make(chan struct{}) + go func() { + defer close(done) + answerNegotiateOnce(t, c, prot.GcsCapabilities{ + RuntimeOsType: "linux", + MinContractVersion: gcscompat.MinCompatibleContractVersion, + MaxContractVersion: gcscompat.GuestHostContractVersion, + GcsCommit: "abc123", + }) + }() + t.Cleanup(func() { <-done }) + + gcc := &GuestConnectionConfig{ + Conn: s, + Log: logrus.NewEntry(logrus.StandardLogger()), + IoListen: npipeIoListen, + } + gc, err := gcc.Connect(context.Background(), true) + if err != nil { + t.Fatalf("compatible contract should connect, got: %v", err) + } + gc.Close() +} + // TestGcsResumeOnConnRenegotiates verifies that ResumeOnConn re-runs the // protocol handshake on the swapped-in transport. The guest resets its GCS // protocol version when it re-dials after a migration blackout, so a resume diff --git a/internal/gcs/prot/protocol.go b/internal/gcs/prot/protocol.go index 536b5100b3..05534ae4f2 100644 --- a/internal/gcs/prot/protocol.go +++ b/internal/gcs/prot/protocol.go @@ -269,6 +269,15 @@ type NegotiateProtocolRequest struct { RequestBase MinimumVersion uint32 MaximumVersion uint32 + // MinContractVersion and MaxContractVersion advertise the host's supported + // guest/host contract range (see internal/gcscompat). They are additive and + // optional: a guest that predates the contract ignores them, and a zero + // MaxContractVersion means the sender does not advertise a contract. + MinContractVersion uint32 `json:",omitempty"` + MaxContractVersion uint32 `json:",omitempty"` + // HostCommit is the hcsshim source commit the host was built from. It is + // diagnostic only, used to make a contract mismatch error actionable. + HostCommit string `json:",omitempty"` } type NegotiateProtocolResponse struct { @@ -400,6 +409,14 @@ type GcsCapabilities struct { SupportedSchemaVersions []hcsschema.Version RuntimeOsType string GuestDefinedCapabilities json.RawMessage + // MinContractVersion and MaxContractVersion advertise the guest's supported + // guest/host contract range (see internal/gcscompat). A zero + // MaxContractVersion means the guest predates the contract. + MinContractVersion uint32 `json:",omitempty"` + MaxContractVersion uint32 `json:",omitempty"` + // GcsCommit is the hcsshim source commit the GCS was built from. It is + // diagnostic only, used to make a contract mismatch error actionable. + GcsCommit string `json:",omitempty"` } type ContainerCreateResponse struct { diff --git a/internal/gcscompat/contract.go b/internal/gcscompat/contract.go new file mode 100644 index 0000000000..40c0e69906 --- /dev/null +++ b/internal/gcscompat/contract.go @@ -0,0 +1,33 @@ +// Package gcscompat defines the guest/host contract version that the GCS +// (guest) and hcsshim (host) exchange during protocol negotiation. +// +// Both the Windows host and the Linux guest compile the same constants from +// this package. Because the file carries no build constraint, the values can +// only differ at runtime when the two binaries were built from source commits +// that changed the contract. That makes the contract version a compact proxy +// for "were these two binaries built from contract-compatible source?", which +// the frozen bridge protocol version (prot.PvV4 = 4) cannot answer: the +// protocol version only bumps for an epochal bridge rewrite, whereas ordinary +// guest/host evolution happens within protocol version 4. +package gcscompat + +const ( + // GuestHostContractVersion is the newest guest/host contract this binary + // implements. Bump it in the same change that alters the guest/host + // message contract in a way both sides must agree on (a change that is not + // backward compatible). + GuestHostContractVersion uint32 = 1 + + // MinCompatibleContractVersion is the oldest peer contract this binary can + // still interoperate with. Raise it only when support for older peers is + // intentionally dropped. + MinCompatibleContractVersion uint32 = 1 +) + +// Compatible reports whether a local contract range [localMin, localMax] and a +// remote contract range [remoteMin, remoteMax] intersect. Two peers can +// interoperate if and only if their advertised ranges overlap. The relation is +// symmetric, so either side can evaluate it. +func Compatible(localMin, localMax, remoteMin, remoteMax uint32) bool { + return localMin <= remoteMax && remoteMin <= localMax +} diff --git a/internal/gcscompat/contract_test.go b/internal/gcscompat/contract_test.go new file mode 100644 index 0000000000..a9cda5759b --- /dev/null +++ b/internal/gcscompat/contract_test.go @@ -0,0 +1,56 @@ +package gcscompat + +import "testing" + +func TestCompatible(t *testing.T) { + cases := []struct { + name string + lMin, lMax, rMin, rMax uint32 + want bool + }{ + {"identical point", 1, 1, 1, 1, true}, + {"identical range", 4, 6, 4, 6, true}, + {"overlap", 4, 6, 2, 5, true}, + {"touching at low edge", 4, 6, 1, 4, true}, + {"touching at high edge", 4, 6, 6, 9, true}, + {"remote strictly below", 4, 6, 1, 3, false}, + {"remote strictly above", 4, 6, 7, 9, false}, + {"remote is a superset", 4, 6, 1, 9, true}, + {"remote is a subset", 1, 9, 4, 6, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := Compatible(tc.lMin, tc.lMax, tc.rMin, tc.rMax); got != tc.want { + t.Fatalf("Compatible(%d,%d,%d,%d) = %v, want %v", + tc.lMin, tc.lMax, tc.rMin, tc.rMax, got, tc.want) + } + }) + } +} + +// TestCompatibleSymmetric verifies that swapping the local and remote ranges +// does not change the verdict, so either the host or the guest can evaluate it. +func TestCompatibleSymmetric(t *testing.T) { + ranges := [][4]uint32{ + {4, 6, 2, 5}, + {4, 6, 1, 3}, + {1, 1, 2, 2}, + {1, 3, 3, 5}, + } + for _, r := range ranges { + forward := Compatible(r[0], r[1], r[2], r[3]) + reverse := Compatible(r[2], r[3], r[0], r[1]) + if forward != reverse { + t.Fatalf("asymmetric verdict for %v: forward=%v reverse=%v", r, forward, reverse) + } + } +} + +// TestSelfCompatible verifies that a binary is always compatible with another +// build of itself (identical range), which must hold for the common case. +func TestSelfCompatible(t *testing.T) { + if !Compatible(MinCompatibleContractVersion, GuestHostContractVersion, + MinCompatibleContractVersion, GuestHostContractVersion) { + t.Fatal("a binary must be compatible with its own contract range") + } +} diff --git a/internal/guest/bridge/bridge_unit_test.go b/internal/guest/bridge/bridge_unit_test.go index e969825ecf..d6b07bd272 100644 --- a/internal/guest/bridge/bridge_unit_test.go +++ b/internal/guest/bridge/bridge_unit_test.go @@ -4,6 +4,7 @@ package bridge import ( + "context" "encoding/binary" "encoding/json" "io" @@ -13,6 +14,7 @@ import ( "testing" "github.com/Microsoft/hcsshim/internal/bridgeutils/gcserr" + "github.com/Microsoft/hcsshim/internal/gcscompat" "github.com/Microsoft/hcsshim/internal/guest/prot" "github.com/Microsoft/hcsshim/internal/guest/transport" "github.com/pkg/errors" @@ -666,3 +668,68 @@ func Test_Bridge_ListenAndServe_HandlersAreAsync_Success(t *testing.T) { t.Error("Incorrect response order for 1st request") } } + +// Test_negotiateProtocolV2_ContractMismatch verifies that the guest rejects a +// host whose advertised contract range cannot overlap the guest's, so a +// mispaired host/GCS pair is refused at negotiation. +func Test_negotiateProtocolV2_ContractMismatch(t *testing.T) { + b := &Bridge{} + badMin := gcscompat.GuestHostContractVersion + 100 + msg, err := json.Marshal(prot.NegotiateProtocol{ + MinimumVersion: uint32(prot.PvV4), + MaximumVersion: uint32(prot.PvMax), + MinContractVersion: badMin, + MaxContractVersion: badMin + 1, + HostCommit: "deadbeefcafe", + }) + if err != nil { + t.Fatal(err) + } + req := &Request{ + Context: context.Background(), + Header: &prot.MessageHeader{ + Type: prot.ComputeSystemNegotiateProtocolV1, + ID: prot.SequenceID(1), + }, + Message: msg, + } + if _, err := b.negotiateProtocolV2(req); err == nil { + t.Fatal("expected an incompatible-contract error, got nil") + } +} + +// Test_negotiateProtocolV2_ContractCompatible verifies that a host advertising a +// compatible contract range negotiates successfully and that the guest +// advertises its own contract range back. +func Test_negotiateProtocolV2_ContractCompatible(t *testing.T) { + b := &Bridge{} + msg, err := json.Marshal(prot.NegotiateProtocol{ + MinimumVersion: uint32(prot.PvV4), + MaximumVersion: uint32(prot.PvMax), + MinContractVersion: gcscompat.MinCompatibleContractVersion, + MaxContractVersion: gcscompat.GuestHostContractVersion, + HostCommit: "abc123", + }) + if err != nil { + t.Fatal(err) + } + req := &Request{ + Context: context.Background(), + Header: &prot.MessageHeader{ + Type: prot.ComputeSystemNegotiateProtocolV1, + ID: prot.SequenceID(1), + }, + Message: msg, + } + resp, err := b.negotiateProtocolV2(req) + if err != nil { + t.Fatalf("compatible contract should negotiate, got: %v", err) + } + npr, ok := resp.(*prot.NegotiateProtocolResponse) + if !ok { + t.Fatalf("unexpected response type %T", resp) + } + if npr.Capabilities.MaxContractVersion != gcscompat.GuestHostContractVersion { + t.Fatalf("guest did not advertise its contract range: %+v", npr.Capabilities) + } +} diff --git a/internal/guest/bridge/bridge_v2.go b/internal/guest/bridge/bridge_v2.go index 19631bef4f..54cdd57309 100644 --- a/internal/guest/bridge/bridge_v2.go +++ b/internal/guest/bridge/bridge_v2.go @@ -14,12 +14,14 @@ import ( "github.com/Microsoft/hcsshim/internal/bridgeutils/commonutils" "github.com/Microsoft/hcsshim/internal/bridgeutils/gcserr" + "github.com/Microsoft/hcsshim/internal/gcscompat" "github.com/Microsoft/hcsshim/internal/guest/prot" "github.com/Microsoft/hcsshim/internal/guest/runtime/hcsv2" "github.com/Microsoft/hcsshim/internal/guest/stdio" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/ot" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" + "github.com/Microsoft/hcsshim/internal/version" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) @@ -42,6 +44,11 @@ var capabilities = prot.GcsCapabilities{ DumpStacksSupported: true, DeleteContainerStateSupported: true, }, + // Advertise this GCS's guest/host contract range so the host can reject a + // mispaired host/GCS pair at negotiation. See internal/gcscompat. + MinContractVersion: gcscompat.MinCompatibleContractVersion, + MaxContractVersion: gcscompat.GuestHostContractVersion, + GcsCommit: version.Commit, } // negotiateProtocolV2 was introduced in v4 so will not be called with a minimum @@ -61,6 +68,19 @@ func (b *Bridge) negotiateProtocolV2(r *Request) (_ RequestResponse, err error) return nil, gcserr.NewHresultError(gcserr.HrVmcomputeUnsupportedProtocolVersion) } + // Enforce guest/host contract compatibility. A host that predates the + // contract advertises no range (MaxContractVersion == 0); skip enforcement + // for it during rollout. Otherwise require the host and guest contract + // ranges to overlap so a mispaired pair is rejected here rather than failing + // confusingly later. + if hostMax := request.MaxContractVersion; hostMax != 0 && + !gcscompat.Compatible( + gcscompat.MinCompatibleContractVersion, gcscompat.GuestHostContractVersion, + request.MinContractVersion, hostMax, + ) { + return nil, gcserr.NewHresultError(gcserr.HrVmcomputeUnsupportedProtocolVersion) + } + min := func(x, y uint32) uint32 { if x < y { return x diff --git a/internal/guest/prot/protocol.go b/internal/guest/prot/protocol.go index ea9e9a7d10..46f35a7ca8 100644 --- a/internal/guest/prot/protocol.go +++ b/internal/guest/prot/protocol.go @@ -270,6 +270,14 @@ type GcsCapabilities struct { // passed to a client of the HCS. This can be useful to pass runtime // specific capabilities not tied to the platform itself. GuestDefinedCapabilities GcsGuestCapabilities `json:",omitempty"` + // MinContractVersion and MaxContractVersion advertise this GCS's supported + // guest/host contract range (see internal/gcscompat). A zero + // MaxContractVersion means the guest predates the contract. + MinContractVersion uint32 `json:",omitempty"` + MaxContractVersion uint32 `json:",omitempty"` + // GcsCommit is the hcsshim source commit this GCS was built from, + // diagnostic only. + GcsCommit string `json:",omitempty"` } // GcsGuestCapabilities represents the customized guest capabilities supported @@ -307,6 +315,14 @@ type NegotiateProtocol struct { MessageBase MinimumVersion uint32 MaximumVersion uint32 + // MinContractVersion and MaxContractVersion advertise the host's supported + // guest/host contract range (see internal/gcscompat). They are additive and + // optional: a zero MaxContractVersion means the host predates the contract. + MinContractVersion uint32 `json:",omitempty"` + MaxContractVersion uint32 `json:",omitempty"` + // HostCommit is the hcsshim source commit the host was built from, + // diagnostic only. + HostCommit string `json:",omitempty"` } // ContainerCreate is the message from the HCS specifying to create a container