Preventing third-party derivation, part one

Preventing third-party derivation, part one

Rate This
  • Comments 27

In this episode of FAIC, a conversation I had with a C# user recently. Next time, some further thoughts on how to use the CLR security system in this sort of scenario.

Him: I have this abstract class defined in one assembly:

// Assembly FooBar.DLL
public abstract class Foo
{
    internal abstract void DoSomething();
// ...
}

I want to create a concrete class derived directly from Foo in another assembly, Blah.DLL.

Me: You are going to have to learn to live with disappointment.

Him: But I should be able to because I have public access to the class Foo!

Me: That statement is false. Having public access to a class absolutely does not mean that it is always legal to create a subclass in another assembly. There are any number of reasons why it might not be legal to create a subclass in another assembly; you happen to have run across just one of them. (There are plenty more -- for example, you could make a public class and mark all the constructors as internal. It's not possible to subclass that from another assembly either, since the derived class constructor does not have an accessible base class constructor that it can call.)

Him: You're right. In this case, I can’t provide an overriding implementation for the internal abstract method DoSomething() because it is not accessible. Therefore the compiler will give an error when I try to subclass Foo. How do I get around this?

Me: You don’t get around it. The type safety system is working as designed; don’t try to defeat it.

If you own the FooBar.DLL assembly then of course there are several ways you can do what you want. Two that immediately come to mind are (1) mark Blah.DLL as a friend assembly of FooBar.DLL using the InternalsVisibleTo attribute (2) change the accessibility of DoSomething to public, protected or protected internal.

Him: Why is it even allowed to have a nonextensible class like this in the first place?

Me: It is sometimes a good thing to make a class that cannot be extended arbitrarily. I have myself occasionally used a similar technique to ensure that no third party can subclass one of my public base classes, though, as we’ll see later, there are other, perhaps better ways.

Look at it this way: if the author of the class wanted you to be able to subclass it, they probably would have made it possible. Clearly they do not want you to subclass this class.

Him: My previous question has been thoroughly begged. Why would someone want to prevent a third party from subclassing?

Me: I can think of a few reasons. Here are two:

1) Designing a class to be extended effectively by third parties is work, and work requires effort. If the class is not intended to be extended by third parties, but must be unsealed (for internal extension, for example) then the implementer is faced with a choice: spend the time/money/effort unnecessarily, provide an extensible but brittle class, or prevent extension by some other method.

This trick is a cheap, easy and straightforward way to prevent extension by arbitrary third parties without preventing extension by internal classes. As we'll see next time, there are other ways you can do this too.

2) A class may need to be extensible internally and used externally, but must not be extended by third parties for security reasons. Imagine you have a method Transfer which takes two instances of your class BankAccount. Suppose BankAccount is an abstract class with two derived classes, CaymanIslandsAccount and SwissAccount. Do you really want arbitrary third parties able to make new objects which fulfill the BankAccount contract but were not implemented by you?

Again, you end up with a tradeoff – either implement Transfer so that it does type checks on the BankAccount passed to it (possibly expensive both in initial implementation and maintenance), implement Transfer so that it accepts any old thing (dangerous!), or prevent anyone from making an unknown kind of BankAccount (cheap, safe).

In general, good security design is “make them do something impossible in order to break the system”. (Or even better “make them do seven impossible things”.) By making it impossible for third parties to fulfill the contract, you add additional security to the system. (By no means enough security, but a little bit more.)

Him: Thanks, I see what's going on here. Clearly I got into this situation because this trick of using access modifiers to prevent extension is insufficient to convey the author of FooBar.DLL's intentions.

Me: Next time we'll talk about more obvious ways to state the intention of "please don't subclass this thing".

  • "Designing a class to be extended effectively by third parties is work, and work requires effort".

    If you don't do this, your overall design suffers, your library is crippled/limited from the start. And a little too much laziness, and the developers will move away from your library. You're limiting the usefulness of your classes to basically some predicted scenarios, while in the real world the shear number of possible "valid" use cases is orders of magnitude higher than what one can predict.

    "A class may need to be extensible internally and used externally, but must not be extended by third parties for security reasons. Imagine you have a method Transfer which takes two instances of your class BankAccount. Suppose BankAccount is an abstract class with two derived classes ..."

    One way around this is to make Transfer non virtual and have a method TransferInternal() marked as "internal virtual", which provides the necessary security while still allowing consumers of the class to use OO techniques to "add" functionality through derivation, and not be forced to use awkward workarounds.  What if someone needs to add some extra data to the SwissAccount class, without modifying it's core functionality, that must be used at higher levels? This scenario is made impossible by preventing derivation.

    Slightly offtopic:

    I'm strongly against sealing of classes to prevent derivation. It's so anti OOP. Instead of sealed, .Net should have had a closed keyword that prevents you from reimplementing interfaces the class already implements, and for virtual methods the final modifier is sufficient.  There were a few times when I really wished some classes are not sealed so I wouldn't have to use some crazy workarounds instead of simple derivation.

    I's true that some classes like System.String need to be sealed because of their special layout, but on other cases I would really preferred a closed approach instead of sealed.

  • Reduced accessibility isn't a security option, you could always get around it.. Don't even think about it this way :)

    I agree with Pop Catalins first paragraph, that attitude "i won't make this extensible beacuse i'm lazy", will definetly return later.. Such "lazy programming" has hidden costs in long-term development.

    Anyway, in my opinion (even little OT), one should use the interface almost everywhere, where is some base abstract class. Let's have an example:

    public abstract class FooBase {

       public abstract void DoSomething();

       public virtual void DoSomething2();

       public void DoSomething3();

       protected void HelperMethod();

    }

    Abstract method definitelly should be in iface, virtual method too, because the possible consumer can't rely on the default implementation (extension don't have to call the base). Helper method can't be consumed, and argument that 'it will be easier to extend that class' is wrong, you still can have base abstract class, with standard implementation etd., that implements iface, but consumer doesn't need to know about it. And at last, the problematic non-virtual method. I think, that consumer should never rely on the actual accurate implementation, because it's just hard to maintance and not flexible. Such method should be also declared by iface, that is, it could be virtual.. if it does sometning, that need's to be "closed", implement it as sealed in that easy-to-use abstract class.

    Sadly, C# alone does not encourage this pattern.. IoC does, and I love it :P

  • First off, who said anything about designing base class libraries? Designing an entire library of functionality that can be extended is even more work than designing a class to be extended. Why would someone do this expensive and difficult work if that wasn't the goal of their business?

    Second, your philosophical belief (or, perhaps, psychological condition! :-) ) is common in this industry. I call the belief that object-oriented design principles are always good principles and should be adhered to when solving every problem, whether the business problem needs OOP or not, "object happiness".

    Writing code that works well in stated, by-design scenarios and fails fast in all other scenarios is frequently my goal. That is a sensible business goal in a world where unnecessary extensibility has large design costs, implementation costs, testing costs, maintenance costs, and more exotic costs like "preventing us from making changes to anything for fear of breaking someone who is using our stuff in a crazy way".

    In short, I reject object happiness. Unnecessary extensibility is pure concentrated evil and should be avoided at all times. It is a huge expense that buys you more huge expenses later.

  • Furthermore, I object to the characterization of a good engineering choice as "lazy". Almost none of us design and implement software for ourselves; we work for someone else, and we are therefore spending their money. Deciding to spending someone else's money responsibly by cutting out all unnecessary features isn't _lazy_, it is _responsible_.

    The time and effort that you save by not designing, implementing, testing and maintaining unnecessary extensibility is time and effort that you can devote to improving your employer's bottom line in other ways.

  • The time an effort saved by building on top of an extensible OO architecture, instead building on top of a closed architecture, using all sorts of workarounds and hacks, is the real time saver. What happened to the open-closed principle?

  • Absolutely! If you are designing an architectural framework, by all means, do the work to make it open and extensible by careful design choices and high-quality implementation. That's obviously goodness. That would be an example of necessary extensibility.

    But most programmers are not designing and implementing architectural frameworks. Most programmers are implementing business logic. When business logic classes are unnecessarily designed and built as though they were base class libraries, they end up over-complicated, over budget, late, hard to test, full of security problems, and have extension points that do not solve any problem ever encountered by their users. That's waste.

    Don't make investments in technology that you don't need and might never need, on the off chance that someone, some day, might want to extend the class. Let that hypothetical person pay the price of unsealing the class and building extensibility into it if that's worth it to them.

  • Fortunately, however, you can use reflection to access just about anything you want (and oh do we ever use that).  You can even use IL to inherit from internal classes (which, yes, I end up doign all the time).  It's a shame people mark all kinds of stuff as internal when its obvious that it could be genericly used by others.  One of the worst examples of this lameness is in the System.ServiceModel.Web assembly where there's all kinds of goodness to create a JavaScript proxy.  For some reason it's all internal forcing people to do reflection and IL tricks to get around it.  If they didn't set their buffer manager as internal, we jQuery guys would feel the love a bit more.

    On the other hand, there are situations where you don't want people to touch it at all.  However, sealing something or setting somethign as internal doesn't do anything for a skilled, determined individual.  Reflector and basic knowledge of how the C# compiler works makes just about anything accessible.  In my work, if I need to protect a redistibutable section of code from extension (kind of like in the bank account example, but for non-secure stuff), I'll be sure to use a static class.  Since it's essentially a sealed, abstract class, there's no way to touch it.  However, if I were doing something that required security and must be protected from IL viewing, there's no way I'm going to package that in an assembly anyhow.  Obfuscation or assembly encryption are well and good, but I seriously prefere designing a system using a software-as-a-service model.  While I'm not ready to go as far as Juval Lowy with his every-class-is-a-service campaign, I normally do follow a every-assembly-is-a-service model.  This protects everyone (and gives all applications some serious flexibility.)  There's no need for the other guys to know what's going on in the black box.  Here's your interface, your endpoint, and your protocol information.  No code protection required.

  • Just wanna highlight an issue with the mentioned approach, because you may make your customers to hate you just because you did not provide them with any point of extensibility.

    For instance, in Silverlight, there is a number of specialized collections derived from the abstract generic collection System.Window.PresentationFrameworkCollection<T>. Some of them are public, some of them internal, but all of them are sealed. At the same time, the PresentationFrameworkCollection<T> has few abstract internal methods, those make you unable to derive your own collection from this one.

    Just because of this, the customer should spend much more time/efforts/money on reinventing the wheel by creating the same functionality in his/her own code.

    So, you have to always remember about your customers, even if there would be just one single customer - you. Because the requirements are going to change, so the software changes follow the requirements. You better be prepared for that with your code structure, then in a future trying to find any workarounds for the limitations you put in yourself.

  • He-he... Seems that everybody is complaing about that the mentioned approach is violating an open/close principle. Wouldn't it be more valuable to tell about how to follow open/close, then how to violate it?

  • He-he... Seems that everybody is complaing about that the mentioned approach is violating an open/close principle. Wouldn't it be more valuable to tell about how to follow open/close, then how to violate it?

  • You can actually create a concrete subclass of an abstract type with an internal constructor.  Just not in C#.

    Basically in IL you would define your new constructor with a body that initializes whatever you need but does not call any superclass constructor directly.  Instead you can use reflection if you really need to invoke that superclass constructor.

    This works because the CLR does not check that a subclass actually calls the constructor of its superclass.

  • well.. accessibility levels are there so that you can structure your code in a way that prevents you and others from trying things you did not plan to be done by anyone. i use them to make sure that one thing (i.e. setting a property value) can only be done in one way (by making the backing field private) so that later when i add functionality (i.e raising an event) to the setter, i can be sure that neither i or someone else did go a way i did not expect. all in all you reduce the number of ways one can go and thus the number of ways something you did not think of could happen.

    oh well.. and i make ugly stuff private /internal so no one can see it ;)

Page 2 of 2 (27 items) 12