Skip to content

Commit 5b34592

Browse files
robertwoj-microsoftrobertwoj-microsoft
committed
Correct logic now
1 parent 9f9d454 commit 5b34592

File tree

2 files changed

+160
-21
lines changed

2 files changed

+160
-21
lines changed

‎src/modules/compliance/src/lib/procedures/ensureFilePermissions.cpp

+14-9
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <Evaluator.h>
44
#include <errno.h>
55
#include <grp.h>
6+
#include <iomanip>
67
#include <pwd.h>
78
#include <string.h>
89
#include <sys/stat.h>
@@ -74,7 +75,7 @@ AUDIT_FN(ensureFilePermissions)
7475
}
7576

7677
mode_t perms = 0x0;
77-
mode_t mask = 0x0;
78+
mode_t mask = 0xFFF;
7879
bool has_perms_or_mask = false;
7980
if (args.find("permissions") != args.end())
8081
{
@@ -95,17 +96,19 @@ AUDIT_FN(ensureFilePermissions)
9596
{
9697
return Error("Invalid mask parameter");
9798
}
98-
if (perms & mask)
99+
if (perms & (~mask))
99100
{
100-
return Error("Permissions and mask overlap");
101+
return Error("Permissions must not have bits set that are not set in the mask");
101102
}
102103

103104
has_perms_or_mask = true;
104105
}
105-
if (has_perms_or_mask && ((statbuf.st_mode & mask) || (statbuf.st_mode & perms) != perms))
106+
if (has_perms_or_mask && ((statbuf.st_mode & mask) != (perms & mask)))
106107
{
107-
logstream << "Invalid permissions - are " << std::oct << (statbuf.st_mode & supportedMask) << " should be " << std::oct
108-
<< (statbuf.st_mode & (~mask) & supportedMask) << " with mask " << std::oct << mask << std::dec;
108+
auto currentPerms = statbuf.st_mode & supportedMask;
109+
auto expectedPerms = ((statbuf.st_mode & ~mask) | (perms & mask)) & supportedMask;
110+
logstream << "Invalid permissions - are " << std::setw(4) << std::setfill('0') << std::oct << currentPerms << " should be " << std::setw(4)
111+
<< std::setfill('0') << std::oct << expectedPerms << " with mask " << std::setw(4) << std::setfill('0') << std::oct << mask << std::dec;
109112
return false;
110113
}
111114

@@ -187,7 +190,7 @@ REMEDIATE_FN(ensureFilePermissions)
187190
}
188191

189192
mode_t perms = 0x0;
190-
mode_t mask = 0x0;
193+
mode_t mask = 0xFFF;
191194
bool has_perms_or_mask = false;
192195
if (args.find("permissions") != args.end())
193196
{
@@ -209,16 +212,18 @@ REMEDIATE_FN(ensureFilePermissions)
209212
logstream << "ERROR: Invalid permissions mask: " << args["mask"];
210213
return Error("Invalid permissions mask: " + args["mask"]);
211214
}
212-
if (perms & mask)
215+
if (perms & (~mask))
213216
{
214-
return Error("Permissions and mask overlap");
217+
return Error("Permissions must not have bits set that are not set in the mask");
215218
}
216219
has_perms_or_mask = true;
217220
}
218221

219222
unsigned short new_perms = (statbuf.st_mode & ~mask) | perms;
223+
OsConfigLogInfo(NULL, "Setting permissions to %o, mask: %o, perms: %o, current: %o", new_perms, mask, perms, statbuf.st_mode);
220224
if (has_perms_or_mask && (new_perms != statbuf.st_mode))
221225
{
226+
OsConfigLogInfo(NULL, "Setting permissions to %o", new_perms);
222227
if (chmod(args["filename"].c_str(), new_perms) < 0)
223228
{
224229
logstream << "ERROR: Chmod error";

‎src/modules/test/recipes/compliance/EnsureFilePermissions.json

+146-12
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,14 @@
7070
"ObjectName": "auditTest",
7171
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp/testfile' } == TRUE"
7272
},
73-
73+
{
74+
"RunCommand": "stat /tmp/testfile | grep 'Access: (0777\/-rwxrwxrwx)'"
75+
},
7476

7577

78+
{
79+
"RunCommand": "touch /tmp/testfile && chmod 777 /tmp/testfile && chown root:root /tmp/testfile"
80+
},
7681
{
7782
"ObjectType": "Desired",
7883
"ComponentName": "Compliance",
@@ -83,7 +88,7 @@
8388
"ObjectType": "Reported",
8489
"ComponentName": "Compliance",
8590
"ObjectName": "auditTest",
86-
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' } == TRUE"
91+
"Payload": "{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' Invalid permissions - are 0777 should be 0644 with mask 7777 } == FALSE"
8792
},
8893
{
8994
"ObjectType": "Desired",
@@ -100,6 +105,9 @@
100105

101106

102107

108+
{
109+
"RunCommand": "touch /tmp/testfile && chmod 777 /tmp/testfile && chown root:root /tmp/testfile"
110+
},
103111
{
104112
"ObjectType": "Desired",
105113
"ComponentName": "Compliance",
@@ -112,9 +120,6 @@
112120
"ObjectName": "auditTest",
113121
"Payload": "{ ensureFilePermissions: ensureFilePermissions for '\/tmp/testfile' Invalid user - is 'root' should be 'foo' } == FALSE"
114122
},
115-
116-
117-
118123
{
119124
"ObjectType": "Desired",
120125
"ComponentName": "Compliance",
@@ -170,9 +175,6 @@
170175
"ObjectName": "auditTest",
171176
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp/testfile' } == TRUE"
172177
},
173-
174-
175-
176178
{
177179
"ObjectType": "Desired",
178180
"ComponentName": "Compliance",
@@ -183,7 +185,7 @@
183185
"ObjectType": "Reported",
184186
"ComponentName": "Compliance",
185187
"ObjectName": "auditTest",
186-
"Payload": "{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' Invalid permissions - are 777 should be 600 with mask 177 } == FALSE"
188+
"Payload": "{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' Invalid permissions - are 0777 should be 0600 with mask 0177 } == FALSE"
187189
},
188190

189191

@@ -213,7 +215,7 @@
213215
"ObjectType": "Reported",
214216
"ComponentName": "Compliance",
215217
"ObjectName": "auditTest",
216-
"Payload": "{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' Invalid permissions - are 777 should be 600 with mask 177 } == FALSE"
218+
"Payload": "{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' Invalid permissions - are 0777 should be 0600 with mask 0177 } == FALSE"
217219
},
218220
{
219221
"ObjectType": "Desired",
@@ -227,7 +229,9 @@
227229
"ObjectName": "auditTest",
228230
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp/testfile' } == TRUE"
229231
},
230-
232+
{
233+
"RunCommand": "stat /tmp/testfile | grep 'Access: (0600\/-rw-------)'"
234+
},
231235

232236

233237
{
@@ -267,14 +271,17 @@
267271
"ObjectType": "Desired",
268272
"ComponentName": "Compliance",
269273
"ObjectName": "remediateTest",
270-
"Payload": "MASK=177 USER=root GROUP=bar"
274+
"Payload": "MASK=133 USER=root GROUP=bar"
271275
},
272276
{
273277
"ObjectType": "Reported",
274278
"ComponentName": "Compliance",
275279
"ObjectName": "auditTest",
276280
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' } == TRUE"
277281
},
282+
{
283+
"RunCommand": "stat /tmp/testfile | grep 'Access: (0644\/-rw-r--r--) Uid: ( 0\/ root) Gid: ( 1001\/ bar)'"
284+
},
278285

279286

280287

@@ -302,6 +309,133 @@
302309
"ObjectName": "auditTest",
303310
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' } == TRUE"
304311
},
312+
{
313+
"RunCommand": "stat /tmp/testfile | grep 'Access: (0600\/-rw-------) Uid: ( 1001\/ foo) Gid: ( 1001\/ bar)'"
314+
},
315+
316+
317+
{
318+
"ObjectType": "Desired",
319+
"ComponentName": "Compliance",
320+
"ObjectName": "procedureTest",
321+
"Payload": {
322+
"audit": {
323+
"ensureFilePermissions": {
324+
"filename": "/tmp/testfile",
325+
"user": "$USER",
326+
"permissions": "$PERMISSIONS",
327+
"group": "$GROUP",
328+
"mask": "$MASK"
329+
}
330+
},
331+
"remediate": {
332+
"ensureFilePermissions": {
333+
"filename": "/tmp/testfile",
334+
"user": "$USER",
335+
"permissions": "$PERMISSIONS",
336+
"group": "$GROUP",
337+
"mask": "$MASK"
338+
}
339+
},
340+
"parameters": {
341+
"USER": "root",
342+
"GROUP": "root",
343+
"PERMISSIONS": "000",
344+
"MASK": "777"
345+
}
346+
}
347+
},
348+
{
349+
"RunCommand": "touch /tmp/testfile && chmod 777 /tmp/testfile && chown root:root /tmp/testfile"
350+
},
351+
{
352+
"ObjectType": "Reported",
353+
"ComponentName": "Compliance",
354+
"ObjectName": "auditTest",
355+
"Payload": "{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' Invalid permissions - are 0777 should be 0000 with mask 0777 } == FALSE"
356+
},
357+
358+
359+
360+
{
361+
"ObjectType": "Desired",
362+
"ComponentName": "Compliance",
363+
"ObjectName": "initTest",
364+
"Payload": "PERMISSIONS=777"
365+
},
366+
{
367+
"ObjectType": "Reported",
368+
"ComponentName": "Compliance",
369+
"ObjectName": "auditTest",
370+
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' } == TRUE"
371+
},
372+
373+
374+
375+
{
376+
"ObjectType": "Desired",
377+
"ComponentName": "Compliance",
378+
"ObjectName": "initTest",
379+
"Payload": "PERMISSIONS=100 MASK=100"
380+
},
381+
{
382+
"ObjectType": "Reported",
383+
"ComponentName": "Compliance",
384+
"ObjectName": "auditTest",
385+
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' } == TRUE"
386+
},
387+
388+
389+
390+
{
391+
"ObjectType": "Desired",
392+
"ComponentName": "Compliance",
393+
"ObjectName": "initTest",
394+
"Payload": "PERMISSIONS=333 MASK=777"
395+
},
396+
{
397+
"ObjectType": "Reported",
398+
"ComponentName": "Compliance",
399+
"ObjectName": "auditTest",
400+
"Payload": "{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' Invalid permissions - are 0777 should be 0333 with mask 0777 } == FALSE"
401+
},
402+
403+
404+
405+
{
406+
"ObjectType": "Desired",
407+
"ComponentName": "Compliance",
408+
"ObjectName": "remediateTest",
409+
"Payload": "PERMISSIONS=333 MASK=777"
410+
},
411+
{
412+
"ObjectType": "Reported",
413+
"ComponentName": "Compliance",
414+
"ObjectName": "auditTest",
415+
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' } == TRUE"
416+
},
417+
{
418+
"RunCommand": "stat /tmp/testfile | grep 'Access: (0333\/--wx-wx-wx)'"
419+
},
420+
421+
422+
423+
{
424+
"ObjectType": "Desired",
425+
"ComponentName": "Compliance",
426+
"ObjectName": "remediateTest",
427+
"Payload": "PERMISSIONS=1000 MASK=1000"
428+
},
429+
{
430+
"ObjectType": "Reported",
431+
"ComponentName": "Compliance",
432+
"ObjectName": "auditTest",
433+
"Payload": "PASS{ ensureFilePermissions: ensureFilePermissions for '\/tmp\/testfile' } == TRUE"
434+
},
435+
{
436+
"RunCommand": "stat /tmp/testfile | grep 'Access: (1333\/--wx-wx-wt)'"
437+
},
438+
305439

306440

307441
{

0 commit comments

Comments
 (0)