CommonAcl is now essentially fully functional. Its descendants DiscretionaryAcl

and SystemAcl need work, but CommonAcl can be manipulated using RawAcls nicely
now and will test canonicity, merge ACEs, remove meaningless ACEs, sort explicit
deny/allow ACEs by SecurityIdentifier... all the nice things CommonAcl is supposed
to do.

Anyway, this patch... Off the top of my head...
Fixed CommonAce and ObjectAce deserialization/roundtripping with opaque data.
Fixed ObjectAce having the wrong BinaryLength depending on ObjectAceFlags.
GenericAce had two protected static SDDL methods which should be internal.

Added some unit tests for the above. One of the tests compares the binary
output with a blob I produced using the *same unit test* on MS.NET (with a
File.WriteAllText creating the byte array to paste in, naturally), and
happily, it produces identical binary output. Very nice.

mcs/class/corlib/System.Security.AccessControl/CommonAce.cs

@@ -71,6 +71,7 @@ namespace System.Security.AccessControl {
 				Array.Copy(binaryForm,
 				           offset + 8 + SecurityIdentifier.BinaryLength,
 				           opaque, 0, opaqueLen);
+				SetOpaque (opaque);
 			}
 		}
 

mcs/class/corlib/System.Security.AccessControl/CommonAcl.cs

@@ -261,7 +261,7 @@ namespace System.Security.AccessControl
 			if (0 != (ace.ObjectAceFlags & ObjectAceFlags.ObjectAceTypePresent))
 				type = ace.ObjectAceType;
 			if (0 != (ace.ObjectAceFlags & ObjectAceFlags.InheritedObjectAceTypePresent))
-				type = ace.InheritedObjectAceType;
+				inheritedType = ace.InheritedObjectAceType;
 		}
 
 		internal abstract void ApplyCanonicalSortToExplicitAces ();

mcs/class/corlib/System.Security.AccessControl/GenericAce.cs

@@ -274,7 +274,7 @@ namespace System.Security.AccessControl {
 				|| type == AceType.SystemAuditObject;
 		}
 
-		protected static string GetSddlAceType (AceType type)
+		internal static string GetSddlAceType (AceType type)
 		{
 			switch (type) {
 			case AceType.AccessAllowed:
@@ -330,7 +330,7 @@ namespace System.Security.AccessControl {
 			}
 		}
 
-		protected static string GetSddlAceFlags (AceFlags flags)
+		internal static string GetSddlAceFlags (AceFlags flags)
 		{
 			StringBuilder result = new StringBuilder ();
 			if ((flags & AceFlags.ObjectInherit) != 0)

mcs/class/corlib/System.Security.AccessControl/ObjectAce.cs

@@ -5,8 +5,10 @@
 //	Dick Porter  <[email protected]>
 //	Atsushi Enomoto  <[email protected]>
 //	Kenneth Bell
+//	James Bellinger  <[email protected]>
 //
 // Copyright (C) 2006-2007 Novell, Inc (http://www.novell.com)
+// Copyright (C) 2012      James Bellinger
 //
 // Permission is hereby granted, free of charge, to any person obtaining
 // a copy of this software and associated documentation files (the
@@ -48,9 +50,9 @@ namespace System.Security.AccessControl
 		{
 			AccessMask = accessMask;
 			SecurityIdentifier = sid;
-			object_ace_flags = flags;
-			object_ace_type = type;
-			inherited_object_type = inheritedType;
+			ObjectAceFlags = flags;
+			ObjectAceType = type;
+			InheritedObjectAceType = inheritedType;
 		}
 
 		internal ObjectAce (AceType type, AceFlags flags, int accessMask,
@@ -60,39 +62,56 @@ namespace System.Security.AccessControl
 		{
 			AccessMask = accessMask;
 			SecurityIdentifier = sid;
-			object_ace_flags = objFlags;
-			object_ace_type = objType;
-			inherited_object_type = inheritedType;
+			ObjectAceFlags = objFlags;
+			ObjectAceType = objType;
+			InheritedObjectAceType = inheritedType;
 		}
 		
 		internal ObjectAce(byte[] binaryForm, int offset)
 			: base(binaryForm, offset)
 		{
 			int len = ReadUShort(binaryForm, offset + 2);
+			int lenMinimum = 12 + SecurityIdentifier.MinBinaryLength;
+			
 			if (offset > binaryForm.Length - len)
 				throw new ArgumentException("Invalid ACE - truncated", "binaryForm");
-			if (len < 44 + SecurityIdentifier.MinBinaryLength)
+			if (len < lenMinimum)
 				throw new ArgumentException("Invalid ACE", "binaryForm");
 			
 			AccessMask = ReadInt(binaryForm, offset + 4);
-			object_ace_flags = (ObjectAceFlags)ReadInt(binaryForm, offset + 8);
-			object_ace_type = ReadGuid(binaryForm, offset + 12);
-			inherited_object_type = ReadGuid(binaryForm, offset + 28);
-			SecurityIdentifier = new SecurityIdentifier(binaryForm, offset + 44);
+			ObjectAceFlags = (ObjectAceFlags)ReadInt(binaryForm, offset + 8);
+			
+			if (ObjectAceTypePresent) lenMinimum += 16;
+			if (InheritedObjectAceTypePresent) lenMinimum += 16;
+			if (len < lenMinimum)
+				throw new ArgumentException("Invalid ACE", "binaryForm");
+
+			int pos = 12;
+			if (ObjectAceTypePresent) {
+				ObjectAceType = ReadGuid(binaryForm, offset + pos); pos += 16;
+			}
+			if (InheritedObjectAceTypePresent) {
+				InheritedObjectAceType = ReadGuid(binaryForm, offset + pos); pos += 16;
+			}
 			
-			int opaqueLen = len - (44 + SecurityIdentifier.BinaryLength);
+			SecurityIdentifier = new SecurityIdentifier(binaryForm, offset + pos);
+			pos += SecurityIdentifier.BinaryLength;
+			
+			int opaqueLen = len - pos;
 			if (opaqueLen > 0) {
 				byte[] opaque = new byte[opaqueLen];
-				Array.Copy(binaryForm, offset + 44 + SecurityIdentifier.BinaryLength,
-				           opaque, 0, opaqueLen);
+				Array.Copy(binaryForm, offset + pos, opaque, 0, opaqueLen);
+				SetOpaque (opaque);
 			}
 		}
 
 		public override int BinaryLength
 		{
 			get {
-				return 44 + SecurityIdentifier.BinaryLength
-					+ OpaqueLength;
+				int length = 12 + SecurityIdentifier.BinaryLength + OpaqueLength;
+				if (ObjectAceTypePresent) length += 16;
+				if (InheritedObjectAceTypePresent) length += 16;
+				return length;
 			}
 		}
 
@@ -101,6 +120,10 @@ namespace System.Security.AccessControl
 			set { inherited_object_type = value; }
 		}
 		
+		bool InheritedObjectAceTypePresent {
+			get { return 0 != (ObjectAceFlags & ObjectAceFlags.InheritedObjectAceTypePresent); }
+		}
+		
 		public ObjectAceFlags ObjectAceFlags {
 			get { return object_ace_flags; }
 			set { object_ace_flags = value; }
@@ -111,26 +134,34 @@ namespace System.Security.AccessControl
 			set { object_ace_type = value; }
 		}
 
-		public override void GetBinaryForm (byte[] binaryForm,
-						    int offset)
+		bool ObjectAceTypePresent {
+			get { return 0 != (ObjectAceFlags & ObjectAceFlags.ObjectAceTypePresent); }
+		}
+		
+		public override void GetBinaryForm (byte[] binaryForm, int offset)
 		{
 			int len = BinaryLength;
-			binaryForm[offset] = (byte)this.AceType;
-			binaryForm[offset + 1] = (byte)this.AceFlags;
-			WriteUShort ((ushort)len, binaryForm, offset + 2);
-			WriteInt (AccessMask, binaryForm, offset + 4);
-			WriteInt ((int)ObjectAceFlags, binaryForm, offset + 8);
-			WriteGuid (ObjectAceType, binaryForm, offset + 12);
-			WriteGuid (InheritedObjectAceType, binaryForm, offset + 28);
+			binaryForm[offset++] = (byte)this.AceType;
+			binaryForm[offset++] = (byte)this.AceFlags;
+			WriteUShort ((ushort)len, binaryForm, offset); offset += 2;
+			WriteInt (AccessMask, binaryForm, offset); offset += 4;
+			WriteInt ((int)ObjectAceFlags, binaryForm, offset); offset += 4;
 			
-			SecurityIdentifier.GetBinaryForm (binaryForm,
-			                                  offset + 44);
+			if (0 != (ObjectAceFlags & ObjectAceFlags.ObjectAceTypePresent)) {
+				WriteGuid (ObjectAceType, binaryForm, offset); offset += 16;
+			}
+			if (0 != (ObjectAceFlags & ObjectAceFlags.InheritedObjectAceTypePresent)) {
+				WriteGuid (InheritedObjectAceType, binaryForm, offset); offset += 16;
+			}
+			
+			SecurityIdentifier.GetBinaryForm (binaryForm, offset);
+			offset += SecurityIdentifier.BinaryLength;
 			
 			byte[] opaque = GetOpaque ();
-			if (opaque != null)
-				Array.Copy (opaque, 0, binaryForm,
-				            offset + 44 + SecurityIdentifier.BinaryLength,
-				            opaque.Length);
+			if (opaque != null) {
+				Array.Copy (opaque, 0, binaryForm, offset, opaque.Length);
+				offset += opaque.Length;
+			}
 		}
 		
 		public static int MaxOpaqueLength (bool isCallback)

mcs/class/corlib/System.Security.Principal/SecurityIdentifier.cs

@@ -152,7 +152,7 @@ namespace System.Security.Principal {
 		// The CompareTo ordering was determined by unit test applied to MS.NET implementation,
 		// necessary because the CompareTo has no details in its documentation.
 		// (See MonoTests.System.Security.AccessControl.DiscretionaryAclTest.)
-		// The comparison was determined to be: authority, then element count, then values.
+		// The comparison was determined to be: authority, then subauthority count, then subauthority.
 		public int CompareTo (SecurityIdentifier sid)
 		{
 			int result;

mcs/class/corlib/Test/System.Security.AccessControl/CommonAclTest.cs

@@ -1,9 +1,9 @@
-// CommonAclTest.cs - NUnit Test Cases for ObjectSecurity<T>
+// CommonAclTest.cs - NUnit Test Cases for CommonAcl
 //
 // Authors:
-//	James Bellinger ([email protected])
+//	James Bellinger  <[email protected]>
 //
-// Copyright 2012 James Bellinger
+// Copyright (C) 2012 James Bellinger
 
 using System;
 using System.Collections.Generic;
@@ -87,7 +87,7 @@ namespace MonoTests.System.Security.AccessControl
 		public void GuidEmptyMergesRegardlessOfFlagsAndOpaqueDataIsNotConsidered ()
 		{
 			SecurityIdentifier sid = new SecurityIdentifier (WellKnownSidType.BuiltinUsersSid, null);
-
+			
 			RawAcl acl = MakeRawAcl (new GenericAce[] {
 				new CommonAce (AceFlags.None, AceQualifier.AccessAllowed, 1, sid, true, new byte[12]),
 				new CommonAce (AceFlags.None, AceQualifier.AccessAllowed, 4, sid, false, new byte[8]), // gets merged
@@ -98,6 +98,7 @@ namespace MonoTests.System.Security.AccessControl
 				new ObjectAce (AceFlags.None, AceQualifier.AccessAllowed, 4, sid,
 				               ObjectAceFlags.InheritedObjectAceTypePresent, Guid.Empty, Guid.NewGuid (), true, new byte[4])
 			});
+			Assert.AreEqual (236, acl.BinaryLength);
 
 			DiscretionaryAcl dacl = new DiscretionaryAcl (false, false, acl);
 			Assert.IsTrue (dacl.IsCanonical);
@@ -212,6 +213,8 @@ namespace MonoTests.System.Security.AccessControl
 			Assert.IsTrue (throws2);
 		}
 
+		// FIXME: Uncomment this once CompoundAce is implemented on Mono.
+		/*
 		[Test]
 		public void CompoundAcesAreNotCanonical ()
 		{
@@ -224,6 +227,7 @@ namespace MonoTests.System.Security.AccessControl
 			DiscretionaryAcl dacl = new DiscretionaryAcl (false, false, acl);
 			Assert.IsFalse (dacl.IsCanonical);
 		}
+		*/
 
 		[Test]
 		public void RemovesMeaninglessAces ()

mcs/class/corlib/Test/System.Security.AccessControl/ObjectAceTest.cs

@@ -0,0 +1,119 @@
+// ObjectAceTest.cs - NUnit Test Cases for ObjectAce
+//
+// Authors:
+//	James Bellinger  <[email protected]>
+//
+// Copyright (C) 2012 James Bellinger
+
+using System;
+using System.Collections.Generic;
+using System.Security.AccessControl;
+using System.Security.Principal;
+using NUnit.Framework;
+
+namespace AccessControl
+{
+	[TestFixture]
+	public class ObjectAceTest
+	{
+		static RawAcl CreateRoundtripRawAcl ()
+		{
+			SecurityIdentifier sid = new SecurityIdentifier (WellKnownSidType.BuiltinUsersSid, null);
+			Assert.AreEqual (16, sid.BinaryLength);
+			
+			GenericAce[] aces = new GenericAce[] {
+				new ObjectAce (AceFlags.None, AceQualifier.AccessAllowed, 1, sid,
+				               ObjectAceFlags.ObjectAceTypePresent,
+				               Guid.Empty, Guid.Empty, false, new byte[8]),
+				new ObjectAce (AceFlags.None, AceQualifier.AccessAllowed, 2, sid,
+				               ObjectAceFlags.InheritedObjectAceTypePresent,
+				               Guid.Empty, Guid.Empty, true, new byte[16]),
+				new ObjectAce (AceFlags.None, AceQualifier.AccessAllowed, 4, sid,
+				               ObjectAceFlags.InheritedObjectAceTypePresent,
+				               Guid.Empty, new Guid ("{8865FB90-A9EB-422F-A8BA-07ECA611D699}"), true, new byte[4]),
+				new ObjectAce (AceFlags.None, AceQualifier.AccessAllowed, 4, sid,
+				               ObjectAceFlags.ObjectAceTypePresent|ObjectAceFlags.InheritedObjectAceTypePresent,
+				               Guid.Empty, new Guid ("{B893007C-38D5-4827-A698-BA25F1E30BAC}"), true, new byte[4]),
+				new ObjectAce (AceFlags.None, AceQualifier.AccessAllowed, 4, sid,
+				               ObjectAceFlags.None,
+				               Guid.Empty, new Guid ("{C0F9DF22-C320-4400-B41F-754F69668640}"), true, new byte[4])
+			};
+			
+			// Make sure this created right, first of all.
+			Assert.AreEqual (AceType.AccessAllowedObject, aces [0].AceType);
+			Assert.AreEqual (AceType.AccessAllowedCallbackObject, aces [1].AceType);
+			Assert.AreEqual (AceType.AccessAllowedCallbackObject, aces [2].AceType);
+			Assert.AreEqual (AceType.AccessAllowedCallbackObject, aces [3].AceType);
+			Assert.AreEqual (AceType.AccessAllowedCallbackObject, aces [4].AceType);
+			Assert.AreEqual (52, aces [0].BinaryLength);
+			Assert.AreEqual (60, aces [1].BinaryLength);
+			Assert.AreEqual (48, aces [2].BinaryLength);
+			Assert.AreEqual (64, aces [3].BinaryLength);
+			Assert.AreEqual (32, aces [4].BinaryLength);
+
+			RawAcl acl = new RawAcl (RawAcl.AclRevision, 0);
+			for (int i = 0; i < aces.Length; i ++)
+				acl.InsertAce (i, aces[i]);
+			return acl;
+		}
+		
+		void CompareBinaryForms (byte[] binaryFormExpected, byte[] binaryFormActual)
+		{
+			Assert.AreEqual (binaryFormExpected.Length, binaryFormActual.Length);
+			for (int i = 0; i < binaryFormExpected.Length; i ++)
+				Assert.AreEqual (binaryFormExpected [i], binaryFormActual [i], "Mismatch at position " + i.ToString ());
+		}
+		
+		[Test]
+		public void BinaryRoundtrip ()
+		{
+			RawAcl acl = CreateRoundtripRawAcl ();
+			byte[] binaryForm1 = new byte[acl.BinaryLength];
+			acl.GetBinaryForm (binaryForm1, 0);
+
+			RawAcl acl2 = new RawAcl (binaryForm1, 0);
+			byte[] binaryForm2 = new byte[acl2.BinaryLength];
+			acl2.GetBinaryForm (binaryForm2, 0);
+
+			CompareBinaryForms (binaryForm1, binaryForm2);
+		}
+		
+		[Test] // This blob produced by binaryForm1 from the BinaryRoundtrip test...
+		public void BlobMatchesMSNet ()
+		{
+			RawAcl acl = CreateRoundtripRawAcl ();
+			byte[] binaryForm1 = new byte[acl.BinaryLength];
+			acl.GetBinaryForm (binaryForm1, 0);
+
+			byte[] binaryForm2 = new byte[] { // 11 per line
+(byte)0x02, (byte)0x00, (byte)0x08, (byte)0x01, (byte)0x05, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x05, (byte)0x00, (byte)0x34,
+(byte)0x00, (byte)0x01, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x01, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x01, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x05,
+(byte)0x20, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x21, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x0B, (byte)0x00, (byte)0x3C, (byte)0x00, (byte)0x02, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x01, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x05, (byte)0x20, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x21, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x0B,
+(byte)0x00, (byte)0x30, (byte)0x00, (byte)0x04, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x90, (byte)0xFB, (byte)0x65, (byte)0x88, (byte)0xEB, (byte)0xA9, (byte)0x2F, (byte)0x42, (byte)0xA8, (byte)0xBA, (byte)0x07,
+(byte)0xEC, (byte)0xA6, (byte)0x11, (byte)0xD6, (byte)0x99, (byte)0x01, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x05, (byte)0x20, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x21, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x0B, (byte)0x00, (byte)0x40, (byte)0x00, (byte)0x04, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x03, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x7C, (byte)0x00,
+(byte)0x93, (byte)0xB8, (byte)0xD5, (byte)0x38, (byte)0x27, (byte)0x48, (byte)0xA6, (byte)0x98, (byte)0xBA, (byte)0x25, (byte)0xF1,
+(byte)0xE3, (byte)0x0B, (byte)0xAC, (byte)0x01, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x05,
+(byte)0x20, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x21, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x0B, (byte)0x00, (byte)0x20, (byte)0x00, (byte)0x04, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+(byte)0x00, (byte)0x00, (byte)0x01, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x05, (byte)0x20,
+(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x21, (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00
+			};
+			
+			CompareBinaryForms (binaryForm2, binaryForm1);
+		}
+	}
+}
+

mcs/class/corlib/corlib_test.dll.sources

@@ -208,6 +208,7 @@ System.Security.AccessControl/AuthorizationRuleTest.cs
 System.Security.AccessControl/CommonAceTest.cs
 System.Security.AccessControl/CommonAclTest.cs
 System.Security.AccessControl/MutexAccessRuleTest.cs
+System.Security.AccessControl/ObjectAceTest.cs
 System.Security.AccessControl/ObjectSecurity_TTest.cs
 System.Security.AccessControl/RawAclTest.cs
 System.Security.AccessControl/RawSecurityDescriptorTest.cs