Fix bit-reversed security descriptor control flag constants (#92)#93
Merged
Conversation
The NT_SECURITY_DESCRIPTOR_CONTROL_* constants were defined following the MS-DTYP packet-diagram convention where bit 0 is the most-significant bit, which is the 16-bit reversal of the actual little-endian wire masks used by Windows (winnt.h SE_* values). As a result every marshaled Control word was wrong (e.g. a self-relative DACL-present descriptor serialized as 0x2001 instead of 0x8004), causing NT servers to reject the descriptor, and parsed Windows descriptors had their flags misinterpreted. Redefine the constants to the correct SE_* wire masks. All consumers use the symbolic names, so marshal, unmarshal, SDDL parsing/serialization, and flag interpretation are corrected together.
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
1 participant
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 #92
Root Cause
The
NT_SECURITY_DESCRIPTOR_CONTROL_*constants insecuritydescriptor/control/NtSecurityDescriptorControl.gowere numbered following the MS-DTYP packet-diagram convention, where "bit 0" is the most-significant bit. That convention is the 16-bit reversal of the little-endian masks actually serialized on the wire and defined inwinnt.h(theSE_*values). For example the library definedSR (Self-Relative) = 0x0001andDP (DACL Present) = 0x2000, whereas the wire/winnt.hvalues areSE_SELF_RELATIVE = 0x8000andSE_DACL_PRESENT = 0x0004. All 16 constants were reversed in the same way.Because the
Control.RawValueis stored and serialized verbatim, this meant the value composed via these constants was emitted directly to the wire in its reversed form.Fix Description
Redefine the 16 constants to the correct
SE_*wire masks (OD=0x0001 … SR=0x8000). Every consumer — SDDL parsing (FromSDDLString), SDDL serialization (ToSDDLString), flag interpretation (Unmarshal/HasControl), and the flag-name maps — references the symbolic names, so marshal, unmarshal, and both SDDL directions are corrected together by this single change. No call site hardcodes a raw control mask.The binary
Unmarshal/Marshalpath copiesRawValuebyte-for-byte, so the existing binary round-trip dataset tests are unaffected; only the meaning assigned to each bit (and the value SDDL parsing composes) changes.How Verified
FromSDDLString("O:SYG:SYD:(A;;FA;;;BA)(A;;FA;;;SY)")thenMarshal()now yields a Control word of0x8004(SE_SELF_RELATIVE | SE_DACL_PRESENT); before the fix it was0x2001, the exact bit-reversal, which NT servers reject withERROR_INVALID_PARAMETER.go test ./...passes, including the existing binary round-trip suite over real Windows SD hex fixtures.Test Coverage
Added:
securitydescriptor/control/NtSecurityDescriptorControl_test.go::TestNtSecurityDescriptorControl_WireValues— asserts each constant equals itsSE_*wire mask.securitydescriptor/NtSecurityDescriptor_sddl_wire_test.go::TestFromSDDLString_MarshalWireControl— asserts the end-to-end SDDL → marshal Control word is0x8004.Scope of Change
securitydescriptor/control/NtSecurityDescriptorControl.go,securitydescriptor/control/NtSecurityDescriptorControl_test.go,securitydescriptor/NtSecurityDescriptor_sddl_wire_test.goRisk and Rollout
The corrected Control word changes the bytes emitted by
Marshal()for any descriptor with control flags set, and changes the flag strings reported for parsed descriptors. This is the intended correction (previous output was non-interoperable with Windows). Any downstream code that compensated for the old reversed output — such as a manual bit-reversal workaround applied afterMarshal()— must drop that workaround once this lands.