diff --git a/cmd/controller_setup.go b/cmd/controller_setup.go index 6b0d3b097..e75119643 100644 --- a/cmd/controller_setup.go +++ b/cmd/controller_setup.go @@ -869,6 +869,11 @@ func setupEnrollmentWatcher( namespace, ) + // Verify agent responses against the key recorded when each agent was + // accepted. The watcher owns those records, so verification reads what + // acceptance wrote and shares its cache. + b.jobClient.SetAgentKeyStore(watcher.KeyStore()) + go func() { if err := watcher.Start(ctx); err != nil { enrollLog.Warn( diff --git a/internal/agent/agent_public_test.go b/internal/agent/agent_public_test.go index 8ebccfdb7..8e3180e92 100644 --- a/internal/agent/agent_public_test.go +++ b/internal/agent/agent_public_test.go @@ -272,6 +272,10 @@ func (s *AgentPublicTestSuite) TestStart() { // wires it into the job client so responses are signed // once the agent is accepted. s.mockJobClient.EXPECT().SetPKISigner(gomock.Any()) + // Enrollment also stamps the agent's identity on what it + // signs, so the controller knows which stored key verifies + // its responses. + s.mockJobClient.EXPECT().SetMachineID(gomock.Any()) return newTestAgent(newTestAgentParams{ appFs: fs, diff --git a/internal/agent/enrollment.go b/internal/agent/enrollment.go index 75b21ad8d..412fa832d 100644 --- a/internal/agent/enrollment.go +++ b/internal/agent/enrollment.go @@ -61,6 +61,10 @@ func (a *Agent) handlePKIEnrollment( // only in tests that exercise enrollment in isolation. if a.jobClient != nil { a.jobClient.SetPKISigner(m) + // Stamp this agent's identity on what it signs, so the controller + // knows which stored key to verify its responses against. Start + // resolves the machine ID before this runs. + a.jobClient.SetMachineID(a.machineID) } a.pkiLogger.Info( diff --git a/internal/controller/enrollment/keystore_adapter.go b/internal/controller/enrollment/keystore_adapter.go new file mode 100644 index 000000000..6a8e91ce3 --- /dev/null +++ b/internal/controller/enrollment/keystore_adapter.go @@ -0,0 +1,68 @@ +// Copyright (c) 2026 John Dewey + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +package enrollment + +import ( + "context" + "errors" + + "github.com/osapi-io/osapi/internal/job/client" +) + +// clientKeyStore adapts the watcher's store to the narrow lookup the job +// client depends on. The job client stays free of this package: the +// dependency runs from the controller to the shared client, not back. +type clientKeyStore struct { + watcher *Watcher +} + +// KeyStore returns the watcher as the job client's agent key store, so +// response verification reads the same records acceptance writes and shares +// their cache. +func (w *Watcher) KeyStore() client.AgentKeyStore { + return clientKeyStore{watcher: w} +} + +// LookupAgentKey implements client.AgentKeyStore, translating this package's +// causes into the client's. The distinction between them is preserved: a +// missing record and an unreadable store never collapse into one answer. +func (s clientKeyStore) LookupAgentKey( + ctx context.Context, + machineID string, +) (*client.AgentKey, error) { + record, err := s.watcher.LookupAgentKey(ctx, machineID) + + switch { + case errors.Is(err, ErrAgentKeyNotFound): + return nil, client.ErrResponseKeyUnknown + case err != nil: + // Every other failure is a store that could not be read. It is + // never reported as "no stored key", which would read as an agent + // that has simply not enrolled yet. + return nil, client.ErrResponseStoreUnavailable + } + + return &client.AgentKey{ + MachineID: record.MachineID, + Hostname: record.Hostname, + PublicKey: record.PublicKey, + }, nil +} diff --git a/internal/controller/enrollment/keystore_adapter_public_test.go b/internal/controller/enrollment/keystore_adapter_public_test.go new file mode 100644 index 000000000..4d3e98775 --- /dev/null +++ b/internal/controller/enrollment/keystore_adapter_public_test.go @@ -0,0 +1,189 @@ +// Copyright (c) 2026 John Dewey + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +package enrollment_test + +import ( + "context" + "crypto/ed25519" + "encoding/json" + "errors" + "log/slog" + "testing" + "time" + + "github.com/nats-io/nats.go/jetstream" + "github.com/stretchr/testify/suite" + "go.uber.org/mock/gomock" + + "github.com/osapi-io/osapi/internal/agent/pki" + "github.com/osapi-io/osapi/internal/controller/enrollment" + enrollMocks "github.com/osapi-io/osapi/internal/controller/enrollment/mocks" + "github.com/osapi-io/osapi/internal/job/client" + jobMocks "github.com/osapi-io/osapi/internal/job/mocks" +) + +type KeyStoreAdapterPublicTestSuite struct { + suite.Suite + + ctx context.Context + mockCtrl *gomock.Controller + mockNC *enrollMocks.MockNATSSubscriber + mockKV *jobMocks.MockKeyValue + mockPKI *enrollMocks.MockPKIProvider + watcher *enrollment.Watcher + fixedTime time.Time + pubKey ed25519.PublicKey +} + +func (s *KeyStoreAdapterPublicTestSuite) SetupTest() { + s.ctx = context.Background() + s.mockCtrl = gomock.NewController(s.T()) + s.mockNC = enrollMocks.NewMockNATSSubscriber(s.mockCtrl) + s.mockKV = jobMocks.NewMockKeyValue(s.mockCtrl) + s.mockPKI = enrollMocks.NewMockPKIProvider(s.mockCtrl) + s.fixedTime = time.Date(2026, 4, 11, 12, 0, 0, 0, time.UTC) + s.pubKey = make(ed25519.PublicKey, ed25519.PublicKeySize) + + s.watcher = enrollment.NewWatcher( + slog.Default(), + s.mockNC, + s.mockKV, + s.mockPKI, + false, + "osapi", + ) +} + +func (s *KeyStoreAdapterPublicTestSuite) TearDownTest() { + s.mockCtrl.Finish() +} + +func (s *KeyStoreAdapterPublicTestSuite) SetupSubTest() { + // A fresh watcher per subtest so a record cached by an earlier case does + // not satisfy a lookup this case expects to reach the store. + s.watcher = enrollment.NewWatcher( + slog.Default(), + s.mockNC, + s.mockKV, + s.mockPKI, + false, + "osapi", + ) +} + +func (s *KeyStoreAdapterPublicTestSuite) TestLookupAgentKey() { + tests := []struct { + name string + machineID string + setupMock func() + validateFunc func(*client.AgentKey, error) + }{ + { + name: "returns the stored key and the enrolled hostname", + machineID: "machine-001", + setupMock: func() { + record := enrollment.AcceptedAgent{ + MachineID: "machine-001", + Hostname: "web-01", + PublicKey: s.pubKey, + Fingerprint: pki.FingerprintOf(s.pubKey), + AcceptedAt: s.fixedTime, + } + data, err := json.Marshal(record) + s.Require().NoError(err) + + entry := jobMocks.NewMockKeyValueEntry(s.mockCtrl) + entry.EXPECT().Value().Return(data) + s.mockKV.EXPECT(). + Get(gomock.Any(), "accepted.machine-001"). + Return(entry, nil) + }, + validateFunc: func(key *client.AgentKey, err error) { + s.Require().NoError(err) + s.Equal("machine-001", key.MachineID) + s.Equal("web-01", key.Hostname) + s.Equal(ed25519.PublicKey(s.pubKey), key.PublicKey) + }, + }, + { + name: "maps a missing record to the client's unknown-key cause", + machineID: "machine-404", + setupMock: func() { + s.mockKV.EXPECT(). + Get(gomock.Any(), "accepted.machine-404"). + Return(nil, jetstream.ErrKeyNotFound) + }, + validateFunc: func(key *client.AgentKey, err error) { + s.Require().Error(err) + s.Nil(key) + s.Require().ErrorIs(err, client.ErrResponseKeyUnknown) + s.NotErrorIs(err, client.ErrResponseStoreUnavailable) + }, + }, + { + name: "maps an unreadable store to the client's unavailable cause", + machineID: "machine-001", + setupMock: func() { + s.mockKV.EXPECT(). + Get(gomock.Any(), "accepted.machine-001"). + Return(nil, errors.New("kv error")) + }, + validateFunc: func(key *client.AgentKey, err error) { + s.Require().Error(err) + s.Nil(key) + s.Require().ErrorIs(err, client.ErrResponseStoreUnavailable) + s.NotErrorIs(err, client.ErrResponseKeyUnknown) + }, + }, + { + name: "maps an undecodable record to the unavailable cause", + machineID: "machine-001", + setupMock: func() { + entry := jobMocks.NewMockKeyValueEntry(s.mockCtrl) + entry.EXPECT().Value().Return([]byte("bad json")) + s.mockKV.EXPECT(). + Get(gomock.Any(), "accepted.machine-001"). + Return(entry, nil) + }, + validateFunc: func(key *client.AgentKey, err error) { + s.Require().Error(err) + s.Nil(key) + s.Require().ErrorIs(err, client.ErrResponseStoreUnavailable) + }, + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + tt.setupMock() + + tt.validateFunc( + s.watcher.KeyStore().LookupAgentKey(s.ctx, tt.machineID), + ) + }) + } +} + +func TestKeyStoreAdapterPublicTestSuite( + t *testing.T, +) { + suite.Run(t, new(KeyStoreAdapterPublicTestSuite)) +} diff --git a/internal/job/client/agent.go b/internal/job/client/agent.go index 941f87af3..596b4763d 100644 --- a/internal/job/client/agent.go +++ b/internal/job/client/agent.go @@ -134,7 +134,11 @@ func (c *Client) WriteJobResponse( // Sign the response when PKI is enabled. kvPayload := responseJSON if c.pkiSigner != nil { - signed, signErr := wrapInSignedEnvelope(c.pkiSigner, responseJSON) + signed, signErr := wrapInSignedEnvelope( + c.pkiSigner, + c.machineID, + responseJSON, + ) if signErr != nil { return fmt.Errorf("failed to sign response: %w", signErr) } diff --git a/internal/job/client/client.go b/internal/job/client/client.go index e81648751..ceec2721a 100644 --- a/internal/job/client/client.go +++ b/internal/job/client/client.go @@ -54,6 +54,15 @@ type Client struct { // targetResolver resolves a target (hostname or machine ID) to the // value used for NATS subject routing. When nil, the target is used as-is. targetResolver func(string) string + // agentKeyStore verifies agent responses against the key recorded when + // the agent was accepted. Nil when the controller is not enforcing, in + // which case responses are handled exactly as they were before + // verification existed. + agentKeyStore AgentKeyStore + // machineID identifies this client's host on payloads it signs, so the + // verifier knows which stored key to check them against. Empty on the + // controller. + machineID string } // Options configures the jobs client. @@ -115,6 +124,22 @@ func (c *Client) SetPKISigner( c.pkiSigner = signer } +// SetAgentKeyStore wires the store used to verify agent responses. See the +// JobClient interface for why this is separate from construction. Passing nil +// leaves responses unverified. +func (c *Client) SetAgentKeyStore( + store AgentKeyStore, +) { + c.agentKeyStore = store +} + +// SetMachineID records the identity stamped on payloads this client signs. +func (c *Client) SetMachineID( + machineID string, +) { + c.machineID = machineID +} + // resolveTarget resolves a target (hostname or machine ID) to the routing // value for NATS subjects. When no resolver is configured, returns the // target unchanged. @@ -292,7 +317,7 @@ func (c *Client) publishAndWait( // Sign the job data when PKI is enabled. kvPayload := jobJSON if c.pkiSigner != nil { - signed, signErr := wrapInSignedEnvelope(c.pkiSigner, jobJSON) + signed, signErr := wrapInSignedEnvelope(c.pkiSigner, c.machineID, jobJSON) if signErr != nil { return "", nil, fmt.Errorf("failed to sign job data: %w", signErr) } @@ -355,7 +380,25 @@ func (c *Client) publishAndWait( // job failure rather than fallen through as unverified data // that happens to fail to unmarshal into a useful response. responseData := entry.Value() - if c.pkiSigner != nil { + + switch { + case c.agentKeyStore != nil: + // Enforcing: the response is verified against the key + // stored for the agent that claims to have sent it, and + // the job fails rather than returning unverified data. + verified, verifyErr := verifyAgentResponse( + ctx, + c.agentKeyStore, + responseData, + ) + if verifyErr != nil { + return "", nil, fmt.Errorf( + "response verification failed for job %s: %w", + jobID, verifyErr, + ) + } + responseData = verified + case c.pkiSigner != nil: unwrapped, _, unwrapErr := unwrapSignedEnvelope( responseData, c.pkiSigner.ControllerPublicKey(), @@ -424,7 +467,7 @@ func (c *Client) publishAndCollect( // Sign the job data when PKI is enabled. kvPayload := jobJSON if c.pkiSigner != nil { - signed, signErr := wrapInSignedEnvelope(c.pkiSigner, jobJSON) + signed, signErr := wrapInSignedEnvelope(c.pkiSigner, c.machineID, jobJSON) if signErr != nil { return "", nil, fmt.Errorf("failed to sign job data: %w", signErr) } @@ -528,7 +571,29 @@ func (c *Client) publishAndCollect( // shows as missing rather than answered, and surfaces as a // timeout for that hostname if it never sends a good response. responseData := entry.Value() - if c.pkiSigner != nil { + + switch { + case c.agentKeyStore != nil: + // Enforcing: a response that does not verify is not this + // agent's reply. It is dropped, so the agent shows as not + // having answered rather than as answered by whoever sent + // it. + verified, verifyErr := verifyAgentResponse( + ctx, + c.agentKeyStore, + responseData, + ) + if verifyErr != nil { + c.logger.WarnContext( + ctx, "broadcast response verification failed", + slog.String("job_id", jobID), + slog.String("error", verifyErr.Error()), + ) + + continue + } + responseData = verified + case c.pkiSigner != nil: unwrapped, _, unwrapErr := unwrapSignedEnvelope( responseData, c.pkiSigner.ControllerPublicKey(), @@ -539,6 +604,7 @@ func (c *Client) publishAndCollect( slog.String("job_id", jobID), slog.String("error", unwrapErr.Error()), ) + continue } responseData = unwrapped diff --git a/internal/job/client/client_public_test.go b/internal/job/client/client_public_test.go index ea9e0f4e8..f0c63c889 100644 --- a/internal/job/client/client_public_test.go +++ b/internal/job/client/client_public_test.go @@ -22,6 +22,8 @@ package client_test import ( "context" + "crypto/ed25519" + "crypto/rand" "encoding/json" "errors" "fmt" @@ -1336,7 +1338,7 @@ func (s *ClientPublicTestSuite) TestQueryWithPKISignerUnwrapPaths() { name: "when response is a valid signed envelope unwraps successfully", responseData: func() []byte { inner := []byte(`{"status":"completed","hostname":"server1"}`) - wrapped, _ := client.ExportWrapInSignedEnvelope(signer, inner) + wrapped, _ := client.ExportWrapInSignedEnvelope(signer, "", inner) return wrapped }, validateFunc: func(_ string, resp *job.Response) { @@ -1349,7 +1351,7 @@ func (s *ClientPublicTestSuite) TestQueryWithPKISignerUnwrapPaths() { responseData: func() []byte { // Build an envelope with a tampered signature so verification fails. inner := []byte(`{"status":"completed","hostname":"server1"}`) - wrapped, _ := client.ExportWrapInSignedEnvelope(signer, inner) + wrapped, _ := client.ExportWrapInSignedEnvelope(signer, "", inner) var envelope job.SignedEnvelope _ = json.Unmarshal(wrapped, &envelope) envelope.Signature[0] ^= 0xFF @@ -1453,7 +1455,7 @@ func (s *ClientPublicTestSuite) TestModifyBroadcastWithPKISigner() { // Return a signed envelope response. inner := []byte(`{"status":"completed","hostname":"server1"}`) - wrapped, _ := client.ExportWrapInSignedEnvelope(signerWithCtrl, inner) + wrapped, _ := client.ExportWrapInSignedEnvelope(signerWithCtrl, "", inner) mockEntry := jobmocks.NewMockKeyValueEntry(s.mockCtrl) mockEntry.EXPECT().Value().Return(wrapped) @@ -1500,7 +1502,7 @@ func (s *ClientPublicTestSuite) TestModifyBroadcastWithPKISigner() { // Return a signed envelope with tampered signature. inner := []byte(`{"status":"completed","hostname":"server1"}`) - wrapped, _ := client.ExportWrapInSignedEnvelope(signerWithCtrl, inner) + wrapped, _ := client.ExportWrapInSignedEnvelope(signerWithCtrl, "", inner) var envelope job.SignedEnvelope _ = json.Unmarshal(wrapped, &envelope) envelope.Signature[0] ^= 0xFF @@ -1869,3 +1871,255 @@ func setupPublishAndWaitMocksWithOpts( Watch(gomock.Any(), gomock.Any()). Return(mockWatcher, nil) } + +// TestQueryWithAgentKeyStoreEnforcing covers the single-target response path +// once the controller holds agents' keys: a response is a result only when it +// verifies against the key stored for the agent that claims to have sent it. +func (s *ClientPublicTestSuite) TestQueryWithAgentKeyStoreEnforcing() { + const ( + target = "server1" + category = "node" + operation = job.OperationType("node.hostname.get") + subject = "jobs.query.host.server1" + ) + + tests := []struct { + name string + responseData func(store *jobmocks.MockAgentKeyStore) []byte + wantErr string + validateFunc func(resp *job.Response) + }{ + { + name: "a response signed by the stored key is a result", + responseData: func(store *jobmocks.MockAgentKeyStore) []byte { + signer, pub := newSigner(s.mockCtrl) + store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(&client.AgentKey{ + MachineID: "machine-001", + Hostname: "server1", + PublicKey: pub, + }, nil) + + inner := []byte(`{"status":"completed","hostname":"server1"}`) + wrapped, _ := client.ExportWrapInSignedEnvelope( + signer, "machine-001", inner, + ) + + return wrapped + }, + validateFunc: func(resp *job.Response) { + s.Require().NotNil(resp) + s.Equal(job.StatusCompleted, resp.Status) + }, + }, + { + name: "a forged response fails the job rather than returning data", + responseData: func(store *jobmocks.MockAgentKeyStore) []byte { + signer, _ := newSigner(s.mockCtrl) + + // The stored record holds a different key, so the signature + // does not verify. + otherPub, _, _ := ed25519.GenerateKey(rand.Reader) + store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(&client.AgentKey{ + MachineID: "machine-001", + Hostname: "server1", + PublicKey: otherPub, + }, nil) + + inner := []byte(`{"status":"completed","hostname":"server1"}`) + wrapped, _ := client.ExportWrapInSignedEnvelope( + signer, "machine-001", inner, + ) + + return wrapped + }, + wantErr: "response verification failed", + }, + { + name: "an agent with no stored key is refused while enforcing", + responseData: func(store *jobmocks.MockAgentKeyStore) []byte { + signer, _ := newSigner(s.mockCtrl) + store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(nil, client.ErrResponseKeyUnknown) + + inner := []byte(`{"status":"completed","hostname":"server1"}`) + wrapped, _ := client.ExportWrapInSignedEnvelope( + signer, "machine-001", inner, + ) + + return wrapped + }, + wantErr: "no stored key for responding agent", + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + store := jobmocks.NewMockAgentKeyStore(s.mockCtrl) + data := tt.responseData(store) + + opts := &client.Options{ + Timeout: 30 * time.Second, + KVBucket: s.mockKV, + StreamName: "JOBS", + } + c, err := client.New(slog.Default(), s.mockNATSClient, opts) + s.Require().NoError(err) + c.SetAgentKeyStore(store) + + s.mockKV.EXPECT(). + Put(gomock.Any(), gomock.Any(), gomock.Any()). + Return(uint64(1), nil) + s.mockNATSClient.EXPECT(). + Publish(gomock.Any(), subject, gomock.Any()). + Return(nil) + + mockEntry := jobmocks.NewMockKeyValueEntry(s.mockCtrl) + mockEntry.EXPECT().Value().Return(data) + ch := make(chan jetstream.KeyValueEntry, 1) + ch <- mockEntry + + mockWatcher := jobmocks.NewMockKeyWatcher(s.mockCtrl) + mockWatcher.EXPECT().Updates().Return(ch).AnyTimes() + mockWatcher.EXPECT().Stop().Return(nil) + + s.mockKV.EXPECT(). + Watch(gomock.Any(), gomock.Any()). + Return(mockWatcher, nil) + + _, resp, err := c.Query(s.ctx, target, category, operation, nil) + if tt.wantErr != "" { + s.Require().Error(err) + s.Contains(err.Error(), tt.wantErr) + + return + } + + s.Require().NoError(err) + tt.validateFunc(resp) + }) + } +} + +// TestModifyBroadcastWithAgentKeyStoreEnforcing covers the broadcast path: a +// response that does not verify is not that agent's reply, so the agent shows +// as not having answered rather than as answered by whoever sent it. +func (s *ClientPublicTestSuite) TestModifyBroadcastWithAgentKeyStoreEnforcing() { + const ( + target = "_all" + category = "node" + operation = job.OperationType("node.hostname.get") + subject = "jobs.modify._all" + ) + + tests := []struct { + name string + responseData func(store *jobmocks.MockAgentKeyStore) []byte + expectedErr string + validateFunc func(responses map[string]*job.Response) + }{ + { + name: "a verified broadcast response counts as the agent's reply", + responseData: func(store *jobmocks.MockAgentKeyStore) []byte { + signer, pub := newSigner(s.mockCtrl) + store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(&client.AgentKey{ + MachineID: "machine-001", + Hostname: "server1", + PublicKey: pub, + }, nil) + + inner := []byte(`{"status":"completed","hostname":"server1"}`) + wrapped, _ := client.ExportWrapInSignedEnvelope( + signer, "machine-001", inner, + ) + + return wrapped + }, + validateFunc: func(responses map[string]*job.Response) { + s.Len(responses, 1) + s.Equal(job.StatusCompleted, responses["server1"].Status) + }, + }, + { + name: "a response from an agent answering for another host is dropped", + responseData: func(store *jobmocks.MockAgentKeyStore) []byte { + signer, pub := newSigner(s.mockCtrl) + + // Signature verifies, but this agent enrolled as evil-01. + store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-evil"). + Return(&client.AgentKey{ + MachineID: "machine-evil", + Hostname: "evil-01", + PublicKey: pub, + }, nil) + + inner := []byte(`{"status":"completed","hostname":"server1"}`) + wrapped, _ := client.ExportWrapInSignedEnvelope( + signer, "machine-evil", inner, + ) + + return wrapped + }, + expectedErr: "no agents responded", + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + store := jobmocks.NewMockAgentKeyStore(s.mockCtrl) + data := tt.responseData(store) + + registryKV := setupRegistryKV(s.mockCtrl, []string{"server1"}) + + opts := &client.Options{ + Timeout: 1 * time.Second, + KVBucket: s.mockKV, + StreamName: "JOBS", + RegistryKV: registryKV, + } + c, err := client.New(slog.Default(), s.mockNATSClient, opts) + s.Require().NoError(err) + c.SetAgentKeyStore(store) + + s.mockKV.EXPECT(). + Put(gomock.Any(), gomock.Any(), gomock.Any()). + Return(uint64(1), nil) + s.mockNATSClient.EXPECT(). + Publish(gomock.Any(), subject, gomock.Any()). + Return(nil) + + mockEntry := jobmocks.NewMockKeyValueEntry(s.mockCtrl) + mockEntry.EXPECT().Value().Return(data) + ch := make(chan jetstream.KeyValueEntry, 1) + ch <- mockEntry + + mockWatcher := jobmocks.NewMockKeyWatcher(s.mockCtrl) + mockWatcher.EXPECT().Updates().Return(ch).AnyTimes() + mockWatcher.EXPECT().Stop().Return(nil) + + s.mockKV.EXPECT(). + Watch(gomock.Any(), gomock.Any()). + Return(mockWatcher, nil) + + _, responses, err := c.ModifyBroadcast( + s.ctx, target, category, operation, nil, + ) + if tt.expectedErr != "" { + s.Require().Error(err) + s.Contains(err.Error(), tt.expectedErr) + + return + } + + s.Require().NoError(err) + tt.validateFunc(responses) + }) + } +} diff --git a/internal/job/client/export_test.go b/internal/job/client/export_test.go index 0dc8f7cbb..2da41b876 100644 --- a/internal/job/client/export_test.go +++ b/internal/job/client/export_test.go @@ -20,7 +20,10 @@ package client -import "encoding/json" +import ( + "context" + "encoding/json" +) // ExportSanitizeKeyForNATS exposes the private sanitizeKeyForNATS for testing. func ExportSanitizeKeyForNATS( @@ -32,9 +35,20 @@ func ExportSanitizeKeyForNATS( // ExportWrapInSignedEnvelope exposes the private wrapInSignedEnvelope for testing. func ExportWrapInSignedEnvelope( signer PKISigner, + machineID string, payload []byte, ) ([]byte, error) { - return wrapInSignedEnvelope(signer, payload) + return wrapInSignedEnvelope(signer, machineID, payload) +} + +// ExportVerifyAgentResponse exposes the private verifyAgentResponse for +// testing. +func ExportVerifyAgentResponse( + ctx context.Context, + store AgentKeyStore, + data []byte, +) ([]byte, error) { + return verifyAgentResponse(ctx, store, data) } // ExportUnwrapSignedEnvelope exposes the private unwrapSignedEnvelope for testing. diff --git a/internal/job/client/jobs.go b/internal/job/client/jobs.go index 4263c925b..c9671e1b7 100644 --- a/internal/job/client/jobs.go +++ b/internal/job/client/jobs.go @@ -95,7 +95,11 @@ func (c *Client) CreateJob( // Sign the job data when PKI is enabled. kvPayload := jobWithStatusJSON if c.pkiSigner != nil { - signed, signErr := wrapInSignedEnvelope(c.pkiSigner, jobWithStatusJSON) + signed, signErr := wrapInSignedEnvelope( + c.pkiSigner, + c.machineID, + jobWithStatusJSON, + ) if signErr != nil { return nil, fmt.Errorf("failed to sign job data: %w", signErr) } @@ -925,9 +929,22 @@ func (c *Client) getJobResponses( continue } - // Unwrap signed envelope if PKI is enabled. + // Verify against the responding agent's stored key when enforcing, + // otherwise unwrap as before. responseData := entry.Value() - if c.pkiSigner != nil { + + switch { + case c.agentKeyStore != nil: + verified, verifyErr := verifyAgentResponse( + ctx, + c.agentKeyStore, + responseData, + ) + if verifyErr != nil { + continue + } + responseData = verified + case c.pkiSigner != nil: unwrapped, _, unwrapErr := unwrapSignedEnvelope( responseData, c.pkiSigner.ControllerPublicKey(), diff --git a/internal/job/client/jobs_public_test.go b/internal/job/client/jobs_public_test.go index 1a33cf5a3..4b9726ccc 100644 --- a/internal/job/client/jobs_public_test.go +++ b/internal/job/client/jobs_public_test.go @@ -2469,7 +2469,7 @@ func (s *JobsPublicTestSuite) TestGetJobStatusWithPKISigner() { inner := []byte( `{"status":"completed","hostname":"server1","data":{"hostname":"web-01"}}`, ) - wrapped, _ := client.ExportWrapInSignedEnvelope(signer, inner) + wrapped, _ := client.ExportWrapInSignedEnvelope(signer, "", inner) respEntry := jobmocks.NewMockKeyValueEntry(s.mockCtrl) respEntry.EXPECT().Value().Return(wrapped) @@ -2513,7 +2513,7 @@ func (s *JobsPublicTestSuite) TestGetJobStatusWithPKISigner() { // Build a valid signed envelope then corrupt the signature. inner := []byte(`{"status":"completed","hostname":"server1"}`) - wrapped, _ := client.ExportWrapInSignedEnvelope(signer, inner) + wrapped, _ := client.ExportWrapInSignedEnvelope(signer, "", inner) var envelope job.SignedEnvelope _ = json.Unmarshal(wrapped, &envelope) envelope.Signature[0] ^= 0xFF @@ -2559,3 +2559,215 @@ func TestJobsPublicTestSuite( ) { suite.Run(t, new(JobsPublicTestSuite)) } + +// TestSetAgentKeyStoreAndMachineID verifies the two wiring calls cmd/ makes +// once identity is known: the controller hands the client the store that +// verifies agent responses, and the agent stamps its machine ID on what it +// signs so the controller knows which stored key to check it against. +func (s *JobsPublicTestSuite) TestSetAgentKeyStoreAndMachineID() { + signer, _ := newSigner(gomock.NewController(s.T())) + + opData := map[string]interface{}{ + "type": "node.hostname.get", + "data": map[string]interface{}{}, + } + + tests := []struct { + name string + wireFn func(c *client.Client) + validateFunc func(storedJobData []byte) + }{ + { + name: "when a machine ID is set it is stamped on signed payloads", + wireFn: func(c *client.Client) { + c.SetPKISigner(signer) + c.SetMachineID("machine-001") + }, + validateFunc: func(storedJobData []byte) { + var envelope job.SignedEnvelope + s.Require().NoError(json.Unmarshal(storedJobData, &envelope)) + s.Equal("machine-001", envelope.MachineID) + }, + }, + { + name: "when no machine ID is set the envelope carries none", + wireFn: func(c *client.Client) { + c.SetPKISigner(signer) + }, + validateFunc: func(storedJobData []byte) { + var envelope job.SignedEnvelope + s.Require().NoError(json.Unmarshal(storedJobData, &envelope)) + s.Empty(envelope.MachineID) + }, + }, + { + name: "when the key store is cleared responses are not verified", + wireFn: func(c *client.Client) { + c.SetPKISigner(signer) + c.SetAgentKeyStore(jobmocks.NewMockAgentKeyStore(s.mockCtrl)) + c.SetAgentKeyStore(nil) + }, + validateFunc: func(storedJobData []byte) { + // Nothing to verify against: the job still signs as before, + // which is the pre-enforcement behaviour. + var envelope job.SignedEnvelope + s.Require().NoError(json.Unmarshal(storedJobData, &envelope)) + s.NotEmpty(envelope.Signature) + }, + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + opts := &client.Options{ + Timeout: 30 * time.Second, + KVBucket: s.mockKV, + StreamName: "JOBS", + } + c, err := client.New(slog.Default(), s.mockNATSClient, opts) + s.Require().NoError(err) + tt.wireFn(c) + + var storedJobData []byte + s.mockKV.EXPECT().Bucket().Return("test-bucket").AnyTimes() + s.mockKV.EXPECT(). + Put(gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, key string, data []byte) (uint64, error) { + if strings.HasPrefix(key, "jobs.") { + storedJobData = data + } + + return uint64(1), nil + }). + Times(2) + s.mockNATSClient.EXPECT(). + Publish(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil) + + _, err = c.CreateJob(s.ctx, opData, "_any") + s.Require().NoError(err) + + tt.validateFunc(storedJobData) + }) + } +} + +// TestGetJobStatusWithAgentKeyStoreEnforcing covers the stored-response path: +// a recorded response is only reported once it verifies against the key held +// for the agent that claims to have written it. +func (s *JobsPublicTestSuite) TestGetJobStatusWithAgentKeyStoreEnforcing() { + jobID := "enforce-job-123" + + setupJobAndKeys := func(responseKey string) { + jobEntry := jobmocks.NewMockKeyValueEntry(s.mockCtrl) + jobEntry.EXPECT().Value().Return([]byte(fmt.Sprintf( + `{"id":"%s","status":"unprocessed","created":"2026-01-01T00:00:00Z","subject":"jobs.query.host.server1","operation":{"type":"node.hostname.get"}}`, + jobID, + ))) + s.mockKV.EXPECT(). + Get(gomock.Any(), "jobs."+jobID). + Return(jobEntry, nil) + + statusLister := newMockKeyLister(s.mockCtrl, []string{}) + s.mockKV.EXPECT(). + ListKeysFiltered(gomock.Any(), "status."+jobID+".>"). + Return(statusLister, nil) + + responseLister := newMockKeyLister(s.mockCtrl, []string{responseKey}) + s.mockKV.EXPECT(). + ListKeysFiltered(gomock.Any(), "responses."+jobID+".>"). + Return(responseLister, nil) + } + + tests := []struct { + name string + setupMocks func(store *jobmocks.MockAgentKeyStore) + validateFunc func(qj *job.QueuedJob) + }{ + { + name: "a response verified against the stored key is reported", + setupMocks: func(store *jobmocks.MockAgentKeyStore) { + responseKey := "responses." + jobID + ".server1.12345" + setupJobAndKeys(responseKey) + + signer, pub := newSigner(s.mockCtrl) + store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(&client.AgentKey{ + MachineID: "machine-001", + Hostname: "server1", + PublicKey: pub, + }, nil) + + inner := []byte( + `{"status":"completed","hostname":"server1","data":{"hostname":"web-01"}}`, + ) + wrapped, _ := client.ExportWrapInSignedEnvelope( + signer, "machine-001", inner, + ) + + respEntry := jobmocks.NewMockKeyValueEntry(s.mockCtrl) + respEntry.EXPECT().Value().Return(wrapped) + s.mockKV.EXPECT(). + Get(gomock.Any(), responseKey). + Return(respEntry, nil) + }, + validateFunc: func(qj *job.QueuedJob) { + s.Require().NotNil(qj) + s.Len(qj.Responses, 1) + resp, ok := qj.Responses["server1"] + s.True(ok) + s.Equal("completed", string(resp.Status)) + }, + }, + { + name: "a response from an agent with no stored key is skipped", + setupMocks: func(store *jobmocks.MockAgentKeyStore) { + responseKey := "responses." + jobID + ".server1.12345" + setupJobAndKeys(responseKey) + + signer, _ := newSigner(s.mockCtrl) + store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(nil, client.ErrResponseKeyUnknown) + + inner := []byte(`{"status":"completed","hostname":"server1"}`) + wrapped, _ := client.ExportWrapInSignedEnvelope( + signer, "machine-001", inner, + ) + + respEntry := jobmocks.NewMockKeyValueEntry(s.mockCtrl) + respEntry.EXPECT().Value().Return(wrapped) + s.mockKV.EXPECT(). + Get(gomock.Any(), responseKey). + Return(respEntry, nil) + }, + validateFunc: func(qj *job.QueuedJob) { + // Not reported as a result: an unverified response is not + // this agent's answer. + s.Require().NotNil(qj) + s.Len(qj.Responses, 0) + }, + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + store := jobmocks.NewMockAgentKeyStore(s.mockCtrl) + tt.setupMocks(store) + + opts := &client.Options{ + Timeout: 30 * time.Second, + KVBucket: s.mockKV, + StreamName: "JOBS", + } + c, err := client.New(slog.Default(), s.mockNATSClient, opts) + s.Require().NoError(err) + c.SetAgentKeyStore(store) + + qj, err := c.GetJobStatus(s.ctx, jobID) + s.NoError(err) + tt.validateFunc(qj) + }) + } +} diff --git a/internal/job/client/signing.go b/internal/job/client/signing.go index 146d411f9..2f1a55365 100644 --- a/internal/job/client/signing.go +++ b/internal/job/client/signing.go @@ -21,6 +21,7 @@ package client import ( + "context" "crypto/ed25519" "encoding/json" "fmt" @@ -33,9 +34,12 @@ import ( var signingMarshalFn = json.Marshal // wrapInSignedEnvelope signs the payload and wraps it in a SignedEnvelope. +// machineID identifies the signer so the verifier can find the key to check +// the signature against; it is empty for controller-signed payloads. // Returns the JSON-encoded envelope. func wrapInSignedEnvelope( signer PKISigner, + machineID string, payload []byte, ) ([]byte, error) { signature := signer.Sign(payload) @@ -43,6 +47,7 @@ func wrapInSignedEnvelope( Payload: payload, Signature: signature, Fingerprint: signer.Fingerprint(), + MachineID: machineID, } envelopeJSON, err := signingMarshalFn(envelope) @@ -85,3 +90,65 @@ func unwrapSignedEnvelope( return envelope.Payload, true, nil } + +// verifyAgentResponse checks a response against the key stored for the agent +// that claims to have sent it, and returns the inner payload. +// +// The envelope's machine ID is self-reported, so it is used only to choose +// which stored record to verify against: a response naming another agent is +// checked against that agent's key and fails unless it was actually signed by +// it. The hostname in the payload must also match the one recorded at +// acceptance, so an accepted agent cannot answer for a host it did not enrol +// as. +// +// Every rejection carries a distinct cause. There is no path that returns the +// payload unverified. +func verifyAgentResponse( + ctx context.Context, + store AgentKeyStore, + data []byte, +) ([]byte, error) { + var envelope job.SignedEnvelope + if err := json.Unmarshal(data, &envelope); err != nil { + return nil, ErrResponseNotSigned + } + + if len(envelope.Payload) == 0 || len(envelope.Signature) == 0 || + envelope.MachineID == "" { + return nil, ErrResponseNotSigned + } + + record, err := store.LookupAgentKey(ctx, envelope.MachineID) + if err != nil { + return nil, err + } + + if !ed25519.Verify(record.PublicKey, envelope.Payload, envelope.Signature) { + return nil, fmt.Errorf( + "%w: machine %s", + ErrResponseSignatureInvalid, + envelope.MachineID, + ) + } + + var response job.Response + if err := json.Unmarshal(envelope.Payload, &response); err != nil { + return nil, fmt.Errorf( + "%w: machine %s payload is not a response", + ErrResponseNotSigned, + envelope.MachineID, + ) + } + + if response.Hostname != record.Hostname { + return nil, fmt.Errorf( + "%w: machine %s enrolled as %q but answered as %q", + ErrResponseHostnameMismatch, + envelope.MachineID, + record.Hostname, + response.Hostname, + ) + } + + return envelope.Payload, nil +} diff --git a/internal/job/client/signing_public_test.go b/internal/job/client/signing_public_test.go index abfbd00e4..3491e59e3 100644 --- a/internal/job/client/signing_public_test.go +++ b/internal/job/client/signing_public_test.go @@ -149,7 +149,7 @@ func (s *SigningPublicTestSuite) TestWrapInSignedEnvelope() { signer, pubKey := newSigner(gomock.NewController(s.T())) - result, err := client.ExportWrapInSignedEnvelope(signer, tt.payload) + result, err := client.ExportWrapInSignedEnvelope(signer, "", tt.payload) tt.validateFunc(result, err, pubKey) }) @@ -172,7 +172,7 @@ func (s *SigningPublicTestSuite) TestUnwrapSignedEnvelope() { name: "when valid signed envelope with correct key", setupData: func() []byte { payload := []byte(`{"id":"test"}`) - wrapped, _ := client.ExportWrapInSignedEnvelope(signer, payload) + wrapped, _ := client.ExportWrapInSignedEnvelope(signer, "", payload) return wrapped }, pubKey: pubKey, @@ -186,7 +186,7 @@ func (s *SigningPublicTestSuite) TestUnwrapSignedEnvelope() { name: "when valid signed envelope with nil key skips verification", setupData: func() []byte { payload := []byte(`{"id":"test"}`) - wrapped, _ := client.ExportWrapInSignedEnvelope(signer, payload) + wrapped, _ := client.ExportWrapInSignedEnvelope(signer, "", payload) return wrapped }, pubKey: nil, @@ -200,7 +200,7 @@ func (s *SigningPublicTestSuite) TestUnwrapSignedEnvelope() { name: "when valid signed envelope with wrong key fails verification", setupData: func() []byte { payload := []byte(`{"id":"test"}`) - wrapped, _ := client.ExportWrapInSignedEnvelope(signer, payload) + wrapped, _ := client.ExportWrapInSignedEnvelope(signer, "", payload) return wrapped }, pubKey: func() ed25519.PublicKey { @@ -278,7 +278,7 @@ func (s *SigningPublicTestSuite) TestRoundTrip() { originalPayload := []byte(`{"id":"round-trip-test","status":"unprocessed"}`) // Wrap - wrapped, err := client.ExportWrapInSignedEnvelope(signer, originalPayload) + wrapped, err := client.ExportWrapInSignedEnvelope(signer, "", originalPayload) s.NoError(err) // Unwrap with correct key diff --git a/internal/job/client/types.go b/internal/job/client/types.go index d22748bf3..18fb2a1f3 100644 --- a/internal/job/client/types.go +++ b/internal/job/client/types.go @@ -23,6 +23,7 @@ package client import ( "context" "crypto/ed25519" + "errors" "github.com/nats-io/nats.go/jetstream" natsclient "github.com/osapi-io/nats-client/pkg/client" @@ -41,6 +42,62 @@ type PKISigner interface { ControllerPublicKey() ed25519.PublicKey } +// AgentKey is what the controller holds for an accepted agent: the key its +// messages are verified against, and the hostname it enrolled under. +type AgentKey struct { + // MachineID is the permanent host identifier the record is keyed by. + MachineID string + // Hostname is the hostname recorded at acceptance. A response claiming + // a different hostname is not this agent's to answer for. + Hostname string + // PublicKey is the key recorded at acceptance. + PublicKey ed25519.PublicKey +} + +// AgentKeyStore looks up an accepted agent's key by machine ID. Nil when the +// controller is not enforcing response verification, which leaves behaviour +// exactly as it was before verification existed. +// +// Implemented on the controller by the enrollment watcher, which is the only +// thing allowed to write the underlying records. +type AgentKeyStore interface { + LookupAgentKey( + ctx context.Context, + machineID string, + ) (*AgentKey, error) +} + +// The causes of a rejected response stay distinct so an operator can tell an +// agent that has not re-enrolled yet from one whose messages are being +// forged, and both from a store that cannot be read. None of them ever means +// "treat as verified". +var ( + // ErrResponseNotSigned means the response carried no signed envelope. + // Expected from an agent that has not been upgraded; never accepted + // while enforcing. + ErrResponseNotSigned = errors.New("response is not a signed envelope") + + // ErrResponseKeyUnknown means no key is stored for the machine ID the + // response claims. Expected during a rollout, before that agent + // re-enrolls. + ErrResponseKeyUnknown = errors.New("no stored key for responding agent") + + // ErrResponseStoreUnavailable means the store could not be read. It is + // never mistaken for "no stored key". + ErrResponseStoreUnavailable = errors.New("agent key store unavailable") + + // ErrResponseSignatureInvalid means a key that is not the stored key + // for that agent signed the response. + ErrResponseSignatureInvalid = errors.New("invalid agent signature on response") + + // ErrResponseHostnameMismatch means the signature verified, but the + // response claims a hostname the signing agent did not enroll under — + // an accepted agent answering for a host that is not its own. + ErrResponseHostnameMismatch = errors.New( + "response hostname does not match the enrolled hostname", + ) +) + const ( // DefaultPageSize is the default number of jobs per page. DefaultPageSize = 10 @@ -194,6 +251,22 @@ type JobClient interface { SetPKISigner( signer PKISigner, ) + + // SetAgentKeyStore wires the store used to verify agent responses. Like + // the signer, it is not available at construction: on the controller it + // arrives with the enrollment watcher. Passing nil leaves responses + // unverified, which is the behaviour before this feature. + SetAgentKeyStore( + store AgentKeyStore, + ) + + // SetMachineID records the identity stamped on payloads this client + // signs, so the verifier knows which stored key to check them against. + // Set on the agent once its identity is resolved; empty on the + // controller, whose payloads agents verify against the controller key. + SetMachineID( + machineID string, + ) } // CreateJobResult represents the result of creating a job. diff --git a/internal/job/client/verify_public_test.go b/internal/job/client/verify_public_test.go new file mode 100644 index 000000000..59502d775 --- /dev/null +++ b/internal/job/client/verify_public_test.go @@ -0,0 +1,286 @@ +// Copyright (c) 2026 John Dewey + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +package client_test + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "encoding/json" + "testing" + + "github.com/stretchr/testify/suite" + "go.uber.org/mock/gomock" + + "github.com/osapi-io/osapi/internal/job" + "github.com/osapi-io/osapi/internal/job/client" + jobmocks "github.com/osapi-io/osapi/internal/job/mocks" +) + +type VerifyAgentResponsePublicTestSuite struct { + suite.Suite + + ctx context.Context + mockCtrl *gomock.Controller + store *jobmocks.MockAgentKeyStore +} + +func (s *VerifyAgentResponsePublicTestSuite) SetupTest() { + s.ctx = context.Background() + s.mockCtrl = gomock.NewController(s.T()) + s.store = jobmocks.NewMockAgentKeyStore(s.mockCtrl) +} + +func (s *VerifyAgentResponsePublicTestSuite) TearDownTest() { + s.mockCtrl.Finish() +} + +// signedResponse returns a response envelope signed by signer and naming +// machineID as the sender. +func (s *VerifyAgentResponsePublicTestSuite) signedResponse( + signer client.PKISigner, + machineID string, + payload []byte, +) []byte { + s.T().Helper() + + wrapped, err := client.ExportWrapInSignedEnvelope(signer, machineID, payload) + s.Require().NoError(err) + + return wrapped +} + +func (s *VerifyAgentResponsePublicTestSuite) TestVerifyAgentResponse() { + tests := []struct { + name string + setupData func() []byte + setupMock func() + validateFunc func([]byte, error) + }{ + { + name: "accepts a response signed by the agent's stored key", + setupData: func() []byte { + signer, pub := newSigner(s.mockCtrl) + s.store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(&client.AgentKey{ + MachineID: "machine-001", + Hostname: "web-01", + PublicKey: pub, + }, nil) + + return s.signedResponse( + signer, + "machine-001", + []byte(`{"status":"completed","hostname":"web-01"}`), + ) + }, + validateFunc: func(payload []byte, err error) { + s.Require().NoError(err) + + var response job.Response + s.Require().NoError(json.Unmarshal(payload, &response)) + s.Equal(job.StatusCompleted, response.Status) + s.Equal("web-01", response.Hostname) + }, + }, + { + name: "rejects a response signed by a key that is not the agent's", + setupData: func() []byte { + signer, _ := newSigner(s.mockCtrl) + + // The stored record holds a different key entirely: this is + // the forged response the advisory describes. + otherPub, _, _ := ed25519.GenerateKey(rand.Reader) + s.store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(&client.AgentKey{ + MachineID: "machine-001", + Hostname: "web-01", + PublicKey: otherPub, + }, nil) + + return s.signedResponse( + signer, + "machine-001", + []byte(`{"status":"completed","hostname":"web-01"}`), + ) + }, + validateFunc: func(payload []byte, err error) { + s.Require().Error(err) + s.Nil(payload) + s.Require().ErrorIs(err, client.ErrResponseSignatureInvalid) + }, + }, + { + name: "rejects an accepted agent answering for another host", + setupData: func() []byte { + signer, pub := newSigner(s.mockCtrl) + + // Signature verifies, but this agent enrolled as evil-01 + // and is claiming to be web-01. + s.store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-evil"). + Return(&client.AgentKey{ + MachineID: "machine-evil", + Hostname: "evil-01", + PublicKey: pub, + }, nil) + + return s.signedResponse( + signer, + "machine-evil", + []byte(`{"status":"completed","hostname":"web-01"}`), + ) + }, + validateFunc: func(payload []byte, err error) { + s.Require().Error(err) + s.Nil(payload) + s.Require().ErrorIs(err, client.ErrResponseHostnameMismatch) + s.Contains(err.Error(), "evil-01") + s.Contains(err.Error(), "web-01") + }, + }, + { + name: "reports an agent with no stored key distinctly", + setupData: func() []byte { + signer, _ := newSigner(s.mockCtrl) + s.store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(nil, client.ErrResponseKeyUnknown) + + return s.signedResponse( + signer, + "machine-001", + []byte(`{"status":"completed","hostname":"web-01"}`), + ) + }, + validateFunc: func(payload []byte, err error) { + s.Require().Error(err) + s.Nil(payload) + s.Require().ErrorIs(err, client.ErrResponseKeyUnknown) + s.NotErrorIs(err, client.ErrResponseSignatureInvalid) + }, + }, + { + name: "reports an unreadable store distinctly", + setupData: func() []byte { + signer, _ := newSigner(s.mockCtrl) + s.store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(nil, client.ErrResponseStoreUnavailable) + + return s.signedResponse( + signer, + "machine-001", + []byte(`{"status":"completed","hostname":"web-01"}`), + ) + }, + validateFunc: func(payload []byte, err error) { + s.Require().Error(err) + s.Nil(payload) + s.Require().ErrorIs(err, client.ErrResponseStoreUnavailable) + s.NotErrorIs(err, client.ErrResponseKeyUnknown) + }, + }, + { + name: "rejects data that is not JSON", + setupData: func() []byte { + return []byte(`not json at all`) + }, + validateFunc: func(payload []byte, err error) { + s.Require().Error(err) + s.Nil(payload) + s.Require().ErrorIs(err, client.ErrResponseNotSigned) + }, + }, + { + name: "rejects an unsigned response", + setupData: func() []byte { + return []byte(`{"status":"completed","hostname":"web-01"}`) + }, + validateFunc: func(payload []byte, err error) { + s.Require().Error(err) + s.Nil(payload) + s.Require().ErrorIs(err, client.ErrResponseNotSigned) + }, + }, + { + name: "rejects an envelope that names no machine", + setupData: func() []byte { + signer, _ := newSigner(s.mockCtrl) + + // An agent that has not been upgraded signs without + // stamping its identity: there is nothing to look up. + return s.signedResponse( + signer, + "", + []byte(`{"status":"completed","hostname":"web-01"}`), + ) + }, + validateFunc: func(payload []byte, err error) { + s.Require().Error(err) + s.Nil(payload) + s.Require().ErrorIs(err, client.ErrResponseNotSigned) + }, + }, + { + name: "rejects a verified payload that is not a response", + setupData: func() []byte { + signer, pub := newSigner(s.mockCtrl) + s.store.EXPECT(). + LookupAgentKey(gomock.Any(), "machine-001"). + Return(&client.AgentKey{ + MachineID: "machine-001", + Hostname: "web-01", + PublicKey: pub, + }, nil) + + return s.signedResponse(signer, "machine-001", []byte(`[1,2,3]`)) + }, + validateFunc: func(payload []byte, err error) { + s.Require().Error(err) + s.Nil(payload) + s.Require().ErrorIs(err, client.ErrResponseNotSigned) + s.Contains(err.Error(), "not a response") + }, + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + data := tt.setupData() + if tt.setupMock != nil { + tt.setupMock() + } + + tt.validateFunc( + client.ExportVerifyAgentResponse(s.ctx, s.store, data), + ) + }) + } +} + +func TestVerifyAgentResponsePublicTestSuite( + t *testing.T, +) { + suite.Run(t, new(VerifyAgentResponsePublicTestSuite)) +} diff --git a/internal/job/mocks/agent_key_store.gen.go b/internal/job/mocks/agent_key_store.gen.go new file mode 100644 index 000000000..333709bcf --- /dev/null +++ b/internal/job/mocks/agent_key_store.gen.go @@ -0,0 +1,57 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/osapi-io/osapi/internal/job/client (interfaces: AgentKeyStore) +// +// Generated by this command: +// +// mockgen -destination=./agent_key_store.gen.go -package=mocks github.com/osapi-io/osapi/internal/job/client AgentKeyStore +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + reflect "reflect" + + client "github.com/osapi-io/osapi/internal/job/client" + gomock "go.uber.org/mock/gomock" +) + +// MockAgentKeyStore is a mock of AgentKeyStore interface. +type MockAgentKeyStore struct { + ctrl *gomock.Controller + recorder *MockAgentKeyStoreMockRecorder + isgomock struct{} +} + +// MockAgentKeyStoreMockRecorder is the mock recorder for MockAgentKeyStore. +type MockAgentKeyStoreMockRecorder struct { + mock *MockAgentKeyStore +} + +// NewMockAgentKeyStore creates a new mock instance. +func NewMockAgentKeyStore(ctrl *gomock.Controller) *MockAgentKeyStore { + mock := &MockAgentKeyStore{ctrl: ctrl} + mock.recorder = &MockAgentKeyStoreMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAgentKeyStore) EXPECT() *MockAgentKeyStoreMockRecorder { + return m.recorder +} + +// LookupAgentKey mocks base method. +func (m *MockAgentKeyStore) LookupAgentKey(ctx context.Context, machineID string) (*client.AgentKey, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LookupAgentKey", ctx, machineID) + ret0, _ := ret[0].(*client.AgentKey) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// LookupAgentKey indicates an expected call of LookupAgentKey. +func (mr *MockAgentKeyStoreMockRecorder) LookupAgentKey(ctx, machineID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LookupAgentKey", reflect.TypeOf((*MockAgentKeyStore)(nil).LookupAgentKey), ctx, machineID) +} diff --git a/internal/job/mocks/generate.go b/internal/job/mocks/generate.go index 16daa2c97..9a56f6b81 100644 --- a/internal/job/mocks/generate.go +++ b/internal/job/mocks/generate.go @@ -25,3 +25,4 @@ package mocks //go:generate go tool go.uber.org/mock/mockgen -destination=./kv.gen.go -package=mocks github.com/nats-io/nats.go/jetstream KeyValue,KeyValueEntry,KeyWatcher,KeyLister //go:generate go tool go.uber.org/mock/mockgen -destination=./job_client.gen.go -package=mocks github.com/osapi-io/osapi/internal/job/client JobClient //go:generate go tool go.uber.org/mock/mockgen -destination=./pki_signer.gen.go -package=mocks github.com/osapi-io/osapi/internal/job/client PKISigner +//go:generate go tool go.uber.org/mock/mockgen -destination=./agent_key_store.gen.go -package=mocks github.com/osapi-io/osapi/internal/job/client AgentKeyStore diff --git a/internal/job/mocks/job_client.gen.go b/internal/job/mocks/job_client.gen.go index ff7c9b94f..58b226d89 100644 --- a/internal/job/mocks/job_client.gen.go +++ b/internal/job/mocks/job_client.gen.go @@ -313,6 +313,18 @@ func (mr *MockJobClientMockRecorder) RetryJob(ctx, jobID, targetHostname any) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RetryJob", reflect.TypeOf((*MockJobClient)(nil).RetryJob), ctx, jobID, targetHostname) } +// SetAgentKeyStore mocks base method. +func (m *MockJobClient) SetAgentKeyStore(store client0.AgentKeyStore) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetAgentKeyStore", store) +} + +// SetAgentKeyStore indicates an expected call of SetAgentKeyStore. +func (mr *MockJobClientMockRecorder) SetAgentKeyStore(store any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAgentKeyStore", reflect.TypeOf((*MockJobClient)(nil).SetAgentKeyStore), store) +} + // SetDrainFlag mocks base method. func (m *MockJobClient) SetDrainFlag(ctx context.Context, hostname string) error { m.ctrl.T.Helper() @@ -327,6 +339,18 @@ func (mr *MockJobClientMockRecorder) SetDrainFlag(ctx, hostname any) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetDrainFlag", reflect.TypeOf((*MockJobClient)(nil).SetDrainFlag), ctx, hostname) } +// SetMachineID mocks base method. +func (m *MockJobClient) SetMachineID(machineID string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetMachineID", machineID) +} + +// SetMachineID indicates an expected call of SetMachineID. +func (mr *MockJobClientMockRecorder) SetMachineID(machineID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMachineID", reflect.TypeOf((*MockJobClient)(nil).SetMachineID), machineID) +} + // SetPKISigner mocks base method. func (m *MockJobClient) SetPKISigner(signer client0.PKISigner) { m.ctrl.T.Helper() diff --git a/internal/job/types.go b/internal/job/types.go index 2c03658dd..db06afabe 100644 --- a/internal/job/types.go +++ b/internal/job/types.go @@ -81,6 +81,13 @@ type SignedEnvelope struct { Signature []byte `json:"signature"` // Fingerprint is the SHA256 fingerprint of the signer's public key. Fingerprint string `json:"fingerprint"` + // MachineID identifies the signer so a verifier can find the key to + // check the signature against. It is self-reported and therefore only + // an index: it selects which stored record to verify against, and a + // signature that does not match that record's key is rejected. Empty + // for controller-signed payloads, which agents verify against the + // controller key they were given at enrollment. + MachineID string `json:"machine_id,omitempty"` } // Request represents a request to perform a job operation.