JACoders / OpenJK

Community effort to maintain and improve Jedi Academy (SP & MP) + Jedi Outcast (SP only) released by Raven Software
GNU General Public License v2.0
2.01k stars 612 forks source link

Linux cgame, ui, jampgame DLLs no longer installed by default #643

Closed smcv closed 9 years ago

smcv commented 9 years ago

Last time I did a build for Debian (from commit 471dd02) I could configure cmake with

    -DBuildJK2SPEngine=ON \
    -DBuildJK2SPRdVanilla=ON \
    -DBuildJK2SPGame=ON \
    -DCMAKE_BUILD_TYPE=RELWITHDEBINFO \
    -DCMAKE_INSTALL_PREFIX=/usr/lib/openjk \
    -DCMAKE_VERBOSE_MAKEFILE=TRUE \

and then make install DESTDIR=$(pwd)/debian/tmp would leave me with these files in debian/tmp, where * is a wildcard for architecture-dependent bits and .so is the Linux equivalent of .dll:

usr/lib/openjk/openjk_sp.*
usr/lib/openjk/rdsp-*.so
usr/lib/openjk/OpenJK/jagame*.so
usr/lib/openjk/openjk.*
usr/lib/openjk/rd-*.so
usr/lib/openjk/openjkded.*
usr/lib/openjk/OpenJK/cgame*.so 
usr/lib/openjk/OpenJK/ui*.so
usr/lib/openjk/OpenJK/jampgame*.so
usr/lib/openjk/openjo_sp.*
usr/lib/openjk/rdjosp-vanilla_*.so
usr/lib/openjk/OpenJK/jospgame*.so

(and then I split up JA and JK2 between base directories /usr/lib/openjk-academy and /usr/lib/openjk-outcast so they don't try to use each other's files, but that's in the Debian packaging not the upstream bit).

In current builds I get a new empty zip file at openjk.pk3, but I no longer get the cgame, ui or jampgame DLLs.

Unpacking DLLs from the pk3 file is not implemented on Linux, so it's OK that it's empty - as far as I'm concerned, if this worked, it would be a security vulnerability (because auto-downloading exists, and arbitrary native code execution from unauthenticated auto-downloaded packages is a bad thing) so I would like to continue to use DLLs that are ordinary files in the filesystem.

smcv commented 9 years ago

I'll try to look into this this evening if nobody has spotted the cause by then - all three MP-relevant DLLs are missing, so it looks as though maybe make install isn't recursing into codemp correctly?

smcv commented 9 years ago

Relatedly, to make this work optimally, should I be installing the DLLs in OpenJK/ or in base/ or both? At the moment I'm using symbolic links to make them appear in both places, but I can drop one or the other if that would be better.

xycaleth commented 9 years ago

This is related to my changes to the CMake files to build the PK3 file at build time. I was working on Windows to get this working and forgot about the implications to Linux and OS X! d4cc979 is where I made the changes.

Relatedly, to make this work optimally, should I be installing the DLLs in OpenJK/ or in base/ or both? At the moment I'm using symbolic links to make them appear in both places, but I can drop one or the other if that would be better

I don't think there's any advantange or disadvantage of having them symlinked. They will be kept in a read-only place anyway, and they can be overridden by individual users so they will almost never need to be touched.

smcv commented 9 years ago

The symlink is an irrelevant implementation detail; my question would be the same if I was just copying the DLLs' file contents instead.

My question is, should I have base/jampgamex86_64.so (etc.) as a file that exists in the installed game's read-only system-wide base path alongside base/assets0.pk3 and so on, or should I have OpenJK/jampgamex86_64.so instead, or should I have both?

smcv commented 9 years ago

OK, I see what's happened. Thanks for pointing me to the relevant commit. The "else" clause for "we are not building a Mac application bundle" has got lost; I'll bring it back.

Is it acceptable for the DLLs to be installed as non-PK3'd files (next to openjk.pk3) for all platforms, or do you specifically want to avoid them existing outside a PK3 file on Windows?

smcv commented 9 years ago

For now I've put it back to how it was before (Windows and non-Mac-bundle Unix both install DLLs, to OpenJK only). If either of those is not ideal, it's easy to change.

ensiform commented 9 years ago

MP won't try to load the DLLs from openjk mod folder unless its set to fs_game OpenJK. And since there was no Linux version of cgame and ui I'm not yet sure what to do for defaults.

smcv commented 9 years ago

On Windows/Mac, is the intention to be treating OpenJK like an unofficial patch for JA (i.e. overwriting bits of JA), or setting it up so you can use Raven's DLLs with the OpenJK engine when playing on (or serving) an OpenJK server, or what?

On platforms where JA/JK2 didn't exist before OpenJK (Linux, or anything else non-Windows non-Mac if people port it) I suspect the best way might be to install the DLLs to base/ - there's no concern about overwriting Raven's versions because there's nothing to overwrite, and getting a not-perfectly-compatible cgame/ui when joining vanilla JA servers is better than nothing.

smcv commented 9 years ago

unless its set to fs_game OpenJK

Is your intention that people should do that, or not?

My assumption had been that the normal setup would be to leave both fs_game and fs_basegame unset, so the engine ends up loading everything from the fallback path (base).

ensiform commented 9 years ago

@Razish @Xycaleth ?

xycaleth commented 9 years ago

I'm still alive!

DLLs in the PK3

I've forgotten which OSs are able to extract the DLLs from PK3 files, so I'll assume that only Windows can extract from the PK3 for now. The intention of my commit was to automatically put the DLLs into a PK3. This is something which our Windows build server does as an extra step after compiling the DLLs, and having it as part of the CMake script means that the PK3 can be auto-generated when running CPack to make the installation files. The Linux and OS X build servers don't do this as far as I know.

For Windows, this is fine as the DLLs can be extracted from the PK3. For OS X, you'll notice that I left the install code as this copies the DLLs into the OpenJK app bundle (if you're not familiar with bundles, they are essentially folders with a special directory structure which OS X treats as an executable application). As OS X cannot extract DLLs from PK3s, the PK3 shouldn't be generated. For Linux, the same applies; it cannot extract DLLs from PK3s so the PK3 shouldn't be generated.

DLL placement

OpenJK is meant to be a drop-in replacement for JK game binaries, but without altering any of the existing gameplay (where possible). This means that the DLLs should be kept in the openjk/ folder, and shouldn't override existing DLLs (the exception here is for JA single player

However, there might not be an existing DLL for some platforms or some architectures so like you say, there needs to be a fallback. In this scenario, a copy of the OpenJK DLLs should be placed in the base/ folder. These scenarios include:

  1. All 64-bit builds.
  2. All OS X builds - the Aspyr version of JKA uses .so files I believe, so they won't work with OpenJK.
  3. Linux is a special case because there already exists a 32-bit jampgamei386.so file, but there are no client-side DLLs. In this case, I think a copy of the MP client-side DLLs (cgamei386.so and uii386.so) and the SP DLL (jagamei386.so) should be placed in the base/ folder.
smcv commented 9 years ago

the DLLs can be extracted from the PK3

What's the intention here? Is the idea that you can send an updated or modded ui/cgame to users of a server, or perhaps even send the OpenJK ui/cgame to vanilla clients?

Although it looks as though this doesn't affect the platform I'm mostly interested in, I should perhaps remind you that in the presence of auto-downloading, this is a security vulnerability: downloading arbitrary native code from a remote server and running it with your own privileges means that the server operator, or anyone who can interfere with the Internet route from the server to you, can control your computer entirely.

In Quake III Arena (and hence ioquake3) it isn't quite so bad, because the QVM bytecode interpreter is somewhat sandboxed, and the engine doesn't unpack DLLs from PK3s or allow QVM bytecode to write to any file with the platform's DLL extension (although it's probably still possible for a determined attacker to break out of the sandbox).

For OS X, you'll notice that I left the install code as this copies the DLLs into the OpenJK app bundle

Right, and you copy them to both base and OpenJK in the bundle. (On case-sensitive platforms it should canonically be OpenJK not openjk, right?)

the Aspyr version of JKA uses .so files I believe, so they won't work with OpenJK

As opposed to OpenJK's .dylib? I suspect you could rename to .so if you wanted to - the extension doesn't actually matter much (even on Windows, where e.g. Python uses .pyd modules that are functionally identical to DLLs). But making a clean break and using a different extension seems reasonable.

In this scenario, a copy of the OpenJK DLLs should be placed in the base/ folder

OK, so turning that around, you basically want to avoid altering base/ on 32-bit Windows, but put the DLLs in both base/ and OpenJK/ everywhere else, potentially with a smaller special case for jampgame on i386 Linux?

At this point, with JA a decade old and (AIUI) known security vulnerabilities in its version of the Q3 engine, I would be inclined to say the official JA Linux dedicated server is just a bad idea, and 32-bit Linux users should copy their PK3s into a "clean" OpenJK installation; so I don't think special-casing i386 Linux is necessary, or a particularly good idea. Many Linux users will prefer open-source code over binaries anyway, and it's not as if anyone not technically-inclined will have been using the existing Linux dedicated server, whereas the Windows and Mac clients have lots of non-technical users.

ensiform commented 9 years ago

Yes the intention is for pure server usage and downloading same as qvm mods in q3. At least on Windows from Raven.

ensiform commented 9 years ago

But generally all servers disable downloading because of the other exploits that were unfixed in the original versions.

xycaleth commented 9 years ago

What's the intention here? Is the idea that you can send an updated or modded ui/cgame to users of a server, or perhaps even send the OpenJK ui/cgame to vanilla clients?

[...] I should perhaps remind you that in the presence of auto-downloading, this is a security vulnerability [...]

The usual convention is for mods to package everything into PK3s, and many existing mods do this already. I guess the security vulnerability is not something we have (or at least, I had) ever considered. It's always been the case that this was possible. In the retail version of JO/JA, the DLLs are stored in the assets*.pk3 files. I suppose it wasn't so much of a problem with existing servers because most servers turned off auto-downloading due to vulnerabilities surrounding it (which OJK has now fixed). As I say, this is a convention which many mods already follow so we can't simply disable DLL extraction. I think we need another GitHub issue for this.

On case-sensitive platforms it should canonically be OpenJK not openjk, right?

I don't think we ever decided on what to do! But I would be okay with using OpenJK as the canonical folder name.

I would be inclined to say the official JA Linux dedicated server is just a bad idea, and 32-bit Linux users should copy their PK3s into a "clean" OpenJK installation; so I don't think special-casing i386 Linux is necessary, or a particularly good idea [...]

Unfortunately there's some political reasons with not doing this. There's a reasonably sized following (I don't have numbers, but at a guess, a third or a quarter of all JKA players) who will not play anything other than the vanilla MP game. They claim that by recompiling the code, the lightsaber combat feels subtly different. Now, this might be due to any number of reasons such as the RNG being slightly different, bugs in the code which exhibit slightly different behaviour with the specific compiler that was used at the time, placebo effect, we don't know and it's not something anybody has fully investigated before. I don't think it's a good idea to rattle the cage so to speak, so I think it's best to leave the existing jampgamei386.so as it is.

Having said that (and not wanting to delete all my text!), I suppose we could replace the jampgame DLL in base and if players do want to setup a vanilla "base" server, they can overwrite our DLL.

smcv commented 9 years ago

I don't think we ever decided on what to do! But I would be okay with using OpenJK as the canonical folder name.

Well, you've made a decision whether you intended to or not :-) The source code says

code/qcommon/q_shared.h:#define OPENJKGAME "OpenJK"

so that's where Linux builds look. Keeping it as-is seems easier than changing it.

As I say, this is a convention which many mods already follow so we can't simply disable DLL extraction. I think we need another GitHub issue for this.

I'll open one.

Having said that (and not wanting to delete all my text!), I suppose we could replace the jampgame DLL in base and if players do want to setup a vanilla "base" server, they can overwrite our DLL.

That's what I'd recommend - it's much simpler, and it gently pushes people in the direction of running code where security flaws are fixable. But if you want the special case, I can just undo it in the Debian package (I'm using symlinks instead of copying anyway, to save a small amount of disk space).

I've pushed some more commits to #644.

smcv commented 9 years ago

Now resolved, except for worries about the security of Sys_UnpackDLL(), which are #646.