FEServer lacks Client Version & Integrity Checking

[ghost]

Issue type: problem

Description:

To my knowledge, as per @eliatlarge 's comment on #45 , FEServer doesn't perform version checking, it's simply trusts that all players have the latest release, didn't mod said release, and are aware said release is available. However, life happens, and you inevitably get that player who missed the memo as noted when @eliatlarge said (emphasis was added by me to highlight the point):

certain incompatibilities can render the clients unusable, but if no one knows this, they complain to me that the normal game is broken.

Case in point, we had two people complaining that the match would end randomly for them just whenever. We came to realize it was because one had been using an old/modified version with different stats, which had a fighter die on one end on not the other, screwing them both over.

We can't just simply assume that everyone is on the same page, as this is begging for trouble both now, and especially down the road when there'll be much larger discrepancies in the form of new classes, weapons, etc. For this very reason, FEServer should be comparing client version numbers to ensure everyone is using the same version, and then if that initial check passes, it should be comparing some form of checksum or hash of the jar to ensure that the player didn't modify it - however, people will want to make change music, make custom maps, etc. which is why #45 also needs to be a thing so that resources like music aren't factored into the integrity check.

Clients that fail the check will be refused and sent back to the title screen (which should clearly display the client's version number)

While FEServer should have this failsafe enabled by default, it should also have an option (at the host's own risk) to disable it so that players and developers alike can playtest preliminary balance ideas.

---

Depends (Partially) On:

45 : Allow Loading of External Resources

---

Examples:

Problem:

certain incompatibilities can render the clients unusable, but if no one knows this, they complain to me that the normal game is broken.

Case in point, we had two people complaining that the match would end randomly for them just whenever. We came to realize it was because one had been using an old/modified version with different stats, which had a fighter die on one end on not the other, screwing them both over.

[rayrobdod]

What I'd do is check the team during the PARTYMESSAGE. Check that each item and unit in the PartyMessage matches those known by the Server then kick the player if the items don't match, but not care about anything else. Any other method would either allow differently-modded games to interact, or prevent compatible games from interacting, or be far too easy to bypass. Physic is going to be absurdly hard to validate as-is though.

The thing about hashing the *.class files is that it requires trusting that the client is actually hashing the *.class files and not handing over a predetermined hardcoded number. And the server and client don't need a lot of the same code, so a properly minified jar wouldn't have a lot of the same classes to hash. The most I can think of being essential to both is maybe the classes in the net.fe.network.message package.

[ghost]

Validating Parties / Stats

Can't it check the contents of each client's stats.txt and weapons.txt to ensure they are identical? Then kick if there's a mismatch? As for Physic, weapons.txt should be displaying the range as 1-Mag/2 instead of having a hardcoded override imo.

Hashes

Good point, I was hoping for some sort of lock and key mechanism using hashes, perhaps generating them in real-time, but that would require the server to have a copy of all the release classes to compare against each clients which might be time consuming. Either way, it should have something to ensure the same version is used so you don't have someone with a build before shoving playing someone with shoving, etc.

[rayrobdod]

Can't it check the contents of each client's stats.txt and weapons.txt to ensure they are identical?

Is the assumption that a player does not know what they're doing? I like the idea of "if a player knows that the server doesn't have crossbows, they can still use their client but not buy crossbows and they'll be fine", and there are things that don't matter like the unbuyable debug weapons. ^(That would mean that the validator shouldn't check item "id"s, but those are only used for icons as far as I can tell) And checking just the raw tsv files wouldn't be able to catch things like a player overspending or inventing new items out of thin air.

[ghost]

Is the assumption that a player does not know what they're doing?

On the contrary, this assumes that the player is highly capable of making honest mistakes along with the chance they may possess a dangerous amount of knowledge and is possibly both willing and able to hack the text files for better stats, growths, and weapons since they're human-readable, which would give them an unfair advantage in tournaments. Ensuring that the contents of both files for all connected players' files are completely identical is meant to protect the game state from both malicious players just as much as general instability from lazy and incompetent players who fail to run the same version since balance patches and independent balance testing are a thing.

What we want is a bullet-proof sanity check to ensure everyone is on the same page / equal footing.

I like the idea of "if a player knows that the server doesn't have crossbows, they can still use their client but not buy crossbows and they'll be fine"

Sadly not everyone pays attention, and it's not good to assume that every end-user is competent. I hate to say it but the best model has been to assume that the end-user is either completely incompetent or malicious and willing to exploit bugs. The server doesn't broadcast it's version number. I'm for having backwards compatibility, BUT the client would need a way of discovering that the server is older and disabling items / features that don't exist for that game on that server.

And checking just the raw tsv files wouldn't be able to catch things like a player overspending or inventing new items out of thin air.

I don't quite follow what you mean by this. Items would be covered by ensuring all copies of weapons.txt are identical, doing a comparing to ensure not even a single character (letter / number / space) is mismatched (this would also include checking for new / removed / moved lines that don't exist universally on all clients and the server) with the server's copy and / or all other copies in play.

player overspending

I'm assuming you mean via hacking the client's source to tamper with the starting gold amount since I noticed that there's at least a sanity check for if the player saves a team with the Treasury modifier then tries to load it in a game without it that removes all items / weapons beyond the allowed gold amount. The server should have the final say in starting gold, if the player loads a team that overspent, it'll employ a fix similar to loading a team saved with a modifier that adjusted gold.

[rayrobdod]

Always assume the client has been compromised.

there's at least a sanity check for if the player saves a team with the Treasury modifier then tries to load it in a game without it that removes all items / weapons beyond the allowed gold amount.

Wait seriously? How... The client checks things but the server doesn't? Why?!?

I suppose pruning illegal elements could be confusing, but probably better than crashing.

[ghost]

Wait seriously? How... The client checks things but the server doesn't? Why?!?

Why? ¯(ツ)/¯ Idk, it just does, ask @eliatlarge.

As for how, my understanding, from a glorified end-user / layman perspective, is that when loading the team file, it parses the equipment in order from top to bottom per unit, per inventory slot, from the Lord down, fulfilling as many requests as possible until funds run out, then the rest of the inventories are ignored. It does the same with EXP when experimenting with an all Lvl 20 team I made via the Veterans modifier which I then loaded into both the Team Builder and a standard game, distributing EXP as defined in order of top to bottom on the unit list until it runs out. I was pretty impressed with this safety as it was one of the first things I attempted in order to break FEMP; and also gave me a false sense of security about the game having all the necessary sanity checks in place; which it doesn't.

Anyways, back on topic, I think a good start would be ensuring the server and clients have completely identical weapons.txt and stats.txt files ensure no strings were edited, added, removed, or reordered - with FEServer running the comparison since the host should obviously be the one deciding what edition everyone who's joining is playing with. Or am I overlooking something? I'm open to suggestions, especially since you and Eli would be the ones implementing it. I'm just here to file reports and bat ideas as for how we should be accomplishing these fixes.

[rayrobdod]

literal explanation of "How" in response to a vague cry of anguish

More, the problem, well "problem", is that the serialized teams, when saved to disk, are just unit and items names. Which is unlike what currently happens during client-server communications and somewhat invalidates my argument against changing client-server to use unit and item names as opposed to unit and item objects.

And I'm mad that anyone anywhere used Java Serialization to read/write a single String[][] to disk instead of serializing the String[][] as a tsv/csv file.

identical weapons.txt and stats.txt

If the only check were for the client to hand a copy of weapons.txt to the server, it would then be trivial to create a client that handed one weapons.txt to the server but used a different weapons.txt for everything else. As long as the client-to-server team information message is not changed to references and the server-to-client team information message remains fully realized.

If the difference between two versions was "Paladins lost access to axes", then hashing stats.txt wouldn't catch that. Hashing the post-deserialization list of units would, though.

But fine. Identical weapons and units. Simple early (I still don't think exhaustive) check for the common case of connecting the wrong client to a server. At that point, the server might as well hand the client a list of weapons and say "These are the weapons we're using today", alas the list of all weapons is a global variable.

The current map needs to be identical too.

[ghost]

And I'm mad that anyone anywhere used Java Serialization to read/write a single String[][] to disk instead of serializing the String[][] as a tsv/csv file.

I don't quite follow what you mean. Wouldn't the end result still be writing to disk if it's being turned into a tsv/csv file? Or do you mean it should be creating something temporary in RAM? I'll openly admit that I'm not knowledgeable about this topic.

Proposed checks aren't sufficient / exhaustive

I never said they were exhaustive, I merely said they're a start. As for units gaining / losing access, new command skills, hacks to the source in general, etc. we'll have to employ some other kind of check as well. As for what, I haven't figured that out yet (aside from a comprehensive, time consuming scan of the client's entire contents) since you said the hash could be compromised / defeated.

[rayrobdod]

Does "Comma Separated Values" mean anything to you? Or "Tab Separated Values"? Incidentally, The units.txt and weapons.txt are Tabbed Separated Values files in disguise. But anyway, CSV and Java Serialization are different formats, and Java Serialization is a versatile format, but none of that versatility is required when writing a sequence of strings and it has disadvantages compared to the simpler alternatives. It's like how images with textual data or sprites shouldn't be stored in a JPEG and photographic images don't belong in a GIF. Or like how a chainsaw can cut lots of stuff, including grass, but one probably still would not use it to mow a lawn. Or how one shouldn't write applications in a text markup language, but my opinion on HTML5 is neither here nor there

So yes, it would still be writing something to disk, but a different thing that, in my opinion, is more suited for the data. It's petty and not that something like that can be changed retroactively, but I still don't like it.

[ghost]

Does "Comma Separated Values" mean anything to you? Or "Tab Separated Values"?

First of all, I'd like to kindly ask you to please refrain from talking down to me. Second, yes, I'm well aware what those file formats are, but my extent of using them has been mostly to import and export from my address book, so forgive me for being unable to connect the dots on this one, as I'm simply trying to understand the situation. Lastly, before reading your reply, I had no idea what any of that has to do with Java Serialization, or what Java Serialization even is for that matter, or what you meant by the difference between "writing to disk" and "writing to a csv / tsv file" as both sounded like disk access to me anyways.

Incidentally, The units.txt and weapons.txt are Tabbed Separated Values files in disguise.

Actually, it seems like a bastardized version of tsv because there's spaces mixed in with those tabs for reasons I cannot begin to comprehend without getting a killer migraine as it looks like garbage no matter what editor I throw at it (other than going to pastebin.com and toying with it there as it somehow ignores the spaces).

Or how one shouldn't write applications in a text markup language, but my opinion on HTML5 is neither here nor there

My brother swears by C++ as the one language to rule them all, but then platform independence becomes a bit harder to achieve as you'd have to distribute a different binary for each OS.

So yes, it would still be writing something to disk, but a different thing that, in my opinion, is more suited for the data.

I understand now. I agree that these should be either of those types of files for many reasons including easier editing in Excel / Calc. As it stands the formatting of the actual files are horrible because they have spaces mixed in with those tabs to the point it displays like shit no matter what editor I throw at it. You definitely have my blessing to address that.

[rayrobdod]

Asking "Do you know what X is" counts as talking down to now. Good to know.

[ghost]

It's the tone / way you asked it, "Does X mean anything to you?" Came off as pretty condescending as it implies incompetence imo. Especially because I wasn't inquiring about the file formats, but java serialization and how writing to the file differs in your opinion from a disk write since you implied there was a difference. Assuming that my lack of knowledge was in regards to something as basic as a commonly used file format was insulting.

But whatever, I'm over it. Let's get back to making magic happen.

[HamaIndustries]

Can we all just agree that we're a group of coders attempting to interface with a group of quality assurance people? It's trying to translate Qualitative analysis vs. Quantitative analysis, and we both need simple terms to explain concepts from one side to correlate with the other. I propose that we all just assume that anything we perceive as talking down is just a crude attempt at explanation, and treat it like we would any other explanation.

[ghost]

Closing as Fixed as per #102 until a more complex solution is needed.

Great job!