Lua Threads for Computer Mod

[aldobr]

If you trully want a sandboxed environment, use minetest.deserialize()

Its was created for other uses. Parse your lua sourcecode and feed it to minetest.deserialize()

You will be only able to talk to it via its return value. But works.

[ninnghazad]

@electrodude512
thanks for taking the time. i don't want to compete, or steal.
i just like coding, i code for a living, i code in my free time, i code in my head when showering.
the main reason i want a mod like this is because i wrote large scripts for computercraft,
and just cannot continue them after what happened to mc. so i want to port it all to mt.
those scripts are kind of hobby, it's relaxing to watch virtual botters and systems do stuff for you.
(https://github.com/ninnghazad/SWARM)

read up upon digilines/mesecons and i like the idea. it would be great if it was kind of a modem-item/node that one could insert or attach to computers and possibly drones too. maybe put an item in a slot of the computer/drone, or have an attachable node-version for computers and a wieldable version for drones (cause they are entities). if there was an extra modem-item/node/whatever, we could plan the mod as a modpack and users with and without digilines could use the computers. same would go for other peripherals.

a computer with a digiline-adapter (digiplug?) with io-expander to mesecons would be a sweet thing. i tried using mesecons to open a door using a computer and it works. but i somehow didn't want to jam the mesecons-stuff directly into the computer, i wanted it more like something you have to craft to get. something like a hub that can be plugged in.

first peripheral i wanted to do was a wireless-modem, because it would be simple. it would have just written data to the target-system's event-queue, and read from it's own.
it is only supposed to work between computers and drones. but same method should work for connecting a digilines-modem to the mod's code.

oh, a harddrive is the only thing i can think of that isn't really needed, as the drones and computers come with an internal harddrive. just because they are rather useless without one anyway,it makes the code simpler, and there has to be a place to store system-specific configs or startup-files. but thats the only thing basic computers should have. the good stuff needs to be extra.

so many possibilities =). anyways, i'd be happy to work with you, but i'm not pissed if you gonna say "screw that ninnghazad and his weak codes! imma make my own!".
in the end it's just a some text for a game.
and it wouldn't be a catastrophe to have multiple computermods - after all mc has CC and OC, which are similar and yet very different.

hm, i will probably make a trunk/dev branch on github, and start to setup the modpack-structure in there.

@aldobr
hm, i don't quite get the idea of deserializing usercode, maybe as a sanitizing measure - but by sandboxing i mean restricting environmental access to functions, filesystem and variables,
while providing enough working features for the sandbox to be meaningfull,
and be able to monitor and control the sandboxed environment from the outside, mainly to kill processes within.
that practically means that _G has to be replaced with a protected version of itself, which does not allow any access to the outside through functions or objects.
but that works quite well right now - i checked what mesecons did for the lua-controller's sandbox,
and it's the same approach, but with more restrictions. they forbid for/while-loops in the lua-controller's sandbox for example,
which i allow - because a computer without loops... meh. when a computer wants for example to access a node next to it,
it cannot just have access to the minetest.* object or the get_node function - the sandbox has to provide a function which calls minetest.get_node in a safe manner,
so the computer can not look up nodes on the other end of the map or stuff like that, or even ban players and so on.
but i'm positive about that part, right now i'm more concerned about that coroutine/stack-bug in minetest.

[aldobr]

Has nothing to do with deserializing.

minetest.deserialize runs inside a trully sandboxed lua. This means that it EXECUTES code that is provided as parameter. But it does so inside a sandbox environment, and returns the value. If you check minetest.serialize, you will see that the serialized format is just a plain string in lua syntax.

If we could hack it in any way, we could very well have a sandbox without all the trouble.

[ninnghazad]

the issue about the coroutine-bug has been assigned to 0.4.11, so thats a good sign.

@aldobr
i have read up upon minetest.serialize/minetest.deserialize and it is not sufficient. as i have written before, this is not about having an empty sandbox, but a sandboxed environment.

[aldobr]

I downloaded minetest source code to search how does minetest.deserialize works.

The idea was to create an alternative minetest.sandbox call that would return a sandboxed lua from C++ code, cutting the complexity of this project.

I couldnt find minetest.deserialize source yet.

[ninnghazad]

that would not make sense. the sandboxed environment has to have access to a secured environment - whatever method you use you will have to write those functions. and it already is working - the work that is left to do would have to be done any way you create a secured environment. here is the part of the source of minetest.deserialize:

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

local function stringtotable(sdata, safe)
        if sdata:byte(1) == 27 then return nil, "binary bytecode prohibited" end
        local f, message = assert(loadstring(sdata))
        if not f then return nil, message end
        if safe then
                setfenv(f, safe_env)
        else
                setfenv(f, env)
        end
        return f()
end

function core.deserialize(sdata, safe)
        local table = {}
        local okay, results = pcall(stringtotable, sdata, safe)
        if okay then
                return results
        end
        core.log('error', 'core.deserialize(): '.. results)
        return nil
end


and again the code that is there already does that, but that is not enough. you're trying to solve a problem you do not seem to understand.
sandbox = easy, safe api for user to use within that sandbox = not easy
but still, it's just work to do and not impossibly hard or anything. just takes some time.

EDIT: that does however remind me that i need to add the bytecode-check to the code.

EDIT2: btw, ShadowNinja said he is to push a fix for the coroutine-issue, which would rock - cause then i can continue coding the drone-stuff.

[aldobr]

"that would not make sense."

That would make a lot of sense. You are still thinking about minetest.deserialize as a deserializing tool. You must understand how that function works. Its a completely sandboxed environment that happen to be used to deserialize things.

If you take that function C++ source code and expand it, you can very well have a general porpuse lua sandbox from C++ code where you can run arbitrary code (not just serialized tables/data).

Thats the whole point.

[ninnghazad]

look at the code. its loadstring and pcall with an empty environment... it's not magic, the mod already does that and more.

[aldobr]

What prevents this from be changed ?

[ninnghazad]

@aldobr
feel free to submit patches or fork the project, nothing is stopping anybody from changing everything.
maybe if you actually tried what you are talking about, you would understand what i am talking about.
i'd think we're talking past each other, and time would be better spend coding, but discussing thin air.
so less talk, more code?

[aldobr]

You are saying that as the sandbox used in deserialize is empty, this would prevent you from having a pre-loaded environment. But i already know that.

You might very well append the whole environment at the header of the code sent to deserialize. The only problem is the lack of multithreading ability (by that i mean the ability to stop execution midway and restart later).

Just patch minetest, and create a persistent sandbox system. Its not exactly the hardest thing to do (if i find the place where minetest.serialize is defined i will do that).

[ninnghazad]

builtin/common/serialize.lua line 17

[electrodude512]

How about we bug the MT devs about adding a minetest.register_on_key and add some way to capture keystrokes before they make it to the game engine, so you can have the normal mode where w is go forward and then another mode where w is type w on the keyboard you're currently pointing at? Then we could drop formspecs entirely and use something like digiline's lcds (but bigger, like ComputerCraft's monitors) for output and a keyboard block for input?

The keyboard would work like this: http://wiki.theoriginalbit.com/moarperipherals/index.php?title=Keyboard

[ninnghazad]

@electrodude512
heho, sounds good.
it would have to be a minetest.register_on_player_receive_key, similar to register_on_player_receive_fields.
we need to be aware that this means more traffic for the server, as every client's keystroke is send immediately,
instead of in a bunch like how it is with the formspecs now.
this may be a reason why core-devs chose not to have it in the first place.
we need that nevermind how exactly it is done however.

like the lcd-display-thingy, but i fear there may just be too little screenspace this way.
i would like to see a 80x24 characters minimum resolution a single basic screen.
guess if the texture used for the lcd can be large that might work and still be readable.
we would need like 640 pixels wide to somewhat nicely print 80 characters per line.
when i imagine using the mod, i like the real-display-way much more then the formspec-stuff.
the formspecs for a start saves us the work of actually having to render characters and stuff,
and they have the benefit of easily includable inventories.
say the drones's inventory or the computer's (possible) upgrade-slots.
i think coolest would be formspecs for kind of an inventory/upgrades/config-gui for the systems,
and the screen itself on the block as a texture like with digilines-lcd.
we could use rightclick for the formspec-menu, and crawl-punch to start/stop using a keyboard,
and punch just to dig up the systems.

we could drop a feature request for the input-thing, and just see how that goes.
we can still try patch it when none of the core's find the time, or make c-plugin if they don't want it in the mt-code.
also, lets check how digiline's does the lcds and how much pixels/characters we could squeeze on a block that way.

EDIT: see https://github.com/Jeija/minetest-mod-d ... d/init.lua - they use a sprite-node with label on it or something - seems rather easy, but not sure if it actually allows for change of fontsize to something usable.

EDIT: changing fontsize in the digilines sourcecode worked, still need to check if something like this can be done with entities (drones)

EDIT: digilines does it by using the "combine" texture-type http://dev.minetest.net/texture, so it should be possible for entities too

[ninnghazad]

using combined textures on entities works, updating the screen works too.
screenshot of drone with a test-screen on its back:

[electrodude512]

When this was still my project, I wanted terminals and hard drives to be separate from computers. This would let you have computers share harddrives (easy persistent data) and make you not need terminals (cheaper) or be able to use alternate terminals (remote login, HUD, etc.). Another reason to separate terminals is so they can be as modular as possible, so they can be easily replaced with something better when possible because we're having problems with them right now. Drones would then be just dumb peripherals that you talk to over some kind of wireless digilines, and you would need a separate computer to control them. A computer's formspec will just ask for the boot drive's channel and the main terminal's channel, and the bios will connect to them and make fake fs and terminal functions that really send stuff to the drives and terminals. What do you think about this idea?

Computers should have a digilines.wrap("channel"), like ComputerCraft's peripheral.wrap("channel"), that returns a table full of functions that send what you pass them to the peripheral on the specified channel and (optionally) waits for a response. We could come up with a standard way to ask a peripheral what methods it supports and how they work, whether they return stuff, etc.

I'm eventually going to fork dronetest on github and do some of this, as well as add some of my peripherals. By the way, mesecons luacontrollers have been very useful for testing digilines, since I never got around to getting my computers to even boot.

EDIT/double post that isn't: (are double posts allowed on this forum?)

Can I have write permission (or whatever github calls it) to your repository, or should I give you some pull requests first? I'm about to send a pull request adding userspace (sandboxed) libraries (os.loadlib) and what I think the digilines peripheral wrapper should look like.

I'm warning you now, I'm going to want to make lots of security changes (none in this coming PR). Why does bootstrap.lua have to worry about any security at all? Should privileged APIs really be in /rom? A computer loading them manually can't do anything, but then they're useless, so why are they even there? Shouldn't activate_by_id load the privileged APIs instead of bootstrap.lua? Nothing a computer shouldn't be able to see should ever be in its environment, even if it gets deleted. It's a security hole just waiting to happen.

[ninnghazad]

@electrodude512
heho. i like the modular approach, and while i have to think about how to implement drones exactly in a approach like this, i changed the computers/drones to not have displays, and added dronetest_display peripheral, which is used per digilines. just a quick shot, but it works well.
only thing i'd like to keep fixed to the computer would be internal storage - the hdd-peripheral could be more of a NAS-peripheral, used for its sharing aspect.

i was planning on implementing something like peripheral.*, with similar functionality. peripherals would accept tables as digilines-msg, those tables contain actions, and each peripheral can act upon a set of actions (defined in the peripheral's code, started from userspace, running in globalspace). furthermore each peripheral has something like a get_capabilities action which returns a table with all information to use the peripheral's actions. the peripheral-api would then use the get_capabilities action to retrieve said table and build functions for a wrapper, which just consist of a wrapped digilines-send to the peripheral.


###

now about the security, it's not where it's supposed to be, obviously. so one-by-one:

Why does bootstrap.lua have to worry about any security at all?
bootstrap.lua is the entrypoint to userspace, it is always what gets called first. however the current contents are more like a placeholder. it is later on supposed to either (file_exists) run a autostart script, or load all needed apis for a shell and drop stdin/stdout (however that will work) to the shell for cli-usage. it's the systems bootloader.
so those bla = nil lines do not belong there, they do belong in activate_by_id (or whereever i setup the userspace_env), and it does not have to worry about security - but read on.

Should privileged APIs really be in /rom?
/rom is the place for privileged stuff - it must always be read-only to userspace. everything in there may be used from userspace but runs in a privileged environment.

A computer loading them manually can't do anything, but then they're useless, so why are they even there?
they are supposed to be loadable from userspace using sys.loadApi. and there will be more apis. those loaded in this monstrosity of bootstrap are just to run the "shell" currently used (that kickass while-loop down there).

Shouldn't activate_by_id load the privileged APIs instead of bootstrap.lua?
Nope see above and below.

Nothing a computer shouldn't be able to see should ever be in its environment, even if it gets deleted. It's a security hole just waiting to happen.
i agree completely. i assume you raised this because you assumed APIs are something from globalspace that is by some mistake accessible from userspace. but this ain't so. however, APIs need to be safe to be run from userspace.
APIs do not need to worry about infinite-loops/syntax-errors-type of security (in general), but need to provide the secure fs-access, secure loading-functions and secure peripheral-wrapps and and and...
APIs are in a parallel-dimension where global/mod-space and userspace meet, they use cool global stuff and wrap it in a way safe for userspace.
APIs are like libraries.


###


I hope i did get the idea across. i know that what is there now does not quite follow those rules, but, hey, project is like a week old. theres some stuff that may seem strange, but is just in place cause of that coroutine-bug, and will be changed once that is fixed (like updating displays in a globalstep instead of when printed to). the bootstrap is just like this cause it has to wait for proper input-solution to create a shell.

what i am planning to do next would be turning the drones into a peripheral, and maybe create a modem or whatever is needed to connect them to the network. in that turn i would also like to add the peripheral-ness to the computers themselfes.
for that a peripheral-over-digilines/dronetest-over-digilines(?) protocol is needed indeed.
what i want to be able to do is for example:

local drone = peripheral.wrap("dronetest:drone:ID") -- some channel leading to a drone
drone.turnLeft()
print("print to stdout")
local secondScreen = peripheral.wrap("dronetest:display:ID") -- some secondary screen on the network
secondScreen.print("print to some other display")

and so on.

im looking forward to seeing your ideas about the peripheral-wrapper. it would be cool if it was loadable as an API.
but do elaborate about the sandboxed-libraries, as i think this might be what sys.loadApi does (loadApi loads privileged code from userspace). if it is to load unpriviliged code from userspace it could be usefull, could however overlap with what shell.run does.

i will set you up as a contributor on github when i get home.
you can add stuff as you want for now, but if you change existing stuff in way that may break stuff please discuss,
and document.

btw,drop by #minetest on irc if you can

[ninnghazad]

thought about peripheral.* some more - would be great if you could directly put it in /rom/apis/peripheral.lua as part of the main mod.
i placed something to start from there with a basic function for wrapping and a test_peripheral example that wraps a display.
to test, use /giveme dronetest:computer and /giveme dronetest_display:display and place computer and display,
connect with digilines.
set display's channel to "dronetest:computer:YOURCOMPUTERIDHERE".
then boot computer and type test_peripheral.

look in dronetest_display/init.lua, rom/bin/test_peripheral and rom/apis/peripheral.lua.
its just a tech-test to see how this may work, so it doesn't really do anything usefull besides the example.
but should be enough to serve as an example of how to put the peripheral-thing as an API.

computers that receive digiline-msgs put them in their event-queue, so they can be easily retrieved in userspace using
sys:receiveDigilineMessage(channel,message_id)

[electrodude512]

Thanks for making me a contributor. I was going to send you a pull request, but Git was being stupid (note to self: don't amend commits after pushing them). I'll talk to you about changing things already in place, but I'm going to go ahead and add stuff that doesn't change stuff you already have. I'm adding a mechanism for userspace (sandboxed) libraries (like for getting a table of functions interface for digilines peripherals).

[ninnghazad]

i see the github term is "collaborator" - be that as it may, i set you up.
hm, that loadlib thingy may be handy as a basis for a userspace-version of loadstring dofile and so on.
want to put it in /rom/apis/os.lua as os.whatever?
remember that the loadstring you use when putting this as an api is the global one, so functions it returns come with privileges - don't forget to put some empty env even if nothing else is specified.
i wanted to have userspace-versions of all those functions (except debug maybe, thats a tricky one) like loadstring and other unsafe functions. that'll be a good start.
it'll be userspace-sandboxes within the main-sandbox, sweet.

about the purpose of loading userspace-apis using that stuff to get functions for peripherals:
take a look at /dronetest_display/init.lua, the on_digilines function and stuff around it. that actions table contains possible peripheral-functions for the display. computers (or whatever is speaking the protocol) can send a GET_CAP over digilines to the display and the display responds in its digiline-callback with a call to that function ('action'), and returns its output.
this all outside of userspace, peripheral api would make that accessible to userspace.
this has a few upside versus extra apis in userspace.
- no need to secure each peripheral's networking api, rather secure peripheral api
- actions of a peripheral are in code, like digilines or mesecons.
- actions can share the object's env they belong to more easily.
- there is something stylish about the method of actually calling the peripheral to have it execute an action.
however, the format of those messages passed is not good yet, don't remember but something about them was missing. works nevertheless.

oh, i changed the formspecs to have a channel field. and removed buttons and stuff from the drone. will try to make dronetest_transceiver, some kind of node that will give a first way of connecting drones to the digilines network, and make them accessible to computers through that.

btw, i gave default digiline-channels like this: dronetest:TYPE:ID
TYPE is computer,drone,display,printer,whatever
ID is either real id or just some running number.

have fun coding

[electrodude512]

I really think we should redo the whole computer environment. All privileged APIs should be loaded into the computer's environment before bootstrap.lua starts, so bootstrap.lua never has access to anything it potentially shouldn't. That way, it's actually completely sandboxed and still will be sandboxed no matter how many bugs I introduce. Should I make a branch and do this in there or what?

You should start by trusting nothing and only permit things that you are absolutely sure are safe (i.e. whitelist rather than blacklist approach).

Also if you were wondering about this in dronetest/init.lua:

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

local ret = {coroutine.resume(co)}
--   dronetest.log("Computer "..id.." result:"..dump(ret))


Computers won't run for me and I can't figure out why:

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

09:53:07: ACTION[ServerThread]: dronetest: STATUS: suspended
09:53:07: ACTION[ServerThread]: dronetest: TYPE: table
09:53:07: ACTION[ServerThread]: dronetest: System #3 has been activated, now 1 systems active.
09:53:07: ACTION[ServerThread]: dronetest: system 3 generated output: System #3 has been activated.
09:53:07: ACTION[ServerThread]: dronetest: STATUS: suspended
09:53:07: ACTION[ServerThread]: dronetest: TYPE: table
09:53:07: ACTION[ServerThread]: dronetest: System #2 has been activated, now 2 systems active.
09:53:07: ACTION[ServerThread]: dronetest: system 2 generated output: System #2 has been activated.
09:53:07: ACTION[ServerThread]: dronetest: System #2's main process has ended! Restarting soon...
09:53:07: ACTION[ServerThread]: dronetest: System #3's main process has ended! Restarting soon...


[ninnghazad]

COMMENCE EPIC QUADPOST!


#1
the resume-line in init.lua is what keeps computers running.
that resume executes the systems' coroutines until they yield again (or quit).
each computer has it's own coroutine.

#2
the server-log you posted looks like the systems crash while booting and try to restart then. which except for the crashing part would be correct. it should look more like this:

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

16:56:04: ACTION[ServerThread]: dronetest: on_receive_fields received ''
16:56:04: ACTION[ServerThread]: dronetest: STATUS: suspended
16:56:04: ACTION[ServerThread]: dronetest: TYPE: table
16:56:04: ACTION[ServerThread]: dronetest: System #1 has been activated, now 1 systems active.
16:56:04: ACTION[ServerThread]: dronetest: system 1 generated output: System #1 has been activated.
16:56:04: ACTION[ServerThread]: dronetest: system 1 generated output: System #1 is booting!
16:56:04: ACTION[ServerThread]: dronetest: system 1 generated output: Finished booting #1, dropping to shell.
SEND


this probably caused by an error in your bootstrap (as it did not get to the "is booting" message).
however, in such cases errors should be thrown - but i too had some cases which would just fail silently in lua.
take out api you last added to bootstrap to check if error lies there. also try if it's same error if you just use one computer in a clean world (if that works there are ID-problems somewhere)

#3
i think you're misunderstanding the way the environments work now - bootstrap does not have access to anything it shouldn't. also the approach is whitelisting, in dronetest/init.lua userspace_environment gets prepared, which is a clean environment to which safe functions are supposed to be added. there are some unsafe ones in there i added for debugging.
bootstrap runs in that userspace-environment.
it is sys.loadApi that brings in stuff from the unsafe global environment, and it can only be used to load apis - which in turn must be save to be accessed from userspace.
so bootstrap is actually very much sandboxed - like all other userspace code it just has the ability to loadApi other stuff.
i added a few lines to bootstrap to print out its current environment, and it looks like this:

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: System #1 is booting!
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: pairs (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: table (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: mod_dir (string)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: fs (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: sys (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: peripheral (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: processInput (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: dump (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: ipairs (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: mod_name (string)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: term (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: print (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: math (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: getId (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: os (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: _G (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: pcall (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: string (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: type (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: count (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: shell (table)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: bootstrap env: xpcall (function)
17:08:17: ACTION[ServerThread]: dronetest: system 1 generated output: Finished booting #1, dropping to shell.


as you can see, this is the whitelisted sandbox environment specified earlier.
if you introduce bugs in apis there will be crashes - its as simple as that - does not matter when you add the apis to userspace.
the fact that there are functions whitelisted to userspace that should not be there (like getfenv) is only for trying out stuff more easily. they can just be removed from dronetest/init.lua userspace_environment definition to have them be gone.
i think that link to the lua-wiki is even in the code somewhere, or at least was.

maybe the name bootstrap is confusing, i also removed the =nil s from bootstrap to make it more obvious this is userspace.
it could also be called init or bios or loader or this-is-the-function-that-runs-first-in-userspace.
it really, really does not have global access.
i don't even really get where you got the idea it might have.

the only way to expose globalspace to userspace are apis and the sys-object.
this can only be breached by using a bad function from an api, that somehow returns its own globalspace.

the system-apis must be accessible to userspace, obviously, but they *must* have access to globalspace to have any use - otherwise it would not be possible to actually interact with the gameworld from userspace in any way.
(you could basically just run 1+1 and not even see the output).
if they do not need access to the minetest.* object and all the other stuff,
they are not system-apis but userspace-apis,
and shall be included from userspace to userspace using your os.loadlib for example.
that is a *very* important distinction.
nothing stops us from having as many userspace-only-apis as we want, but the basic system-apis that need to have access to the global env are simply a must, not by preference, but by cold logic - these simply must be callable from userspace but have access to globalspace.

however, if you don't believe me (which you should seeing the output of that print), branch the project and try a different way.
branch for large changes to test them out without confusing the rest of the team, well, me ;)
also that way we can compare branches to see how functionality actually differs.

in this case we would probably see that there aren't many ways to sandbox a coroutine, and doing it differently would most likely end up doing it the same way but just calling functions differently.
it basically always is this:

local f = function() print("hello from userspace") end
setfenv(f,{print=print}) -- nothing can be done in that env but print stuff
local co = coroutine.create(f)
coroutine.resume(co)

and this is what we do right now too.

excuse that wall-of-text, but i do think this is an important matter.

#4
by the way - to get access to the current userspace from an api's globalspace, you can do getfenv(2) if the apifunction was called from userspace (like in shell.run i think) to get the safe environment. thats usefull to have an api load functions and have them run in userspace, instead of an empty environment for example.

also shadowninja has pushed the fix for issue #1709, and if it works, a lot of stuff can move on.

[electrodude512]

the resume-line in init.lua is what keeps computers running.
that resume executes the systems' coroutines until they yield again (or quit).
each computer has it's own coroutine.

Yeah, I know. I changed it to what I quoted it as, and you put it back. I was just saying I changed it to figure out why computers weren't running and accidentally committed it.

Computers still don't run for me. bootstrap.lua makes it past the compiler, but the first line, a print statement, never gets run.

<rest of your post>

You're right, bootstrap.lua only gets whitelisted functions. However, they have dangerous environments. You should do this to all of the functions:

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

function wrapfunc(func, env)
   return setfenv(function(...) return func(...) end, env)
end


That way, computers can't run getfenv(sys.receiveDigilineMessage).minetest and get the global minetest table.
I already mostly did this and will push it soon.

About computers not running, I added this to the beginning of bootstrap.lua:

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

print("Booting...")

coroutine.yield()

coroutine.yield()


That didn't even work. However, I tried replacing the loadstring(bootstrap) with function() print("Booting!") end and it worked.

EDIT:

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

...
-- Load bootstrap code once
local bootstrap = readFile(mod_dir.."/bootstrap.lua")
dronetest.log("bootstrap:"..dump(bootstrap))
...


That prints 'boostrap:""'.
I made readFile print out error messages and return nil instead of silently fail. Here's the result and culprit:

dronetest: readFile: "/home/<username>/src/minetest/bin/../mods/dronetest/dronetest/bootstrap.lua" is not a valid filename.

I'm now fixing readFile to allow .. but still enforce sandboxing.

UPDATE:
Computers now boot for me!

The root of the problem was that my minetest is installed in my home folder, not /usr/..., so it used a relative path and not an absolute path. That means mod_dir had a .., meaning that all calls to readFile failed!

I replaced is_filename(filename) with is_filename_in_sandbox(filename, sandbox), which allows .. but doesn't allow filename to be outside of sandbox. It knows about the . and .. files. I made readFile and writeFile take another parameter, sandbox, but am still updating stuff to use it. Passing nil for a sandbox means no sandbox.

[ninnghazad]

That way, computers can't run getfenv(sys.receiveDigilineMessage).minetest and get the global minetest table.
I already mostly did this and will push it soon.

i know and was planning on doing that checking in userspace-version of getfenv (or disable it). the reason for this plan was me assuming that taking away the env from those functions may stop them from working. for example functions that use dronetest.*. but give it a try and we'll see, if your way works we'll just use it.
the other way could be wrapping getfenv and check all envs for a minetest.* object, and just return userspace if found.

but more urgent, your systems not booting:
firstoff it confuses me a little, because i don't have any problems using clean mt, clean dt and clean world.
how the system fails is usually caused by a syntax-error, in this case bootstrap. error catching is not complete yet ;). runtime errors should be caught, but syntax-errors that cause compile-fails aren't in many places.
so it might actually work to comment out or remove all of bootstrap and leave only

Your phone or window isn't wide enough to display the code box. If it's a phone, try rotating it to landscape mode.

Code: Select all

print("HEHO!")
return true

or try a fresh bootstrap from git. i'm commiting quite often, so it might be you pulled some rancid version between commits or so. the syntax-error may be anywhere in the file, so thats usually the case when line-1-print won't do.

I'm now fixing readFile to allow .. but still enforce sandboxing.

sounds good, readFile actually isn't supposed to have this check, it's meant for userspace-filereading, and only as a quick n dirty measure. so it should be used in fs.* os.* and so on sys-apis, there is _makePath but thats all ugly, and not usuable in a release - so fix away there too if that ugliness hinders your stuff.
as you may have noticed, there's lfs included, which should give all the tools to make cross-platform (as in win/linux/android) fs.* os.* and so on for userspace. readFile in main-init is meant as blunt tool to read configs or stuff that ofc should never reach userspace.

i've been at it at multiple spots, mainly drones, userspace-version of coroutines and general stuff. the last version i pushed enabled me to setup a computer, multiple drones, multiple displays and a transceiver. i then used test_drone command on the computer, and have 2 drones moving a bit. pretty cool. computer runs the command and connects to the drones, that traffic is routed through the transceiver, and the stdout from computer is send over computers channel. displays may receive if listening on that channel.
transceiver forwards all traffic to all drones that does not come on it's own channel, that channel is to be used to controll the transceiver later on to restrict sending or so.
drones answer with trick over transceiver to computer.
oh i also just added forceloading for computers and drones.

EDIT:
just read update. that sounds even better. that function also nice for os.* fs.* in userspace.

EDIT:
when stuff works try build like above and try the test_drone with 2 turtles, u may have to change the ids/channels in test_drone

[electrodude512]

Are you using "userspace" to refer to sandboxed or normal code? You mean sandboxed code (code the end user writes), right?

Don't bother to take away the env from any functions. It will lead to nothing but headaches and subtle security flaws. Instead, wrap the function inside another function with a safe env. The sandbox function I wrote but haven't committed yet goes through a table you pass it and does this to all of the functions inside. I also modified your table.copy to have a third parameter, safeenv, that it will use for wrapper functions' environments. It won't wrap functions or mess with their environments if you don't pass it a safeenv.

I've already made a whitelist for setfenv/getfenv and setmetatable/getmetatable, but I've disabled it, via "if false and illegal then return end", because I haven't tested it yet.

This won't happen today, but I plan to make a function that creates a userspace environment for each system, instead of copying one table. This will make system-specific closures possible and not a headache. It will also fix the the sys.id bug.

[ninnghazad]

Are you using "userspace" to refer to sandboxed or normal code? You mean sandboxed code (code the end user writes), right?

yes, i refer to the environment and coroutine the player writes and executes code in as userspace.

The sandbox function I wrote but haven't committed yet goes through a table you pass it and does this to all of the functions inside.

that sounds proper. however, we must make sure that whatever that function returns does not contain a loophole via a returned function or object. so we need to inspect lua apis we want in userspace for unsafe return values, and wrap these in apis (where we do something with the return-value and return a userspace-version or interpretation of it)
- the other we add to the table-o-userspace-functions and walk through with your function.
coroutine for example needed an api - math.* or string.* may not need one, as they return rather predictable stuff.

This won't happen today, but I plan to make a function that creates a userspace environment for each system, instead of copying one table. This will make system-specific closures possible and not a headache. It will also fix the the sys.id bug.

Ye to that - that is how it is supposed to be, but right now it's more a mess of copies and links. system-specific stuff at that point may be usefull later on, to have different access-permissions per player/system or fractions of systems and so on.

EDIT: in case you didn't notice in the readme-change, theres a texture-bug somewhere, so updating displays leaks a bit of mem everytime - switch displays to empty channel to stop updating if that becomes a problem.

[electrodude512]

I think we should separate the computer innards from init.lua into their own file, for manageability. A 1200 line file is too big, and there's no excuse not to split it because the computer innards seem like it would be fairly easy to separate them from node definitions and such. I would make it modular if we were to do that, so it will be easier for us and other developers (by making their mod depend on ours) to do things like attach computers to items or blocks other than the standard computer block or have non-physical computers floating around in lualand doing magical (probably server admin) stuff.

If you don't mind telling me, which timezone do you live in, so we can coordinate not making big changes like splitting the main file in two at the same time and having to deal with huge merges conflicts? I live in GMT-4:00 (EDT).

yes, i refer to the environment and coroutine the player writes and executes code in as userspace.

OK, good.

that sounds proper. however, we must make sure that whatever that function returns does not contain a loophole via a returned function or object. so we need to inspect lua apis we want in userspace for unsafe return values, and wrap these in apis (where we do something with the return-value and return a userspace-version or interpretation of it)
- the other we add to the table-o-userspace-functions and walk through with your function.
coroutine for example needed an api - math.* or string.* may not need one, as they return rather predictable stuff.

For functions returning functions, we can either have the wrapper sandbox things itself (slower but foolproof) or expect API functions to do it themselves (faster but more dangerous).

Ye to that - that is how it is supposed to be, but right now it's more a mess of copies and links. system-specific stuff at that point may be usefull later on, to have different access-permissions per player/system or fractions of systems and so on.

Should you or I make environments have constructor functions and not just table copies? You seem to be making good progress on peripherals right now, which I haven't even looked at yet, while I'm only really familiar with what goes on inside the actual computer block.

Whoever does this should also split it into two files at the same time.

EDIT: in case you didn't notice in the readme-change, theres a texture-bug somewhere, so updating displays leaks a bit of mem everytime - switch displays to empty channel to stop updating if that becomes a problem.

Wonderful! Do digilines LCDs have this problem too? Does breaking and re-placing a display free the lost memory?

[ninnghazad]

I think we should separate the computer innards from init.lua into their own file, for manageability.

Ye, that init.lua has grown quickly. Lets split it up.

which timezone do you live in

GMT +1:00 (CET)

we can either have the wrapper sandbox things itself (slower but foolproof) or expect API functions to do it themselves (faster but more dangerous).

apis in that regard not only have the purpose of safely getting data to userspace, but also to provide it in a form the player may use, or combine several global-calls to form some userspace-information. if it is just about a function to be used more or less directly, theres no need. but if it is about producing complex output, a specific wrapper is needed, what i consider apis for. also we have to remember that being able not load parts of the userspace may be good when number of functions grows, to reduce server-time needed to boot systems. but i would consider those apis that exist in normal lua also to be part of a fixed minimal set.

Whoever does this should also split it into two files at the same time.

Well, you said you had stuff uncommited, so you do it i'd say right after commiting (even if not completely done). As it's weekend however that would have to happen today, so i can code myself silly over the weekend. well if i feel like it. i can split tonight too if you can't find the time.
thought of leaving init.lua with actual init and includes, and then have computer/drone/util and probably more as we go on.

Do digilines LCDs have this problem too? Does breaking and re-placing a display free the lost memory?

Probably yes, but due to their tiny texture, it will not be nearly as noticable. And nope, not even exiting to menu does. Only quitting game. I assume this will piss of other people too, so it will sooner than later be fixed.

[electrodude512]

Ye, that init.lua has grown quickly. Lets split it up.

I think I can do it tonight.

apis in that regard not only have the purpose of safely getting data to userspace, but also to provide it in a form the player may use, or combine several global-calls to form some userspace-information. if it is just about a function to be used more or less directly, theres no need. but if it is about producing complex output, a specific wrapper is needed, what i consider apis for. also we have to remember that being able not load parts of the userspace may be good when number of functions grows, to reduce server-time needed to boot systems. but i would consider those apis that exist in normal lua also to be part of a fixed minimal set.

Maybe we could put a metatable.__index on _G to automatically load apis when they're first needed?

Well, you said you had stuff uncommited, so you do it i'd say right after commiting (even if not completely done). As it's weekend however that would have to happen today, so i can code myself silly over the weekend. well if i feel like it. i can split tonight too if you can't find the time.
thought of leaving init.lua with actual init and includes, and then have computer/drone/util and probably more as we go on.

I've committed everything. If you decide to split init.lua, please tell me as soon as you decide to, so I don't also do it and make a mess. I might be able to do it tonight, so please commit and push all of your changes when you're done working on stuff tonight.

Probably yes, but due to their tiny texture, it will not be nearly as noticable. And nope, not even exiting to menu does. Only quitting game. I assume this will piss of other people too, so it will sooner than later be fixed.

This leak is only on the client, right? Restarting the client will fix it?

[ninnghazad]

Maybe we could put a metatable.__index on _G to automatically load apis when they're first needed?

sure, that's a good idea - either way we should add an easy way to list available apis. a command and function maybe. if you add an __index to userspace's _G, just add some viewer-function for the table it uses also. maybe read list of /rom/apis/ on modload and store name=>filename in table or something.

If you decide to split init.lua, please tell me as soon as you decide to, so I don't also do it and make a mess.

no prob, i think i shall do this today, at least for some stuff. but not immediately, have to make food for my woman first. also wanted to add more drone stuff.

This leak is only on the client, right? Restarting the client will fix it?

well, i actually haven't checked as i use singleplayer for testing - but i assume so, cause i see no reason for the server to load textures.