Securing Breeze on the server to prevent malicious updates to foreign keys

Question

The Problem

I'm just trying to figure out exactly how much of my own security I need to implement on the server side when saving changes in Breeze. In particular, I'm thinking about how a malicious user could manually hack the SaveChanges request, or hack the javascript in the client, to bypass my normal business rules - for example, to maliciously alter foreign key IDs on my entities.

I want to understand exactly where I need to focus my security efforts; I don't want to waste time implementing layers of security that are not required.

I'm using Breeze with .net and Entity Framework on the server side.

Example

Here's a trivial example. ObjectA has a reference to an ObjectB, and ObjectA is owned by a particular User. So, my database looks like this:

ObjectA:

Id    ObjectB_Id    SomeField          User_Id
1     1             Alice's ObjectA    1
2     2             Bob's ObjectA      2

ObjectB:

Id    SomeOtherField
1     Foo
2     Bar

User:

Id    Name
1     Alice
2     Bob

From this model, the security concerns I have are:

1. I don't want unauthenticated users to be changing any data
2. I don't want Bob to be able to make any changes to Alice's ObjectA
3. I don't want Alice to try to point her ObjectA at Bob's ObjectB.
4. I don't want Bob to try to change the User_Id on his ObjectA to be Alice.

The solution for (1) is trivial; I'll ensure that my SaveChanges method has an [Authorize] attribute.

I can easily use Fiddler to build a SaveChanges request to reproduce issues 2 to 4 - for example, I can build a request which changes Alice's ObjectA to point to Bob's ObjectB. This is what the message content might look like:

"entities":
[
    {
        "Id":1,
        "ObjectB_Id":2,
        "SomeField":"Alice's ObjectA",
        "User_Id":1,
        "entityAspect":
        {
            "entityTypeName":"ObjectA:#MyNamespace",
            "defaultResourceName":"ObjectAs",
            "entityState":"Modified",
            "originalValuesMap":
            {
                "ObjectB_Id":"1"
            },
            "autoGeneratedKey":
            {
                "propertyName":"Id",
                "autoGeneratedKeyType":"Identity"
            }
        }
    }
],

As I'd expect, when no security is implemented on the server side, this persists the updated value for ObjectB_Id into the database.

However, I've also confirmed that if there is no entry for ObjectB_Id in the originalValuesMap, then even if I change the value for ObjectB_Id in the main body of the message it is NOT updated in the database.

General Rules?

So, I think this means that the general security rules I need to follow on the server are:

[Edited 4 July 2013 - rewritten for clarity]

In general:

• Nothing in the message can be trusted: neither values in the originalValuesMap nor supposedly "unchanged" values
  • The only exception is the identity of the entity, which we can assume is correct.
  • Supposedly "unchanged" properties may have been tampered with even if they are not in the originalValuesMap

For "Unchanged" properties (properties which are not also on the originalValuesMap):

• When "using" any "unchanged" property, we must NOT use the value from the message; we must retrieve the object from the database and use the value from that.
  • for example, when checking owenership of an object to ensure that the user is allowed to change it, we cannot trust a UserId on the message; we must retrieve the entity from the database and use the UserId value from that
• For any other "unchanged" property, which we are not using in any way, we don't need to worry if it has been tampered with because, even if it has, the tampered value will not be persisted to the database

For changed properties (properties which are also on the originalValuesMap):

• Business rules may prevent particular properties being changed. If this is the case, we should implement a check for each such rule.

• If a value is allowed to be changed, and it is a foreign key, we should probably perform a security check to ensure that the new value is allowed to be used by the session identity

• We must not use any of the original values in the originalValuesMap, as these may have been tampered with

[End of edit]

Implementing the Rules

Assuming that these rules are correct, I guess there are a couple of options to implement security around the changed foreign keys:

• If the business rules do not allow changes to a particular field, I will reject the SaveChanges request
• If the business rules DO allow changes to a particular field, I will check that the new value is allowed. In doing this, CANNOT use the originalValuesMap; I'll need to go to the database (or other trusted source, eg session Cookie)

Applying these rules to the security concerns that I gave above,

• security concern (2). I'll need to check the user identity on the session against the User_ID on the ObjectA that is currently in the database. This is because I cannot trust the User_ID on the request, even if it is not in the originalValuesMap.

• security concern (3). If the business rules allow a change of ObjectB, I will need to check who owns the new value of ObjectB_Id; I'll do this by retrieving the specified ObjectB from the database. If this ObjectB is not owned by ObjectA's owner, I probably want to reject the changes.

• security concern (4). If the business rules allow a change of User, this is already covered by (2).

Questions

So, really, I'm looking for confirmation that I'm thinking along the right lines.

1. Are my general rules correct?
2. Does my implementation of the rules sound reasonable?
3. Am I missing anything?
4. Am I over complicating things?

Placida

A session cookie is not a trusted source. – Uranie

You have a security hole in "For any other unchanged property, which we are not using in any way, we don't need to worry if it has been tampered with because, even if it has, the tampered value will not be persisted to the database". If you send a request with modified property values but originalValuesMap is completely empty, then Breeze will save all the properties to the database. – Legendary

Answer 1

Phil ... you are absolutely on the right track here. You've done a nice job of laying out the issues and the threats and the general approach to mitigating those threats. It is almost as if you had written the introduction to the Breeze security chapter ... which we haven't gotten to yet.

I do not think that you are "over complicating things"

Someone reading this might think "wow ... that's a lot of work ... that Breeze stuff must be insecure".

Well it is a lot of work. But it isn't Breeze that is making it difficult. This is the necessary thinking for every web application in existence. Authentication is only the first step ... the easiest step ... in securing an application.

You shouldn't trust any client request ... even if the client is authenticated. That means making sure the client is authorized to make the request and that the content entering and exiting the server is consistent with what the client is both claiming to do and is allowed to do. These are general principles that apply to all web applications, not just Breeze applications. Adhering to these principles is no more difficult in Breeze than in any other technology.

One Breeze technicality you may have overlooked. The EFContextProvider.Context should only hold the entities to save; don't use it to retrieve original entities.You'll need a separate DbContext to retrieve the original entities to compare with the change-set entities from the client.

We are working on samples that demonstrate ways to handle the issues you described. For example, we're recommending (and demo'ing) a "validation rules engine" that plugs into the BeforeSaveEntitiesDelegate; this "engine" approach makes it easier to write bunches of server-side rules and have them applied automatically.

Our samples and guidance aren't quite ready for publication. But they are coming along.

Meanwhile, follow your instincts as you've described them here. Blog about your progress. Tell us about it ... and we'll be thrilled to highlight your posts.

Casteel

I guess the only thing here particular to Breeze is how it chooses which values to save in the database; it is strictly based on those properties that appear in the OriginalValuesMap. Before I discovered this I was concerned I'd have to perform security checks on all properties on the entity, not just those in the OriginalValuesMap. As an aside, what is the thinking behind passing in the original value on the request? Given that they can't be trusted, and given that they are not sent back to the client in the response, why not just pass through a list of names of the changed properties? – Placida

Regarding your comments about creating another DbContext from which to read the clean entity from the database: yes, I actually spotted this after writing the original comment. For the benefit of everyone else, there's an explanation of the rationale behind this at https://mcmap.net/q/1320287/-using-this-context-inside-beforesaveentity – Placida

Do you have the samples available yet? Or better yet, do you have a way of white-listing the properties to save like asp.net mvc's TryUpdateModel method? – Pitchman

Answer 2

I've been looking for guidance on the same matter and I am very happy to find your brilliant analysis. In my opinion the answer to our problem is different though, assuming that we are talking about applications which are to be composed of more than a few modules and are to live longer than a year.

If rules become too complicated it means that we might be using inappropriate approach. I'm sure many brilliant developers would cope following these rules but the sad truth is that most of our peers would either get it wrong or would forget about some of them under pressure.

I'd say that we need to go back to Fowler's, Evans' and Nilssons' publications and repeat after them that in larger applications (and these have strong security requirements) the entity model is not something that should be exposed to the client at all (for other reasons than security too - e.g. maintainability).

On the other hand it is worth looking at revisions to these original ideas proposed later by Greg Young and Udi Dahan. These in essence say that model for reading does not have to and often is not the same as model for writing 'data'.

To sum this up I'd say that the base rule should be DON'T use Breeze for writing and DO use it for reading (with DTOs/Projections), provided you don't query the 'real' model but the model built specially for reading (e.g. Views not Tables).

All this quite naturally emerges if you follow your domain and use cases and above all if you follow Test-Driven approach. Would you really end up with BeforeSaveEntities solution for business rules while following Test-Driven-Development?

Spoken