Skip to content

Commit 55950ab

Browse files
Nagendra TomarNagendra Tomar
Nagendra Tomar
authored and
Nagendra Tomar
committed
Don't fail outright for ACL revision 4.
Though our supported ACL types must carry ACL revision 2 as per the doc https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dtyp/20233ed8-a6c6-4097-aafa-dd545ed24428 but I've seen some dirs have ACL revision 4 but ACL types are still supported ones. So instead of failing upfront, let it fail with unsupported ACE type. Also hexadecimal aceRights are more commonly seen than I expected, so removing a log.
1 parent 96fe100 commit 55950ab

File tree

2 files changed

+23
-9
lines changed

2 files changed

+23
-9
lines changed

sddl/sddlHelper_linux.go

+22-8
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,11 @@ import (
4545
* Are they used for filesystem objects?
4646
*/
4747
const (
48-
SDDL_REVISION = 1 // SDDL Revision MUST always be 1.
49-
SID_REVISION = 1 // SID Revision MUST always be 1.
50-
ACL_REVISION = 2 // Higher ACL revisions support stuff like Object ACE.
48+
SDDL_REVISION = 1 // SDDL Revision MUST always be 1.
49+
SID_REVISION = 1 // SID Revision MUST always be 1.
50+
ACL_REVISION = 2 // ACL revision for support basic ACE type used for filesystem ACLs.
51+
ACL_REVISION_DS = 4 // ACL revision for supporting stuff like Object ACE. This should ideally not be used with the ACE
52+
// types we support, but I've seen some objects like that.
5153
)
5254

5355
type SECURITY_INFORMATION uint32
@@ -856,12 +858,16 @@ func aceRightsToString(aceRights uint32) string {
856858
}
857859

858860
// Use stringified rights only if *all* available rights can be represented with a shorthand name.
861+
// The else part is commented as it's being hit too often. One such common aceRights value is 0x1200a9.
859862
if allRights == aceRights {
860863
return aceRightsString
861-
} else if allRights != 0 {
862-
fmt.Printf("aceRightsString: Only partial rights could be stringified (aceRights=0x%x, allRights=0x%x)",
863-
aceRights, allRights)
864864
}
865+
/*
866+
else if allRights != 0 {
867+
fmt.Printf("aceRightsString: Only partial rights could be stringified (aceRights=0x%x, allRights=0x%x)",
868+
aceRights, allRights)
869+
}
870+
*/
865871

866872
// Fallback to integral mask value.
867873
return fmt.Sprintf("0x%x", aceRights)
@@ -1113,9 +1119,17 @@ func getDaclString(sd []byte) (string, error) {
11131119

11141120
// ACL.AclRevision.
11151121
aclRevision := sd[dacloffset]
1116-
if aclRevision != ACL_REVISION {
1122+
1123+
//
1124+
// Though we support only ACCESS_ALLOWED_ACE_TYPE and ACCESS_DENIED_ACE_TYPE which as per docs should be
1125+
// present with ACL revision 2, but I've seen some objects with these ACE types but acl revision 4.
1126+
// Instead of failing here, we let it proceed. Later isUnsupportedAceType() will catch unsupported ACE types.
1127+
//
1128+
// https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-dtyp/20233ed8-a6c6-4097-aafa-dd545ed24428
1129+
//
1130+
if aclRevision != ACL_REVISION && aclRevision != ACL_REVISION_DS {
11171131
// More importantly we don't support Object ACEs (ACL_REVISION_DS).
1118-
return "", fmt.Errorf("Unsupported ACL Revision (%d), supported revision is %d", aclRevision, ACL_REVISION)
1132+
return "", fmt.Errorf("Invalid ACL Revision (%d), valid values are 2 and 4.", aclRevision)
11191133
}
11201134

11211135
// ACL.AceCount.

ste/sourceInfoProvider-Local_linux.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func (f localFileSourceInfoProvider) GetSDDL() (string, error) {
3232
sdStr, err := sddl.SecurityDescriptorToString(sd)
3333
if err != nil {
3434
// Panic, as it's unexpected and we would want to know.
35-
panic("Cannot parse binary Security Descriptor returned by QuerySecurityObject")
35+
panic(fmt.Errorf("Cannot parse binary Security Descriptor returned by QuerySecurityObject(%s, 0x%x): %v", f.jptm.Info().Source, securityInfoFlags, err))
3636
}
3737

3838
fSDDL, err := sddl.ParseSDDL(sdStr)

0 commit comments

Comments
 (0)