Partager via


Using SecAnnotate to Analyze Your Assemblies for Transparency Violations – An Example

SecAnnotate (available in the final .NET 4 SDK, and in beta form here) can be used to analyze your assemblies, especially APTCA assemblies in order to find transparency violations without needing code coverage from a test case.  Instead, the static analysis provided by SecAnnotate is valuable in ensuring that your assembly is fully correct from a transparency perspective.  Let’s take a look at how it might be used for a simple APTCA library.

SecAnnotate runs in two different modes.  By default, it runs in annotation mode where it loops over your assembly looking for transparency violations and creating suggestions for how they might be fixed.  It then applies these suggestions to its model of the transparency of your assembly and loops again finding any violations that this updated set of annotations would have caused.  This process is repeated until there are no further transparency violations found.  That is, SecAnnotate’s algorithm is basically:

  1. Make a pass over the assembly finding any transparency violations
  2. Generate a set of annotations that would fix the violations found in (1)
  3. Apply the fixes from (2) in memory to the assembly, with the more recent annotations taking precedent over annotations found in previous passes.
  4. If any updates were made in step (3), repeat from step (1)

It’s important to note that SecAnnotate is making suggestions for annotations that would fix the transparency violations.  A real human does need to scan the suggested annotations to ensure that they make sense for your code.  For example, SecAnnotate will tend to be conservative and if a violation could be fixed by making a method either critical or safe critical, SecAnnotate will recommend that the method be critical.  However, if you look at the method maybe it really is safe critical (using unverifiable code as an implementation detail, or issuing a demand, etc).  In that case you can mark the method as safe critical and avoid having the criticalness of the method fan-out.

Similarly, in some cases SecAnnotate will indicate that a method must be safe critical (to satisfy inheritance rules for instance).  In those cases, it’s important to make sure that the method really is safe critical – that it is validating inputs, outputs, and ensuring that the operation being performed is safe.

Let’s look at an example of using SecAnnotate on a Buffer class (used here to illustrate use of SecAnnotate, not as an example of a world-class Buffer type :-):

 

using System;

using System.Runtime.InteropServices;

using System.Security;

using System.Security.Permissions;

[assembly: AllowPartiallyTrustedCallers]

[assembly: SecurityRules(SecurityRuleSet.Level2)]

public sealed class Buffer : IDisposable

{

    private IntPtr m_buffer;

    private int m_size;

    public Buffer(int size)

    {

        if (size <= 0)

            throw new ArgumentException("size");

        m_size = size;

        m_buffer = Marshal.AllocCoTaskMem(size);

    }

    ~Buffer()

    {

        Dispose(false);

    }

    public void Dispose()

    {

        Dispose(true);

        GC.SuppressFinalize(this);

    }

    private void Dispose(bool disposing)

    {

        if (m_buffer != IntPtr.Zero)

        {

            Marshal.FreeCoTaskMem(m_buffer);

            m_buffer = IntPtr.Zero;

        }

    }

    public IntPtr NativePointer

    {

        [SecurityPermission(SecurityAction.LinkDemand, UnmanagedCode = true)]

        get

        {

            if (m_buffer == IntPtr.Zero)

                throw new ObjectDisposedException(GetType().FullName);

            return m_buffer;

        }

    }

    public int Size

    {

        get

        {

            if (m_buffer == IntPtr.Zero)

                throw new ObjectDisposedException(GetType().FullName);

            return m_size;

        }

    }

}

 

We can then build this into Buffer.dll and run SecAnnotate.exe over it as follows:

 

C:\blog>c:\Windows\Microsoft.NET\Framework\v4.0.21006\csc.exe /debug /t:library Buffer.cs

Microsoft (R) Visual C# 2010 Compiler version 4.0.21006.1

Copyright (C) Microsoft Corporation. All rights reserved.

C:\blog>"c:\Program Files\SecAnnotate\SecAnnotate.exe" Buffer.dll

Microsoft (R) .NET Framework Security Transparency Annotator 4.0.21105.0

Copyright (c) Microsoft Corporation.  All rights reserved.

Annotating 'buffer'.

Beginning pass 1.

Pass complete, 4 new annotation(s) generated.

Beginning pass 2.

Pass complete, 2 new annotation(s) generated.

Beginning pass 3.

Pass complete, 2 new annotation(s) generated.

Beginning pass 4.

Pass complete, 0 new annotation(s) generated.

Annotating complete. 8 errors found.

  MethodsMustOverrideWithConsistentTransparency : 2 violation(s)

  SecurityRuleSetLevel2MethodsShouldNotBeProtectedWithLinkDemands : 1

  violation(s)

  TransparentMethodsMustNotReferenceCriticalCode : 4 violation(s)

  TransparentMethodsShouldNotBeProtectedWithLinkDemands : 1 violation(s)

Writing annotation report 'TransparencyAnnotations.xml'.

We can see from the output that Buffer.dll required 3 passes to annotate and on the fourth pass no new violations were found.

In the first pass, SecAnnotate found four annotations:

  1. Buffer’s constructor calls Marshal.AllocCoTaskMem which is security critical.  Therefore, the suggested annotation is that Buffer’s constructor also become critical.
  2. Dispose(bool) calls Marshal.FreeCoTaskMem which is security critical.  SecAnnotate suggests that Dispose(bool) become security critical
  3. The NativePointer property getter is protected with a LinkDemand.  Level 2 transparency deprecates LinkDemands in favor of security critical APIs, so the LinkDemand should be removed and the getter be made critical.
  4. The NativePointer property additionally is security transparent, but is protected with a LinkDemand.  This is a strange pattern since transparent code shouldn’t be doing anything that needs protecting.

Those four annotations lead SecAnnotate in pass 1 to update the constructor, Dispose(bool) and NativePointer getter to be security critical and move on to pass 2.  The second pass results in the following violations:

  1. Dispose() calls Dispose(bool) which was made critical in pass 1.  Since Dispose() is transparent, this is a violation.  SecAnnotate will now make Dispose() critical.
  2. Buffer’s Finalizer also calls Dispose(bool), which means that it also must now become security critical.

After applying those annotations to its in-memory model of Buffer.dll, SecAnnotate continues onto pass 3:

  1. Dispose() is critical from pass 2.  However, it implements security transparent interface member IDisposable.Dispose.  This is a violation of the transparency rules – so SecAnnotate suggests Dispose() become safe critical.
  2. Similarly, the finalizer is critical from pass 2, however it overrides transparent virtual method Object.Finalize. This is also a violation of transparency rules – so SecAnnotate suggests that the finalizer become safe critical.

Applying this final set of annotations leads to a pass with no new errors detected, and so SecAnnotate writes out its final report.  This report is divided into two sections – the required annotations section listing methods and types that need to be updated to fix transparency violations and the rule information section with details about each transparency rule that was tripped by this assembly.

I’ve attached the output from SecAnnotate.exe to this blog post so that you can see an example report even if you haven’t run SecAnnotate yourself.

Let’s look first at the required annotations section.  For each type, method, or field that needs to have a transparency annotation added, there will be an XML section with the suggested annotations, the reason for the annotation, and the pass number that the annotation was detected in.

For example, the XML for Buffer’s constructor looks like this:

 

<method name=".ctor(Int32)">

  <annotations>

    <critical>

      <rule name="TransparentMethodsMustNotReferenceCriticalCode">

        <reason pass="1" sourceFile="c:\blog\buffer.cs" sourceLine="20">Transparent method 'Buffer..ctor(System.Int32)' references security critical method 'System.Runtime.InteropServices.Marshal.AllocCoTaskMem(System.Int32)'.  In order for this reference to be allowed under the security transparency rules, either 'Buffer..ctor(System.Int32)' must become security critical or safe-critical, or 'System.Runtime.InteropServices.Marshal.AllocCoTaskMem(System.Int32)' become security safe-critical or transparent.</reason>

      </rule>

    </critical>

  </annotations>

</method>

 

Which indicates that on pass 1, SecAnnotate detected that Buffer’s constructor was transparent but called Marshal.AllocCoTaskMem.  Since we have symbols available, SecAnnotate also pointed out the source file and line number that made this API call.  Because of this call, pass 1 suggests that the constructor (taking an Int32 parameter – in case you have multiple overloads) become security critical.

The Dispose(bool) section looks very similar:

 

<method name="Dispose(Boolean)">

  <annotations>

    <critical>

      <rule name="TransparentMethodsMustNotReferenceCriticalCode">

        <reason pass="1" sourceFile="c:\blog\buffer.cs" sourceLine="38">Transparent method 'Buffer.Dispose(System.Boolean)' references security critical method 'System.Runtime.InteropServices.Marshal.FreeCoTaskMem(System.IntPtr)'.  In order for this reference to be allowed under the security transparency rules, either 'Buffer.Dispose(System.Boolean)' must become security critical or safe-critical, or 'System.Runtime.InteropServices.Marshal.FreeCoTaskMem(System.IntPtr)' become security safe-critical or transparent.</reason>

      </rule>

    </critical>

  </annotations>

</method>

 

As we expect NativePointer getter has two violations in pass 1 – both of which suggest that the method become security critical.

 

<method name="get_NativePointer()">

  <annotations>

    <critical>

      <rule name="SecurityRuleSetLevel2MethodsShouldNotBeProtectedWithLinkDemands">

        <reason pass="1" sourceFile="c:\blog\buffer.cs" sourceLine="47">'Buffer.get_NativePointer()' is protected with a LinkDemand for 'SecurityPermissionAttribute'.  In the level 2 security rule set, it should be protected by being security critical instead.  Remove the LinkDemand and mark 'Buffer.get_NativePointer()' security critical.</reason>

      </rule>

      <rule name="TransparentMethodsShouldNotBeProtectedWithLinkDemands">

        <reason pass="1" sourceFile="c:\blog\buffer.cs" sourceLine="47">Transparent method 'Buffer.get_NativePointer()' is protected with a LinkDemand for 'SecurityPermissionAttribute'.  Remove this LinkDemand, or make the method security critical or safe-critical.</reason>

      </rule>

    </critical>

  </annotations>

</method>

 

The rules violated here are different, but both suggest that the method become critical and so are both listed in the <critical> section of the method’s report.

More interesting is the report output for Dispose() – remember, pass 2 detected that this method should be critical because it is calling the critical Dispose(bool) overload from pass 1.  However, pass 3 detected that being critical actually tripped an inheritance rule violation and the method should really be safe critical:

 

<method name="Dispose()">

  <annotations>

    <safeCritical>

      <rule name="MethodsMustOverrideWithConsistentTransparency">

        <reason pass="3" sourceFile="c:\blog\buffer.cs" sourceLine="29">Critical method 'Buffer.Dispose()' is overriding transparent or safe critical method 'System.IDisposable.Dispose()' in violation of method override rules.  'Buffer.Dispose()' must become transparent or safe-critical in order to override a transparent or safe-critical virtual method or implement a transparent or safe-critical interface method.</reason>

      </rule>

    </safeCritical>

    <critical>

      <rule name="TransparentMethodsMustNotReferenceCriticalCode">

        <reason pass="2" sourceFile="c:\blog\buffer.cs" sourceLine="30">Transparent method 'Buffer.Dispose()' references security critical method 'Buffer.Dispose(System.Boolean)'.  In order for this reference to be allowed under the security transparency rules, either 'Buffer.Dispose()' must become security critical or safe-critical, or 'Buffer.Dispose(System.Boolean)' become security safe-critical or transparent.</reason>

      </rule>

    </critical>

  </annotations>

</method>

 

This section contains both a <safeCritical> and a <critical> section – so SecAnnotate is making two different suggestions for this method.  The way to read this output is to scan for the pass numbers.  Remember that the SecAnnotate algorithm has the later passes override the earlier passes for transparency purposes so the final suggested annotation is the one with the largest pass number.  In this case we have a safe critical suggestion in pass 3 – which means that SecAnnotate is suggesting this method be safe critical.

If SecAnnotate is recommending the annotation from the largest pass number, then why does it output all of the lower passes as well?   The reason is to provide the most context to the person analyzing the report.  By tracing through the passes from lowest number to highest we can see why SecAnnotate first decided that the method must be critical before later deciding to make it safe critical (and the reason behind that switch).   As we’ll see later, that information can be quite useful when deciding upon which annotations to add to your source code.

In this case, SecAnnotate is saying that the second pass caused Dispose() to be marked critical due to calling Dispose(bool).  In the third pass, in order to satisfy inheritance rules, Dispose() needed to be marked safe critical.

Similar analysis applies to the finalizer section of the report.

Now that we’ve finished looking through the report, let’s update the Buffer code to use SecAnnotate’s recommendations in order to fix its transparency violations.

To start with, the final set of annotations that SecAnnotate recommends for us is:

  • Constructor –> critical (from pass 1)
  • Dispose() –> safe critical (from pass 3)
  • Dispose(bool) –> critical (from pass 1)
  • Finalizer –> safe critical (from pass 3)
  • NativePointer getter –> critical (from pass 1)

As I mentioned earlier, SecAnnotate is making recommendations for a set of annotations that will fix up transparency violations in the assembly being analyzed.  However, a real human should always look at the recommendations to ensure that they are optimal and correct for the particular code.

Let’s look at them one by one.  Generally, it’s convenient to work from APIs identified in earlier passes out to APIs that were identified in later passes (as we’ll see here).  In fact one technique for using SecAnnotate is to limit it to only one pass, fix up that pass, and then rerun SecAnnotate to completion.  I’ll talk more about using that technique in a later post.

With that in mind, let’s start with the constructor which was flagged in pass 1.  The constructor must be critical because it calls a critical API to allocate memory.  However, making the constructor itself security critical has an important consequence – transparent code can no longer use the constructor code.  If we want to continue to allow transparent access to the Buffer class then we need to find a way to make the constructor safe critical instead.

The critical operation that was identified is calling a critical API to allocate unmanaged memory.  We might want to prevent transparent code from being able to do this if they could access the address of this memory, however there is no direct transparent access to that memory.  Further, the allocated buffer will be released when the finalizer or dispose method is called – which means that a leak can only be caused by transparent code if it holds onto a strong reference to the Buffer object.  That is not a threat that this API cares about because partial trust code could already cause the same effect by simply holding onto a strong reference to a large managed array.

You might notice that it doesn’t make much sense to make the constructor safe critical in order to expose it to partial trust code, after all you must be critical to do anything interesting with a Buffer object in the first place.   While that’s true, being safe critical also opens another important use case – full trust callers do not need to be critical to allocate a Buffer object either.  Their code that accesses the buffer itself may still be required to be critical, however we don’t expand that out to the code that is simply allocating the buffer in the first place.  By not requiring the allocation methods to become critical, we’ve reduced the audit burden on them, as we’ve done that work here lower in the stack to prove that this is a safe operation.

From this analysis, it turns out to be safe to make the constructor safe critical.

Next let’s look at the Dispose(bool) method.  This is also flagged as being security critical because it references a security critical API.  That API is exposing the ability to release the memory used by the Buffer.  However, since we allow transparent code to allocate the buffer, it stands to reason that we also want it to be able to free the buffer.  Our threat model shows that there is a threat here – since the Buffer class is not thread safe, it is possible for critical code to be using the memory address that the buffer allocated at the same time that the buffer is released.  This could lead to transparent code causing critical code to trash memory on the heap.

That threat might be mitigated by thoroughly documenting that this Buffer type is not thread safe, and that critical code should not expose it out to transparent code that might trigger a race condition on the use of the object.   Analysis of the type makes us believe that this would be the most common use pattern anyway (there’s not any compelling functionality that is gained by allowing outside code access to your buffer, especially malicious partial trust code).

With that in mind, it turns out that safe critical might be a better annotation for Dispose(bool) rather than critical.

The final pass 1 annotation is the NativePointer getter.  This is flagged as being critical because it used to be protected with a LinkDemand which is now obsolete in the level 2 security transparency model.  Making the getter be critical makes sense because we don’t want to expose the address of the pointer out to unaudited or untrusted code.

However, that leads us to an interesting thought – if we don’t want to expose the unaudited address to partial trust code, and that address is stored in the m_buffer field, then it might make sense to make that field itself security critical.  In general SecAnnotate cannot make a suggestion like this because it doesn’t know what fields store sensitive information.  However, we know that this field is sensitive, so we should make it critical.

This will have the side effect of causing SecAnnotate (and the runtime) to flag any code that gets added to the Buffer class later which accesses m_buffer directly, rather than going through the NativePointer property getter.  Since that code is touching a sensitive field, SecAnnotate will flag it for accessing a critical piece of data and ensure that it is either critical or safe critical and audited.

Now, let’s move to the later passes.   Both Dispose() and the finalizer got flagged in pass 2 to be security safe critical because they were using security critical method Dispose(bool).  However, we previously decided that Dispose(bool) should be safe critical rather than critical.  That means that both Dispose() and the finalizer can stay transparent (since pass 2 would now be satisfied by transparent calling critical code).   This is an example of using the full SecAnnotate report in order to come up with a better set of annotations – we know why the method was flagged to be critical in pass 2, so we know we may not need to consider later passes.

With all that in mind, our final set of proposed annotations based upon the SecAnnotate report is:

  • m_buffer –> critical
  • Constructor –> safe critical
  • Dispose(bool) –> safe critical
  • NativePointer getter –> critical

Putting these updates into place, we can rerun SecAnnotate to check our work.  In this case, SecAnnotate will find that there is one remaining violation to correct which stemmed from our decision to push the security critical attribute down from the NativePointer property onto the field that it is exposing (note that had we gone with the original SecAnnotae suggestion of only marking the NativePointer getter as critical, this violation wouldn’t have shown up – instead it was our decision to mark the underlying sensitive data as critical that flagged a new violation):

 

<method name="get_Size()">

  <annotations>

    <critical>

      <rule name="TransparentMethodsMustNotReferenceCriticalCode">

        <reason pass="1" sourceFile="c:\blog\buffer.cs" sourceLine="72">Transparent method 'Buffer.get_Size()' references security critical field 'Buffer.m_buffer'.  In order for this reference to be allowed under the security transparency rules, either 'Buffer.get_Size()' must become security critical or safe-critical, or 'Buffer.m_buffer' become security safe-critical or transparent.</reason>

      </rule>

    </critical>

  </annotations>

</method>

 

Since the Size getter is accessing the critical m_buffer field, SecAnnotate flags it to be critical.  However, since it is not exposing this field and simply using it to make sure that the buffer hasn’t yet been cleaned up (the end user can never figure out anything other than the buffer has a non-null field value from calling this property), we can safely make this a safe critical field as well.

With that final update in place:

 

using System;

using System.Runtime.InteropServices;

using System.Security;

using System.Security.Permissions;

[assembly: AllowPartiallyTrustedCallers]

[assembly: SecurityRules(SecurityRuleSet.Level2)]

public sealed class Buffer : IDisposable

{

    [SecurityCritical]

    private IntPtr m_buffer;

    private int m_size;

    // Safe critical because we're only exposing the ability to allocate native memory, not

    // access that memory directly (access is gated through security critical APIs). Since

    // the threat of using up all of the memory for a process is not something that we're

    // looking to mitigate (after all, holding a large managed array has the same effect)

    // we don't need to gate access to this constructor.

    [SecuritySafeCritical]

    public Buffer(int size)

    {

        if (size <= 0)

            throw new ArgumentException("size");

        m_size = size;

        m_buffer = Marshal.AllocCoTaskMem(size);

    }

    ~Buffer()

    {

        Dispose(false);

    }

    public void Dispose()

    {

        Dispose(true);

        GC.SuppressFinalize(this);

    }

    // Safe critical because we're simply releasing the memory held by the buffer.

    // This is not safe to use cross-thread, so it is important to document that

    // trusted code not give access to their Buffer classes to untrusted code

    // which may trigger race conditions between use of the Buffer and release of

    // it.

    [SecuritySafeCritical]

    private void Dispose(bool disposing)

    {

        if (m_buffer != IntPtr.Zero)

        {

            Marshal.FreeCoTaskMem(m_buffer);

            m_buffer = IntPtr.Zero;

        }

    }

    public IntPtr NativePointer

    {

        [SecurityCritical]

        get

        {

            if (m_buffer == IntPtr.Zero)

                throw new ObjectDisposedException(GetType().FullName);

            return m_buffer;

        }

    }

    public int Size

    {

        // Safe critical since we aren't exposing the m_buffer field, and just use it

        // as an internal implementation detail to detect if the buffer is disposed

        // or not.

        [SecuritySafeCritical]

        get

        {

            if (m_buffer == IntPtr.Zero)

                throw new ObjectDisposedException(GetType().FullName);

            return m_size;

        }

    }

}

 

We should now be done annotating this class.  To ensure that we are, in fact, done we can run SecAnnotate in verification mode.  Unlike the default annotation mode, verification mode does not attempt to make multiple passes over the assembly and figure out a suggested set of annotations to fix any errors.  Instead, it just runs to ensure that there are no existing transparency violations in the assembly.

Its return value is equivalent to the number of violations found, so running in verification mode can be used as a post-build step to ensure that assemblies contain no transparency violations:

 

C:\blog>"c:\Program Files\SecAnnotate\SecAnnotate.exe" /v Buffer.dll

Microsoft (R) .NET Framework Security Transparency Annotator 4.0.21105.0

Copyright (c) Microsoft Corporation.  All rights reserved.

Verifying 'buffer'.

Verification complete. 0 error(s) found.

Transparency annotations on all assemblies successfully verified.

 

And with that we’re done – Buffer.dll is now verified to contain no violations of security transparency rules.  When we go to ship this assembly, the SecuritySafeCritical surface area will be subject to a security audit to ensure that they are safe and secure, and that our threat model has sufficient mitigation for any threats exposed. 

TransparencyAnnotations.xml