Avoiding ugly afterthoughts. Part b. Coding for Security, Coding for i18n, Testing as a Part of Development

 
Author:  Follow: TwitterFacebook
Job Title:Sarcastic Architect
Hobbies:Thinking Aloud, Arguing with Managers, Annoying HRs,
Calling a Spade a Spade, Keeping Tongue in Cheek
 
 

Pages: 1 2 3

Jump to Coding for i18n
i18n, error handling, DfT, security, cross-platform while coding

#DDMoG, Vol. IV
[[This is Chapter 12(b) from “beta” Volume IV of the upcoming book “Development&Deployment of Multiplayer Online Games”, which is currently being beta-tested. Beta-testing is intended to improve the quality of the book, and provides free e-copy of the “release” book to those who help with improving; for further details see “Book Beta Testing“. All the content published during Beta Testing, is subject to change before the book is published.

To navigate through the book, you may want to use Development&Deployment of MOG: Table of Contents.]]

We continue our discussion of “things you need to keep in mind during development to avoid rewriting the whole thing later”. Next on our list is “Coding for Security”

Coding for Security

DON’T trust the client!

For pretty much any distributed system, RULE #0 when it comes to security is

DON’T trust the client!

Not surprisingly, this rule applies to multiplayer games too. In particular, it means that you SHOULD make a big effort and

make sure that ALL your gameplay-affecting decisions are made on the server-side.

As it was mentioned in Chapter II, for really fast-paced games this is not always possible, but for each and every violation of this rule, you MUST understand that any decision made on the client, WILL be abused, so that you need to realize what kind of benefits the cheater will be able to obtain via abusing client-made decisions.

Sanitize, then sanitize some more

Hare pointing out:If your code assumes that a certain string doesn’t contain a null character within – this is an assumption which SHOULD be validated not by the code itself (where it is very easy to forget about), but in a separate “sanitization” layer which comes before your code kicks in.As a part of dealing with “DON’T trust the client” rule, one thing you MUST do is to “sanitize” the data which comes from the client. “Sanitizing” is a very well-known idea, and the logic behind goes as follows. If your code assumes that a certain string doesn’t contain a null character within – this is an assumption which SHOULD be validated not by the code itself (where it is very easy to forget about), but in a separate “sanitization” layer which comes before your code kicks in. Exactly the same logic applies to each and every assumption about the data coming from the client – they need to be enforced by “sanitising” layer.

As a very rough classification, we can separate sanitizing into two different areas: “field-level sanitization” and “inter-field sanitization”.

Field-Level Sanitization

If you pass over the network an enum field, which can take only five different values, it is still often encoded with 3 bits (or even the whole byte), which can take 8 (or even 256) different values. In such cases, simply taking these bits and casting them into enum type is dangerous (you can easily get an invalid value which can cause all kinds of trouble to your unsuspecting code down the road). Proper field-level sanitization should take a look at the enum field, and to reject any messages which go beyond allowed 5 values.

If you have your own IDL (as was recommended in Chapter III), I further recommend to extend your IDL-generated code to perform sanitisation. To implement sanitization, you’ll need to work in two directions simultaneously:

  • allow more fine-grain description of the fields so that the contract between client and server becomes better defined (and allows for less loopholes). For example, to enable sanitization described above, your IDL SHOULD support a notion of enum (to avoid passing enums as ints, which won’t allow you to sanitize your enums properly).
    • While you’re at it – make sure to allow specifying whether you allow NaN values for a specific float/double field
    • For integer and floating-points fields, specifying allowable ranges (as in “from -1 to 1000” or “from -180.0 to +180.0”) is generally significantly better than simplistic “int16_t” or “float”.
  • enforce field-level sanitization within the code generated by your IDL compiler.
    • As a Really Big Fat rule of thumb, messages failing sanitization SHOULD be ignored
    • What to do with a connection where offending message has arrived from, is not that obvious, but usually it is a good idea to drop the connection altogether (regardless of the connection being TCP one or your-own-simulated-over-UDP one).

Rationale behind this suggestion (the one to enforce sanitization within IDL-generated code) is three-fold:

  • first of all, IDL is a contract between the parties, so that if somebody violates the contract – well, it shouldn’t pass through this layer
  • Hare thumb up:second, doing sanitization at IDL level automates quite a bit of tedious-and-error-prone work, which is always a Good Thing™second, doing sanitization at IDL level automates quite a bit of tedious-and-error-prone work, which is always a Good Thing™
  • third – as soon as you specifiy your data types, IDL can throw away all the data-failing-sanitization-rules easily and silently for you (“as if it didn’t arrive at all”)
Inter-Field Sanitization

When we’re going beyond the field-level restrictions, there can be some inter-field restrictions too. One example of such restriction is “this field X within struct S can take value Y only if struct-S0-which-contains-S has field E equal to either E0 or E1”. Such restrictions happen more often than it might seem, and it is really important to enforce them during sanitization stage (before your main code).

Ideally (from security point of view), you would enforce such inter-field restrictions within your IDL too. Unfortunately, adding them into your home-grown IDL is difficult (in general case, it will require your own description language to specify them – and an elaborated one at that).

As a result, inter-field sanitization is usually implemented as a separate layer, written in your usual programming language and sitting right behind the IDL unmarshalling (that is, assuming that IDL unmarshalling performs field-level sanitization too).

Other Develop-for-Security Best Practices

In addition to sanitization, there is a pretty long list of best practices which need to be followed for the development of reasonably secure distributed programs. Below you’ll find a very brief version of such a list.

For the list below, I’ve tried to find some kind of balance between the list being too long (risking that nobody will read it) and missing something important; admittedly, this list is very far from being complete from security point of view (for a much more comprehensive version, see [OWASP]), but on the positive side it is still possible to follow it while coding:

  • DO separate your server-side code into different event-driven objects. It will allow you to keep interfaces clean, and to specify different permissions during deployment.
    • Threatening hare:Between different event-driven objects, DO restrict sensitive information on need-to-know basis. In particular, DON’T send whole-user-account-including-password to an object which doesn’t need the password.Between different event-driven objects, DO restrict sensitive information on need-to-know basis. In particular, DON’T send whole-user-account-including-password to an object which doesn’t need the password.
  • DO restrict sizes of the stuff you receive over the network. In particular, allocating 256 bytes because you’ve got “size=256” from the client is usually fine, but allocating 256GBytes because you’ve got size=”256000000000” is usually not.
  • DON’T rely on HTTP(S) sessions for security. And if you do – at the very least make sure to read [OWASP] and follow its “Session Management” session to the letter.
    • In general, ANY session mechanism (HTTP or not) is a Big Can of Worms security-wise and needs to be analyzed very carefully for potential security holes (in other words: DO consider hijacking of session token as possible until you have proven otherwise).
  • DON’T open local files where file names have anything to do with input-coming-over-the-network. If you absolutely cannot avoid it – make sure to sanitize path files properly (eliminating all the things such as ‘../’ etc., etc.. etc.)
  • DO use prepared statements (and ONLY prepared statements) for your SQL. You Really Really don’t want any of those injections attacks to get through. And while we’re at it – escaping (especially DIY escaping) does not qualify as a good substitute for prepared statements.
  • DON’T write the information which is Really Sensitive, to your text log files (or delay it until it becomes not-so-sensitive). Three real-world examples:
    • NEVER EVER log passwords
    • If you happen to process credit cards – DON’T write whole credit card numbers into your text logs (not even as a part of “whole-message-received-from-the-other-party”). Instead, print the masked ones (with all-digits-except-for-last-four replaced with asterisks).
    • If you happen to run a poker site – DON’T log pocket cards right away; instead – simply log something like “2 pocket cards dealt to player X” at this moment, and log “Pocket cards for player X were such and such” at the very end of the hand. This will ensure that even if an attacker has managed to reach your server, he cannot get an advantage of knowing-opponents-cards by merely looking at your log files (and getting the data from memory, while possible, is much more difficult, so you might be able to get him before he gets there).
  • Surprised hare:DO make sure NOT to show your server-side error messages and exceptions to the client.DO make sure NOT to show your server-side error messages and exceptions to the client.
  • DO log all security events (if you can handle it, this also includes logging validation failures coming from the client, BUT make sure to avoid being DoS-ed via overloading your log facility)
  • DO both log (to text file etc.) and audit (to the special DB table) all the administrative actions taken by your support (like “user ban” or anything else to that effect).
  • DO use RAII (or try-with-resources/using-statement/with-statement in Java/C#/Python). It is a security feature too, as it prevents DoS attacks (via causing your server to exhaust resources).
  • There are also specific guidelines depending on your programming language. For C++-specific security guidelines, see Chapter [[TODO]] (yes, it will include discussion on buffer overflows), for Java – see [Oracle], for other programming languages you will have to Google it and/or to compile your own list out of these two).

As usual for security stuff, the list above is non-exhaustive, but I hope it is a reasonably good starting point.

Keep reading for Coding for i18n
Join our mailing list:

Comments

    • "No Bugs" Hare says

      > I think point 3. is key to me.

      I don’t have any disagreements with point 3. However, unit tests, at the very least when applied to distributed systems, are extremely far from producing any kind of proof (and are much more like “false sense of security”); see above about unexpected-sequences stuff, which is dominating non-trivial bug space for any distributed system I know.

      And then, given very limited help from unit testing, I refuse to change code just to enable some of the weirder unit tests. If design is bad – it should be changed regardless of tests, that’s it, but if it is tests which dictate design – there is something very wrong in the picture.

      > I don’t think you have to sacrifice readability for tests.

      As long as you don’t sacrifice readability – I have no problems with tests whatsoever (and encourage them too :-)). The Big Fat Problem with TDD is that way too many people have started to treat it as a kind of religion (see my earlier “Best Practices vs Witch Hunts” article here: http://ithare.com/best-practices-vs-witch-hunts/ )

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.