views:

124

answers:

2

I'm attempting to add in an access rule to a RegistryKey like so:

using ( RegistryKey registry = OpenOrCreateMyKey() )
{
    RegistrySecurity security = registry.GetAccessControl();
    security.AddAccessRule( new RegistryAccessRule(
        new SecurityIdentifier( WellKnownSidType.BuiltinUsersSid, null ),
        RegistryRights.WriteKey,
        InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit,
        PropagationFlags.None,
        AccessControlType.Allow ) );
    registry.SetAccessControl( security );
}

However, if the registry is mangled, AddAccessRule throws an exception (Reflector shows that the GetAccessControl call determines that they are not canonical up-front and you trip the fail-safe when you attempt to do a write when the RegistrySecurity instance is in that state):

System.InvalidOperationException: This access control list is not in canonical form and therefore cannot be modified.

Running regedt32 (and probably regedit) then shows a popup saying the permissions on <key> are incorrectly ordered, which may cause some entries to be ineffective <paraphrasing>Do you want to unmangle them now? Y/N</paraprasing>

The most authorative piece I've seen on this issue is http://www.codeproject.com/KB/system/accessctrl3.aspx, which says:

However, the control flags are implemented as properties (talk about inconsistency!). You can retrieve the auto inheritance settings from the AreAccessRulesProtected / AreAuditRulesProtected (recall if an ACL is protected, it does not auto-inherit). If you read part 2, you'll know that some of the Windows APIs did not support inheritance, hence could corrupt the inheritance settings of your machine. The good news is that .NET fully supports the inheritance mechanism and will properly preserve the inheritance settings. If you opened a security descriptor that somehow got a disordered ACL (maybe from some rogue Win32 app), an InvalidOperationException will be thrown if you tried to edit it.

Generally, such non canonical ACLs results from use of the [since retired] NewSID tool, and people write KB articles to say "well stop doing that then".

But, critically, this isn't always the reason, and sometimes the code just needs to work. What's a good clean safe way of handling this?

I'll provide two answers and people can vote and pick holes, vote, comment and nitpick.

A: 

Approach 1 is to ignore the inherited permissions and blindly write in what you wanted to anyway:-

using ( RegistryKey registry = OpenOrCreateMyKey() )
{
    RegistrySecurity security = new RegistrySecurity();
    security.AddAccessRule( new RegistryAccessRule( ... ));
    registry.SetAccessControl( security );
}

Problem is, I have no idea what the potential negative impacts are. I'm happy that all people that need access will gain it from my rule. However, does it play nice with the rest of the OS infrastructure?

Ruben Bartelink
A: 

Approach 2 is to try the first way and fall-back to the dirty way only if necessary:-

using ( RegistryKey registry = OpenOrCreateMyKey() )
{
    RegistrySecurity security = new RegistrySecurity();
    if ( !security.AreAccessRulesCanonical )
    {
        Low.WriteWarning( "Registry permissions (likely ones inherited from parent) are inconsistent (RegistrySecurity.AreAccessRulesCanonical is false), so using alternate permissioning algorithm, Has NewSID or another tool mangled permissions? regedt32 can be used to analyze the issue." );
         // Ignore parent ACLs and just blindly stuff in this ACL (which will continue to be inherited)
         security = new RegistrySecurity();
     }

    security.AddAccessRule( new RegistryAccessRule( ... ));
    registry.SetAccessControl( security );
}

In addition to the weaknesses in Approach 1, this has unpredictable behavior, but at least it's bug-compatible with the original approach to a degree.

Ruben Bartelink