gcs: negotiate a guest/host contract version to detect drift

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"

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

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 {

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
+}

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")
+	}
+}

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)
+	}
+}