Fix missing SDDL mapping for ZD object-deny callback ACE (Fixes #129) - #130
Merged
Conversation
Add the SDDL_CALLBACK_OBJECT_ACCESS_DENIED = "ZD" constant and its SDDLToACETypeMap entry mapping to ACE_TYPE_ACCESS_DENIED_CALLBACK_OBJECT (0x0C). Because SDDLToACETypeMap is used bidirectionally -- sddlParseACE looks up token->type and sddlACETypeToString reverse-iterates it for type->token -- this one entry enables both parsing and serializing ZD ACEs. The rest of the pipeline (isConditionalACEType, sddlGetACLRevision, binary Marshal/Unmarshal, condition codec) already handled 0x0C. Also update the stale comment listing conditional callback ACE types to include ZD, and add a round-trip test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked Issue
Closes #129Root Cause
ZD(ACCESS_DENIED_CALLBACK_OBJECT, ACE type0x0C) was omitted from theSDDL_*constants and fromSDDLToACETypeMapinsddl/ace/acetype/AccessControlEntryType.gowhen the sibling callback types (XA/0x09,XD/0x0A,ZA/0x0B) were added. That map is the single source of truth for the SDDL↔ACE-type translation in both directions, so the omission made 0x0C unrepresentable on parse and serialize even though the rest of the descriptor pipeline already supported it.Fix Description
Add the
SDDL_CALLBACK_OBJECT_ACCESS_DENIED = "ZD"constant and itsSDDLToACETypeMapentry mapping toACE_TYPE_ACCESS_DENIED_CALLBACK_OBJECT. BecausesddlParseACE()looks up token→type via this map andsddlACETypeToString()reverse-iterates the same map for type→token, the single entry enables both parsing and serialization. No other production code needed changing:isConditionalACEType()andsddlGetACLRevision()already list 0x0C, the object-GUID field parsing is type-agnostic, and the conditional-expression trailer path already routes throughisConditionalACEType(). A stale comment listing the conditional callback types (XA/XD/XU/ZA) is updated to includeZD.How Verified
TestObjectCallbackACE_ZD_SDDLRoundTrip(see below);go test ./...passes with no regressions.FromSDDLString("D:P(ZD;;WP;bf967950-0de6-11d0-a285-00aa003049e2;;S-1-1-0;(Member_of {SID(BA)}))")now parses to a 0x0C ACE with the ObjectType GUID andartxconditional expression intact; the descriptor binary-marshals and unmarshals unchanged; andToSDDLString()re-emitsD:P(ZD;;WP;bf967950-0de6-11d0-a285-00aa003049e2;;WD;(Member_of {SID(S-1-5-32-544)}))(previously it emitted nothing for the 0x0C ACE and parsing errored withunknown ACE type: ZD).Test Coverage
Added:
securitydescriptor/NtSecurityDescriptor_conditional_test.go→TestObjectCallbackACE_ZD_SDDLRoundTrip, asserting the ACE type (0x0C), the ObjectType GUID, and the conditional expression survive both an SDDL round-trip and a binaryMarshal/Unmarshalround-trip.Scope of Change
sddl/ace/acetype/AccessControlEntryType.go,securitydescriptor/NtSecurityDescriptor_sddl.go(comment only),securitydescriptor/NtSecurityDescriptor_conditional_test.go(new test)Risk and Rollout
Low blast radius: adds a previously-absent map key and a test. It cannot change behavior for any ACE type other than 0x0C (whose SDDL handling was previously non-functional), so it is safe to merge without staged rollout.