KSP-ModularManagement / KSPe

Extensions and utilities for Kerbal Space Program
http://ksp.lisias.net/add-ons/KSPAPIExtensions
Other
11 stars 6 forks source link

The Sandboxed File System is borking on symlinks #6

Closed Lisias closed 3 years ago

Lisias commented 4 years ago

As explained here

The problem is that I'm using the absolute pathname to make sure no one is breaking out of the cage, preventing Add'Ons to access user's files. But the stunt rendered the symlinks unusable.

TODO: Cook a way of allowing symlinks when checking against breaking out of the sandbox filesystem.

Lisias commented 4 years ago

I was wrong on thinking I was wrong.

The Absolute stunt is working as intended, at least on MacOS.

I will have to tackled this down on a knife fight - i.e., trying to reproduce the problem locally.

In the mean time, a ScreenShot with the problem will be helpful, because I will see what the user got and can compare with what I expected, and try to figure out where things gone down through the tubes.

Lisias commented 4 years ago

Deleted my last post, it was wrong - I reproduced the problem on my machine this time (I had fired up the wrong test bed, shame on me).

This is what's happening on my rig (where the /Users is a symlink itself to /.users - a hack to convince MacOS Mojave to allow me to move the Users to another volume -- DAMN IT, APPLE!).

Screen Shot 2020-08-16 at 07 39 52

v-evets commented 4 years ago

Or, in my case, as I was already loading KSP to snap a pic when you updated the issue:

Screenshot_20200816_224353

The (relative) symlink there should be fairly obvious: ./GameData -> ./Profiles/Test/GameData GameData/Squad is also a symlink to ../../Stock/Squad, but I guess KSP Recall doesn't police that one :P

Lisias commented 4 years ago

It's not KSPe. It's Unity or Mono 3.5. (sigh).

The nice things on building decent APIs, however, is that once I fix this, everthing else that uses is fixed too.

I'm working on my standard solution for this: throw away the trash and writing a proper solution. :)

v-evets commented 4 years ago

It's Unity or Mono 3.5. (sigh).

Of course it is. They're "cross-platform" which means they don't work as expected on any platform.

Lisias commented 4 years ago

The freaking runtime is resolving symlinks and changing the directory to the "normalized" one when the binary starts. Then the AppDomain does the same on every assembly you load.

This is my Environment.GetCommandLineArgs()[0] on a commandline application:

/.users/lisias/Workspaces/KSP/GIT/net-lisias/ksp/KSPAPIExtensions/bin/Debug/Tests/Tests.exe

They just fucked up on this. This shit is unneeded and uncalled for.

Lisias commented 4 years ago

And it's Microsoft. A minimal console testbed on C# 4.0 had the same behaviour. Holy crap.

the Criminal Code forbids me to express my sentiments to the idiot that wrote this requirement.

Lisias commented 4 years ago

On a second review, I noticed something interesting.

The APPDOMAIN also does the stunt, but only on one level. On my screenshot, you will notice that the TweakScale symlink was resolved, but not the /Users one. This means that chaining symlinks will, effectivelly, fool the runtime!

Microsoft didn't even implemented the idiocy the right way - jizuis.

Lisias commented 4 years ago

I could not prevent the runtime from reparsing the executable pathname, but I can unreparse the assemblies location once they are loaded from inside the KSP file hirarchy.

The idiot from Microsoft that will force me to do things their way is still pooping on diapers.

Fixed on commits:

Lisias commented 4 years ago

@v-evets , please unzip this file on the TweakScale's Plugins folder, replacing the older KSPe.Light.TweakScale.

This stunt is working for me, I need to be sure it will work on Linux now.

download removed. Use this one instead

v-evets commented 4 years ago

Well, no warning no more... But the log appears a little unhappy: KSP.log

readlink returning non-zero is to be expected for things that are not links, so it probably should be a warning rather than an error. As for that big fat ArgumentException... You're the .NET guy :D

Layout for clarity: $ find ./ -type l -exec echo -n {} "-> " \; -exec readlink {} \; ./Logs -> Profiles/Test/Logs ./GameData -> Profiles/Test/GameData ./Screenshots -> Profiles/Test/Screenshots ./CKAN -> Profiles/Test/CKAN ./thumbs -> Profiles/Test/thumbs ./Profiles/Test/GameData/Squad -> ../../Stock/GameData/Squad ./Profiles/Main/GameData/SquadExpansion -> ../../Stock/GameData/SquadExpansion ./Profiles/Main/GameData/Squad -> ../../Stock/GameData/Squad ./UserLoadingScreens -> Profiles/Test/UserLoadingScreens ./KSP.log -> Profiles/Test/KSP.log ./saves -> Profiles/Test/saves ./settings.cfg -> Profiles/Test/settings.cfg

readlink will return relative paths for relative links, not sure if that's what you want, and exit 1 on non-links. realpath returns absolute path (and works on non-links too), but it's not part of coreutils so it may not be available on non-debian systems.

Lisias commented 4 years ago

Yep, it's the reason I decided to go along with readlink.

I intend to use realpath if it's available, but I still need to work a bit on that code (it may even works as it is, but that would be make me worried - life is not that easy).

And, of course, I still need to check if Windows will need similar stunts. My Windows machine doesn't have a GPU card (damn), but now that I know what I'm fighting, I can try some unit tests running on console there.

I'm downloading the log file and giving it a peek right now.

Lisias commented 4 years ago

Holy crap...

[WRN 12:12:45.087] Failed to reparse /home/steve/Games/KSP_linux/Profiles/Stock/GameData/SquadExpansion.
[ERR 12:12:45.087] /usr/bin/readlink returned 1 ()

[WRN 12:12:45.294] Failed to reparse /home/steve/Games/KSP_linux/Profiles/Stock/GameData/Squad.
[ERR 12:12:45.294] /usr/bin/readlink returned 1 ()

[EXC 12:12:45.379] ArgumentException: An item with the same key has already been added. Key: /home/steve/Games/KSP_linux/Profiles/Stock/GameData/Squad/
    System.Collections.Generic.Dictionary`2[TKey,TValue].TryInsert (TKey key, TValue value, System.Collections.Generic.InsertionBehavior behavior) (at <ad04dee02e7e4a85a1299c7ee81c79f6>:0)
    System.Collections.Generic.Dictionary`2[TKey,TValue].Add (TKey key, TValue value) (at <ad04dee02e7e4a85a1299c7ee81c79f6>:0)
    KSPe.IO.Path.Origin () (at <a9f6970cc9394c63b174c59d0fc99a2e>:0)
    KSPe.Util.Installation.CheckForWrongDirectoy (System.Type type, System.String name, System.String vendor) (at <a9f6970cc9394c63b174c59d0fc99a2e>:0)
    KSPe.Util.Installation.Check[T] (System.String name, System.String vendor) (at <a9f6970cc9394c63b174c59d0fc99a2e>:0)
    KSPe.Util.Installation.Check[T] (System.Type versionClass) (at <a9f6970cc9394c63b174c59d0fc99a2e>:0)
    TweakScale.Startup.Start () (at <fe30b98943a242f997bae5d1715f2076>:0)
    UnityEngine.DebugLogHandler:LogException(Exception, Object)
    ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
    UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)

I didn't saw that coming - symlinks inside the KSP_ROOT pinpointing to inside KSP_ROOT. The argument exception happened because the symlink was resolved and the target directory was already added to the UNREPARSE dictionary.

But I failed to undertand why the real directories were being processed at all - I check the path for being a symlink (ReparsePoint, on MS lingo) before deciding to call the readlink, it should not had happened.

Can you send me a ls -lR of the whole /home/steve/Games/KSP_linux directory?

I need to see how you used the symlinks in order to understand where I did wrong (if at all, this can be yet another MS' idiosyncrasy ruining my day).

v-evets commented 4 years ago

symlinks inside the KSP_ROOT pinpointing to inside KSP_ROOT

The symlinks involved never leave KSP_ROOT at all, and they're relative to it so they don't break if the whole KSP_ROOT is renamed or moved.

send me a ls -lR of the whole /home/steve/Games/KSP_linux directory

ls.txt.gz I hope you like verbosity...

I'm not totally convinced that calling external binaries is a good idea at all TBH, doesn't Mono.Posix have symlink handling stuff in it?

Lisias commented 4 years ago

I hope you like verbosity...

Not too much, but grep is my friend! :D

I'm not totally convinced that calling external binaries is a good idea at all TBH, doesn't Mono.Posix have symlink handling stuff in it?

I gave a look on it, but it relies on native libraries - what is an incredible pain in the ass on MacOS - I'm trying to load native DLLs on this piece of crap for months (in order to use Joysticks), but this is just impossible on signed APPs, as it appears. It's the reason I couldn't had tried Principia yet.

So I took the quick&dirty path - if it is good enough for BASH, is good enough for me, and it is platform agnostic (you can use it on Windows, if you are using mingw).

The stunt is only used once at KSP startup, with the results cached on memory from that point - so anything going wrong is caught early.

Lisias commented 4 years ago

In time, @v-evets , you probably will want to symlink the PartDatabase.cfg too.

EDIT: what would be a waste of time, because it's constantly removed and recreated.

v-evets commented 4 years ago

Indeed it is. But you did remind me to add Physics.cfg to the $includefile array in my script. Ta.

Lisias commented 4 years ago

I finally understood an interesting log entry on your KSP.log

[LOG 01:35:07.613] [AddonLoader]: Instantiating addon 'ModuleManager' from assembly 'ModuleManager'
[LOG 01:35:07.636] [ModuleManager] INFO: version 4.1.3.2 at /.users/lisias/Workspaces/KSP/runtime/1.10.1-SYM/Profiles/Test/GameData/ModuleManager.dll won the election against
Version 4.1.3.2 /.users/lisias/Workspaces/KSP/runtime/1.10.1-SYM/GameData/ModuleManager.dll

The AssemblyResolver from Squad is getting lost on this symlinks fest.

I reproduced your setup on my rig, and coerced the stunt into working - things works better when you do your job properly, instead of hacking and slicing your way out of the mess. :D

Lisias commented 4 years ago

@v-evets , here is the latest build, with binaries for everything that would need it.

download removed. Use this one instead

Where to install the files should be trivial.

You have a hell of a heterodox installment :D It's probable that if this works fine to you, it will work fine for everybody else. :)

v-evets commented 4 years ago

The AssemblyResolver from Squad is getting lost on this symlinks fest.

Nah, it's just trying both forks in the road, ending up in the same place, and going "ahh, screw it, eeny, meeny, miny, moe..." :P Since both options yield the same assembly, nothing explodes.

Really, one should either always get the real path or always get the link (the former being more common), but mono seems to be kinda schizophrenic when it comes to symlinks unless you explicitly resolve them.

This only becomes a problem when you're actively trying to check and compare paths of course :P

You have a hell of a heterodox installment :D It's probable that if this works fine to you, it will work fine for everybody else. :)

Thanks, I make a habit of illuminating corner-cases... And yes, yes it does work fine. :) Also, I run Gentoo, of course I like to be different. :D

I created that mess to speed up testing and bug reporting while saving disk space, my OCD couldn't stand separate installs just for a clean GameData directory. The script is about 300 lines of bash now, with support for kdialog, zenity and dialog (or none), fancy GUI progressbars for backup / restore, and a few other miscellaneous features... Scope might have crept ever so slightly. ;)

May I serve as a warning to others...

Lisias commented 4 years ago

The AssemblyResolver from Squad is getting lost on this symlinks fest.

Nah, it's just trying both forks in the road, ending up in the same place, and going "ahh, screw it, eeny, meeny, miny, moe..." :P

=D But empirical tests while writing my own AssemblyResolver revealed that additional assemblies are loaded on their own AppDomain, what inherently ends up forcing RPC when crossing domains.

C# has this annoying feature of silently abstracting you from important things. You make a simple call inside your AppDomain, it's a simple ASM call and that's it. You do the same on the same code "living" on other AddDomain, that call ends up passing through a marshaling in the same way you would expect on a RPC to the other side of the universe.

It's a interesting feature, but it should not be transparent like this. Marshaling is a fscking heavy way of doing business, it should not be transparently applied.

And.. Yeah. I think I finally understood why Squad introduced that "nasty" change on 1.8 on loading duplicated Assemblies: as a measure to improve performance. What's not necessarily a bad thing, but they should made the change obviously clear and had explained exactly what's happening and why. My ass got beautifully bitten by this stunt.

Since both options yield the same assembly, nothing explodes.

Yet. Give me some time, I will think on something. =D

7ranceaddic7 commented 4 years ago

When you play in the belly of the beast you will get bitten.

Lisias commented 4 years ago

When you play in the belly of the beast you will get bitten.

Being the reason we usually tie them before playing! :D

Lisias commented 4 years ago

Whoops... Forgot to close this. Doing it.

Lisias commented 4 years ago

I'm reopening this due a missing usecase, detected by boribori on forum.

I need to extent the readlink stunt to the KSP's root directory itself.

Lisias commented 4 years ago

Weird. I could not reproduce the problem on MacOS. =/

I created a symlink pinpointing a top-level KSP folder (ln -s ~/yadayadayada/1.3.1 KSP_Version), but everything worked as I (didn't) expected on MacOs.

This means that besides screwing up royally on this normalising path stunt, Microsoft is screwing up differently on Linux and MacOS - otherwise I would had the same result boribori got on his rig on mine.

Oh, well...

Lisias commented 4 years ago

Created a branch to handle the mess: https://github.com/net-lisias-ksp/KSPAPIExtensions/tree/issues/6

Lisias commented 4 years ago

Boribori, this is a new DEBUG compile withe the stunt implemented on the last branch compiled for everything I publish.

Please use these binaries to see what you get. I will need the KSP.log once you run the stunt, no matter you have a success or not - I need to understand what's happening on your machine, and the debugging logs will pinpoint me where the thing failed (or worked).

download removed. Use this one instead

for the records:

This is what I'm getting on my rig using these binaries on a KSP being called from a symlink (see post above)

[LOG 13:25:08.329] [KSPe.IO.Path] Calculating Origin for /.users/lisias/Workspaces/KSP/runtime/1.8.1/KSP.app/Contents/Resources/Data/Managed/Assembly-CSharp.dll
[LOG 13:25:08.329] [KSPe.IO.Path] Normalized path /.users/lisias/Workspaces/KSP/runtime/1.8.1
[LOG 13:25:08.505] [KSPe.IO.Path] Origin is /.users/lisias/Workspaces/KSP/runtime/1.8.1/
[LOG 13:25:08.506] [KSPe.IO.Path] PWD is /.users/lisias/Workspaces/KSP/runtime/1.8.1
[LOG 13:25:08.523] [KSPe Binder] Hooked.
codebori commented 4 years ago

Still the same warning KSP.log

Lisias commented 4 years ago

Still the same warning

Not a surprise:

[LOG 18:30:41.475] [KSPe.IO.Path] Calculating Origin for /home/boris/Games/KSP/KSP_linux/KSP_Data/Managed/Assembly-CSharp.dll
[LOG 18:30:41.475] [KSPe.IO.Path] Normalized path /home/boris/Games/KSP/KSP_linux
[LOG 18:30:42.036] [KSPe.IO.Path] UNREPARSE /home/boris/Games/KSP/RealSolarFiction/GameData/RSF/ <- /home/boris/Games/KSP/KSP_linux/GameData/RSF/
[LOG 18:30:42.353] [KSPe.IO.Path] UNREPARSE /home/boris/Games/KSP/KSP_linux_1.10.1_LRTR_RF/GameData/LRTR/PluginData/ <- /home/boris/Games/KSP/KSP_linux/GameData/LRTR/pluginData/
[LOG 18:30:42.366] [KSPe.IO.Path] Origin is /home/boris/Games/KSP/KSP_linux/
[LOG 18:30:42.366] [KSPe.IO.Path] PWD is /home/boris/Games/KSP/KSP_linux_1.10.1_LRTR_RF

Things are way different on your rig. Damn you, Microsoft!!!

Lisias commented 4 years ago

On the other hand, things may be less ugly as I was thinking... Wait a minute, I have an idea...

Lisias commented 4 years ago

@codebori , new attempt. Please download this file and redo the tests.

download removed. Use this one instead

(and publish the new KSP.log, the thing working or not!)

codebori commented 4 years ago

No warning this time, but I'm afraid it might not be a good sign. KSP.log

Lisias commented 4 years ago

Pretty embarrassing... "What could possible go wrong"? Yeah, right.

New release.

I spend some time testing this one, it should at least bork on something else than a NRE on your rig.

download removed. Use this one instead

codebori commented 4 years ago

Looks good so far. KSP.log I'll play for a bit and see if it works.

Lisias commented 4 years ago

Looks good so far. I'll play for a bit and see if it works.

Excellent!

Monitor any file being created (or existing file that was taken as missing). The change I made was somewhat deep on the stack, so any bork will screw up everything - on the bright side, any bork will be pretty damn hard to go unnoticed, as everything relies on that code to anything related to a file (from configs to textures, and even DLLs.)

Lisias commented 4 years ago

Of course, I missed something...

[LOG 10:34:23.448] [KSPe.IO.Path] UNREPARSE /home/boris/Games/KSP/KSP_linux_1.10.1_LRTR_RF/GameData/LRTR/PluginData/ <- /home/boris/Games/KSP/KSP_linux_1.10.1_LRTR_RF/GameData/LRTR/pluginData/

This directory is being unreparsed to itself - clearly a small flaw on the logic, probably induced because I waved the restriction of the unreparsing to happen only inside GameData, so I never had faced this new situation before.

A bit of additional coding is on the works.

codebori commented 4 years ago

Monitor any file being created (or existing file that was taken as missing). The change I made was somewhat deep on the stack, so any bork will screw up everything - on the bright side, any bork will be pretty damn hard to go unnoticed, as everything relies on that code to anything related to a file (from configs to textures, and even DLLs.)

I'm not sure what to look for exactly, but I haven't noticed anything out of the ordinary yet.

This directory is being unreparsed to itself - clearly a small flaw on the logic, probably induced because I waved the restriction of the unreparsing to happen only inside GameData, so I never had faced this new situation before.

I don't know what "unreparsed" means. I do have more symlinks in GameData/ This one is because of a case bug in LRTR.

Lisias commented 4 years ago

I'm not sure what to look for exactly, but I haven't noticed anything out of the ordinary yet.

On Recall and TweakScale, you should find no problems. my kspu "line" of add'ons are the ones that will suffer if I bork on something, as they reliy on KSPe for everything.

On the bottom line, I'm using the stunt myself on the test bed I built for @v-evets use case - anything wrong will affect both use cases, so in the end there's not much you can see unless you are inclined to embrace the dark side of the modding. :)

I don't know what "unreparsed" means. I do have more symlinks in GameData/ This one is because of a case bug in LRTR.

"Reparse" is how Microsoft calls solving symlinks into absolute directories. So what I'm doing is "unreparsing" by brute force.

You got hit because, in order to make things easier, I had confined the unreparsing stunt to GameData, as it would satisfy @v-evets 's use case without too much refactoring on the KSPe thingy. Expanding to KSP root itself would be a hell of a pain - if you see the new code I wrote, you will see I had to change some things on some critical parts of the code that I hoped to to ever touch again :)

When I initially wrote all of that, I didn't knew about this idiocy of reparsing the application root (as I'm a sane person, not a egotist that don't care about other people's need and do things the easiest way for me without caring about the consequences), so I was hopeful that "unreparsing" only the GameData contents would be enough.

Well... I was wrong. :D

I will keep this open for some time, just in case you or even me finds something. I made some changes on some critical parts that I wrote almost an year ago, and these code needs to be proven on the field somehow before risking it into the mainstream.

Lisias commented 4 years ago

Unsurprisingly, I found some loose ends on the code. Unlikely it will affect @codebori , as the use cases involved are related to the KSPU line of Add'Ons.

This zip file contains all the current changes. (I will remove the old ones, they are a liability by now).

download removed. Use this one instead

Lisias commented 4 years ago

Somewhat surprinsgly, I found yet another loose end on the code. This will not affect @codebori , as the use cases involved are (again) related to the KSPU line of Add'Ons.

This zip file contains all the current changes. (I will remove the old ones, they are a liability by now).

KSPe-Issue-6.zip

Lisias commented 3 years ago

Apparently this problem is bitting again, this time on Manjaro Linux. From the bug report on forum:

Game Ver.: 1.11.1.3066 64-bit from GOG

OS: Manjaro Linux 20.2.1

Mod Vers.: Module Manager 4.1.4.0, KSP Recall 0.0.5.0, TweakScale 2.4.4.5

Player.log: BugReport-TweakScale-2-4-4-5-KSPRecall-0-0-5-0-Player.log.zip

Lisias commented 3 years ago

Humm... Found the problem. :(

[TweakScale] ERROR: Scale should be installed on [/mnt/LinuxData/KSP/KSP1-11-1-BUGTEST/game/KSP1-11-1-BUGTEST/game/GameData/TweakScale/], not on [/mnt/LinuxData/KSP/KSP1-11-1-BUGTEST/game/KSP1-11-1-BUGTEST/game/GameData/TweakScale]. at error:0

A missingPath.DirectorySeparatorChar on the target path.... But I (think I) had solved this some time ago...

Lisias commented 3 years ago

TheKurgan, can you please run the following command

which readlink

and post the results here? Until this moment, this is the only thing that can go wrong and change the results from my machine (and from the Gentoo's one above) to your Manjaro...

Lisias commented 3 years ago

Fixed (again) on commit https://github.com/net-lisias-ksp/KSPAPIExtensions/commit/c43b32f1b366914a3364537a7a3485128456e77e .