[Mod] Interkom - Exchange stuff between servers [Interkom]

u18398

[Mod] Interkom - Exchange stuff between servers [Interkom]

by u18398 » Post

Image
Last edited by u18398 on Wed Jul 20, 2022 12:59, edited 7 times in total.

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] Exchange stuff between servers [Interkom]

by sofar » Post

This code looks super iffy. A fun read to find input validation issues so people can send arbitrary stuff they don't have to players on other servers. Are you sure you're checking all the input correctly? Did you protect against races and properly lock the transfer files?

You should post the dependencies and license in the forum thread. See sticky topic.

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

sofar wrote:This code looks super iffy.

just for the record:
  • -I do not post something "iffy" in mods section, this works for months without iussues on my two public servers
    -We are here in wip mods section.Wip means: work in progress.
    Do not expect something here what would not needed to be worked on.
sofar wrote:A fun read to find input validation issues so people can send arbitrary stuff they don't have to players on other servers. Are you sure you're checking all the input correctly? Did you protect against races and properly lock the transfer files?
Exaggerated a bit ?
But I am always happy other people have fun with my developments. Any proove, example maybe ?
I am repeating myself, this mod is running for months on my servers without trouble.

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] Exchange stuff between servers [Interkom]

by sofar » Post

Gundul wrote:
sofar wrote:This code looks super iffy.

just for the record:
  • -I do not post something "iffy" in mods section, this works for months without iussues on my two public servers
    -We are here in wip mods section.Wip means: work in progress.
    Do not expect something here what would not needed to be worked on.
sofar wrote:A fun read to find input validation issues so people can send arbitrary stuff they don't have to players on other servers. Are you sure you're checking all the input correctly? Did you protect against races and properly lock the transfer files?
Exaggerated a bit ?
But I am always happy other people have fun with my developments. Any proove, example maybe ?
I am repeating myself, this mod is running for months on my servers without trouble.
If I reply with concerns then it means (1) I generally like the code/mod/idea, and (2) I care enough to voice my concerns in case that I think it's warranted. It's not a personal attack, obviously, I never said "Oh gundul has the nerve to tickle a wombat!", instead I said "this code looks suspect (iffy)" with the intent of making you critically think about whether this is actually secure or not.

It obviously works, I don't doubt it. What I doubt is that if players send requests at the same time that both actually arrive intact. Since the receiving server can't verify that the items are in the senders' inventory, it's not hard to think of a possible exploit to generate some message that gets the receiver lots of items that the sender never had, or just corrupt the messages. There's also some splitting of user input going on without escaping, so there's a possibility of people throwing weird characters at you that you didn't think of ("\n") and may end up in your transfer file.

It's not my code, if you don't want to secure it more, that's your perogative. But now that you've posted the code, if there is an exploit possible, anyone can find it.

User avatar
Krock
Developer
Posts: 4649
Joined: Thu Oct 03, 2013 07:48
GitHub: SmallJoker
Location: Switzerland
Contact:

Re: [Mod] Exchange stuff between servers [Interkom]

by Krock » Post

I wondered how "iffy" the code actually is and only figured out some minor issues:

1) Unreliable file writing. I'd rather use `minetest.safe_write_file` to ensure everything is written correctly in case the server is not shut down correctly.
2) Writing to the file at the same time: If server 1 writes to the file whereas server 2 deletes an entry from the file, there may be a file corruption. Not sure though, how this could be solved.
3) If you ever allow metadata: New lines and commas would break the system. There's no escape handling in your custom string.split implementation and new lines would destroy the format. Luckily you treat items with metadata like tools (disallowed), even though they could be stackable.

It was however interesting to see this mix of tabs and spaces. I think you're using four spaces, but the editor does not convert existing tabs when you indent the code.
Look, I programmed a bug for you. >> Mod Search Engine << - Mods by Krock - DuckDuckGo mod search bang: !mtmod <keyword here>

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

sofar wrote:
If I reply with concerns then it means (1) I generally like the code/mod/idea, and (2) I care enough to voice my concerns in case that I think it's warranted. It's not a personal attack, obviously, I never said "Oh gundul has the nerve to tickle a wombat!", instead I said "this code looks suspect (iffy)" with the intent of making you critically think about whether this is actually secure or not.
Sorry for getting you wrong, engish is not my native language and "iffy" is not the word we would use in a situation like this.
sofar wrote: It obviously works, I don't doubt it. What I doubt is that if players send requests at the same time that both actually arrive intact. Since the receiving server can't verify that the items are in the senders' inventory, it's not hard to think of a possible exploit to generate some message that gets the receiver lots of items that the sender never had, or just corrupt the messages. There's also some splitting of user input going on without escaping, so there's a possibility of people throwing weird characters at you that you didn't think of ("\n") and may end up in your transfer file.
The security is always at my mind. But it is only about sharing some stuff between servers. Worst that could happen is someone getting lots of stuff he did not have. And I get the debug for free and can work with that. :)
I already updated some parts of the code checking more detailed for nodenames.
sofar wrote: It's not my code, if you don't want to secure it more, that's your perogative. But now that you've posted the code, if there is an exploit possible, anyone can find it.
I will make it as secure as I am able to do, that is why I post it now and not already did when I first put the mod
online. That is also why I put the warning caution and do not describe more in detail how to synchronize folders.
Everybody must decide this for his own. Not to forget that exploits also can have positve effects when server and
running software are well monitored.

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

Krock wrote:I wondered how "iffy" the code actually is and only figured out some minor issues:

1) Unreliable file writing. I'd rather use `minetest.safe_write_file` to ensure everything is written correctly in case the server is not shut down correctly.
Thank you, I did not know that exists in the minetest api.
Krock wrote: 2) Writing to the file at the same time: If server 1 writes to the file whereas server 2 deletes an entry from the file, there may be a file corruption. Not sure though, how this could be solved.
I had this in my mind. But I must confess I have no solution for this, too. I was thinking that the software which synchs the folders would take care of this maybe.
Krock wrote: 3) If you ever allow metadata: New lines and commas would break the system. There's no escape handling in your custom string.split implementation and new lines would destroy the format. Luckily you treat items with metadata like tools (disallowed), even though they could be stackable.
I do not plan to allow metadata. This is not wanted now and in the near future.
Maybe all this can be done with a gui in the future, but only if this really runs reliable.

Items with metadata could be stackable, yes. But in that case the metadata would be lost if sent to an other server.
Krock wrote: It was however interesting to see this mix of tabs and spaces. I think you're using four spaces, but the editor does not convert existing tabs when you indent the code.
Sorry, which tabs and spaces you mean ?

User avatar
Krock
Developer
Posts: 4649
Joined: Thu Oct 03, 2013 07:48
GitHub: SmallJoker
Location: Switzerland
Contact:

Re: [Mod] Exchange stuff between servers [Interkom]

by Krock » Post

Gundul wrote:Items with metadata could be stackable, yes. But in that case the metadata would be lost if sent to an other server.
Ah okay. I didn't look at the code for a too long time to notice that behaviour.
Gundul wrote:Sorry, which tabs and spaces you mean ?
These particular lines are interesting: https://github.com/berengma/interkom/bl ... #L233-L239
space-only indents: 3/7 lines
1x tab, spaces: 1/7 lines
tab-only indents: 3/7 lines

Not important in terms of functionality but looks a bit as if you copies&pasted multiple mods into this one.
Look, I programmed a bug for you. >> Mod Search Engine << - Mods by Krock - DuckDuckGo mod search bang: !mtmod <keyword here>

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] Exchange stuff between servers [Interkom]

by sofar » Post

My initial thoughts on this mod are to secure the wire protocol and not use files as they are obviously prone to locking issues (as lua does none) and 2 servers could write the same file at the same time. A better implementation would be to use sockets to do all the communication, where each server has a "receive" socket, and the sender transmits by sending some json encoded structural content.

The chatcommands and parsing of input are difficult things to get right (does it handle utf8 properly?) so I would think about replacing the chatcommands with a properly secured formspec where players need to put the actual items in a send slot, which also takes the trouble of knowing the item names away for players. You could use the sockets to send player lists to each server so that players know who is online or not and have the form display this, or maintain a friend list or something like that.

User avatar
Lone_Wolf
Member
Posts: 2576
Joined: Sun Apr 09, 2017 05:50
GitHub: LoneWolfHT
IRC: LandarVargan
In-game: LandarVargan

Re: [Mod] Exchange stuff between servers [Interkom]

by Lone_Wolf » Post

Gundul wrote:
Krock wrote: 2) Writing to the file at the same time: If server 1 writes to the file whereas server 2 deletes an entry from the file, there may be a file corruption. Not sure though, how this could be solved.
I had this in my mind. But I must confess I have no solution for this, too. I was thinking that the software which synchs the folders would take care of this maybe.
I'm not sure of how everything works in your system but you might be able to create a 'status file' for each server in the folder and set it up so that before a server writes anything it checks the other server's status files.
If none of the other servers is writing have it set the contents of its status file to something that says it's writing to a file.
Make it write to the file it was originally going to write to, and when it finishes you can make it rewrite it's status file to something saying it's not writing

I hope that's readable...
My ContentDB -|- Working on CaptureTheFlag -|- Minetest Forums Dark Theme!! (You need it)

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] Exchange stuff between servers [Interkom]

by sofar » Post

Lone_Wolf wrote: I'm not sure of how everything works in your system but you might be able to create a 'status file' for each server in the folder and set it up so that before a server writes anything it checks the other server's status files.
If none of the other servers is writing have it set the contents of its status file to something that says it's writing to a file.
Make it write to the file it was originally going to write to, and when it finishes you can make it rewrite it's status file to something saying it's not writing

I hope that's readable...
Now you just created a second locking problem with the state file. :)

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

Lone_Wolf wrote: Make it write to the file it was originally going to write to, and when it finishes you can make it rewrite it's status file to something saying it's not writing

I hope that's readable...
Thank you it was readable :)
But I think sofar already answered this. :)

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

sofar wrote:My initial thoughts on this mod are to secure the wire protocol and not use files as they are obviously prone to locking issues (as lua does none) and 2 servers could write the same file at the same time. A better implementation would be to use sockets to do all the communication, where each server has a "receive" socket, and the sender transmits by sending some json encoded structural content.

The chatcommands and parsing of input are difficult things to get right (does it handle utf8 properly?) so I would think about replacing the chatcommands with a properly secured formspec where players need to put the actual items in a send slot, which also takes the trouble of knowing the item names away for players. You could use the sockets to send player lists to each server so that players know who is online or not and have the form display this, or maintain a friend list or something like that.
You are right with the formspecs. I chose the chatcommands because experience from my servers says
players are lazy in writing. That way I could test my mod online on my servers more relaxed in the beginning.

I must confess I have very little knowledge about sockets, but that can change. Always happy to learn something.
A quick search returned a "lua socket api" for lua 5.1. No idea if that could help. If someone knows any good source
or book please feel free to post/tell me.

The next steps for interkom will be:
  • - use minetest.safe_write_file if possible
    - restrict use of chatcommands to players with interkom(server ?) priv
    - add formspecs to handle items and send them to connected servers
    - find a way to use sockets
These steps do not have to be static. I am happy for any serious idea someone could have.

u18398

save file writing

by u18398 » Post

created a new branch using minetest.safe_file_write
https://github.com/berengma/interkom/tree/savewrite

u18398

GUI and save_write_file

by u18398 » Post

Finished the gui. No more writing needed. Just type /stuff. On the left see the connected servers and on the right
you can see the players on that sever. To the four slots in the middle you can move the stuff you want to send.
Press send and watch chat messages if everything went well.


Now we have pictures :)
Image

I will add support for unified_inventory shortly, so you can use directly from your inv.

User avatar
Otter
Member
Posts: 152
Joined: Fri May 12, 2017 21:17
GitHub: InfiniteOtter
In-game: Otter

Re: [Mod] Exchange stuff between servers [Interkom]

by Otter » Post

I just want to support that this is quite tested and seems to work without hitch. I am quite particular about things disappearing in mods. The exchange using this software ends up being quite robust and when there were issues, the items did not get sent.
I have sent quite a bit of stuff between the servers. The stacks can be large on these servers so a lot can be sent at once. Between the servers there is a strong advantage to exchanges. Some things common enough on one are quite rare on the other. This worked out well.

It was an entirely different mod and not one made by Gundul that showed a bug and deleted weeks of productivity. In retrospect, sending your most valuable resources from multiple servers to a single server is probably foolish. Loosing large stacks of diamond, and mese blocks that you spent weeks accumulating can be pretty discouraging. Especially when you need them to continue researching how to use the mod that destroyed the stuff. :)

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

Otter wrote:I just want to support that this is quite tested and seems to work without hitch. I am quite particular about things disappearing in mods. The exchange using this software ends up being quite robust and when there were issues, the items did not get sent.
I have sent quite a bit of stuff between the servers. The stacks can be large on these servers so a lot can be sent at once. Between the servers there is a strong advantage to exchanges. Some things common enough on one are quite rare on the other. This worked out well.
I can confirm, no issues so far since half a year running this on my servers. No lost stuff at all.
Now even everything can be sent, not only default:xxx and
with the new gui pretty easy for everyone to handle.
Otter wrote: It was an entirely different mod and not one made by Gundul that showed a bug and deleted weeks of productivity. In retrospect, sending your most valuable resources from multiple servers to a single server is probably foolish. Loosing large stacks of diamond, and mese blocks that you spent weeks accumulating can be pretty discouraging. Especially when you need them to continue researching how to use the mod that destroyed the stuff. :)
Maybe interesting to know which mod that was ? I did not know that there was a mod like mine before.

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

fixed crash if server just started and playerlist was still empty

besides from that, mod runs reliably on the three connected servers without issues for
more than 6 months now. Tons of stuff and messages have been sent.
And with a gui it is much easier to use now.

But that does still not mean, the mod is 100% safe to use. If you feel like try and find security holes,
please do so. You find two of the three servers in the minetest serverlist. Feel free to test.

Gundul

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

If stuff was rejected you do not need to whitelist "Customs". Stuff is returned
automatically. Also playername "Customs" is forbidden now.

Added a third server to Jungle universe.

Now the following three servers are connected and you can exchange stuff and
private messages:

neoh4x0r
Member
Posts: 82
Joined: Wed Aug 29, 2018 20:16
GitHub: neoh4x0r

Re: [Mod] Exchange stuff between servers [Interkom]

by neoh4x0r » Post

Gundul wrote:I do not post something "iffy" in mods section, this works for months without iussues on my two public servers
There have been security bugs and vulnerabilities in mainstream software that went unnoticed for years (even for a few decades in some cases) [and for a large majority of them they were simple mistakes involving input validation, typographical, and logical errors].

To be honest, just because there have not been any issues yet -- does not mean that there won't be any in the near future.....all that it means is that the right person(s) have not come along yet to find and/or take advantage of them.

PS: Just from taking a cursory look at the code.....
Other than using the "shared" folder Lilly -- it is not clear how you are actually sending data to remote servers.

I see the following in your first post:
You need to setup a synchronized folder if you want to connect servers.
By default this folder must be in your world directory named "Lilly"
for example ~/.minetest/worlds/myworld/Lilly

With this mod it is possible to transfer stuff from one server to an other. It also works
on one server only, but /give command is a bit faster :)
But still, in the code, it is not clear how the servers are actually communicating...(whether they are remote servers, etc)

I guess what I am asking for is clarification on how the severs are actually communicating (to read/write a remote file in folder Lilly)

Because, the exact technical nature of how this file gets written to would be of use to anyone that wished to abuse it (ie to trash it or give themselves an unfair advantage by inserting whatever they wanted) --

EG: What security measures are there to ensure that an unauthorized third-party cannot write to the file either locally or remotely ?

neoh4x0r
Member
Posts: 82
Joined: Wed Aug 29, 2018 20:16
GitHub: neoh4x0r

Re: [Mod] Exchange stuff between servers [Interkom]

by neoh4x0r » Post

Krock wrote:
Gundul wrote:Items with metadata could be stackable, yes. But in that case the metadata would be lost if sent to an other server.
Ah okay. I didn't look at the code for a too long time to notice that behaviour.
Gundul wrote:Sorry, which tabs and spaces you mean ?
These particular lines are interesting: https://github.com/berengma/interkom/bl ... #L233-L239
space-only indents: 3/7 lines
1x tab, spaces: 1/7 lines
tab-only indents: 3/7 lines

Not important in terms of functionality but looks a bit as if you copies&pasted multiple mods into this one.
Also those lines (if block) is redundant:

Code: Select all

if supported and not tool then
interkom.saveAC(sname,"GIV,"..name..","..interkom.name..","..pname..","..message)
minetest.chat_send_player(name,core.colorize(green,">> Stuff send to: ")..core.colorize(orange,pname.."@"..sname))
return true
else
       -- the following line is false if we are in the outer-else block, and because of this, the
       -- the message about metadata will never be seen because it will always evaluate to false.
	if supported and not tool then
	minetest.chat_send_player(name,core.colorize(red,">> ERROR: ")..core.colorize(green,"You do not have (or contains  meta) ")..core.colorize(orange,message))
	return false
	else -- this will always be true
		if not tool then
		minetest.chat_send_player(name,core.colorize(red,">> ERROR: ")..core.colorize(orange,"> "..message.." <")..core.colorize(green," -- Enter stackname like modname:name # (example: default:stone 25)"))
		return false
		else
			minetest.chat_send_player(name,core.colorize(red,">> ERROR: ")..core.colorize(green,"You can not send tools"))
			return false
		end
	end
end
It could be reduced to this (50% less code):

Code: Select all

if not supported or tool then
 minetest.chat_send_player(name,core.colorize(red,">> ERROR: ")..core.colorize(green,"You can not send tools or items with metadata"))
 return false
end
if supported and not tool then
 interkom.saveAC(sname,"GIV,"..name..","..interkom.name..","..pname..","..message)
 minetest.chat_send_player(name,core.colorize(green,">> Stuff send to: ")..core.colorize(orange,pname.."@"..sname))
 return true
end
EG: (De Morgan's laws) -- take the negation of (not S or T) = S and not T

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

neoh4x0r wrote: To be honest, just because there have not been any issues yet -- does not mean that there won't be any in the near future.....all that it means is that the right person(s) have not come along yet to find and/or take advantage of them.
I never said it is safe, did I ? Use at your own risk I said. Nobody ever stated that this wip mod is free of bugs or
can not be abused in some way. Welcome to the WIP section :D
Only thing which is proved so far: it works for a very long time and in a every day use. That is all.
neoh4x0r wrote: But still, in the code, it is not clear how the servers are actually communicating...(whether they are remote servers, etc)

I guess what I am asking for is clarification on how the severs are actually communicating (to read/write a remote file in folder Lilly)

Because, the exact technical nature of how this file gets written to would be of use to anyone that wished to abuse it (ie to trash it or give themselves an unfair advantage by inserting whatever they wanted) --

EG: What security measures are there to ensure that an unauthorized third-party cannot write to the file either locally or remotely ?
How file is read and written you can read in code.
How the servers communicate is not part of the mod, but is provided by your operation system.
For example you have three servers on three different locations like I do:

you chose one server as a master server and make a folder "Lilly" in your world folder: ~/.minetest/worlds/test/Lilly

on the two other servers you now need to mount that remote folder to a local one on each machine and link it to
each world folder. How this can be done depends on you and your operation system. A quick google search how to mount remote folders will tell you more.

neoh4x0r
Member
Posts: 82
Joined: Wed Aug 29, 2018 20:16
GitHub: neoh4x0r

Re: [Mod] Exchange stuff between servers [Interkom]

by neoh4x0r » Post

Gundul wrote:
neoh4x0r wrote: To be honest, just because there have not been any issues yet -- does not mean that there won't be any in the near future.....all that it means is that the right person(s) have not come along yet to find and/or take advantage of them.
I never said it is safe, did I ? Use at your own risk I said. Nobody ever stated that this wip mod is free of bugs or
can not be abused in some way. Welcome to the WIP section :D
Only thing which is proved so far: it works for a very long time and in a every day use. That is all.
neoh4x0r wrote: But still, in the code, it is not clear how the servers are actually communicating...(whether they are remote servers, etc)

I guess what I am asking for is clarification on how the severs are actually communicating (to read/write a remote file in folder Lilly)

Because, the exact technical nature of how this file gets written to would be of use to anyone that wished to abuse it (ie to trash it or give themselves an unfair advantage by inserting whatever they wanted) --

EG: What security measures are there to ensure that an unauthorized third-party cannot write to the file either locally or remotely ?
How file is read and written you can read in code.
How the servers communicate is not part of the mod, but is provided by your operation system.
For example you have three servers on three different locations like I do:

you chose one server as a master server and make a folder "Lilly" in your world folder: ~/.minetest/worlds/test/Lilly

on the two other servers you now need to mount that remote folder to a local one on each machine and link it to
each world folder. How this can be done depends on you and your operation system. A quick google search how to mount remote folders will tell you more.
Yeah it was easy enough to see how the file was written and read on a server....just not how the servers were actually connected / communicated with one another.

I see when you write a file you are using safe_file_write (as Krock pointed out) to solve "Unreliable file writing".

However, there might another potential issue with concurrent writes (a situation where the data is no longer in sync between the servers).

I found this about it in lua_api.txt:
* `minetest.safe_file_write(path, content)`: returns boolean indicating success
* Replaces contents of file at path with new contents in a safe (atomic)
way. Use this instead of below code when writing e.g. database files:
`local f = io.open(path, "wb"); f:write(content); f:close()`
The line that reads: Replaces contents of file at path with new contents in a safe (atomic) way. would be good for handing issues like Krock pointed out.

However, it does not take into consideration if another server is currently writing to the file when another server also tries to write to it (concurrent writes, race conditions, etc).

Atomic operations are useful for solving problems with mutual exclusion, but they are only effective if they are executed on the same system (eg for a variable in memory to be safely updated when two or more processes are attempting to update it) -- the atomic part means that the operation only takes a single execution / instruction cycle to preform the update such that is no chance that the instructions from the two processes would be interleaved.

These atomic operations do not apply to external access.....for instance: for remote synchronization you may want to use lock files that are created at the start of a write and removed after the end of the write -- the servers would check for the presence of a lock file before writing and if a lock was found they would wait for the file to be written before attempting to update it.

Considering the atomic nature of safe_file_write, it is likely that it would not take very long for the write to be complete.

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

neoh4x0r wrote:for instance: for remote synchronization you may want to use lock files that are created at the start of a write and removed after the end of the write -- the servers would check for the presence of a lock file before writing and if a lock was found they would wait for the file to be written before attempting to update it.

Considering the atomic nature of safe_file_write, it is likely that it would not take very long for the write to be complete.
You are not the first with the lock files idea. Please read Lone Wolf's post and Sofar's answer. to it.
Sofar suggested to use sockets instead, but this is still beyond my knowlegde.

We did some stress testing. Three people on the three servers send stuff to each other very quickly. As far as the gui allows. Nothing happened. No corrupted files, nothing. Everythings was sent and recieved like it should. There might still be theoretical problems, in practice there are none, so far.

If you would like to find the first issue, you are invited to do so. The three servers are in the public list.

u18398

Re: [Mod] Exchange stuff between servers [Interkom]

by u18398 » Post

Fixed crash when typed /ok without someone having
sent something to you.

Post Reply

Who is online

Users browsing this forum: No registered users and 18 guests