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