Skip to content

Commit 71511f2

Browse files
add windows machine GUID to hardware identifiers (#2282)
Co-authored-by: Rebecca Mahany-Horton <rebeccamahany@gmail.com>
1 parent 20a09c2 commit 71511f2

File tree

4 files changed

+170
-9
lines changed

4 files changed

+170
-9
lines changed
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//go:build !windows
2+
// +build !windows
3+
4+
package agent
5+
6+
import (
7+
"context"
8+
9+
"github.com/kolide/launcher/ee/agent/types"
10+
)
11+
12+
// currentMachineGuid is only implemented for windows, where the hardware_uuid
13+
// cannot be used as a stable identifier
14+
func currentMachineGuid(_ context.Context, _ types.Knapsack) (string, error) {
15+
return "", nil
16+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
//go:build windows
2+
// +build windows
3+
4+
package agent
5+
6+
import (
7+
"context"
8+
"fmt"
9+
"log/slog"
10+
11+
"github.com/kolide/launcher/ee/agent/types"
12+
"golang.org/x/sys/windows/registry"
13+
)
14+
15+
const (
16+
regKeyMachineGuidParent string = `SOFTWARE\Microsoft\Cryptography`
17+
regValueMachineGuid string = `MachineGuid`
18+
)
19+
20+
// currentMachineGuid retrieves the current value of HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Cryptography\MachineGuid
21+
// from the windows registry, to be used as a stable hardware identifier for windows
22+
func currentMachineGuid(ctx context.Context, k types.Knapsack) (string, error) {
23+
parentKey, err := registry.OpenKey(registry.LOCAL_MACHINE, regKeyMachineGuidParent, registry.READ)
24+
if err != nil {
25+
return "", fmt.Errorf("opening registry key '%s': %w", regKeyMachineGuidParent, err)
26+
}
27+
28+
defer func() {
29+
if err := parentKey.Close(); err != nil {
30+
k.Slogger().Log(ctx, slog.LevelInfo,
31+
"could not close registry key",
32+
"key_name", regKeyMachineGuidParent,
33+
"err", err,
34+
)
35+
}
36+
}()
37+
38+
machGuid, _, err := parentKey.GetStringValue(regValueMachineGuid)
39+
if err != nil {
40+
return "", fmt.Errorf("reading registry value '%s' for key '%s': %w", regValueMachineGuid, regKeyMachineGuidParent, err)
41+
}
42+
43+
return machGuid, nil
44+
}

ee/agent/reset.go

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ var (
3535
hostDataKeyHardwareUuid = []byte("hardware_uuid")
3636
hostDataKeyMunemo = []byte("munemo")
3737
hostDataKeyResetRecords = []byte("reset_records")
38+
hostDataKeyMachineGuid = []byte("machine_guid") // used for windows only, because the hardware_uuid is not stable
3839
)
3940

4041
const (
@@ -52,7 +53,9 @@ func (use UninitializedStorageError) Error() string {
5253
// stored data in the HostDataStore. If the hardware- or enrollment-identifying information
5354
// has changed, it logs the change. In the future, it will take a backup of the database, and
5455
// then clear all data from it.
55-
func DetectAndRemediateHardwareChange(ctx context.Context, k types.Knapsack) {
56+
// returns a bool of whether remediation occurred (for now, just whether it would have occurred
57+
// given the detection parameters in place)
58+
func DetectAndRemediateHardwareChange(ctx context.Context, k types.Knapsack) bool {
5659
ctx, span := observability.StartSpan(ctx)
5760
defer span.End()
5861

@@ -61,11 +64,13 @@ func DetectAndRemediateHardwareChange(ctx context.Context, k types.Knapsack) {
6164
serialChanged := false
6265
hardwareUUIDChanged := false
6366
munemoChanged := false
67+
machineGuidChanged := false
6468

6569
defer func() {
6670
slogger.Log(ctx, slog.LevelDebug, "finished check to see if database should be reset",
6771
"serial", serialChanged,
6872
"hardware_uuid", hardwareUUIDChanged,
73+
"machine_guid", machineGuidChanged,
6974
"munemo", munemoChanged,
7075
)
7176
}()
@@ -78,6 +83,13 @@ func DetectAndRemediateHardwareChange(ctx context.Context, k types.Knapsack) {
7883
hardwareUUIDChanged = valueChanged(ctx, k, slogger, currentHardwareUUID, hostDataKeyHardwareUuid)
7984
}
8085

86+
currentMachineGuid, err := currentMachineGuid(ctx, k)
87+
if err != nil {
88+
slogger.Log(ctx, slog.LevelWarn, "could not get current machine GUID", "err", err)
89+
} else {
90+
machineGuidChanged = valueChanged(ctx, k, slogger, currentMachineGuid, hostDataKeyMachineGuid)
91+
}
92+
8193
// Collect munemo for logging/record-keeping purposes only -- we don't use it to determine whether
8294
// we should perform a reset
8395
currentTenantMunemo, err := currentMunemo(k)
@@ -87,11 +99,14 @@ func DetectAndRemediateHardwareChange(ctx context.Context, k types.Knapsack) {
8799
munemoChanged = valueChanged(ctx, k, slogger, currentTenantMunemo, hostDataKeyMunemo)
88100
}
89101

90-
if serialChanged && hardwareUUIDChanged {
102+
// note that machineGuid is only collected for windows. machineGuidChanged will only ever have a meaningful value for Windows
103+
remediationRequired := (serialChanged && hardwareUUIDChanged) || machineGuidChanged
104+
if remediationRequired {
91105
slogger.Log(ctx, slog.LevelWarn, "detected hardware change",
92106
"serial_changed", serialChanged,
93107
"hardware_uuid_changed", hardwareUUIDChanged,
94108
"tenant_munemo_changed", munemoChanged,
109+
"machine_guid_changed", machineGuidChanged,
95110
)
96111
// In the future, we can proceed with backing up and resetting the database.
97112
// For now, we are only logging that we detected the change until we have a dependable
@@ -100,6 +115,7 @@ func DetectAndRemediateHardwareChange(ctx context.Context, k types.Knapsack) {
100115
slogger.Log(ctx, slog.LevelWarn, "resetting the database",
101116
"serial_changed", serialChanged,
102117
"hardware_uuid_changed", hardwareUUIDChanged,
118+
"machine_guid_changed", machineGuidChanged,
103119
"tenant_munemo_changed", munemoChanged,
104120
)
105121
@@ -125,6 +141,13 @@ func DetectAndRemediateHardwareChange(ctx context.Context, k types.Knapsack) {
125141
slogger.Log(ctx, slog.LevelWarn, "could not set munemo in host data store", "err", err)
126142
}
127143
}
144+
if machineGuidChanged {
145+
if err := k.PersistentHostDataStore().Set(hostDataKeyMachineGuid, []byte(currentMachineGuid)); err != nil {
146+
slogger.Log(ctx, slog.LevelWarn, "could not set machine GUID in host data store", "err", err)
147+
}
148+
}
149+
150+
return remediationRequired
128151
}
129152

130153
func GetResetRecords(ctx context.Context, k types.Knapsack) ([]dbResetRecord, error) {
@@ -240,7 +263,7 @@ func valueChanged(ctx context.Context, k types.Knapsack, slogger *slog.Logger, c
240263
return false // assume no change
241264
}
242265

243-
if len(storedValue) == 0 {
266+
if len(storedValue) == 0 && len(currentValue) > 0 {
244267
slogger.Log(ctx, slog.LevelDebug, "value not previously stored, storing now", "key", string(dataKey))
245268
if err := k.PersistentHostDataStore().Set(dataKey, []byte(currentValue)); err != nil {
246269
slogger.Log(ctx, slog.LevelError, "could not store value", "err", err, "key", string(dataKey))

ee/agent/reset_test.go

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
8686
serialChanged bool
8787
hardwareUUIDSetInStore bool
8888
hardwareUUIDChanged bool
89+
machineGUIDSetInStore bool
90+
machineGUIDChanged bool
8991
osquerySuccess bool
9092
munemoSetInStore bool
9193
munemoChanged bool
@@ -98,6 +100,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
98100
serialChanged: false,
99101
hardwareUUIDSetInStore: true,
100102
hardwareUUIDChanged: false,
103+
machineGUIDSetInStore: false,
104+
machineGUIDChanged: false,
101105
osquerySuccess: true,
102106
munemoSetInStore: true,
103107
munemoChanged: false,
@@ -110,6 +114,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
110114
serialChanged: true,
111115
hardwareUUIDSetInStore: true,
112116
hardwareUUIDChanged: false,
117+
machineGUIDSetInStore: false,
118+
machineGUIDChanged: false,
113119
osquerySuccess: true,
114120
munemoSetInStore: true,
115121
munemoChanged: false,
@@ -122,6 +128,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
122128
serialChanged: false,
123129
hardwareUUIDSetInStore: true,
124130
hardwareUUIDChanged: true,
131+
machineGUIDSetInStore: false,
132+
machineGUIDChanged: false,
125133
osquerySuccess: true,
126134
munemoSetInStore: true,
127135
munemoChanged: false,
@@ -134,6 +142,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
134142
serialChanged: false,
135143
hardwareUUIDSetInStore: true,
136144
hardwareUUIDChanged: false,
145+
machineGUIDSetInStore: false,
146+
machineGUIDChanged: false,
137147
osquerySuccess: true,
138148
munemoSetInStore: true,
139149
munemoChanged: true,
@@ -146,6 +156,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
146156
serialChanged: true,
147157
hardwareUUIDSetInStore: true,
148158
hardwareUUIDChanged: true,
159+
machineGUIDSetInStore: true,
160+
machineGUIDChanged: true,
149161
osquerySuccess: true,
150162
munemoSetInStore: true,
151163
munemoChanged: true,
@@ -158,6 +170,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
158170
serialChanged: false,
159171
hardwareUUIDSetInStore: true,
160172
hardwareUUIDChanged: false,
173+
machineGUIDSetInStore: false,
174+
machineGUIDChanged: false,
161175
osquerySuccess: false,
162176
munemoSetInStore: true,
163177
munemoChanged: false,
@@ -170,6 +184,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
170184
serialChanged: false,
171185
hardwareUUIDSetInStore: true,
172186
hardwareUUIDChanged: false,
187+
machineGUIDSetInStore: false,
188+
machineGUIDChanged: false,
173189
osquerySuccess: true,
174190
munemoSetInStore: true,
175191
munemoChanged: false,
@@ -182,6 +198,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
182198
serialChanged: false,
183199
hardwareUUIDSetInStore: true,
184200
hardwareUUIDChanged: false,
201+
machineGUIDSetInStore: false,
202+
machineGUIDChanged: false,
185203
osquerySuccess: true,
186204
munemoSetInStore: true,
187205
munemoChanged: false,
@@ -194,6 +212,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
194212
serialChanged: false,
195213
hardwareUUIDSetInStore: false,
196214
hardwareUUIDChanged: false,
215+
machineGUIDSetInStore: false,
216+
machineGUIDChanged: false,
197217
osquerySuccess: true,
198218
munemoSetInStore: true,
199219
munemoChanged: false,
@@ -206,6 +226,8 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
206226
serialChanged: false,
207227
hardwareUUIDSetInStore: true,
208228
hardwareUUIDChanged: false,
229+
machineGUIDSetInStore: false,
230+
machineGUIDChanged: false,
209231
osquerySuccess: true,
210232
munemoSetInStore: false,
211233
munemoChanged: false,
@@ -218,24 +240,63 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
218240
serialChanged: false,
219241
hardwareUUIDSetInStore: true,
220242
hardwareUUIDChanged: true,
243+
machineGUIDSetInStore: false,
244+
machineGUIDChanged: false,
221245
osquerySuccess: true,
222246
munemoSetInStore: true,
223247
munemoChanged: false,
224248
registrationsExist: true,
225249
expectDatabaseWipe: false,
226250
},
251+
{
252+
name: "machine GUID previously stored, all other data unchanged, no database wipe",
253+
serialSetInStore: true,
254+
serialChanged: false,
255+
hardwareUUIDSetInStore: true,
256+
hardwareUUIDChanged: false,
257+
machineGUIDSetInStore: true,
258+
machineGUIDChanged: false,
259+
osquerySuccess: true,
260+
munemoSetInStore: true,
261+
munemoChanged: false,
262+
registrationsExist: true,
263+
expectDatabaseWipe: false,
264+
},
265+
{
266+
name: "machine GUID not previously stored, all other data unchanged, no database wipe",
267+
serialSetInStore: true,
268+
serialChanged: false,
269+
hardwareUUIDSetInStore: true,
270+
hardwareUUIDChanged: false,
271+
machineGUIDSetInStore: false,
272+
machineGUIDChanged: true,
273+
osquerySuccess: true,
274+
munemoSetInStore: true,
275+
munemoChanged: false,
276+
registrationsExist: true,
277+
expectDatabaseWipe: false,
278+
},
279+
{
280+
name: "machine GUID previously stored, then changed, expect database wipe",
281+
serialSetInStore: true,
282+
serialChanged: false,
283+
hardwareUUIDSetInStore: true,
284+
hardwareUUIDChanged: false,
285+
machineGUIDSetInStore: true,
286+
machineGUIDChanged: true,
287+
osquerySuccess: true,
288+
munemoSetInStore: true,
289+
munemoChanged: false,
290+
registrationsExist: true,
291+
expectDatabaseWipe: true,
292+
},
227293
}
228294

229295
for _, tt := range testCases {
230296
tt := tt
231297
t.Run(tt.name, func(t *testing.T) {
232298
t.Parallel()
233299

234-
// For now, we never expect the database to be wiped. In the future, when we
235-
// decide to proceed with resetting the database, we can remove this line from
236-
// the tests and they will continue to validate expected behavior.
237-
tt.expectDatabaseWipe = false
238-
239300
slogger := multislogger.NewNopLogger()
240301

241302
// Set up dependencies: data store for hardware-identifying data
@@ -270,6 +331,9 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
270331
actualHardwareUUID = "test-hardware-uuid"
271332
}
272333

334+
actualMachineGUID, err := currentMachineGuid(context.TODO(), mockKnapsack)
335+
require.NoError(t, err, "expected to be able to read machine GUID")
336+
273337
if tt.serialSetInStore {
274338
if tt.serialChanged {
275339
require.NoError(t, testHostDataStore.Set(hostDataKeySerial, []byte("some-old-serial")), "could not set serial in test store")
@@ -286,6 +350,14 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
286350
}
287351
}
288352

353+
if tt.machineGUIDSetInStore {
354+
if tt.machineGUIDChanged {
355+
require.NoError(t, testHostDataStore.Set(hostDataKeyMachineGuid, []byte("some-old-machine-guid")), "could not set machine guid in test store")
356+
} else {
357+
require.NoError(t, testHostDataStore.Set(hostDataKeyMachineGuid, []byte(actualMachineGUID)), "could not set machine guid in test store")
358+
}
359+
}
360+
289361
// Set up dependencies: ensure that retrieved tenant matches current data
290362
munemoValue := []byte("test-munemo")
291363

@@ -331,7 +403,13 @@ func TestDetectAndRemediateHardwareChange(t *testing.T) {
331403
require.NoError(t, testConfigStore.Set([]byte("localEccKey"), testLocalEccKeyRaw))
332404

333405
// Make test call
334-
DetectAndRemediateHardwareChange(context.TODO(), mockKnapsack)
406+
remediationRequired := DetectAndRemediateHardwareChange(context.TODO(), mockKnapsack)
407+
require.Equal(t, tt.expectDatabaseWipe, remediationRequired, "expected remediation to be required when database should be wiped")
408+
409+
// For now, we never expect the database to be wiped. In the future, when we
410+
// decide to proceed with resetting the database, we can remove this line from
411+
// the tests and they will continue to validate expected behavior.
412+
tt.expectDatabaseWipe = false
335413

336414
// Confirm backup occurred, if database got wiped
337415
if tt.expectDatabaseWipe {

0 commit comments

Comments
 (0)