[mono] Implement IgnoresAccessChecksToAttribute support (#48558)

* Implement IgnoresAccessChecksToAttribute support

   System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute is like the reverse of InternalsVisibleTo - it's added to an assembly to indicate that that assembly can call non-public methods from the specified assembly.

   The main difference is that while InternalsVisibleTo allows access to `internal` members, IgnoresAccessChecksTo also allows access to `private` members.

   The interesting thing is that the DynamicProxy dynamically generates the attribute, so it is not explicitly declared in CoreLib.

   Fixes https://github.com/dotnet/runtime/issues/47989

* Add System.Reflection.Emit regression tests that throw MethodAccessException

   check that calling private/internal methods in another assembly throws.
   check that calling a private method in the same assembly throws.

src/libraries/System.Reflection.DispatchProxy/tests/DispatchProxyTests.cs

@@ -137,28 +137,24 @@ public static void Create_Using_BaseType_Without_Default_Ctor_Throws_ArgumentExc
         }
 
         [Fact]
-        [SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
         public static void Create_Using_PrivateProxy()
         {
             Assert.NotNull(TestType_PrivateProxy.Proxy<TestType_IHelloService>());
         }
 
         [Fact]
-        [SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
         public static void Create_Using_PrivateProxyAndInternalService()
         {
             Assert.NotNull(TestType_PrivateProxy.Proxy<TestType_InternalInterfaceService>());
         }
 
         [Fact]
-        [SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
         public static void Create_Using_PrivateProxyAndInternalServiceWithExternalGenericArgument()
         {
             Assert.NotNull(TestType_PrivateProxy.Proxy<TestType_InternalInterfaceWithNonPublicExternalGenericArgument>());
         }
 
         [Fact]
-        [SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
         public static void Create_Using_InternalProxy()
         {
             Assert.NotNull(DispatchProxy.Create<TestType_InternalInterfaceService, InternalInvokeProxy>());
@@ -171,7 +167,6 @@ public static void Create_Using_ExternalNonPublicService()
         }
 
         [Fact]
-        [SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
         public static void Create_Using_InternalProxyWithExternalNonPublicBaseType()
         {
             Assert.NotNull(DispatchProxy.Create<TestType_IHelloService, TestType_InternalProxyInternalBaseType>());
@@ -190,7 +185,6 @@ public static void Create_Using_InternalServiceWithGenericArgumentBeingNonPublic
         }
 
         [Fact]
-        [SkipOnMono("https://github.com/dotnet/runtime/issues/47989")]
         public static void Create_Using_InternalProxyWithBaseTypeImplementingServiceWithgenericArgumentBeingNonPublicExternalService()
         {
             Assert.NotNull(DispatchProxy.Create<TestType_IHelloService, TestType_InternalProxyImplementingInterfaceWithGenericArgumentBeingNonPublicExternalType>());

src/libraries/System.Reflection.Emit/tests/AssemblyBuilderTests.cs

@@ -340,5 +340,91 @@ private static void VerifyAssemblyBuilder(AssemblyBuilder assembly, AssemblyName
             Assert.Empty(assembly.DefinedTypes);
             Assert.Empty(assembly.GetTypes());
         }
+
+	private static void SamplePrivateMethod ()
+	{
+	}
+
+	internal static void SampleInternalMethod ()
+	{
+	}
+
+	[Fact]
+	void Invoke_Private_CrossAssembly_ThrowsMethodAccessException()
+	{
+	    TypeBuilder tb = Helpers.DynamicType(TypeAttributes.Public);
+	    var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] {  });
+
+	    var ilg = mb.GetILGenerator ();
+
+	    var callee = typeof (AssemblyTests).GetMethod ("SamplePrivateMethod", BindingFlags.Static | BindingFlags.NonPublic);
+
+	    ilg.Emit (OpCodes.Call, callee);
+	    ilg.Emit (OpCodes.Ret);
+
+	    var ty = tb.CreateType ();
+
+	    var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);
+
+	    var d = (Action) mi.CreateDelegate (typeof(Action));
+
+	    Assert.Throws<MethodAccessException>(() => d ());
+	}
+
+	[Fact]
+	void Invoke_Internal_CrossAssembly_ThrowsMethodAccessException()
+	{
+	    TypeBuilder tb = Helpers.DynamicType(TypeAttributes.Public);
+	    var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] {  });
+
+	    var ilg = mb.GetILGenerator ();
+
+	    var callee = typeof (AssemblyTests).GetMethod ("SampleInternalMethod", BindingFlags.Static | BindingFlags.NonPublic);
+
+	    ilg.Emit (OpCodes.Call, callee);
+	    ilg.Emit (OpCodes.Ret);
+
+	    var ty = tb.CreateType ();
+
+	    var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);
+
+	    var d = (Action) mi.CreateDelegate (typeof(Action));
+
+	    Assert.Throws<MethodAccessException>(() => d ());
+	}
+	
+	[Fact]
+	void Invoke_Private_SameAssembly_ThrowsMethodAccessException()
+	{
+	    ModuleBuilder modb = Helpers.DynamicModule();
+	    
+	    string calleeName = "PrivateMethod";
+
+	    TypeBuilder tbCalled = modb.DefineType ("CalledClass", TypeAttributes.Public);
+	    var mbCalled = tbCalled.DefineMethod (calleeName, MethodAttributes.Private | MethodAttributes.Static);
+	    mbCalled.GetILGenerator().Emit (OpCodes.Ret);
+
+	    var tyCalled = tbCalled.CreateType();
+	    var callee = tyCalled.GetMethod (calleeName, BindingFlags.NonPublic | BindingFlags.Static);
+
+	    TypeBuilder tb = modb.DefineType("CallerClass", TypeAttributes.Public);
+	    var mb = tb.DefineMethod ("MyMethod", MethodAttributes.Public | MethodAttributes.Static, typeof(void), new Type[] {  });
+
+	    var ilg = mb.GetILGenerator ();
+
+	    ilg.Emit (OpCodes.Call, callee);
+	    ilg.Emit (OpCodes.Ret);
+
+	    var ty = tb.CreateType ();
+
+	    var mi = ty.GetMethod ("MyMethod", BindingFlags.Static | BindingFlags.Public);
+
+	    var d = (Action) mi.CreateDelegate (typeof(Action));
+
+	    Assert.Throws<MethodAccessException>(() => d ());
+	}
+
+
+
     }
 }

src/mono/mono/metadata/assembly.c

@@ -2080,8 +2080,9 @@ mono_assembly_request_open (const char *filename, const MonoAssemblyOpenRequest
 }
 
 static void
-free_item (gpointer val, gpointer user_data)
+free_assembly_name_item (gpointer val, gpointer user_data)
 {
+	mono_assembly_name_free_internal ((MonoAssemblyName *)val);
 	g_free (val);
 }
 
@@ -2105,7 +2106,6 @@ mono_assembly_load_friends (MonoAssembly* ass)
 	ERROR_DECL (error);
 	int i;
 	MonoCustomAttrInfo* attrs;
-	GSList *list;
 
 	if (ass->friend_assembly_names_inited)
 		return;
@@ -2126,7 +2126,9 @@ mono_assembly_load_friends (MonoAssembly* ass)
 	}
 	mono_assemblies_unlock ();
 
-	list = NULL;
+	GSList *visible_list = NULL;
+	GSList *ignores_list = NULL;
+
 	/* 
 	 * We build the list outside the assemblies lock, the worse that can happen
 	 * is that we'll need to free the allocated list.
@@ -2138,7 +2140,16 @@ mono_assembly_load_friends (MonoAssembly* ass)
 		uint32_t data_length;
 		gchar *data_with_terminator;
 		/* Do some sanity checking */
-		if (!attr->ctor || attr->ctor->klass != mono_class_try_get_internals_visible_class ())
+		if (!attr->ctor)
+			continue;
+		gboolean has_visible = FALSE;
+		gboolean has_ignores = FALSE;
+		has_visible = attr->ctor->klass == mono_class_try_get_internals_visible_class ();
+		/* IgnoresAccessChecksToAttribute is dynamically generated, so it's not necessarily in CoreLib */
+		/* FIXME: should we only check for it in dynamic modules? */
+		has_ignores = (!strcmp ("IgnoresAccessChecksToAttribute", m_class_get_name (attr->ctor->klass)) &&
+			       !strcmp ("System.Runtime.CompilerServices", m_class_get_name_space (attr->ctor->klass)));
+		if (!has_visible && !has_ignores)
 			continue;
 		if (attr->data_size < 4)
 			continue;
@@ -2152,7 +2163,10 @@ mono_assembly_load_friends (MonoAssembly* ass)
 		aname = g_new0 (MonoAssemblyName, 1);
 		/*g_print ("friend ass: %s\n", data);*/
 		if (mono_assembly_name_parse_full (data_with_terminator, aname, TRUE, NULL, NULL)) {
-			list = g_slist_prepend (list, aname);
+			if (has_visible)
+				visible_list = g_slist_prepend (visible_list, aname);
+			if (has_ignores)
+				ignores_list = g_slist_prepend (ignores_list, aname);
 		} else {
 			g_free (aname);
 		}
@@ -2163,11 +2177,14 @@ mono_assembly_load_friends (MonoAssembly* ass)
 	mono_assemblies_lock ();
 	if (ass->friend_assembly_names_inited) {
 		mono_assemblies_unlock ();
-		g_slist_foreach (list, free_item, NULL);
-		g_slist_free (list);
+		g_slist_foreach (visible_list, free_assembly_name_item, NULL);
+		g_slist_free (visible_list);
+		g_slist_foreach (ignores_list, free_assembly_name_item, NULL);
+		g_slist_free (ignores_list);
 		return;
 	}
-	ass->friend_assembly_names = list;
+	ass->friend_assembly_names = visible_list;
+	ass->ignores_checks_assembly_names = ignores_list;
 
 	/* Because of the double checked locking pattern above */
 	mono_memory_barrier ();
@@ -3318,7 +3335,6 @@ mono_assembly_release_gc_roots (MonoAssembly *assembly)
 gboolean
 mono_assembly_close_except_image_pools (MonoAssembly *assembly)
 {
-	GSList *tmp;
 	g_return_val_if_fail (assembly != NULL, FALSE);
 
 	if (assembly == REFERENCE_MISSING)
@@ -3343,12 +3359,10 @@ mono_assembly_close_except_image_pools (MonoAssembly *assembly)
 	if (!mono_image_close_except_pools (assembly->image))
 		assembly->image = NULL;
 
-	for (tmp = assembly->friend_assembly_names; tmp; tmp = tmp->next) {
-		MonoAssemblyName *fname = (MonoAssemblyName *)tmp->data;
-		mono_assembly_name_free_internal (fname);
-		g_free (fname);
-	}
+	g_slist_foreach (assembly->friend_assembly_names, free_assembly_name_item, NULL);
+	g_slist_foreach (assembly->ignores_checks_assembly_names, free_assembly_name_item, NULL);
 	g_slist_free (assembly->friend_assembly_names);
+	g_slist_free (assembly->ignores_checks_assembly_names);
 	g_free (assembly->basedir);
 
 	MONO_PROFILER_RAISE (assembly_unloaded, (assembly));

src/mono/mono/metadata/class.c

@@ -5855,6 +5855,23 @@ is_valid_family_access (MonoClass *access_klass, MonoClass *member_klass, MonoCl
 	return TRUE;
 }
 
+static gboolean
+ignores_access_checks_to (MonoAssembly *accessing, MonoAssembly *accessed)
+{
+	if (!accessing || !accessed)
+		return FALSE;
+
+	mono_assembly_load_friends (accessing);
+	for (GSList *tmp = accessing->ignores_checks_assembly_names; tmp; tmp = tmp->next) {
+		MonoAssemblyName *victim = (MonoAssemblyName *)tmp->data;
+		if (!victim->name)
+			continue;
+		if (!g_ascii_strcasecmp (accessed->aname.name, victim->name))
+			return TRUE;
+	}
+	return FALSE;
+}
+
 static gboolean
 can_access_internals (MonoAssembly *accessing, MonoAssembly* accessed)
 {
@@ -5880,7 +5897,7 @@ can_access_internals (MonoAssembly *accessing, MonoAssembly* accessed)
 		}
 		return TRUE;
 	}
-	return FALSE;
+	return ignores_access_checks_to (accessing, accessed);
 }
 
 /*
@@ -5979,7 +5996,9 @@ can_access_type (MonoClass *access_klass, MonoClass *member_klass)
 		return member_klass_nested_in && can_access_type (access_klass, member_klass_nested_in);
 
 	case TYPE_ATTRIBUTE_NESTED_PRIVATE:
-		return is_nesting_type (member_klass, access_klass) && member_klass_nested_in && can_access_type (access_klass, member_klass_nested_in);
+		if (is_nesting_type (member_klass, access_klass) && member_klass_nested_in && can_access_type (access_klass, member_klass_nested_in))
+			return TRUE;
+		return ignores_access_checks_to (access_klass_assembly, member_klass_assembly);
 
 	case TYPE_ATTRIBUTE_NESTED_FAMILY:
 		return mono_class_has_parent_and_ignore_generics (access_klass, m_class_get_nested_in (member_klass)); 
@@ -6028,7 +6047,7 @@ can_access_member (MonoClass *access_klass, MonoClass *member_klass, MonoClass*
 		/* same compilation unit */
 		return m_class_get_image (access_klass) == member_klass_image;
 	case FIELD_ATTRIBUTE_PRIVATE:
-		return access_klass == member_klass;
+		return (access_klass == member_klass) || ignores_access_checks_to (access_klass_assembly, member_klass_image->assembly);
 	case FIELD_ATTRIBUTE_FAM_AND_ASSEM:
 		if (is_valid_family_access (access_klass, member_klass, context_klass) &&
 		    can_access_internals (access_klass_assembly, member_klass_image->assembly))

src/mono/mono/metadata/metadata-internals.h

@@ -219,6 +219,7 @@ struct _MonoAssembly {
 	MonoAssemblyName aname;
 	MonoImage *image;
 	GSList *friend_assembly_names; /* Computed by mono_assembly_load_friends () */
+	GSList *ignores_checks_assembly_names; /* Computed by mono_assembly_load_friends () */
 	guint8 friend_assembly_names_inited;
 	guint8 dynamic;
 	MonoAssemblyContext context;