New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

making auto-downloading safer #130

Open
smcv opened this Issue Jul 17, 2015 · 39 comments

Comments

Projects
None yet
9 participants
@smcv

smcv commented Jul 17, 2015

ioquake3 developers currently recommend that auto-downloading should always be disabled. For instance, in the advisory for CVE-2011-2764:

Quake3 was never really designed to be secure against malicious 3rd party content, and probably isn't even in latest revisions of ioquake3. So downloading of untrusted content is still discouraged.
...
workaround: Don't download and install untrusted addons. Set cl_allowdownload to 0

This is a shame, because auto-downloading is clearly something that people use - if they didn't, there would have been no point in making it better by integrating cURL support - and it's very convenient to be able to download map packs, models/skins and other non-executable content.

ioquake3 has historically treated it as a security vulnerability if there is a known exploit allowing an auto-downloadable mod to escape from the QVM sandbox and execute arbitrary native code (e.g. CVE-2011-3012). However, some Quake III-derived games use native-code for their mods and thus cannot limit mods' capabilities at all, e.g. Jedi Knight II, Jedi Academy and RTCW. Open-source versions of those games continue to support unpacking native-code DLLs from PK3 files and loading them, so that existing mods can continue to work, e.g. JACoders/OpenJK#646.

At the moment, a mod that has been auto-downloaded from a server to a client is indistinguishable from a manually-installed mod. I think this is the key thing preventing auto-downloading from being made safer, and I would like to improve on it. Would ioquake3 developers be interested in merging that change? Whether it gets integrated into ioquake3 master or not, it would be best if this happened in some fork of ioquake3 (GPL-2+), so that the relevant code can legally be imported into both GPL-2-only projects like OpenJK, and GPL-3-only projects like iortcw.

Here is a sketch of one design that I think would address this:

  • When auto-downloading mods, "mark" the auto-downloaded PK3 in some way. The easiest way would be a different file extension, perhaps .dlpk3; or for better compatibility with current ioquake3, a double extension like .dl.pk3 or .auto.pk3 might work.
  • When loading files that are believed to be safe - maps, textures, models - treat all PK3s as equivalent.
  • Are shaders and GLSL in the safe set? I don't know their capabilities.
  • When loading executable content - QVM bytecode, and native DLLs in derived games that do that - refuse to load from auto-downloaded mods, only from manually-installed mods. This could be controlled by a config option, or it could be mandatory; iortcw could potentially have an intermediate setting where QVM bytecode is allowed but native DLLs are not.

Or does anyone have better ideas for solving this?

smcv referenced this issue in JACoders/OpenJK Oct 2, 2015

MP: HTTP downloads now working. This is still WIP though as we want
a global repository option that always works even if the server has
the download feature disabled.
@sago007

This comment has been minimized.

sago007 commented Jun 20, 2016

This sounds like a really great feature. I always imagined a mappk3-format that only allowed selected white listed files to load but this sounds better.
I believe that a dedicated download folder with file names like "orignal-base-name.HASH_ID.dlpk3" would be ideal. It is good with the original name in case the user wants to promote a map to manual installed state and the hash would solve file conflicts.
Files should only be loaded if referenced by the server. This prevents the user not understanding why all the textures are different then they start the game the next time.

@zturtleman

This comment has been minimized.

Member

zturtleman commented Jun 20, 2016

I agree with having a download folder, possibly in each mod directory (i.e., baseq3/downloads), and only using the pk3 if referenced by the server.

It does prevent being able to switch to / use new mods offline that get downloaded. I think it's an unfortunate lost, but I don't think anything can be done about that without a central authority of what the default pk3s are.

It would be nice if the pk3s could be enabled / disabled via the ioq3 launcher or UI but I'm not sure that's a realistic expectation.

A better alternative to disabling loading downloaded QVMs would be making the QVM interpreters safe to use. (Though, that's easier said than done as one can probably never be 100% sure it's safe.) Otherwise client can download 100+ MB and then be kicked because pure server is enabled and client is using the wrong CGame or UI QVM or CGame not being compatible with Game VM. Which seems like a really poor user experience, though possibly better than malicious code execution.

@sago007

This comment has been minimized.

sago007 commented Jun 20, 2016

I believe placing the downloaded files in "downloads/baseq3/pakname.hash_id.dlpk3" should make it easy to disable and enable. It just needs to be moved to/from "baseq3/pakname.pk3" and "downloads/baseq3/pakname.hash_id.dlpk3". I think the launcher is the best way to do that if the user wants to make it permanent. I remember a tool for UnrealTorunament 1999 that did just that.

Even if the interpreter could be made safe by checking all its call (and I doubt that) the download directory would still provide value.

@wtfbbqhax

This comment has been minimized.

wtfbbqhax commented Jun 20, 2016

The engine should not care about the pk3 file's name vs the hash.

Please clarify, what is the point of a "separate" download directory? Can a rogue server cause me to overwrite my on disk files?

@ensiform

This comment has been minimized.

ensiform commented Jun 20, 2016

Yeah it can. In the quake directory

@wtfbbqhax

This comment has been minimized.

wtfbbqhax commented Jun 20, 2016

Holy crap, why is that?

I'd rather the client disconnected me and warn of evil server behavior.

@ensiform

This comment has been minimized.

ensiform commented Jun 21, 2016

It's the way the filesystem works. Not designed for malicious servers but it's how pak7 replaces the qvm files from pak6 and 5 .....

@wtfbbqhax

This comment has been minimized.

wtfbbqhax commented Jun 21, 2016

are you sure? my experience has always been that last pk3 asciibetical order is king; directory search order mattered also.

@ensiform

This comment has been minimized.

ensiform commented Jun 21, 2016

Yes that's what I just said. It's in reverse order of the pk3 file name. And it depends on load order of the directories too but I'm not clear on the best way to describe that.

@wtfbbqhax

This comment has been minimized.

wtfbbqhax commented Jun 21, 2016

Ya, but a server downloaded pak7.pk3 should be overriding the pak0.pk3 (short of special case in FS).

I abused this quite heavily in Tremulous with ziptool in the day.

@smcv

This comment has been minimized.

smcv commented Jun 21, 2016

A better alternative to disabling loading downloaded QVMs would be making the QVM interpreters safe to use. (Though, that's easier said than done as one can probably never be 100% sure it's safe.)

In vanilla Q3 they were always meant to be safe to use, and I don't know of any specific vulnerabilities right now - but the ioquake3 team have fixed several vulnerabilities in code that was already meant to be safe and sandboxed, which makes me quite confident that there's still something there that an attacker could turn into arbitrary code execution. It could either be a flaw in the bytecode interpreter/JIT on some platform, or a "syscall" that doesn't or can't validate its arguments and can be used to escape the sandbox.

In some Q3 derivatives (OpenJK, iortcw) the engine explicitly supports unpacking arbitrary native DLLs from the (potentially auto-downloaded!) PK3 file, which is never going to be something we can make safe. Native DLLs are not an issue for ioquake3, but I would prefer to solve this problem in ioquake3 rather than the derived games for copyright reasons: ioquake3 has a permissive enough license (GPL-2+) for it to be legal to copy ioquake3 code into either OpenJK or iortcw. OpenJK is GPL-2 and iortcw is GPL-3, so code cannot be copied from either of those games into the other.

@wtfbbqhax

This comment has been minimized.

wtfbbqhax commented Jun 21, 2016

It's pointless to consider the QVM a safe sandbox (not that anyone ever should have); AFAICT it's half-baked abstraction for portable/compatible server/clients.

but to re-iterate QVM is not safe or secure.

Regarding native DLL's, there have been research etc.. into sandboxing processes in general; though you might get more value by distributing signed packages?

@smcv

This comment has been minimized.

smcv commented Jun 21, 2016

Regarding native DLL's, there have been research etc.. into sandboxing processes in general

That's certainly a mitigation, but not a solution. I'm experimenting with confining the Quake series in Debian using AppArmor, and at some point I'll try doing the same thing with Bubblewrap, but that doesn't (and fundamentally can't) put up a privilege boundary between the engine and the game-code: any game-code that can escape the QVM sandbox will be able to do anything the engine can do, including arbitrary writes to ~/.q3a and, until Wayland or Mir is sufficiently widely available to be relied on, spying on any other X11 window.

you might get more value by distributing signed packages

I package ioquake3, OpenJK and iortcw in Debian. Our source and binary packages are already distributed with a signature-based trust chain, and the tool that processes the proprietary game data (game-data-packager) is a signed package containing strong hashes for "known-good" data: that isn't the problem. The problem is how we deal with third-party add-ons to the games, and I think we can confidently say that the majority of existing game-mods are never going to do this. A signed version of Threewave CTF, for instance, seems vanishingly unlikely.

Signing also requires some sort of trust model to distinguish between content that is validly signed by someone you trust, and content that is validly signed by someone malicious. Reputable Linux distributions usually have this, with a trust-root controlled by the distribution's core developers, and a suitable choice of compromise between enough decentralization to scale up and enough centralization to be secure. Games and game-mods usually don't.

@lrq3000

This comment has been minimized.

lrq3000 commented Mar 4, 2017

Why not split autodownload in two options: allow resources autodownload (maps/textures/etc) and allow qvm autodownload (for mods)? The first one could be enabled by default but the second one disabled. Only players that are aware and want to download a mod would enable it (temporarily).

@ensiform

This comment has been minimized.

ensiform commented Mar 4, 2017

Because without first downloading and inspecting the downloaded file, you can't tell what it has.

@lrq3000

This comment has been minimized.

lrq3000 commented Mar 4, 2017

Ah then the rejection can be done after downloading.

@smcv

This comment has been minimized.

smcv commented Mar 11, 2017

I believe placing the downloaded files in "downloads/baseq3/pakname.hash_id.dlpk3" should make it easy to disable and enable. It just needs to be moved to/from "baseq3/pakname.pk3" and "downloads/baseq3/pakname.hash_id.dlpk3".

This seems a good structure, and I like downloads/threewave/pak05.abcdef12.pk3 better than threewave/downloads/pak05.abcdef12.pk3. It's definitely easier to implement (because what the server actually sends to the client to trigger auto-downloading is threewave/pak05), so that's what I'm prototyping. It also has the advantage that the normal path traversal checks prevent us from overwriting anything that isn't in downloads, even if the server sends monstrously long filenames in an attempt to get them truncated to end with an extension that we treat as trusted, which seems like a big win. If we're using that structure, then the extension doesn't necessarily need changing at all: we can use "was found in the downloads directory" as the special marker.

We might still want to use an altered extension, or some marker just before the extension, as the marker of "this is not trusted". That would make it safe for users to drag maps/models/other "safe" mods from downloads/ into the corresponding mod directory without enabling arbitrary code execution by those mods, while they would have to rename the file to mark a particular auto-downloaded mod as trusted (able to execute code). I'm using .auto.pk3 for now - the implementation is a lot easier if the files still end with .pk3.

Files should only be loaded if referenced by the server. This prevents the user not understanding why all the textures are different then they start the game the next time.

That's a good idea, and I'm implementing it.

It's in reverse order of the pk3 file name. And it depends on load order of the directories too but I'm not clear on the best way to describe that.

If you're connected to a pure server, you follow whatever load order the server specifies, and your local view of the filesystem is (temporarily!) not relevant.

If you are the server (including single-player), or you're a client of an impure server, then the load order of directories is the most important factor: files in the per-user "home" directory override files found next to the executable, or in a system-wide path on Unix.

For files in the same directory, @wtfbbqhax is correct to say that later filenames (in asciibetical order) override earlier filenames.

smcv added a commit to smcv/ioq3 that referenced this issue Mar 11, 2017

Don't load executable code from auto-downloaded PK3s
Fixes: ioquake#130
Signed-off-by: Simon McVittie <smcv@debian.org>
@ensiform

This comment has been minimized.

ensiform commented Mar 11, 2017

I think a standardized solution that can be used across the idTech3 chain would be nice.

@smcv

This comment has been minimized.

smcv commented Mar 11, 2017

Here is a prototype: https://github.com/smcv/ioq3/commits/issue130

I haven't tried to apply it to OpenJK or iortcw yet, but it's all in core code, so it should hopefully apply reasonably well to other engines. Because ioquake3 is more permissively licensed than other Quake 3 forks (ioquake3 is GPL-2+, OpenJK is GPL-2 only, iortcw is GPL-3 only), the legalities are clearer if original work is done in ioquake3 and ported to the others afterwards.

Unresolved issues that need resolving before merge:

How best to "mark" filenames as untrusted?

An infix before .pk3 seems best to me - for now I've used naming like %s.%08x.auto.pk3, but .auto. is not particularly clear in its meaning. Other options include .noexec., .restrict., .dl (but lower case L is really unclear when not in a real word, so I'd prefer to avoid that one).

A prefix (auto.%s.pk3) is probably bad because prefixes are the most important thing when deciding sort order, so we want the extra noise we insert into the filename to appear as late as possible.

A different extension (*.dlpk3, *.dpk3) is probably bad because FS_AddGameDirectory() gained a lot of complexity to implement .pk3dir without extra malloc(), and would essentially need rewriting to cope with a third option. Also, *.pk3 is what people already know about.

Do we need a cvar to say "continue to execute arbitrary code from servers?"

Feature parity with vanilla Q3 would require us to at least have an option to keep executing QVM code found in auto-downloaded files. However, on today's Internet I don't think that's a good idea at all. I'm sure it was fine 20 years ago, when the most likely attacks were basically vandalism by individuals, and there probably wasn't much valuable or sensitive on the average gamer's computer; but in a world with botnets and ransomware run by organised crime, and where a computer is likely to contain a large proportion of your life, unauthenticated arbitrary code execution on joining a game server just doesn't seem a responsible thing to have any more.

On the other hand, if maintainers of ioquake3-derived engines are more likely to merge this change if it's accompanied by an option, then having the option to be secure is better than being unconditionally insecure... iortcw/iortcw#3 was pretty clear about it being unacceptable to knock out either auto-download or code-execution in that project, and something is better than nothing.

@smcv

This comment has been minimized.

smcv commented Mar 11, 2017

How best to "mark" filenames as untrusted?

Of the ideas I've had, I'm currently leaning towards changing the current .auto. infix to .noexec. but leaving it as an infix.

Do we need a cvar?

I would rather not implement one, but if that's what it takes to get something like this into multiple idTech3 forks then I can. Ideally it would have a name that at least makes people think twice, like cl_trustServersCompletely or cl_runDownloadedCode.

@ensiform

This comment has been minimized.

ensiform commented Mar 11, 2017

I think some kind of rudimentary UI using rendering of a SCR_FillRect and text in engine code would be nice. You could keep it at the interrupted stage from the download but I think the text would be drawn.

@TimeDoctor

This comment has been minimized.

Member

TimeDoctor commented Mar 12, 2017

presenting the choice to run unsigned code might be much easier in https://github.com/ioquake/launch

@smcv

This comment has been minimized.

smcv commented Mar 12, 2017

If possible I would like to avoid having too much letting the perfect be the enemy of the good here - the current implementation of auto-downloading is just not safe, but turning it off makes custom maps a lot less useful, so any compromise between the two seems better than the extremes we currently have available.

I think some kind of rudimentary UI using rendering of a SCR_FillRect and text in engine code would be nice. You could keep it at the interrupted stage from the download but I think the text would be drawn.

I wondered about raising a Com_Error (ERR_DROP) (which gets displayed somewhat nicely) on attempts to load QVM code from an untrusted pk3, instead of just doing a Com_Printf and falling back to the next implementation of the corresponding QVM in the search path. Would that be better?

If we expect that most modded servers have cgame/ui code that is optional and broadly compatible with the fs_basegame, such that falling back to vanilla cgame/ui is fine, then I think failing over to the vanilla implementation (as is done now) might be better. If we expect that most modded cgame/ui code is a hard requirement for joining the game successfully, then an ERR_DROP is better.

The message that's currently presented in my prototype is something like this:

WARNING: Refusing to load executable code "vm/ui.qvm" from auto-downloaded package "/home/me/.q3a/downloads/threewave/pak05.abcdef12.auto.pk3". Please download this mod from a trusted source.

Suggestions for better wording welcome, but if possible please make concrete suggestions rather than just saying it should be better :-)

If you meant putting an "are you sure? y/n" dialog in this code path, I would rather not do that because it will be hard to phrase it in a way people will understand, and in any case we'll just get the dancing pigs problem - people will answer "yes, continue" against their best interests, and probably continue to blame the game for not having safe behaviour if the bad situation we warned about happens to them.

I understand that engine maintainers don't want to reduce convenience for players, but on the modern Internet I just don't think executing unauthenticated code is a safe or responsible thing to do.

presenting the choice to run unsigned code might be much easier in https://github.com/ioquake/launch

What UX do you have in mind? A checkbox in the launcher that results in enabling/disabling the cl_pwnMyComputer cvar, or what?

I don't think we can know whether a server will present executable code to us until we actually try joining it, although perhaps the referenced pak information available to the server browser is enough to make a reasonable guess?

@smcv

This comment has been minimized.

smcv commented Mar 14, 2017

Other things that need fixing on this branch before I can propose it for merge:

  • Treat *.cfg and maybe *.conf as executable like *.qvm
  • Treat *.dll, *.so, *.dylib as potentially trying to be malicious too
  • Possibly list the contents of the PK3 in advance and reject anything that looks potentially executable, even if we aren't loading it right now? (not sure about this one)
  • When disconnected from a server (any reason, including it trying to pwn us with executable code), take all auto-downloads off the search path; otherwise auto-downloaded changes "leak" into the main menu (contradicting what @sago007 suggested), issuing an ERR_DROP results in a recursive error when we try to reload ui.qvm and that's also found in the auto-downloaded package

I wondered about raising a Com_Error (ERR_DROP) (which gets displayed somewhat nicely) on attempts to load QVM code from an untrusted pk3, instead of just doing a Com_Printf and falling back to the next implementation of the corresponding QVM in the search path

I tried this, but it's more awkward than it sounds, because:

  • Com_Error uses a big blocky font in which it's hard to convey enough detail to be actionable
  • If the user moves z_awesome_map.12345678.noexec.pk3 from downloads/baseq3 into baseq3 and it turns out to contain unwanted executable code, we don't want their client to go into a loop of trying to load the UI, finding that the preferred candidate is the one bundled with that map and refusing to load it, until the offending file is removed manually; instead we probably want to fail over to the vanilla UI, if only to display a message

So I'm leaning towards just doing a Com_Printf, and maybe having a temporary flag that upgrades it to an ERR_DROP if we are actively trying to join a server.

@Chomenor

This comment has been minimized.

Chomenor commented Mar 15, 2017

My filesystem project already has a basic download directory implementation, and I noticed that it uses the opposite directory order from your proposal, i.e. mod/downloads/pak instead of downloads/mod/pak. The line of reasoning I had when I decided to use mod/downloads/pak is that it would be more easier friendly:

  • With mod/downloads/pak, a user browsing into a mod directory can clearly see that there is a download directory as well, and easily enter it.
  • With downloads/mod/pak, a user looking at the main mod directory could forget or be unaware the download location even exists if they didn't follow the update, and be confused as to where paks are coming from. Even if they know what's going on, they have to navigate backwards and into the downloads folder to find out if a corresponding download directory for that mod even exists.

These are just my thoughts anyway, it is certainly possible to change my project to do it the other way if it becomes standardized that way.

Also, I wonder in general if my project would be a better platform for developing these download directory changes. It was designed with this kind of feature in mind and the selection/precedence model can deal with complex "search order" problems far more cleanly than the existing code, along with many other advantages.

@smcv

This comment has been minimized.

smcv commented Mar 15, 2017

I noticed that it uses the opposite directory order from your proposal

If there is consensus among engine maintainers that your way round is better, I'll try to change it. If there is no consensus, I'm sticking to the one that was easiest to implement, which is putting downloads/ first.

Putting downloads/ before any potentially attacker-supplied string (in particular the game name) does have the advantage that if a malicious server tries to push the .pk3 extension outside the buffer so that they can write a file whose name ends with .dll or whatever, the worst case is that we have mitigated the attack by making sure that file can only be in downloads/. (Consider baseq3/aaaaaaa...aaaaaa.dll.pk3 or aaaaaaa...aaaaaaaaa/pwned.dll.pk3 where the number of a is chosen to make .dll the last thing that fits in the buffer.)

I wonder in general if my project would be a better platform for developing these download directory changes

What I am primarily aiming for is for the various forks of the Quake 3 engine that are present in Linux distributions to be safer to use. If your project is too much of a behavioural change for the ioquake3, iortcw, OpenArena, OpenJK, etc. maintainers to want to merge it, then I still have to fix those separately if I'm going to achieve my goal; so I might as well start with those.

If I was writing the file-handling/netcode part of a game engine from first principles to be secure and safe, I'd write it in a high-level language like Python or something, or in C with dynamically allocated strings provided by GLib or similar (I write GLib code in my day job), or in some language where dynamically allocated strings are a first-class feature like C++ . However, this completely contradicts many of the design decisions made in the Quake 3 engine (in particular it's "pure C" with no special libraries, and it minimizes use of dynamic allocation), so the result would not be the Quake 3 engine. When modifying the Quake 3 engine I'm working from the basic assumption that the result has to be something the Quake 3 engine's de facto maintainers might accept.

The server-side pure validation routine is no longer supported, since it has no security value in modern conditions.

This is probably one of the things that makes the ioquake3 maintainers unwilling to merge your reimplementation. You are right that the pure server mechanism has no security value against an attacker who can modify their engine (or brute-force CRC32, which is not difficult), but mods "in the wild" rely on it for correctness: I know at least OpenArena 0.8.5+ relies on the pure server mechanism to be able to stay compatible with 0.8.1 servers (by loading older bytecode there).

@smcv

This comment has been minimized.

smcv commented Mar 15, 2017

Branch updated:

  • unload auto-downloaded files before processing Com_Error, so that if we have to reload the UI QVM it comes from the normal search path
  • change infix indicating untrusted (data-only) PK3s from .auto. to .noexec.
  • do not allow abusively long auto-download filenames that might cause truncation (game name and PK3 basename each limited to MAX_QPATH = 64 chars)
  • for extra paranoia, also check for truncation when opening the auto-downloaded file itself, and ERR_DROP if too long
  • move check for executable code into FS_FOpenFileReadDir() so that it applies to all reads
  • treat .qvm, .cfg, DLL_EXT as potentially executable
  • treat .so, .dylib, .dll as potentially executable (even if this particular platform won't)
  • continue to ERR_DROP if auto-downloaded files in downloads/ contain code that we try to execute
  • log a warning but do not call Com_Error if previously auto-downloaded files outside downloads/ contain code that we would try to execute (in this case we move on to the next search path entry as though the executable code had not existed)

Considered but not yet done (please say if you feel strongly about these):

  • list the contents of auto-downloaded PK3s in advance and reject anything that looks potentially executable, even if we aren't loading it right now
  • a cvar to trust any of the executable extensions in auto-downloaded files (I think CVE-2017-6903 demonstrates that there are lots of ways bytecode can escape its sandbox, so my focus here is on not executing attacker-supplied code at all, rather than on trying to prevent attacker-supplied code from doing bad things)
@Chomenor

This comment has been minimized.

Chomenor commented Mar 15, 2017

@smcv Thanks for the feedback; ill try to address the different points:

Putting downloads/ before any potentially attacker-supplied string (in particular the game name) does have the advantage that if a malicious server tries to push the .pk3 extension outside the buffer

I would think this would be mitigated by a simple length check, but I suppose if you feel that's important for your implementation. I think my filesystem sets a defined limit of 64 characters for the main part of the pk3 filename, and the directory, hash tag, etc. can take it above that but it stays well below the 256 characters where overflows are usually an issue.

What I am primarily aiming for is for the various forks of the Quake 3 engine that are present in Linux distributions to be safer to use.

That makes sense. I was just concerned about how it could affect my project, for example requiring a change to the download folder structure standard. This is all stuff I will have to track and update if I want my project viable for ioquake3 inclusion in the future.

If your project is too much of a behavioural change for the ioquake3, iortcw, OpenArena, OpenJK, etc. maintainers to want to merge it

The behavioral changes in my project are limited, and mostly cvar controlled. In my testing map/mod/server breakage is very rare (although things working better on the new filesystem is very common). My project is not intended for OpenJK (which is pretty different from ioquake3 anyway) but iortcw, OpenArena, and various others should not be a problem.

If I was writing the file-handling/netcode part of a game engine from first principles to be secure and safe, I'd write it in a high-level language like Python or something

My project is pure C for straightforward integration in ioquake3 and other projects. I think I achieved pretty decent results despite the language restriction, even when it comes to security.

This is probably one of the things that makes the ioquake3 maintainers unwilling to merge your reimplementation.

We are talking about the SV_VerifyPaks_f function, right? If an ioquake3 maintainer asked I could have that function restored within a day (even though it's normally useless and just wastes CPU time). The rest of the pure server system is fully supported in my project. Better than supported, since you can do things like set sv_paks but not sv_pure to create a priority pak list and still allow loading from other paks.

I know at least OpenArena 0.8.5+ relies on the pure server mechanism to be able to stay compatible with 0.8.1 servers (by loading older bytecode there).

This isn't pure verification related, right? I don't see anything special in the verification function.

@smcv

This comment has been minimized.

smcv commented Mar 16, 2017

I know at least OpenArena 0.8.5+ relies on the pure server mechanism to be able to stay compatible with 0.8.1 servers (by loading older bytecode there).

This isn't pure verification related, right? I don't see anything special in the verification function.

Specifically, the part it relies on is that a new client connecting to an old server will not load pak6-patch085.pk3 and pak6-patch088.pk3 even though they are on its search path, because their CRCs are not on the list of "pure" packages presented by the remote server. The OpenArena 0.8.1 QVMs resemble Quake III Arena, whereas 0.8.5's QVMs are more like Team Arena, and for some reason Team Arena inserted new items into the middle of structs/enums rather than at the end.

If I'm understanding the mechanisms correctly, then you are correct to say that after the client calculates a checksum over everything on its search path, the verification step (where the server recalculates that same checksum and checks that the client gave it the right answer) is not necessary. (It's also useless, of course, because a cheating client would maintain two search paths, one for the files it will tell the server it's going to load and another for the files it's actually going to load.)

@Chomenor

This comment has been minimized.

Chomenor commented Mar 16, 2017

@smcv Yes, this is supported in my project. Thanks for clarifying. I updated my readme to make that section more clear.

Just as an FYI, the hashes used to identify paks aren't exactly CRCs, in fact they're even less robust than that. I could explain the format if you'd like. Obviously if you want to do things like avoiding spoofed paks or doing whitelisting at the download level you would use a secure hash like SHA256. Although there is trickiness there because you have to account for paks that are functionally identical but with different comment data in the zip, which float around in practice because of applications (think badly behaved zip tools and antivirus programs) automatically messing with them.

@TimeDoctor

This comment has been minimized.

Member

TimeDoctor commented Apr 13, 2017

I'm tempted just to edit Chromenor posts in this thread since every time I try to read this thread I end up hearing about his project instead. Which should not at all impact the development of any other issues, diffs, or pull requests.

What UX do you have in mind? A checkbox in the launcher that results in enabling/disabling the cl_pwnMyComputer cvar, or what?

The launcher would act as a server browser (there isn't a coherent xplat one available) and an intermediary to download (because downloading large files in-engine with it fullscreen isn't fun) and present mods or new games to the user from a trusted source if available. Similar to how the Blizzard (formerly battle.net) launcher represents different games, except with the server browser that gives you options when you connect to a server that is running files you don't have. If a trusted source for a map or mod or new game isn't available, it could have a two-step warning system per-item similar to smart screen on Windows 10. "This mod isn't trusted. In order to protect your computer it won't run or download" with a 'more info' option that gives more of an explanation and access to download/run it anyway, and a 'cancel' option that is ever present and cancels connecting to the server with the unknown mod. There could be a global flag in some advanced options that could be disabled with a note that your computer might get owned.

When auto-downloading mods, "mark" the auto-downloaded PK3 in some way. The easiest way would be a different file extension, perhaps .dlpk3; or for better compatibility with current ioquake3, a double extension like .dl.pk3 or .auto.pk3 might work.

Asking users to browse the file system and notice that some files are marked with a special extension is not something that modern players are going to want to do or understand. It isn't necessarily a bad idea to do it still for advanced users, just keep in mind that the focus should be in making Quake 3 easy across platforms for new players as well as more secure

I agree with having a download folder, possibly in each mod directory (i.e., baseq3/downloads), and only using the pk3 if referenced by the server.

Yes. Mod directory.

It does prevent being able to switch to / use new mods offline that get downloaded. I think it's an unfortunate lost, but I don't think anything can be done about that without a central authority of what the default pk3s are.

It would be nice if the pk3s could be enabled / disabled via the ioq3 launcher or UI but I'm not sure that's a realistic expectation.

I believe that both of these things are possible, the first is more difficult, the second is less difficult except in the design sense of how to present options without turning into the overloaded option buffet that is something like the otherwise extremely useful Nexus Mod Manager.

@smcv

This comment has been minimized.

smcv commented Apr 13, 2017

The launcher would act as [...] an intermediary to download

That's great as a long term plan, but rather more ambitious than what I've been working on here. I would really prefer for it not to become a blocker for making current ioquake3 (and its derivatives, which don't have launchers either) safe to use on the Internet - I'm trying to improve the situation incrementally rather than waiting for a future that might or might not happen.

For the versions of ioquake3, iortcw, OpenJK in Debian, my choices at the moment are: let the user shoot themselves in the foot by enabling unrestricted auto-downloading, or remove functionality by force-disabling auto-downloading, or remove the engine from the distribution altogether and hope that everyone downloads a nightly autobuild and not 1.36. If I can, I would like to limit the foot-shooting while keeping the ability to auto-download maps.

Sorry, I don't have the amount of UI design skill or spare time necessary to write a cross-platform launcher for you.

I am aware that for ioquake3 upstream, and particularly for releasing to Windows and macOS users, distributing new engine binaries needs significant infrastructure; but in the more limited context of a Linux distribution, we already know how to ship binaries corresponding to some updated source code to all our users (that being the point of a distribution). So that part of launcher isn't directly relevant to us.

"This mod isn't trusted. In order to protect your computer it won't run or download"

Are you thinking of this as working with the current two-level trust (PK3s are either fully trusted to execute code, or not opened at all), or with the three-level structure that I proposed above? (PK3s are either fully trusted to execute code, or only used for inert data like maps and textures (new thing), or not opened at all)

@Chomenor

This comment has been minimized.

Chomenor commented Apr 13, 2017

I'm tempted just to edit Chromenor posts in this thread since every time I try to read this thread I end up hearing about his project instead. Which should not at all impact the development of any other issues, diffs, or pull requests.

My project implements almost the exact functionality requested in this issue, which is why I thought it was relevant to the discussion. I won't post about it any more if you don't want me to.

@TimeDoctor

This comment has been minimized.

Member

TimeDoctor commented Apr 14, 2017

That's great as a long term plan, but rather more ambitious than what I've been working on here. I would really prefer for it not to become a blocker for making current ioquake3 (and its derivatives, which don't have launchers either) safe to use on the Internet

I am aware that for ioquake3 upstream, and particularly for releasing to Windows and macOS users, distributing new engine binaries needs significant infrastructure; but in the more limited context of a Linux distribution, we already know how to ship binaries corresponding to some updated source code to all our users (that being the point of a distribution). So that part of launcher isn't directly relevant to us.

The experience for playing games and staying on their current code under Linux through a distribution is miserable. People complain about bugs we have fixed with our test builds under Linux. People using a distribution are at the whims of the package maintainer and distribution's choices about keeping up with security fixes. This is an issue for Linux, too. It is today less safe for Linux servers and clients to run through whatever a distribution chooses to package of ioquake3.

@robo9k

This comment has been minimized.

robo9k commented Apr 14, 2017

People complain about bugs we have fixed with our test builds under Linux. People using a distribution are at the whims of the package maintainer and distribution's choices about keeping up with security fixes.

Our primary target for users is on Windows. We need a build engineer to maintain installers for Windows, Mac, and Linux. It's more than a zip file.

It would be a lot easier for distributors if you actually tagged some Git revisions with version numbers so they have the feeling of being somewhat more stable than HEAD. You can't complain about users not running bleeding edge Linux builds or packagers to adopt whatever current state your SCM tree is in. This was topic in #271 among others and is still a gripe on various distribution bug trackers.
Having released source code versions is exactly what distributors use to build their own, blessed rpm/deb/ebuild/whatever. If you do not have stable releases or no releases at all, then you end up with "whatever a distribution chooses to package".

Furthermore, like @smcv pointed out, ioquake3 is also the code base for other standalone games, which might or might not have their own releases, build system and distribution packages.

@TimeDoctor

This comment has been minimized.

Member

TimeDoctor commented Apr 14, 2017

It would be a lot easier for distributors if you actually tagged some Git revisions with version numbers so they have the feeling of being somewhat more stable than HEAD. You can't complain about users not running bleeding edge Linux builds or packagers to adopt whatever current state your SCM tree is in.

This was topic in #271 among others and is still a gripe on various distribution bug trackers.

What Linux distributions want is irrelevant to this project, ioquake3's own packages for Linux are more important. Our "bleeding edge" builds are going to be more stable than whatever they package. I want to make ioquake3 easy for players and new game developers, not distributions to package.

@ensiform

This comment has been minimized.

ensiform commented Apr 14, 2017

Unfortunately it's also made more complicated by the impossible ability to get the assets for Quake3 on Linux/Mac with a cdkey now. So while you​ can make a simple binary release, a full installer which handles finding and or gathering assets won't be happening anytime soon.

Steamcmd tool can get the windows assets but you have to login to your steam account with it including steam guard timed code. And to top it off, it doesn't give you the cdkey & q3key file that's generated for your account like it does on Windows.

Also, team arena support via steam CMD doesn't work as intended afaik and doesn't provide 1.32 version of team Arena and only the pak0.

The only way to fix these issues I would imagine is if we could convince Zenimax to add them to steam on Linux and Mac and Gog properly. Either through a petition campaign or if someone has other ideas. Sponge has said that they personally can't do anything about it and that zenimax is unlikely to contract aspyr due to some licensing requirements they demand plus at this point it might not make any sense to them.

@smcv

This comment has been minimized.

smcv commented Jun 3, 2017

Getting back to the original topic of this issue, the current status is: The branch at #274 works, but currently uses a precedence order that I know is not correct (auto-downloads are in an order that is the opposite of what the server said). I'll fix that when I know what I should be aiming for, but there are open questions at #274 (comment) about the desired behaviour.

If the ioquake3 maintainers consider the approach described in the first few comments here to be a useful one, please answer the questions on #274. Thanks!

@smcv

This comment has been minimized.

smcv commented Jun 3, 2017

Also, team arena support via steam CMD doesn't work as intended afaik and doesn't provide 1.32 version of team Arena and only the pak0.

If it helps, everything except pak0.pk3 and mp-pak0.pk3 is available from idgames mirrors. linuxq3apoint-1.32b-3.x86.run contains the Team Arena patch PK3s, and can be unpacked on any platform without running it by skipping the first 8251 bytes and treating the rest as a gzipped tarball (for instance the Python standard library is enough to unpack it, and probably so is libarchive).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment